Skip to content

Commit 18a8444

Browse files
Yufei WuYufei Wu
authored andcommitted
testing is done, fixing algo now
1 parent 13445c3 commit 18a8444

File tree

2 files changed

+88
-9
lines changed

2 files changed

+88
-9
lines changed

src/rateLimiters/fixedWindow.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,21 @@ class FixedWindow implements RateLimiter {
7070
await this.client.setex(uuid, keyExpiry, JSON.stringify(newUserWindow));
7171
return { success: true, tokens: this.capacity - newUserWindow.currentTokens };
7272
}
73-
73+
// optional
7474
await this.client.setex(uuid, keyExpiry, JSON.stringify(newUserWindow));
75+
return { success: false, tokens: this.capacity };
7576
}
7677

7778
const window: RedisWindow = await JSON.parse(windowJSON as string);
7879

7980
const updatedUserWindow = this.updateTimeWindow(window, timestamp);
80-
if (window.currentTokens > this.capacity) {
81+
if (tokens > this.capacity) {
8182
await this.client.setex(uuid, keyExpiry, JSON.stringify(updatedUserWindow));
82-
return { success: false, tokens: window.currentTokens };
83+
return { success: false, tokens: this.capacity - updatedUserWindow.currentTokens };
8384
}
8485
await this.client.setex(uuid, keyExpiry, JSON.stringify(updatedUserWindow));
8586

86-
return { success: false, tokens: updatedUserWindow.currentTokens };
87+
return { success: true, tokens: updatedUserWindow.currentTokens };
8788
}
8889

8990
/**
@@ -98,7 +99,8 @@ class FixedWindow implements RateLimiter {
9899
currentTokens: window.currentTokens,
99100
fixedWindowStart: window.fixedWindowStart,
100101
};
101-
if (timestamp > window.fixedWindowStart + this.windowSize) {
102+
if (timestamp >= window.fixedWindowStart + this.windowSize) {
103+
// TODO: should base on user's current timestamp
102104
updatedUserWindow.fixedWindowStart = window.fixedWindowStart + this.windowSize;
103105
updatedUserWindow.currentTokens = 0;
104106
}

test/rateLimiters/fixedWindow.test.ts

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ async function getWindowFromClient(redisClient: ioredis.Redis, uuid: string): Pr
2626
async function setTokenCountInClient(
2727
redisClient: ioredis.Redis,
2828
uuid: string,
29-
tokens: number,
30-
time: number
29+
currentTokens: number,
30+
fixedWindowStart: number
3131
) {
32-
const value: RedisWindow = { currentTokens: tokens, fixedWindowStart: time };
32+
const value: RedisWindow = { currentTokens, fixedWindowStart };
3333
await redisClient.set(uuid, JSON.stringify(value));
3434
}
3535
describe('Test FixedWindow Rate Limiter', () => {
@@ -50,7 +50,84 @@ describe('Test FixedWindow Rate Limiter', () => {
5050
CAPACITY - withdraw5
5151
);
5252
const tokenCountFull = await getWindowFromClient(client, user1);
53-
expect(tokenCountFull.currentTokens).toBe(CAPACITY - withdraw5);
53+
expect(tokenCountFull.currentTokens).toBe(5);
54+
});
55+
test('reached 40% capacity in current time window and still can pass request', async () => {
56+
const initial = 5;
57+
const partialWithdraw = 2;
58+
expect(
59+
(await limiter.processRequest(user2, timestamp, initial + partialWithdraw))
60+
.tokens
61+
).toBe(CAPACITY - initial - partialWithdraw);
62+
// TODO: using setWindowClient first and update the capacity (not working)
63+
// TODO: might need to look into why is not taking second request...
64+
65+
const tokenCountPartial = await getWindowFromClient(client, user2);
66+
expect(tokenCountPartial.currentTokens).toBe(initial + partialWithdraw);
67+
});
68+
69+
// TODO: need to fix processRequest Fn()
70+
71+
xtest('window is partially full and request has no leftover tokens', async () => {
72+
const initial = 6;
73+
const partialWithdraw = 5;
74+
await setTokenCountInClient(client, user2, initial, timestamp);
75+
expect(
76+
(await limiter.processRequest(user2, timestamp, partialWithdraw)).success
77+
).toBe(false);
78+
const tokenCountPartialToEmpty = await getWindowFromClient(client, user2);
79+
expect(tokenCountPartialToEmpty.currentTokens).toBe(10);
80+
});
81+
});
82+
describe('after a BLOCKED request...', () => {
83+
afterEach(() => {
84+
client.flushall();
85+
});
86+
test('initial request is greater than capacity', async () => {
87+
// expect remaining tokens to be 10, b/c the 11 token request should be blocked
88+
expect((await limiter.processRequest(user1, timestamp, 11)).success).toBe(false);
89+
// expect current tokens in the window to still be 0
90+
expect((await getWindowFromClient(client, user1)).currentTokens).toBe(0);
91+
});
92+
xtest('window is partially full but not enough time elapsed to reach new window', async () => {
93+
const requestedTokens = 9;
94+
95+
await setTokenCountInClient(client, user2, requestedTokens, timestamp);
96+
// expect remaining tokens to be 1, b/c the 2-token-request should be blocked
97+
// TODO: fix processRequest function
98+
const result = await limiter.processRequest(user2, timestamp + WINDOW_SIZE - 1, 2);
99+
100+
expect(result.success).toBe(false);
101+
// TODO: fix this, (expect 1, but get 9 -- which is # of currentTokens)
102+
expect(result.tokens).toBe(CAPACITY - requestedTokens);
103+
104+
// expect current tokens in the window to still be 9
105+
expect((await getWindowFromClient(client, user2)).currentTokens).toBe(9);
106+
});
107+
});
108+
describe('updateTimeWindow function works as expect', () => {
109+
afterEach(() => {
110+
client.flushall();
111+
});
112+
xtest('New window is initialized after reaching the window size', async () => {
113+
const fullRequest = 10;
114+
await setTokenCountInClient(client, user3, fullRequest, timestamp);
115+
const noAccess = await limiter.processRequest(
116+
user3,
117+
timestamp + WINDOW_SIZE - 1,
118+
1
119+
);
120+
121+
// TODO: fix processRequest function
122+
// expect cannot pass any request
123+
expect(noAccess.success).toBe(false);
124+
125+
const newRequest = 3;
126+
await setTokenCountInClient(client, user3, newRequest, timestamp + WINDOW_SIZE + 1);
127+
const newAccess = await limiter.processRequest(user3, timestamp, 1);
128+
129+
expect(newAccess.success).toBe(true);
130+
expect(newAccess.tokens).toBe(4);
54131
});
55132
});
56133
});

0 commit comments

Comments
 (0)