Skip to content

Commit

Permalink
Ensure cached objects are cloned first (#389)
Browse files Browse the repository at this point in the history
* don't use resource from cache when retrieved, allow controlling cache ttl

* allow optional resources without error

* linting

* ensure cached objects are isolated from object modifications

* increase cache size, tidy
  • Loading branch information
carrolp authored Feb 21, 2023
1 parent 105e4ce commit 0fd5359
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions lib/FetchEnvs.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

const merge = require('deepmerge');
const clone = require('clone');
const log = require('./bunyan-api').createLogger('fetchEnvs');

const STRING = 'string';
Expand All @@ -29,7 +30,7 @@ const KIND_MAP = new Map([

const LRU = require('lru-cache');
const LruOptions = {
maxSize: parseInt(process.env.FETCHENVS_CACHE_SIZE) || 100000, // the max cache size
maxSize: parseInt(process.env.FETCHENVS_CACHE_SIZE) || 1000000, // the max cache size
sizeCalculation: (r) => { return( JSON.stringify(r).length ); }, // how to determine the size of a resource added to the cache
ttl: 1000 * 60 * 3, // max time to cache (LRU does not directly enforce, but maxSize will eventually push them out)
updateAgeOnGet: false, // Don't update ttl when an item is retrieved from cache
Expand Down Expand Up @@ -62,21 +63,20 @@ module.exports = class FetchEnvs {
this.resourceCache = {
has: (key) => {
const hit = globalResourceCache.has(`${user}/${key}`);
if( hit ) log.info( `FetchEnvs cache HIT: '${user}/${key}'` );
return hit;
},
set: (key, value) => {
log.info( `FetchEnvs cache MISS: '${user}/${key}'` );
// When setting a key, keep track of users to allow later deletion
globalResourceCacheUsers.add( user );
globalResourceCache.set(`${user}/${key}`, value);
globalResourceCache.set( `${user}/${key}`, clone(value) ); // Cache a deep copy of the value, the original may be modified after caching (modifications must not alter the cache)
log.info( `FetchEnvs cached '${user}/${key}'` );
},
get: (key) => {
if( globalResourceCache.has(`${user}/${key}`) ) {
log.info( `FetchEnvs cache HIT: '${user}/${key}'` );
}
return globalResourceCache.get(`${user}/${key}`);
return clone( globalResourceCache.get(`${user}/${key}`) ); // Return a deep copy of the value, the returned value may be modified (modifications must not alter the cache)
},
};
}
Expand Down Expand Up @@ -129,7 +129,7 @@ module.exports = class FetchEnvs {

/*
Single-resource queries are cacheable. If it's in the cache, use it.
If not in the cache, start an api call to populate the cache if needed, wait for it to finish, then use it from the cache.
If not in the cache, start an api call to populate the cache if needed, wait for it to finish, then use the result.
*/
async #getSingleResource( resource ) {
const { apiVersion, kind, namespace, name } = resource;
Expand All @@ -139,8 +139,9 @@ module.exports = class FetchEnvs {
if( this.resourceCache.has( cacheKey ) ) {
resource = this.resourceCache.get( cacheKey );
}
// Single-resource queries are cacheable. If not in the cache, start an api call to populate the cache if needed, wait for it to finish, then use it from the cache.
// If not in the cache...
else {
// If there isn't already an outstanding api call to populate the cache, start one
if( !singleResourceQueryCache[cacheKey] ) {
singleResourceQueryCache[cacheKey] = ( async () => {
try {
Expand All @@ -150,6 +151,8 @@ module.exports = class FetchEnvs {
if( resource ) {
this.resourceCache.set( cacheKey, resource ); // Cache this resource
}
// Resource may be optional, so undefined resource is not treated as an error.
return( resource );
}
}
finally {
Expand All @@ -158,9 +161,8 @@ module.exports = class FetchEnvs {
} )();
}

await singleResourceQueryCache[cacheKey];

resource = this.resourceCache.get( cacheKey );
// Wait for the outstanding api call to complete, use it's value
resource = await singleResourceQueryCache[cacheKey];
}

return resource;
Expand Down

0 comments on commit 0fd5359

Please sign in to comment.