Skip to content

Commit

Permalink
some small fixes and improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@w-ensink
w-ensink committed Dec 16, 2024
1 parent 3003d93 commit b6670bc
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 21 deletions.
29 changes: 17 additions & 12 deletions src/main/java/foundation/privacybydesign/sms/ratelimit/RedisRateLimit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ public class RedisRateLimit extends RateLimit {
private static Logger LOG = LoggerFactory.getLogger(RedisRateLimit.class);
private static RedisRateLimit instance;

final String ipLimitsNamespace = "ip-limits";
final String phoneLimitsNamespace = "phone-limits:";
private static final String ipLimitsNamespace = "ip-limits";
private static final String phoneLimitsNamespace = "phone-limits:";
private static final String timestampFieldName = "timestamp";
private static final String triesFieldName = "tries";

public static RateLimit getInstance() {
if (instance == null) {
Expand Down Expand Up @@ -59,7 +61,7 @@ protected long nextTryIP(String ip, long now) {
}
}

long startLimit = startLimitIP(now);
final long startLimit = startLimitIP(now);
if (limit < startLimit) {
// First visit or previous visit was long ago.
// Act like the last try was 3 periods ago.
Expand Down Expand Up @@ -122,7 +124,7 @@ protected synchronized long nextTryPhone(String phone, long now) {

@Override
protected synchronized void countIP(String ip, long now) {
long nextTry = nextTryIP(ip, now);
final long nextTry = nextTryIP(ip, now);
if (nextTry > now) {
throw new IllegalStateException("counting rate limit while over the limit");
}
Expand All @@ -137,7 +139,7 @@ protected synchronized void countIP(String ip, long now) {
// phone number.
@Override
protected synchronized void countPhone(String phone, long now) {
long nextTry = nextTryPhone(phone, now);
final long nextTry = nextTryPhone(phone, now);

try (var jedis = pool.getResource()) {
final String key = Redis.createKey(phoneLimitsNamespace, phone);
Expand Down Expand Up @@ -182,8 +184,8 @@ static Limit limitFromRedis(Jedis jedis, String key) {
jedis.watch(key);
Transaction transaction = jedis.multi();

final Response<String> tsResult = transaction.hget(key, "timestamp");
final Response<String> triesResponse = transaction.hget(key, "tries");
final Response<String> tsResult = transaction.hget(key, timestampFieldName);
final Response<String> triesResponse = transaction.hget(key, triesFieldName);

final List<Object> transactionResult = transaction.exec();

Expand Down Expand Up @@ -214,8 +216,8 @@ static void limitToRedis(Jedis jedis, String key, Limit limit) {
jedis.watch(key);
Transaction transaction = jedis.multi();

transaction.hset(key, "timestamp", Long.toString(limit.timestamp));
transaction.hset(key, "tries", Integer.toString(limit.tries));
transaction.hset(key, timestampFieldName, Long.toString(limit.timestamp));
transaction.hset(key, triesFieldName, Integer.toString(limit.tries));

final List<Object> results = transaction.exec();
if (results == null) {
Expand All @@ -230,8 +232,10 @@ static void limitToRedis(Jedis jedis, String key, Limit limit) {
}
}

// TODO: This is not the idiomatic way to delete expired items in Redis,
// use the built in `expire` command instead
private void cleanUpIpLimits() {
long now = System.currentTimeMillis();
final long now = System.currentTimeMillis();

final String pattern = Redis.createNamespace(ipLimitsNamespace) + "*";
ScanParams scanParams = new ScanParams().match(pattern);
Expand All @@ -258,8 +262,10 @@ private void cleanUpIpLimits() {
}
}

// TODO: This is not the idiomatic way to delete expired items in Redis,
// use the built in `expire` command instead
private void cleanUpPhoneLimits() {
long now = System.currentTimeMillis();
final long now = System.currentTimeMillis();

final String pattern = Redis.createNamespace(phoneLimitsNamespace) + "*";
ScanParams scanParams = new ScanParams().match(pattern);
Expand All @@ -280,5 +286,4 @@ private void cleanUpPhoneLimits() {
} while (!cursor.equals("0")); // continue until the cursor wraps around
}
}

}
24 changes: 15 additions & 9 deletions src/main/java/foundation/privacybydesign/sms/tokens/RedisTokenRequestRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
*/
class RedisTokenRequestRepository implements TokenRequestRepository {
private static Logger LOG = LoggerFactory.getLogger(RedisTokenRequestRepository.class);
private static String namespace = "request";
private static final String namespace = "request";
private static final String tokenFieldName = "token";
private static final String triesFieldName = "tries";
private static final String createdFieldName = "tries";

JedisSentinelPool pool;

RedisTokenRequestRepository() {
Expand All @@ -34,9 +38,9 @@ public void store(String phone, TokenRequest request) {
jedis.watch(key);

Transaction transaction = jedis.multi();
transaction.hset(key, "token", request.token);
transaction.hset(key, "tries", Integer.toString(request.tries));
transaction.hset(key, "created", String.valueOf(request.created));
transaction.hset(key, tokenFieldName, request.token);
transaction.hset(key, triesFieldName, Integer.toString(request.tries));
transaction.hset(key, createdFieldName, String.valueOf(request.created));

final List<Object> result = transaction.exec();

Expand All @@ -61,6 +65,8 @@ public void remove(String phone) {
}
}

// TODO: This is not the idiomatic way to delete expired items in Redis,
// use the built in `expire` command instead
@Override
public void removeExpired() {
final String pattern = Redis.createNamespace(namespace) + "*";
Expand All @@ -81,10 +87,10 @@ public void removeExpired() {
}

private void removeIfExpired(Jedis jedis, String key) {
String createdStr = jedis.hget(key, "created");
final String createdStr = jedis.hget(key, createdFieldName);
if (createdStr != null) {
try {
long created = Long.parseLong(createdStr);
final long created = Long.parseLong(createdStr);
if (TokenRequest.isExpiredForCreationDate(created)) {
jedis.del(key);
}
Expand All @@ -103,9 +109,9 @@ public TokenRequest retrieve(String phone) {
jedis.watch(key);
Transaction transaction = jedis.multi();

final Response<String> tokenRes = transaction.hget(key, "token");
final Response<String> triesRes = transaction.hget(key, "tries");
final Response<String> createdRes = transaction.hget(key, "created");
final Response<String> tokenRes = transaction.hget(key, tokenFieldName);
final Response<String> triesRes = transaction.hget(key, triesFieldName);
final Response<String> createdRes = transaction.hget(key, createdFieldName);

final List<Object> execRes = transaction.exec();

Expand Down

0 comments on commit b6670bc

Please sign in to comment.