From 44af41547ed1df437ca8183ecb680dae075fbfb1 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 23 Feb 2021 13:50:22 +0100 Subject: [PATCH] feat: make client features endpoint memoizable (#734) --- package.json | 1 + src/lib/options.js | 6 ++ src/lib/routes/client-api/feature.js | 35 +++++++++-- src/lib/routes/client-api/feature.test.js | 48 +++++++++++++++ src/lib/routes/client-api/index.js | 5 +- src/lib/routes/index.ts | 5 +- yarn.lock | 72 ++++++++++++++++++++++- 7 files changed, 157 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index de7eb888ea..bb8f04d9db 100644 --- a/package.json +++ b/package.json @@ -126,6 +126,7 @@ "prettier": "^1.19.1", "proxyquire": "^2.1.3", "source-map-support": "^0.5.19", + "sinon": "^9.2.4", "superagent": "^6.1.0", "supertest": "^5.0.0", "ts-node": "^9.1.1", diff --git a/src/lib/options.js b/src/lib/options.js index 12cd6c45aa..6666482be3 100644 --- a/src/lib/options.js +++ b/src/lib/options.js @@ -90,6 +90,12 @@ function defaultOptions() { version, secureHeaders: process.env.SECURE_HEADERS || false, enableOAS: process.env.ENABLE_OAS || false, + experimental: { + clientFeatureMemoize: { + enabled: process.env.CLIENT_FEATURE_MEMOIZE || false, + maxAge: process.env.CLIENT_FEATURE_MAXAGE || 1000, + }, + }, }; } diff --git a/src/lib/routes/client-api/feature.js b/src/lib/routes/client-api/feature.js index 4e97174625..369d229b61 100644 --- a/src/lib/routes/client-api/feature.js +++ b/src/lib/routes/client-api/feature.js @@ -2,6 +2,8 @@ import { handleErrors } from '../admin-api/util'; +const memoizee = require('memoizee'); + const Controller = require('../controller'); const version = 1; @@ -16,20 +18,43 @@ const FEATURE_COLUMNS_CLIENT = [ ]; class FeatureController extends Controller { - constructor({ featureToggleService }, getLogger) { + constructor({ featureToggleService }, { getLogger, experimental }) { super(); this.toggleService = featureToggleService; this.logger = getLogger('client-api/feature.js'); this.get('/', this.getAll); this.get('/:featureName', this.getFeatureToggle); + if (experimental && experimental.clientFeatureMemoize) { + this.cache = experimental.clientFeatureMemoize.enabled; + this.cachedFeatures = memoizee( + (query, fields) => + this.toggleService.getFeatures(query, fields), + { + promise: true, + maxAge: experimental.clientFeatureMemoize.maxAge, + normalizer(args) { + // args is arguments object as accessible in memoized function + return JSON.stringify(args[0]); + }, + }, + ); + } } async getAll(req, res) { try { - const features = await this.toggleService.getFeatures( - req.query, - FEATURE_COLUMNS_CLIENT, - ); + let features; + if (this.cache) { + features = await this.cachedFeatures( + req.query, + FEATURE_COLUMNS_CLIENT, + ); + } else { + features = await this.toggleService.getFeatures( + req.query, + FEATURE_COLUMNS_CLIENT, + ); + } res.json({ version, features }); } catch (e) { handleErrors(res, this.logger, e); diff --git a/src/lib/routes/client-api/feature.test.js b/src/lib/routes/client-api/feature.test.js index b3222f7caf..cc23d71203 100644 --- a/src/lib/routes/client-api/feature.test.js +++ b/src/lib/routes/client-api/feature.test.js @@ -3,10 +3,12 @@ const test = require('ava'); const supertest = require('supertest'); const { EventEmitter } = require('events'); +const sinon = require('sinon'); const store = require('../../../test/fixtures/store'); const getLogger = require('../../../test/fixtures/no-logger'); const getApp = require('../../app'); const { createServices } = require('../../services'); +const FeatureController = require('./feature'); const eventBus = new EventEmitter(); @@ -40,6 +42,52 @@ test('should get empty getFeatures via client', t => { }); }); +test('if caching is enabled should memoize', t => { + const getFeatures = sinon.fake.returns([]); + + const featureToggleService = { + getFeatures, + }; + const controller = new FeatureController( + { featureToggleService }, + { + getLogger, + experimental: { + clientFeatureMemoize: { + enabled: true, + maxAge: 10000, + }, + }, + }, + ); + controller.getAll({ query: {} }, { json: () => {} }); + controller.getAll({ query: {} }, { json: () => {} }); + t.is(getFeatures.callCount, 1); +}); + +test('if caching is not enabled all calls goes to service', t => { + const getFeatures = sinon.fake.returns([]); + + const featureToggleService = { + getFeatures, + }; + const controller = new FeatureController( + { featureToggleService }, + { + getLogger, + experimental: { + clientFeatureMemoize: { + enabled: false, + maxAge: 10000, + }, + }, + }, + ); + controller.getAll({ query: {} }, { json: () => {} }); + controller.getAll({ query: {} }, { json: () => {} }); + t.is(getFeatures.callCount, 2); +}); + test('fetch single feature', t => { t.plan(1); const { request, featureToggleStore, base } = getSetup(); diff --git a/src/lib/routes/client-api/index.js b/src/lib/routes/client-api/index.js index 388c876251..0a28350148 100644 --- a/src/lib/routes/client-api/index.js +++ b/src/lib/routes/client-api/index.js @@ -13,10 +13,7 @@ class ClientApi extends Controller { const { getLogger } = config; this.get('/', this.index); - this.use( - '/features', - new FeatureController(services, getLogger).router, - ); + this.use('/features', new FeatureController(services, config).router); this.use('/metrics', new MetricsController(services, getLogger).router); this.use( '/register', diff --git a/src/lib/routes/index.ts b/src/lib/routes/index.ts index f7ffd53b08..e29e5d7bf0 100644 --- a/src/lib/routes/index.ts +++ b/src/lib/routes/index.ts @@ -22,10 +22,7 @@ class IndexRouter extends Controller { // legacy support (remove in 4.x) if (config.enableLegacyRoutes) { - const featureController = new FeatureController( - services, - config.getLogger, - ); + const featureController = new FeatureController(services, config); this.use('/api/features', featureController.router); } } diff --git a/yarn.lock b/yarn.lock index 7b2364698e..cc3b55afb6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -461,6 +461,13 @@ resolved "https://registry.npmjs.org/@sindresorhus/is/-/is-0.14.0.tgz" integrity sha512-9NET910DNaIPngYnLLPeg+Ogzqsi9uM4mSboU5y6p8S5DzMTVEsJZrawi+BoDNUVBa2DhJqQYUFvMDfgU062LQ== +"@sinonjs/commons@^1.6.0", "@sinonjs/commons@^1.8.1": + version "1.8.2" + resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.8.2.tgz#858f5c4b48d80778fde4b9d541f27edc0d56488b" + integrity sha512-sruwd86RJHdsVf/AtBoijDmUqJp3B6hF/DGC23C+JaegnDHaZyewCjoVGTdg3J0uz3Zs7NnIT05OBOmML72lQw== + dependencies: + type-detect "4.0.8" + "@sinonjs/commons@^1.7.0": version "1.8.1" resolved "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.8.1.tgz" @@ -468,6 +475,27 @@ dependencies: type-detect "4.0.8" +"@sinonjs/fake-timers@^6.0.0", "@sinonjs/fake-timers@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz#293674fccb3262ac782c7aadfdeca86b10c75c40" + integrity sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA== + dependencies: + "@sinonjs/commons" "^1.7.0" + +"@sinonjs/samsam@^5.3.1": + version "5.3.1" + resolved "https://registry.yarnpkg.com/@sinonjs/samsam/-/samsam-5.3.1.tgz#375a45fe6ed4e92fca2fb920e007c48232a6507f" + integrity sha512-1Hc0b1TtyfBu8ixF/tpfSHTVWKwCBLY4QJbkgnE7HcwyvT2xArDxb4K7dMgqRm3szI+LJbzmW/s4xxEhv6hwDg== + dependencies: + "@sinonjs/commons" "^1.6.0" + lodash.get "^4.4.2" + type-detect "^4.0.8" + +"@sinonjs/text-encoding@^0.7.1": + version "0.7.1" + resolved "https://registry.yarnpkg.com/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz#8da5c6530915653f3a1f38fd5f101d8c3f8079c5" + integrity sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ== + "@szmarczak/http-timer@^1.1.2": version "1.1.2" resolved "https://registry.npmjs.org/@szmarczak/http-timer/-/http-timer-1.1.2.tgz" @@ -1914,7 +1942,7 @@ dicer@0.2.5: readable-stream "1.1.x" streamsearch "0.1.2" -diff@^4.0.1: +diff@^4.0.1, diff@^4.0.2: version "4.0.2" resolved "https://registry.yarnpkg.com/diff/-/diff-4.0.2.tgz#60f3aecb89d5fae520c11aa19efc2bb982aade7d" integrity sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A== @@ -3840,6 +3868,11 @@ jsprim@^1.2.2: json-schema "0.2.3" verror "1.10.0" +just-extend@^4.0.2: + version "4.1.1" + resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-4.1.1.tgz#158f1fdb01f128c411dc8b286a7b4837b3545282" + integrity sha512-aWgeGFW67BP3e5181Ep1Fv2v8z//iBJfrvyTnq8wG86vEESwmonn1zPBJ0VfmT9CJq2FIT0VsETtrNFm2a+SHA== + jwa@^1.4.1: version "1.4.1" resolved "https://registry.npmjs.org/jwa/-/jwa-1.4.1.tgz" @@ -4059,6 +4092,11 @@ lodash.flattendeep@^4.4.0: resolved "https://registry.npmjs.org/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz" integrity sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI= +lodash.get@^4.4.2: + version "4.4.2" + resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" + integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk= + lodash.isequal@^4.5.0: version "4.5.0" resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" @@ -4441,6 +4479,17 @@ nice-try@^1.0.4: resolved "https://registry.npmjs.org/nice-try/-/nice-try-1.0.5.tgz" integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ== +nise@^4.0.4: + version "4.1.0" + resolved "https://registry.yarnpkg.com/nise/-/nise-4.1.0.tgz#8fb75a26e90b99202fa1e63f448f58efbcdedaf6" + integrity sha512-eQMEmGN/8arp0xsvGoQ+B1qvSkR73B1nWSCh7nOt5neMCtwcQVYQGdzQMhcNscktTsWB54xnlSQFzOAPJD8nXA== + dependencies: + "@sinonjs/commons" "^1.7.0" + "@sinonjs/fake-timers" "^6.0.0" + "@sinonjs/text-encoding" "^0.7.1" + just-extend "^4.0.2" + path-to-regexp "^1.7.0" + node-cleanup@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/node-cleanup/-/node-cleanup-2.1.2.tgz#7ac19abd297e09a7f72a71545d951b517e4dde2c" @@ -4925,6 +4974,13 @@ path-to-regexp@0.1.7: resolved "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.7.tgz" integrity sha1-32BBeABfUi8V60SQ5yR6G/qmf4w= +path-to-regexp@^1.7.0: + version "1.8.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.8.0.tgz#887b3ba9d84393e87a0a0b9f4cb756198b53548a" + integrity sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA== + dependencies: + isarray "0.0.1" + path-to-regexp@^2.2.1: version "2.4.0" resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-2.4.0.tgz#35ce7f333d5616f1c1e1bfe266c3aba2e5b2e704" @@ -5714,6 +5770,18 @@ signal-exit@^3.0.2: resolved "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.3.tgz" integrity sha512-VUJ49FC8U1OxwZLxIbTTrDvLnf/6TDgxZcK8wxR8zs13xpx7xbG60ndBlhNrFi2EMuFRoeDoJO7wthSLq42EjA== +sinon@^9.2.4: + version "9.2.4" + resolved "https://registry.yarnpkg.com/sinon/-/sinon-9.2.4.tgz#e55af4d3b174a4443a8762fa8421c2976683752b" + integrity sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg== + dependencies: + "@sinonjs/commons" "^1.8.1" + "@sinonjs/fake-timers" "^6.0.1" + "@sinonjs/samsam" "^5.3.1" + diff "^4.0.2" + nise "^4.0.4" + supports-color "^7.1.0" + slash@^3.0.0: version "3.0.0" resolved "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz" @@ -6345,7 +6413,7 @@ type-check@~0.3.2: dependencies: prelude-ls "~1.1.2" -type-detect@4.0.8: +type-detect@4.0.8, type-detect@^4.0.8: version "4.0.8" resolved "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz" integrity sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==