Fixing critical defect in InMemorySlidingWindowRequestRateLimiter and…

… HazelcastSlidingWindowRequestRateLimiter that was resulting in incorrect behaviour beyond a trivial use case
mokies · Apr 28, 2019 · a0b780d · a0b780d
1 parent 2b1b4b4
commit a0b780d
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 2 deletions.
diff --git a/.../src/main/java/es/moki/ratelimitj/hazelcast/HazelcastSlidingWindowRequestRateLimiter.java b/.../src/main/java/es/moki/ratelimitj/hazelcast/HazelcastSlidingWindowRequestRateLimiter.java
@@ -128,7 +128,7 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) {
             List<String> dele = new ArrayList<>();
             long trim = Math.min(savedKey.trimBefore, oldTs + savedKey.blocks);
 
-            for (long oldBlock = oldTs; oldBlock == trim - 1; oldBlock++) {
+            for (long oldBlock = oldTs; oldBlock <= trim - 1; oldBlock++) {
                 String bkey = savedKey.countKey + oldBlock;
                 Long bcount = hcKeyMap.get(bkey);
                 if (bcount != null) {

diff --git a/...ain/java/es/moki/ratelimitj/inmemory/request/InMemorySlidingWindowRequestRateLimiter.java b/...ain/java/es/moki/ratelimitj/inmemory/request/InMemorySlidingWindowRequestRateLimiter.java
@@ -132,7 +132,7 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) {
             List<String> dele = new ArrayList<>();
             long trim = Math.min(savedKey.trimBefore, oldTs + savedKey.blocks);
 
-            for (long oldBlock = oldTs; oldBlock == trim - 1; oldBlock++) {
+            for (long oldBlock = oldTs; oldBlock <= trim - 1; oldBlock++) {
                 String bkey = savedKey.countKey + oldBlock;
                 Long bcount = keyMap.get(bkey);
                 if (bcount != null) {

diff --git a/...main/java/es/moki/ratelimitj/test/limiter/request/AbstractSyncRequestRateLimiterTest.java b/...main/java/es/moki/ratelimitj/test/limiter/request/AbstractSyncRequestRateLimiterTest.java
@@ -11,6 +11,7 @@
 
 import java.time.Duration;
 import java.util.*;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BiFunction;
 import java.util.stream.IntStream;
 
@@ -118,6 +119,29 @@ void shouldResetLimit() {
         assertThat(requestRateLimiter.overLimitWhenIncremented(key)).isFalse();
     }
 
+
+    @Test
+    void shouldRateLimitOverTime() {
+        RequestLimitRule rule1 = RequestLimitRule.of(Duration.ofSeconds(5), 250).withPrecision(Duration.ofSeconds(1)).matchingKeys("ip:127.3.9.3");
+        RequestRateLimiter requestRateLimiter = getRateLimiter(ImmutableSet.of(rule1), timeBandit);
+        AtomicLong timeOfLastOperation = new AtomicLong();
+
+        IntStream.rangeClosed(1, 50).forEach(loop -> {
+
+            IntStream.rangeClosed(1, 250).forEach(value -> {
+                timeBandit.addUnixTimeMilliSeconds(14L);
+                boolean overLimit = requestRateLimiter.overLimitWhenIncremented("ip:127.3.9.3");
+                if (overLimit) {
+                    long timeSinceLastOperation = timeBandit.get() - timeOfLastOperation.get();
+                    assertThat(timeSinceLastOperation).isLessThan(3);
+                } else {
+                    timeOfLastOperation.set(timeBandit.get());
+                }
+            });
+
+        });
+    }
+
     @Test @Disabled
     void shouldPreventThunderingHerdWithPrecision() {