-
Notifications
You must be signed in to change notification settings - Fork 92
Add incrementer #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add incrementer #18
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| .settings | ||
| .classpath | ||
| .project | ||
| bin/ | ||
|
|
||
| # Package Files # | ||
| *.jar | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ public boolean overLimitWhenIncremented(String key) { | |
| // TODO support muli keys | ||
| @Override | ||
| public boolean overLimitWhenIncremented(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, true); | ||
| return eqOrGeLimit(key, weight, true, false); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -60,20 +60,19 @@ public boolean geLimitWhenIncremented(String key) { | |
|
|
||
| @Override | ||
| public boolean geLimitWhenIncremented(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, false); | ||
| return eqOrGeLimit(key, weight, false, false); | ||
| } | ||
|
|
||
| // @Override | ||
| // public boolean isOverLimit(String key) { | ||
| // return overLimitWhenIncremented(key, 0); | ||
| // } | ||
| // | ||
| // @Override | ||
| // public boolean isGeLimit(String key) { | ||
| // return geLimitWhenIncremented(key, 0); | ||
| // } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| @Override | ||
| public boolean incrementRegardless(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, false, true); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| @Override | ||
| public boolean resetLimit(String key) { | ||
| IMap<Object, Object> map = hz.getMap(key); | ||
| if (map == null || map.isEmpty()) { | ||
|
|
@@ -92,7 +91,7 @@ private IMap<String, Long> getMap(String key, int longestDuration) { | |
| return hz.getMap(key); | ||
| } | ||
|
|
||
| private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) { | ||
| private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater, boolean increment_anyway) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For me |
||
|
|
||
| requireNonNull(key, "key cannot be null"); | ||
| requireNonNull(rules, "rules cannot be null"); | ||
|
|
@@ -152,7 +151,10 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) { | |
| // check our limits | ||
| long count = coalesce(cur, 0L) + weight; | ||
| if (count > rule.getLimit()) { | ||
| return true; // over limit, don't record request | ||
| geLimit = true; | ||
| if (!increment_anyway) { | ||
| return true; // over limit, don't record request | ||
| } | ||
| } else if (!strictlyGreater && count == rule.getLimit()) { | ||
| geLimit = true; // at limit, do record request | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,10 +59,41 @@ void shouldEventuallyCleanUpExpiredKeys() throws Exception { | |
| }); | ||
|
|
||
| IMap<Object, Object> map = hz.getMap(key); | ||
|
|
||
| while (map.size() != 0) { | ||
| Thread.sleep(10); | ||
| } | ||
| assertThat(map.size()).isZero(); | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| void shouldCheckIncrementAnyway() throws Exception { | ||
|
|
||
| ImmutableSet<RequestLimitRule> rules = ImmutableSet.of(RequestLimitRule.of(20, TimeUnit.SECONDS, 5)); | ||
| RequestRateLimiter requestRateLimiter = getRateLimiter(rules, timeBandit); | ||
|
|
||
| String key = "ip:127.0.0.5"; | ||
|
|
||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| // Counter should be 0 now | ||
| assertThat(requestRateLimiter.overLimitWhenIncremented(key, 1)).isFalse(); | ||
| // Counter should be 1 now and 'false', meaning not over limit | ||
|
|
||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| assertThat(requestRateLimiter.incrementRegardless(key, 10)).isTrue(); | ||
| // Counter should be 11 now and 'true', meaning over limit | ||
|
|
||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| assertThat(requestRateLimiter.overLimitWhenIncremented(key, 1)).isTrue(); | ||
| // Counter should be 11 now and 'true', meaning over limit | ||
| // If the increment regardless worked, then the value would have been 2 | ||
| // which would not be under the limit. | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
Comment on lines
+92
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Way too many new lines :) |
||
| } | ||
|
|
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,18 +59,24 @@ public boolean overLimitWhenIncremented(String key) { | |
|
|
||
| @Override | ||
| public boolean overLimitWhenIncremented(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, true); | ||
| return eqOrGeLimit(key, weight, true, false); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean incrementRegardless(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, true, true); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean geLimitWhenIncremented(String key) { | ||
| return geLimitWhenIncremented(key, 1); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean geLimitWhenIncremented(String key, int weight) { | ||
| return eqOrGeLimit(key, weight, false); | ||
| return eqOrGeLimit(key, weight, false, false); | ||
| } | ||
|
|
||
|
|
||
| // @Override | ||
| // public boolean isOverLimit(String key) { | ||
|
|
@@ -100,7 +106,9 @@ private ConcurrentMap<String, Long> getMap(String key, int longestDuration) { | |
| }); | ||
| } | ||
|
|
||
| private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) { | ||
| // Chris Fauerbach, adding a way to override the increment even if over limit | ||
| // @chrisfauerbach , https://fauie.com , github.com/chrisfauerbach | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these comments present? |
||
| private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater, boolean increment_regardless) { | ||
|
|
||
| requireNonNull(key, "key cannot be null"); | ||
| requireNonNull(rules, "rules cannot be null"); | ||
|
|
@@ -157,7 +165,11 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) { | |
| // check our limits | ||
| long count = coalesce(cur, 0L) + weight; | ||
| if (count > rule.getLimit()) { | ||
| return true; // over limit, don't record request | ||
| if (increment_regardless) { | ||
| geLimit = true; | ||
| }else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return true; // over limit, don't record request | ||
| } | ||
| } else if (!strictlyGreater && count == rule.getLimit()) { | ||
| geLimit = true; // at limit, do record request | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,20 +4,30 @@ | |
| import es.moki.ratelimitj.core.limiter.request.RequestRateLimiter; | ||
| import es.moki.ratelimitj.core.time.TimeSupplier; | ||
| import es.moki.ratelimitj.test.limiter.request.AbstractSyncRequestRateLimiterTest; | ||
| import es.moki.ratelimitj.test.time.TimeBanditSupplier; | ||
| import io.lettuce.core.RedisClient; | ||
| import io.lettuce.core.api.StatefulRedisConnection; | ||
| import org.junit.jupiter.api.AfterAll; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.BeforeAll; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import com.google.common.collect.ImmutableSet; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.util.Set; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
|
|
||
| public class RedisSlidingWindowSyncRequestRateLimiterTest extends AbstractSyncRequestRateLimiterTest { | ||
|
|
||
| private static RedisClient client; | ||
| private static StatefulRedisConnection<String, String> connect; | ||
| private static Logger LOG = LoggerFactory.getLogger(RedisSlidingWindowSyncRequestRateLimiterTest.class); | ||
|
|
||
| @BeforeAll | ||
| static void beforeAll() { | ||
|
|
@@ -42,4 +52,33 @@ void afterEach() { | |
| protected RequestRateLimiter getRateLimiter(Set<RequestLimitRule> rules, TimeSupplier timeSupplier) { | ||
| return new RedisSlidingWindowRequestRateLimiter(connect, rules, timeSupplier); | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| void shouldCheckIncrementAnyway() throws Exception { | ||
|
|
||
| ImmutableSet<RequestLimitRule> rules = ImmutableSet.of(RequestLimitRule.of(20, TimeUnit.SECONDS, 5)); | ||
| TimeBanditSupplier timeBandit = new TimeBanditSupplier(); | ||
|
|
||
| RequestRateLimiter requestRateLimiter = getRateLimiter(rules, timeBandit); | ||
|
|
||
| String key = "ip:127.0.0.5"; | ||
|
|
||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| // Counter should be 0 now | ||
| assertThat(requestRateLimiter.overLimitWhenIncremented(key, 1)).isFalse(); | ||
| // Counter should be 1 now and 'false', meaning not over limit | ||
| LOG.debug("About to print add 10 to go over."); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO the log debug messages are not needed, and they could be Java comments if they bring any value |
||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| assertThat(requestRateLimiter.incrementRegardless(key, 10)).isTrue(); | ||
| // Counter should be 11 now and 'true', meaning over limit | ||
|
|
||
| LOG.debug("About to print add 1 that should be over.."); | ||
| timeBandit.addUnixTimeMilliSeconds(100L); | ||
| assertThat(requestRateLimiter.overLimitWhenIncremented(key, 1)).isTrue(); | ||
| // Counter should be 11 now and 'true', meaning over limit | ||
| // If the increment regardless worked, then the value would have been 2 | ||
| // which would not be under the limit. | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
towareds -> towards