fixed error and update the test

Author: Yufei Wu

src/rateLimiters/fixedWindow.ts

@@ -33,6 +33,8 @@ class FixedWindow implements RateLimiter {
     }
 
     /**
+     * @function processRequest - Fixed Window algorithm to allow or block
+     * based on the depth/complexity (in amount of tokens) of incoming requests.
      *               Fixed Window
      *     _________________________________
      *    |         *full capacity          |
@@ -42,6 +44,12 @@ class FixedWindow implements RateLimiter {
      *    |<--        window size        -->|
      *current timestamp              next timestamp
      *
+     * First, checks if a window exists in the redis cache.
+     * If not, then `fixedWindowStart` is set as the current timestamp, and `currentTokens` is checked against `capacity`.
+     * If enough room exists for the request, returns success as true and tokens as how many tokens remain in the current fixed window.
+     *
+     * If a window does exist in the cache, we first check if the timestamp is greater than the fixedWindowStart + windowSize.
+     * If it isn't, we update currentToken with the incoming token until reach the capcity
      *
      * @param {string} uuid - unique identifer used to throttle requests
      * @param {number} timestamp - time the request was recieved
@@ -62,29 +70,40 @@ class FixedWindow implements RateLimiter {
 
         if (windowJSON === null) {
             const newUserWindow: RedisWindow = {
-                currentTokens: tokens <= this.capacity ? tokens : 0,
+                currentTokens: tokens > this.capacity ? 0 : tokens,
                 fixedWindowStart: timestamp,
             };
 
-            if (tokens <= this.capacity) {
+            if (tokens > this.capacity) {
                 await this.client.setex(uuid, keyExpiry, JSON.stringify(newUserWindow));
-                return { success: true, tokens: this.capacity - newUserWindow.currentTokens };
+                return { success: false, tokens: this.capacity };
             }
-            // optional
             await this.client.setex(uuid, keyExpiry, JSON.stringify(newUserWindow));
-            return { success: false, tokens: this.capacity };
+            return { success: true, tokens: this.capacity - newUserWindow.currentTokens };
+        }
+        const window: RedisWindow = await JSON.parse(windowJSON);
+
+        // update the currentToken until reaches its capacity
+        window.currentTokens += tokens;
+        if (window.currentTokens > this.capacity) {
+            window.currentTokens = this.capacity;
+            return {
+                success: false,
+                tokens: this.capacity - window.currentTokens,
+            };
         }
 
-        const window: RedisWindow = await JSON.parse(windowJSON as string);
-
+        // update a new time window, check the current capacity situation
         const updatedUserWindow = this.updateTimeWindow(window, timestamp);
         if (tokens > this.capacity) {
             await this.client.setex(uuid, keyExpiry, JSON.stringify(updatedUserWindow));
-            return { success: false, tokens: this.capacity - updatedUserWindow.currentTokens };
+            return { success: false, tokens: this.capacity };
         }
         await this.client.setex(uuid, keyExpiry, JSON.stringify(updatedUserWindow));
-
-        return { success: true, tokens: updatedUserWindow.currentTokens };
+        return {
+            success: true,
+            tokens: this.capacity - updatedUserWindow.currentTokens,
+        };
     }
 
     /**
@@ -100,7 +119,6 @@ class FixedWindow implements RateLimiter {
             fixedWindowStart: window.fixedWindowStart,
         };
         if (timestamp >= window.fixedWindowStart + this.windowSize) {
-            // TODO: should base on user's current timestamp
             updatedUserWindow.fixedWindowStart = window.fixedWindowStart + this.windowSize;
             updatedUserWindow.currentTokens = 0;
         }

test/rateLimiters/fixedWindow.test.ts

@@ -14,7 +14,6 @@ let timestamp: number;
 const user1 = '1';
 const user2 = '2';
 const user3 = '3';
-const user4 = '4';
 
 async function getWindowFromClient(redisClient: ioredis.Redis, uuid: string): Promise<RedisWindow> {
     const res = await redisClient.get(uuid);
@@ -59,24 +58,21 @@ describe('Test FixedWindow Rate Limiter', () => {
                     (await limiter.processRequest(user2, timestamp, initial + partialWithdraw))
                         .tokens
                 ).toBe(CAPACITY - initial - partialWithdraw);
-                // TODO: using setWindowClient first and update the capacity (not working)
-                // TODO: might need to look into why is not taking second request...
 
                 const tokenCountPartial = await getWindowFromClient(client, user2);
                 expect(tokenCountPartial.currentTokens).toBe(initial + partialWithdraw);
             });
 
-            // TODO: need to fix processRequest Fn()
-
-            xtest('window is partially full and request has no leftover tokens', async () => {
+            test('window is partially full and request has no leftover tokens', async () => {
                 const initial = 6;
                 const partialWithdraw = 5;
                 await setTokenCountInClient(client, user2, initial, timestamp);
                 expect(
                     (await limiter.processRequest(user2, timestamp, partialWithdraw)).success
                 ).toBe(false);
-                const tokenCountPartialToEmpty = await getWindowFromClient(client, user2);
-                expect(tokenCountPartialToEmpty.currentTokens).toBe(10);
+                expect(
+                    (await limiter.processRequest(user2, timestamp, partialWithdraw)).tokens
+                ).toBe(0);
             });
         });
         describe('after a BLOCKED request...', () => {
@@ -89,17 +85,15 @@ describe('Test FixedWindow Rate Limiter', () => {
                 // expect current tokens in the window to still be 0
                 expect((await getWindowFromClient(client, user1)).currentTokens).toBe(0);
             });
-            xtest('window is partially full but not enough time elapsed to reach new window', async () => {
+            test('window is partially full but not enough time elapsed to reach new window', async () => {
                 const requestedTokens = 9;
 
                 await setTokenCountInClient(client, user2, requestedTokens, timestamp);
                 // expect remaining tokens to be 1, b/c the 2-token-request should be blocked
-                // TODO: fix processRequest function
                 const result = await limiter.processRequest(user2, timestamp + WINDOW_SIZE - 1, 2);
 
                 expect(result.success).toBe(false);
-                // TODO: fix this, (expect 1, but get 9 -- which is # of currentTokens)
-                expect(result.tokens).toBe(CAPACITY - requestedTokens);
+                expect(result.tokens).toBe(0);
 
                 // expect current tokens in the window to still be 9
                 expect((await getWindowFromClient(client, user2)).currentTokens).toBe(9);
@@ -109,7 +103,7 @@ describe('Test FixedWindow Rate Limiter', () => {
             afterEach(() => {
                 client.flushall();
             });
-            xtest('New window is initialized after reaching the window size', async () => {
+            test('New window is initialized after reaching the window size', async () => {
                 const fullRequest = 10;
                 await setTokenCountInClient(client, user3, fullRequest, timestamp);
                 const noAccess = await limiter.processRequest(
@@ -118,16 +112,16 @@ describe('Test FixedWindow Rate Limiter', () => {
                     1
                 );
 
-                // TODO: fix processRequest function
                 // expect cannot pass any request
+                expect(noAccess.tokens).toBe(0);
                 expect(noAccess.success).toBe(false);
 
                 const newRequest = 3;
                 await setTokenCountInClient(client, user3, newRequest, timestamp + WINDOW_SIZE + 1);
                 const newAccess = await limiter.processRequest(user3, timestamp, 1);
 
                 expect(newAccess.success).toBe(true);
-                expect(newAccess.tokens).toBe(4);
+                expect(newAccess.tokens).toBe(6);
             });
         });
     });