updated comments. enabled skipped slidingwindowlog tests

Author: shalarewicz

src/rateLimiters/slidingWindowLog.ts

@@ -96,32 +96,33 @@ class SlidingWindowLog implements RateLimiter {
         // Stop at the first timestamp that's expired and cut the rest.
 
         const cutoff = timestamp - this.windowSize;
-        let tokensInLog = 0;
+        let tokensInLog = 0; // total active tokens in the log
         let cutoffIndex = 0; // index of oldest active request
-        // TODO: Provide a timestamp for when the request will succeeed.
-        // Compute time between response and this timestamp later on
-        // FIXME: What should this be if the complexity is too big?
-        let retryIndex = requestLog.length; // time the user must wait before a request can be allowed.
+        let lastAllowedIndex = requestLog.length; // Index of oldest request in the log for which this request would be allowed.
 
         for (let index = requestLog.length - 1; index >= 0; index--) {
             if (cutoff >= requestLog[index].timestamp) {
+                // we reached the first expired request
                 cutoffIndex = index + 1;
                 break;
             } else {
                 // the request is active
                 tokensInLog += requestLog[index].tokens;
                 if (this.capacity - tokensInLog >= tokens) {
-                    // the log is able to accept this request
-                    retryIndex = index;
+                    // the log is able to accept the current request
+                    lastAllowedIndex = index;
                 }
             }
         }
 
+        // Time (ms) after which the current request would succeed if it is blocked.
         let retryAfter: number;
+
+        // Request will never be allowed
         if (tokens > this.capacity) retryAfter = Infinity;
-        // need the request before retryIndex
-        else if (retryIndex > 0)
-            retryAfter = this.windowSize + requestLog[retryIndex - 1].timestamp;
+        // need the request before lastAllowedIndex
+        else if (lastAllowedIndex > 0)
+            retryAfter = this.windowSize + requestLog[lastAllowedIndex - 1].timestamp;
         else retryAfter = 0; // request is allowed
 
         // Conditional check to avoid unecessary slice

test/rateLimiters/slidingWindowLog.test.ts

@@ -467,7 +467,7 @@ describe('SlidingWindowLog Rate Limiter', () => {
             expect(retryAfter).toBe(timestamp + 100 + WINDOW_SIZE);
         });
     });
-    xtest('users have their own logs', async () => {
+    test('users have their own logs', async () => {
         const requested = 6;
         const user3Tokens = 8;
         // // Add log for user 3 so we have both a user that exists in the store (3) and one that doesn't (2)
@@ -477,12 +477,14 @@ describe('SlidingWindowLog Rate Limiter', () => {
         await limiter.processRequest(user1, timestamp + 100, requested);
 
         // // Check that each user has the expected log
-        expect(await getLogFromClient(client, user1)).toEqual({
-            timestamp: timestamp + 100,
-            tokens: requested,
-        });
+        expect(await getLogFromClient(client, user1)).toEqual([
+            {
+                timestamp: timestamp + 100,
+                tokens: requested,
+            },
+        ]);
         expect(await getLogFromClient(client, user2)).toEqual([]);
-        expect(await getLogFromClient(client, user3)).toEqual([{ timestamp, tokens: requested }]);
+        expect(await getLogFromClient(client, user3)).toEqual([{ timestamp, tokens: user3Tokens }]);
 
         await limiter.processRequest(user2, timestamp + 200, 1);
         expect(await getLogFromClient(client, user1)).toEqual([
@@ -497,7 +499,7 @@ describe('SlidingWindowLog Rate Limiter', () => {
                 tokens: 1,
             },
         ]);
-        expect(await getLogFromClient(client, user3)).toEqual([{ timestamp, tokens: requested }]);
+        expect(await getLogFromClient(client, user3)).toEqual([{ timestamp, tokens: user3Tokens }]);
     });
 
     test('is able to be reset', async () => {
@@ -533,7 +535,7 @@ describe('SlidingWindowLog Rate Limiter', () => {
             );
         });
 
-        xtest('...allows custom window size and capacity', async () => {
+        test('...allows custom window size and capacity', async () => {
             const customWindow = 500;
             const customSizelimiter = new SlidingWindowLog(customWindow, CAPACITY, client);
 
@@ -551,18 +553,21 @@ describe('SlidingWindowLog Rate Limiter', () => {
                 .processRequest(user1, timestamp + customWindow, CAPACITY)
                 .then((res) => res.success);
 
-            const customCapacitylimiter = new SlidingWindowLog(WINDOW_SIZE, 5, client);
+            // Reset the redis store
+            customSizelimiter.reset();
+
             const customCapacity = 5;
+            const customCapacitylimiter = new SlidingWindowLog(WINDOW_SIZE, customCapacity, client);
 
-            let customWindowSuccess = await customCapacitylimiter
+            let customCapacitySuccess = await customCapacitylimiter
                 .processRequest(user1, timestamp, customCapacity + 1)
                 .then((res) => res.success);
-            expect(customSizeSuccess).toBe(false);
+            expect(customCapacitySuccess).toBe(false);
 
-            customWindowSuccess = await customCapacitylimiter
+            customCapacitySuccess = await customCapacitylimiter
                 .processRequest(user1, timestamp + 100, customCapacity)
                 .then((res) => res.success);
-            expect(customWindowSuccess).toBe(true);
+            expect(customCapacitySuccess).toBe(true);
         });
     });
 });