Skip to content

Commit 0b485b4

Browse files
tmacrotmacro
committed
impr(CLDSRV-852): Refator cache helpers to allow for namespaced keys
1 parent 2b47a81 commit 0b485b4

4 files changed

Lines changed: 64 additions & 56 deletions

File tree

‎lib/api/apiUtils/rateLimit/cache.js‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,66 @@
11
const configCache = new Map();
22

3-
// Load tracking for adaptive burst capacity
4-
// Map<bucketKey, Array<timestamp>> - rolling 1-second window
5-
const requestTimestamps = new Map();
3+
const namespace = {
4+
bucket: 'bkt',
5+
};
66

7-
function setCachedConfig(key, limitConfig, ttl) {
7+
function cacheSet(cache, key, value, ttl) {
88
const expiry = Date.now() + ttl;
9-
configCache.set(key, { expiry, config: limitConfig });
9+
cache.set(key, { expiry, value });
1010
}
1111

12-
function getCachedConfig(key) {
13-
const value = configCache.get(key);
14-
if (value === undefined) {
12+
function cacheGet(cache, key) {
13+
const cachedValue = cache.get(key);
14+
if (cachedValue === undefined) {
1515
return undefined;
1616
}
1717

18-
const { expiry, config } = value;
18+
const { expiry, value } = cachedValue;
1919
if (expiry <= Date.now()) {
20-
configCache.delete(key);
20+
cache.delete(key);
2121
return undefined;
2222
}
2323

24-
return config;
24+
return value;
25+
}
26+
27+
function cacheDelete(cache, key) {
28+
cache.delete(key);
2529
}
2630

27-
function expireCachedConfigs(now) {
31+
function cacheExpire(cache) {
32+
const now = Date.now();
33+
2834
const toRemove = [];
29-
for (const [key, { expiry }] of configCache.entries()) {
35+
for (const [key, { expiry }] of cache.entries()) {
3036
if (expiry <= now) {
3137
toRemove.push(key);
3238
}
3339
}
3440

3541
for (const key of toRemove) {
36-
configCache.delete(key);
42+
cache.delete(key);
3743
}
3844

3945
return toRemove.length;
4046
}
4147

42-
/**
43-
* Invalidate cached config for a specific bucket
44-
*
45-
* @param {string} bucketName - Bucket name
46-
* @returns {boolean} True if entry was found and removed
47-
*/
48-
function invalidateCachedConfig(bucketName) {
49-
const cacheKey = `bucket:${bucketName}`;
50-
return configCache.delete(cacheKey);
48+
function formatKeyDecorator(fn) {
49+
return (resourceClass, resourceId, ...args) => fn(`${resourceClass}:${resourceId}`, ...args);
5150
}
5251

52+
const getCachedConfig = formatKeyDecorator(cacheGet.bind(null, configCache));
53+
const setCachedConfig = formatKeyDecorator(cacheSet.bind(null, configCache));
54+
const deleteCachedConfig = formatKeyDecorator(cacheDelete.bind(null, configCache));
55+
const expireCachedConfigs = cacheExpire.bind(null, configCache);
56+
5357
module.exports = {
58+
namespace,
5459
setCachedConfig,
5560
getCachedConfig,
5661
expireCachedConfigs,
57-
invalidateCachedConfig,
58-
62+
deleteCachedConfig,
5963
// Do not access directly
6064
// Used only for tests
6165
configCache,
62-
requestTimestamps,
6366
};

‎lib/api/bucketDeleteRateLimit.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const metadata = require('../metadata/wrapper');
44
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
55
const collectCorsHeaders = require('../utilities/collectCorsHeaders');
66
const { isRateLimitServiceUser } = require('./apiUtils/authorization/serviceUser');
7-
const { invalidateCachedConfig } = require('./apiUtils/rateLimit/cache');
7+
const cache = require('./apiUtils/rateLimit/cache');
88
const { removeTokenBucket } = require('./apiUtils/rateLimit/tokenBucket');
99

1010
/**
@@ -52,7 +52,7 @@ function bucketDeleteRateLimit(authInfo, request, log, callback) {
5252
return callback(err, corsHeaders);
5353
}
5454
// Invalidate cache and remove token bucket
55-
invalidateCachedConfig(bucketName);
55+
cache.deleteCachedConfig(cache.namespace.bucket, bucketName);
5656
removeTokenBucket(bucketName);
5757
log.debug('invalidated rate limit cache and token bucket for bucket', { bucketName });
5858
// TODO: implement Utapi metric support

‎lib/api/bucketPutRateLimit.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const { config } = require('../Config');
66
const metadata = require('../metadata/wrapper');
77
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
88
const { isRateLimitServiceUser } = require('./apiUtils/authorization/serviceUser');
9-
const { invalidateCachedConfig } = require('./apiUtils/rateLimit/cache');
9+
const cache = require('./apiUtils/rateLimit/cache');
1010

1111
function parseRequestBody(requestBody, callback) {
1212
try {
@@ -94,7 +94,7 @@ function bucketPutRateLimit(authInfo, request, log, callback) {
9494
return callback(err, corsHeaders);
9595
}
9696
// Invalidate cache so new limit takes effect immediately
97-
invalidateCachedConfig(bucketName);
97+
cache.deleteCachedConfig(cache.namespace.bucket, bucketName);
9898
log.debug('invalidated rate limit cache for bucket', { bucketName });
9999
// TODO: implement Utapi metric support
100100
return callback(null, corsHeaders);

‎tests/unit/api/apiUtils/rateLimit/cache.js‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ const sinon = require('sinon');
33

44
const constants = require('../../../../../constants');
55
const {
6+
namespace,
67
configCache,
78
getCachedConfig,
89
setCachedConfig,
910
expireCachedConfigs,
10-
invalidateCachedConfig,
11+
deleteCachedConfig,
1112
} = require('../../../../../lib/api/apiUtils/rateLimit/cache');
1213

1314
describe('test limit config cache storage', () => {
@@ -22,66 +23,70 @@ describe('test limit config cache storage', () => {
2223
clock.restore();
2324
});
2425

26+
beforeEach(() => {
27+
configCache.clear();
28+
});
29+
2530
it('should add config to cache', () => {
26-
setCachedConfig('foo', 10, constants.rateLimitDefaultConfigCacheTTL);
31+
setCachedConfig(namespace.bucket, 'foo', 10, constants.rateLimitDefaultConfigCacheTTL);
2732
assert.deepStrictEqual(
28-
configCache.get('foo'),
33+
configCache.get(`${namespace.bucket}:foo`),
2934
{
3035
expiry: now + constants.rateLimitDefaultConfigCacheTTL,
31-
config: 10,
36+
value: 10,
3237
}
3338
);
3439
});
3540

3641
it('should get a non expired config', () => {
37-
setCachedConfig('foo', 10, constants.rateLimitDefaultConfigCacheTTL);
38-
assert.strictEqual(getCachedConfig('foo'), 10);
42+
setCachedConfig(namespace.bucket, 'foo', 10, constants.rateLimitDefaultConfigCacheTTL);
43+
assert.strictEqual(getCachedConfig(namespace.bucket, 'foo'), 10);
3944
});
4045

4146
it('should return undefined and delete the key for an expired config', () => {
42-
configCache.set('foo', {
47+
configCache.set(`${namespace.bucket}:foo`, {
4348
expiry: now - 10000,
44-
config: 10,
49+
value: 10,
4550
});
46-
assert.strictEqual(getCachedConfig('foo'), undefined);
51+
assert.strictEqual(getCachedConfig(namespace.bucket, 'foo'), undefined);
4752
});
4853

49-
it('should expire configs less than or equal to the given timestamp', () => {
54+
it('should expire configs less than or equal to current time', () => {
5055
configCache.set('past', {
5156
expiry: now - 10000,
52-
config: 10,
57+
value: 10,
5358
});
5459
configCache.set('present', {
5560
expiry: now,
56-
config: 10,
61+
value: 10,
5762
});
5863
configCache.set('future', {
5964
expiry: now + 10000,
60-
config: 10,
65+
value: 10,
6166
});
62-
expireCachedConfigs(now);
67+
// expireCachedConfigs uses Date.now() internally; fake clock is set to `now`
68+
expireCachedConfigs();
6369
assert.strictEqual(configCache.get('past'), undefined);
6470
assert.strictEqual(configCache.get('present'), undefined);
6571
assert.deepStrictEqual(configCache.get('future'), {
6672
expiry: now + 10000,
67-
config: 10,
73+
value: 10,
6874
});
6975
});
7076

71-
it('should invalidate cached config for a specific bucket', () => {
72-
setCachedConfig('bucket:my-bucket', { limit: 100 }, constants.rateLimitDefaultConfigCacheTTL);
73-
setCachedConfig('bucket:other-bucket', { limit: 200 }, constants.rateLimitDefaultConfigCacheTTL);
77+
it('should delete cached config for a specific resource', () => {
78+
setCachedConfig(namespace.bucket, 'my-bucket', { limit: 100 }, constants.rateLimitDefaultConfigCacheTTL);
79+
setCachedConfig(namespace.bucket, 'other-bucket', { limit: 200 }, constants.rateLimitDefaultConfigCacheTTL);
7480

75-
const result = invalidateCachedConfig('my-bucket');
81+
deleteCachedConfig(namespace.bucket, 'my-bucket');
7682

77-
assert.strictEqual(result, true);
78-
assert.strictEqual(getCachedConfig('bucket:my-bucket'), undefined);
79-
assert.deepStrictEqual(getCachedConfig('bucket:other-bucket'), { limit: 200 });
83+
assert.strictEqual(getCachedConfig(namespace.bucket, 'my-bucket'), undefined);
84+
assert.deepStrictEqual(getCachedConfig(namespace.bucket, 'other-bucket'), { limit: 200 });
8085
});
8186

82-
it('should return false when invalidating non-existent bucket', () => {
83-
const result = invalidateCachedConfig('non-existent-bucket');
84-
85-
assert.strictEqual(result, false);
87+
it('should be a no-op when deleting a non-existent key', () => {
88+
assert.doesNotThrow(() => {
89+
deleteCachedConfig(namespace.bucket, 'non-existent-bucket');
90+
});
8691
});
8792
});

0 commit comments

Comments
 (0)