Fix incorrect logic for limit test

.test() can only reliably assert whether a rate limit is currently exceeded (i.e. num requests in window >= total allowed)
alisaifee · May 12, 2015 · 28fba41 · 28fba41
1 parent 7eba8ce
commit 28fba41
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 23 deletions.
diff --git a/limits/strategies.py b/limits/strategies.py
@@ -86,11 +86,10 @@ def test(self, item, *identifiers):
          limit
         :return: True/False
         """
-        return self.storage().acquire_entry(
+        return self.storage().get_moving_window(
             item.key_for(*identifiers),
             item.amount, item.get_expiry(),
-            no_add=True
-        )
+        )[1] < item.amount
 
     def get_window_stats(self, item, *identifiers):
         """
@@ -136,7 +135,7 @@ def test(self, item, *identifiers):
          limit
         :return: True/False
         """
-        return self.storage().get(item.key_for(*identifiers)) <= item.amount
+        return self.storage().get(item.key_for(*identifiers)) < item.amount
 
     def get_window_stats(self, item, *identifiers):
         """

diff --git a/tests/test_strategy.py b/tests/test_strategy.py
@@ -139,32 +139,23 @@ def xest_moving_window_memcached(self):
 
 
     def test_test_fixed_window(self):
-        stores = [
-            MemoryStorage(),
-            RedisStorage("redis:/localhost:6379"),
-            MemcachedStorage("memcached://localhost:11211")
-        ]
-        limit = RateLimitItemPerSecond(1,1)
-        for store in stores:
+        with hiro.Timeline().freeze() as timeline:
+            store = MemoryStorage()
             limiter = FixedWindowRateLimiter(store)
+            limit = RateLimitItemPerSecond(2,1)
             self.assertTrue(limiter.hit(limit), store)
-            self.assertFalse(limiter.hit(limit), store)
-            self.assertFalse(limiter.test(limit), store)
-            time.sleep(1)
             self.assertTrue(limiter.test(limit), store)
             self.assertTrue(limiter.hit(limit), store)
+            self.assertFalse(limiter.test(limit), store)
+            self.assertFalse(limiter.hit(limit), store)
 
     def test_test_moving_window(self):
-        stores = [
-            MemoryStorage(),
-            RedisStorage("redis:/localhost:6379"),
-        ]
-        limit = RateLimitItemPerSecond(1,1)
-        for store in stores:
+        with hiro.Timeline().freeze() as timeline:
+            store = MemoryStorage()
+            limit = RateLimitItemPerSecond(2,1)
             limiter = MovingWindowRateLimiter(store)
             self.assertTrue(limiter.hit(limit), store)
-            self.assertFalse(limiter.hit(limit), store)
-            self.assertFalse(limiter.test(limit), store)
-            time.sleep(1)
             self.assertTrue(limiter.test(limit), store)
             self.assertTrue(limiter.hit(limit), store)
+            self.assertFalse(limiter.test(limit), store)
+            self.assertFalse(limiter.hit(limit), store)