From 6713f93f4de025b64e8b7b9670e84b07e27e7271 Mon Sep 17 00:00:00 2001 From: Varsha Adiga Date: Tue, 9 Apr 2024 15:15:12 +0530 Subject: [PATCH] adding cache expiry mechanisms and fixing PD-237465, PD-237445 (#139) * adding cache expiry mechanisms and fixing PD-237465, PD-237445 * 2.9.2 * resolving review comments * adding a debounce cache clean up function and and making changes to improve performance * updating unit test cases with deletion test case * resolving review comments --- lib/bvFetch/README.md | 51 +++++- lib/bvFetch/index.js | 308 ++++++++++++++++++++++++-------- package-lock.json | 2 +- package.json | 2 +- test/unit/bvFetch/index.spec.js | 79 ++++++-- 5 files changed, 353 insertions(+), 89 deletions(-) diff --git a/lib/bvFetch/README.md b/lib/bvFetch/README.md index 31245c1..4149cb6 100644 --- a/lib/bvFetch/README.md +++ b/lib/bvFetch/README.md @@ -7,23 +7,68 @@ The BvFetch module provides methods to cache duplicate API calls and interact wi ## BvFetch Parameters `shouldCache (Function):` A function that takes the API response JSON as input and returns a boolean indicating whether to cache the response or not. This allows you to implement custom logic based on the response content. If caching is desired, the function should return true; otherwise, false. - `cacheName (String):` Optional. Specifies the name of the cache to be used. If not provided, the default cache name 'bvCache' will be used. +`cacheLimit (Integer)`: Optional. Specifies the cache size limit for the cache storage. Its value should be in MB. Default value is 10 MB. ## bvFetchFunc Method Parameters `url (String):` The URL of the API endpoint to fetch data from. - `options (Object):` Optional request options such as headers, method, etc., as supported by the Fetch API. ## bvFetchFunc Return Value `Promise:` A promise that resolves to the API response. If the response is cached, it returns the cached response. Otherwise, it fetches data from the API endpoint, caches the response according to the caching logic, and returns the fetched response. -## flushCache Method Parameters +## generateCacheKey Method Parameters: +`url (String):` The URL of the API endpoint. +`options (Object):` Optional request options. +## generateCacheKey Return Value: +`string:` The generated cache key. + +## retrieveCachedUrls Method +Retrieves cached URLs from the cache storage associated with the provided cache name. +## retrieveCachedUrls Parameters This method takes no parameters. +## retrieveCachedUrls Return Value +`void:` This method does not return anything. + +## fetchDataAndCache Method +Fetches data from the specified URL, caches the response, and returns the response. +## Parameters +`url (String):` The URL from which to fetch data. +`options (Object):` Optional request options such as headers, method, etc., as supported by the +Fetch API. +`cacheKey (String):` + The cache key associated with the fetched data. +## Return Value +`Promise:` A promise that resolves to the fetched response. + +## fetchFromCache Method +Function to fetch data from cache. +## Parameters +`cacheKey (String):` The cache key to fetch data from the cache. +## Return Value +Promise: A Promise that resolves with a Response object if the data is found in cache, or null if the data is not cached or expired. +## cacheData Method +Caches the provided response with the specified cache key if it meets the criteria for caching. +## Parameters +`response (Response):` The response object to be cached. +`cacheKey (String):` The cache key associated with the response. +## Return Value +`void:` This method does not return anything. + + +## flushCache Method Parameters +This method takes no parameters. ## flushCache Return Value `Promise:` A promise indicating the completion of cache flush operation. +## manageCache Method +Manages the cache by deleting expired cache entries and maintaining the cache size limit. +## Parameters +This method takes no parameters. +## Return Value +`void:` This method does not return anything. + ## Usage with of `BvFetch`: diff --git a/lib/bvFetch/index.js b/lib/bvFetch/index.js index 6ead4ed..bcd17b3 100644 --- a/lib/bvFetch/index.js +++ b/lib/bvFetch/index.js @@ -6,10 +6,12 @@ const { fetch } = require('../polyfills/fetch') -module.exports = function BvFetch ({ shouldCache, cacheName }) { +module.exports = function BvFetch ({ shouldCache, cacheName, cacheLimit }) { this.shouldCache = shouldCache; this.cacheName = cacheName || 'bvCache'; + this.cacheLimit = cacheLimit * 1024 * 1024 || 10 * 1024 * 1024; this.fetchPromises = new Map(); + this.cachedUrls = new Set(); /** * Generates a unique cache key for the given URL and options. @@ -24,6 +26,128 @@ module.exports = function BvFetch ({ shouldCache, cacheName }) { return key; }; + /** + * Retrieves cached URLs from the cache storage associated with the provided cache name. + * @returns {void} + */ + + this.retrieveCachedUrls = () => { + // Open the Cache Storage + caches.open(this.cacheName).then(cache => { + // Get all cache keys + cache.keys().then(keys => { + keys.forEach(request => { + this.cachedUrls.add(request.url); + }); + }); + }); + + } + + //callretrieveCachedUrls function to set the cache URL set with the cached URLS + this.retrieveCachedUrls(); + + /** + * Fetches data from the specified URL, caches the response, and returns the response. + * @param {string} url - The URL from which to fetch data. + * @param {string} cacheKey - The cache key associated with the fetched data. + * @returns {Promise} A Promise that resolves with the fetched response. + * @throws {Error} Throws an error if there's any problem fetching the data. + */ + this.fetchDataAndCache = (url, options = {}, cacheKey) => { + return fetch(url,options) + .then((response) => { + // initiate caching of response and return the response + this.cacheData(response, cacheKey); + return response.clone(); + }) + .catch(function (error) { + throw new Error('Error fetching data: ' + error); + }); + } + + /** + * Caches the provided response with the specified cache key if it meets the criteria for caching. + * @param {Response} response - The response object to be cached. + * @param {string} cacheKey - The cache key associated with the response. + * @returns {void} + */ + + this.cacheData = (response, cacheKey) => { + const errJson = response.clone(); + let canBeCached = true; + // Check for error in response obj + errJson.json().then(json => { + if (typeof this.shouldCache === 'function') { + canBeCached = this.shouldCache(json); + } + }).then(() => { + if (canBeCached) { + const clonedResponse = response.clone() + const newHeaders = new Headers(); + clonedResponse.headers.forEach((value, key) => { + newHeaders.append(key, value); + }); + newHeaders.append('X-Bazaarvoice-Cached-Time', Date.now()) + // Get response text to calculate its size + clonedResponse.text().then(text => { + // Calculate size of response text in bytes + const sizeInBytes = new Blob([text]).size; + + // Append response size to headers + newHeaders.append('X-Bazaarvoice-Response-Size', sizeInBytes); + + // Create new Response object with modified headers + const newResponse = new Response(clonedResponse._bodyBlob || clonedResponse.body, { + status: clonedResponse.status, + statusText: clonedResponse.statusText, + headers: newHeaders + }); + // Cache the response + caches.open(this.cacheName).then(cache => { + cache.put(cacheKey, newResponse); + //add key to cachedUrls set + this.cachedUrls.add(cacheKey); + }); + }); + } + }) + } + + /** + * Function to fetch data from cache. + * @param {string} cacheKey - The cache key to fetch data from the cache. + * @returns {Promise} A Promise that resolves with a Response object if the data is found in cache, + * or null if the data is not cached or expired. + * @throws {Error} Throws an error if there's any problem fetching from cache. + */ + this.fetchFromCache = (cacheKey) => { + // Check if the URL is in the set of cached URLs + if (!this.cachedUrls.has(cacheKey)) { + return Promise.resolve(null); + } + + // Open the cache and try to match the URL + return caches.open(this.cacheName) + .then((cache) => { + return cache.match(cacheKey) + .then((cachedResponse) => { + + const cachedTime = cachedResponse.headers.get('X-Bazaarvoice-Cached-Time'); + const ttl = cachedResponse.headers.get('Cache-Control').match(/max-age=(\d+)/)[1]; + const currentTimestamp = Date.now(); + const cacheAge = (currentTimestamp - cachedTime) / 1000; + if (cacheAge < ttl) { + // Cached response found + return cachedResponse.clone(); + } + }) + }) + .catch((error) => { + throw new Error('Error fetching from cache: ' + error); + }); + } + /** * Fetches data from the API endpoint, caches responses, and handles caching logic. * @param {string} url - The URL of the API endpoint. @@ -32,84 +156,38 @@ module.exports = function BvFetch ({ shouldCache, cacheName }) { */ this.bvFetchFunc = (url, options = {}) => { - // get the key + const cacheKey = this.generateCacheKey(url, options); + // If an ongoing fetch promise exists for the URL, return it + if (this.fetchPromises.has(cacheKey)) { + return this.fetchPromises.get(cacheKey).then(res => res.clone()); + } - // check if its available in the cache - return caches.open(this.cacheName) - .then(currentCache => currentCache.match(cacheKey)) - .then(cachedResponse => { + // Check if response is available in cache + const newPromise = this.fetchFromCache(cacheKey) + .then((cachedResponse) => { + // If response found in cache, return it if (cachedResponse) { - const cachedTime = cachedResponse.headers.get('X-Cached-Time'); - const ttl = cachedResponse.headers.get('Cache-Control').match(/max-age=(\d+)/)[1]; - const currentTimestamp = Date.now(); - const cacheAge = (currentTimestamp - cachedTime) / 1000; - - if (cacheAge < ttl) { - // Cached response found - return cachedResponse.clone(); - } + return cachedResponse; } + // If response not found in cache, fetch from API and cache it + return this.fetchDataAndCache(url, options, cacheKey); + }); - // check if there is an ongoing promise - if (this.fetchPromises.has(cacheKey)) { - return this.fetchPromises.get(cacheKey).then(res => res.clone()); - } + // Store the ongoing fetch promise + this.fetchPromises.set(cacheKey, newPromise); - // Make a new call - const newPromise = fetch(url, options); - - // Push the newPromise to the fetchPromises Map - this.fetchPromises.set(cacheKey, newPromise); - - return newPromise - .then(response => { - const clonedResponse = response.clone(); - const errJson = clonedResponse.clone() - let canBeCached = true; - return errJson.json().then(json => { - if (typeof this.shouldCache === 'function') { - canBeCached = this.shouldCache(json); - } - return response - }).then(res => { - if (canBeCached) { - const newHeaders = new Headers(); - clonedResponse.headers.forEach((value, key) => { - newHeaders.append(key, value); - }); - newHeaders.append('X-Cached-Time', Date.now()); - - const newResponse = new Response(clonedResponse._bodyBlob, { - status: clonedResponse.status, - statusText: clonedResponse.statusText, - headers: newHeaders - }); - //Delete promise from promise map once its resolved - this.fetchPromises.delete(cacheKey); - - return caches.open(this.cacheName) - .then(currentCache => - currentCache.put(cacheKey, newResponse) - ) - .then(() => res); - } - else { - //Delete promise from promise map if error exists - this.fetchPromises.delete(cacheKey); - - return res - } + //initiate cache cleanUp + this.debounceCleanupExpiredCache(); + + // When fetch completes or fails, remove the promise from the store + newPromise.finally(() => { + this.fetchPromises.delete(cacheKey); + }); + + return newPromise.then(res => res.clone()); + } - }); - }) - }) - .catch(err => { - // Remove the promise that was pushed earlier - this.fetchPromises.delete(cacheKey); - throw err; - }); - }; /** * Clears all cache entries stored in the cache storage. @@ -124,5 +202,89 @@ module.exports = function BvFetch ({ shouldCache, cacheName }) { }); }); }; + + this.manageCache = () => { + // Delete expired cache entries + caches.open(this.cacheName).then(cache => { + cache.keys().then(keys => { + keys.forEach(key => { + cache.match(key).then(response => { + const cachedTime = response.headers.get('X-Bazaarvoice-Cached-Time'); + const ttl = response.headers.get('Cache-Control').match(/max-age=(\d+)/)[1]; + const currentTimestamp = Date.now(); + const cacheAge = (currentTimestamp - cachedTime) / 1000; + if (cacheAge >= ttl) { + cache.delete(key); + this.cachedUrls.delete(key); + } + }); + }); + }); + }); + + // Calculate total size of cached responses + let totalSize = 0; + caches.open(this.cacheName).then(cache => { + cache.keys().then(keys => { + // Create an array of promises for cache match operations + const matchPromises = keys.map(key => + cache.match(key).then(response => { + const sizeHeader = response.headers.get('X-Bazaarvoice-Response-Size'); + return parseInt(sizeHeader, 10); + }) + ); + + // wait for all match promises to resolve + return Promise.all(matchPromises) + .then(sizes => sizes.reduce((acc, size) => acc + size, 0)); + }).then(size => { + totalSize = size; + // If total size exceeds 10 MB, delete old cache entries + if (totalSize > this.cacheLimit) { + + // create an array of cached responses + const cacheEntries = []; + return cache.keys().then(keys => { + const cachesResEntries = keys.map(key => + cache.match(key).then(response => { + const sizeHeader = response.headers.get('X-Bazaarvoice-Response-Size'); + const lastAccessedTime = response.headers.get('X-Bazaarvoice-Cached-Time'); + cacheEntries.push({ key, size: parseInt(sizeHeader, 10), lastAccessedTime }); + }) + ); + + return Promise.all(cachesResEntries) + .then(() => { + // Sort cache entries by last accessed time in ascending order + cacheEntries.sort((a, b) => a.lastAccessedTime - b.lastAccessedTime); + + // Delete older cache entries until total size is under 10 MB + let currentSize = totalSize; + cacheEntries.forEach(entry => { + if (currentSize > this.cacheLimit) { + cache.delete(entry.key); + this.cachedUrls.delete(entry.key); + currentSize -= entry.size; + } + }); + }); + }); + } + }); + }); + }; + + + function debounce (func, delay) { + let timer; + return function () { + clearTimeout(timer); + timer = setTimeout(() => { + func.apply(this, arguments); + }, delay); + }; + } + + this.debounceCleanupExpiredCache = debounce(this.manageCache, 8000); -} +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 3e942be..51c76cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "bv-ui-core", - "version": "2.9.1", + "version": "2.9.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index d0e4cc2..5465847 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bv-ui-core", - "version": "2.9.1", + "version": "2.9.2", "license": "Apache 2.0", "description": "Bazaarvoice UI-related JavaScript", "repository": { diff --git a/test/unit/bvFetch/index.spec.js b/test/unit/bvFetch/index.spec.js index f412e08..931db40 100644 --- a/test/unit/bvFetch/index.spec.js +++ b/test/unit/bvFetch/index.spec.js @@ -65,7 +65,7 @@ describe('BvFetch', function () { caches.open.resolves({ match: (key) => { expect(key).to.equal(cacheKey); - Promise.resolve(mockResponse) + return Promise.resolve(mockResponse) }, put: (key, response) => { cacheStorage.set(key, response); @@ -73,6 +73,10 @@ describe('BvFetch', function () { } }); + // Simulate that the response is cached + bvFetchInstance.cachedUrls.add(cacheKey); + + // Call the function under test bvFetchInstance.bvFetchFunc(url, options) .then(response => { // Check if response is fetched from cache @@ -91,19 +95,18 @@ describe('BvFetch', function () { done(error); // Call done with error if any }) }); - + it('should fetch from network when response is not cached', function (done) { const url = 'https://jsonplaceholder.typicode.com/todos'; const options = {}; - const cacheKey = bvFetchInstance.generateCacheKey(url, options); - + const matchSpy = sinon.spy((key) => { + expect(key).to.equal(cacheKey); + Promise.resolve(null) + }); caches.open.resolves({ - match: (key) => { - expect(key).to.equal(cacheKey); - Promise.resolve(null) - }, + match: matchSpy, put: (key, response) => { cacheStorage.set(key, response); return Promise.resolve(); @@ -118,7 +121,7 @@ describe('BvFetch', function () { console.log(response.body) // Check if caches.match was called - expect(cacheStub.called).to.be.true; + expect(matchSpy.called).to.be.false; done(); }) @@ -133,7 +136,7 @@ describe('BvFetch', function () { bvFetchInstance.shouldCache = (res) => { return false }; - + bvFetchInstance.bvFetchFunc(url, options) .then(response => { // Check if response is fetched from network @@ -141,7 +144,7 @@ describe('BvFetch', function () { console.log(response.body) // Check if caches.match was called - expect(cacheStub.calledOnce).to.be.true; + expect(cacheStub.calledOnce).to.be.false; // Check if response is not cached const cachedResponse = cacheStorage.get(url); @@ -152,5 +155,59 @@ describe('BvFetch', function () { .catch(done); }); + it('should delete cache when size is greater than 10 MB', function (done) { + // Mock cache entries exceeding 10 MB + const mockCacheEntries = [ + { key: 'key1', size: 6000000 }, // 6 MB + { key: 'key2', size: 6000000 } // 6 MB + // Add more entries as needed to exceed 10 MB + ]; + + // Stub cache operations + const deleteSpy = sinon.spy( + (key) => { + const index = mockCacheEntries.findIndex(entry => entry.key === key); + if (index !== -1) { + mockCacheEntries.splice(index, 1); // Delete entry from mock cache entries + } + return Promise.resolve(true); + } + ) + caches.open.resolves({ + keys: () => Promise.resolve(mockCacheEntries.map(entry => entry.key)), + match: (key) => { + const entry = mockCacheEntries.find(entry => entry.key === key); + if (entry) { + return Promise.resolve({ + headers: new Headers({ + 'X-Bazaarvoice-Response-Size': entry.size.toString(), + 'X-Bazaarvoice-Cached-Time': Date.now(), + 'Cache-Control': 'max-age=3600' + }) + }); + } + else { + return Promise.resolve(null); + } + }, + delete: deleteSpy + }); + + // Create a new instance of BvFetch + const bvFetchInstance = new BvFetch({ shouldCache: true }); + + // Call manageCache function + bvFetchInstance.manageCache() + setTimeout(() => { + // Ensure cache deletion occurred until the total size is under 10 MB + const totalSizeAfterDeletion = mockCacheEntries.reduce((acc, entry) => acc + entry.size, 0); + expect(totalSizeAfterDeletion).to.be.at.most(10 * 1024 * 1024); // Total size should be under 10 MB + // Ensure cache.delete was called for each deleted entry + expect(deleteSpy.called).to.be.true; + expect(deleteSpy.callCount).to.equal(mockCacheEntries.length); + done(); + }, 500); + + }); });