From 5996dacd3950258de6f9eddecd2bfcb43ea6dd31 Mon Sep 17 00:00:00 2001 From: Richard Lindner Date: Mon, 30 Sep 2024 10:23:14 +0200 Subject: [PATCH] Fb/redis read local mode (#75) * better defaulting for constructor/reset * jsdoc for constructor * some more jsdoc * make redisRead local mode work similar to state --- src/featureToggles.js | 105 +++++++++++------ .../featureToggles.service.test.js.snap | 107 +++++++++++++++++- .../featureToggles.service.test.js | 99 +++++++++------- test/uniqueName.test.js | 4 +- 4 files changed, 241 insertions(+), 74 deletions(-) diff --git a/src/featureToggles.js b/src/featureToggles.js index dba0f06..2dbd3d2 100644 --- a/src/featureToggles.js +++ b/src/featureToggles.js @@ -119,6 +119,30 @@ class FeatureToggles { // START OF CONSTRUCTOR SECTION // ======================================== + static _getDefaultUniqueName() { + if (ENV_UNIQUE_NAME) { + return ENV_UNIQUE_NAME; + } + let cfApp; + try { + cfApp = cfEnv.cfApp; + if (cfApp.application_name) { + return cfApp.application_name; + } + } catch (err) { + throw new VError( + { + name: VERROR_CLUSTER_NAME, + cause: err, + info: { + cfApp: JSON.stringify(cfApp), + }, + }, + "error determining cf app name" + ); + } + } + _processValidations(featureKey, validations, configFilepath) { const configDir = configFilepath ? pathlib.dirname(configFilepath) : process.cwd(); @@ -274,7 +298,16 @@ class FeatureToggles { ); } - _reset({ uniqueName, redisChannel = DEFAULT_REDIS_CHANNEL, redisKey = DEFAULT_REDIS_KEY }) { + /** + * Implementation for {@link constructor}. + * + * @param {ConstructorOptions} [options] + */ + _reset({ + uniqueName = FeatureToggles._getDefaultUniqueName(), + redisChannel = DEFAULT_REDIS_CHANNEL, + redisKey = DEFAULT_REDIS_KEY, + } = {}) { this.__redisChannel = uniqueName ? redisChannel + "-" + uniqueName : redisChannel; this.__redisKey = uniqueName ? redisKey + "-" + uniqueName : redisKey; @@ -292,9 +325,19 @@ class FeatureToggles { this.__isConfigProcessed = false; } - // NOTE: constructors cannot be async, so we need to split this state preparation part from the initialize part - constructor({ uniqueName = undefined, redisChannel = DEFAULT_REDIS_CHANNEL, redisKey = DEFAULT_REDIS_KEY } = {}) { - this._reset({ uniqueName, redisChannel, redisKey }); + /** + * @typedef ConstructorOptions + * @type object + * @property {string} [uniqueName] unique name to prefix both Redis channel and key + * @property {string} [redisChannel] channel for Redis pub/sub to propagate changes across servers + * @property {string} [redisKey] key in Redis to save non-fallback values + */ + /** + * NOTE: constructors cannot be async, so we need to split this state preparation part from the initialize part + * @param {ConstructorOptions} [options] + */ + constructor(options) { + this._reset(options); } // ======================================== @@ -304,30 +347,6 @@ class FeatureToggles { // START OF SINGLETON SECTION // ======================================== - static _getInstanceUniqueName() { - if (ENV_UNIQUE_NAME) { - return ENV_UNIQUE_NAME; - } - let cfApp; - try { - cfApp = cfEnv.cfApp; - if (cfApp.application_name) { - return cfApp.application_name; - } - } catch (err) { - throw new VError( - { - name: VERROR_CLUSTER_NAME, - cause: err, - info: { - cfApp: JSON.stringify(cfApp), - }, - }, - "error determining cf app name" - ); - } - } - /** * Get singleton instance * @@ -335,8 +354,7 @@ class FeatureToggles { */ static getInstance() { if (!FeatureToggles.__instance) { - const uniqueName = FeatureToggles._getInstanceUniqueName(); - FeatureToggles.__instance = new FeatureToggles({ uniqueName }); + FeatureToggles.__instance = new FeatureToggles(); } return FeatureToggles.__instance; } @@ -713,6 +731,11 @@ class FeatureToggles { ); } + /** + * Implementation for {@link initializeFeatures}. + * + * @param {InitializeOptions} [options] + */ async _initializeFeatures({ config: configRuntime, configFile: configFilepath, configAuto } = {}) { if (this.__isInitialized) { return; @@ -840,9 +863,23 @@ class FeatureToggles { return this; } + /** + * TODO + * @typedef Config + * @type object + */ + /** + * @typedef InitializeOptions + * @type object + * @property {Config} [config] + * @property {string} [configFile] + * @property {Config} [configAuto] + */ /** * Initialize needs to run and finish before other APIs are called. It processes the configuration, sets up * related internal state, and starts communication with redis. + * + * @param {InitializeOptions} [options] */ async initializeFeatures(options) { if (!this.__initializePromise) { @@ -919,11 +956,15 @@ class FeatureToggles { */ async getRemoteFeaturesInfos() { this._ensureInitialized(); + + let remoteStateScopedValues; + // NOTE: for NO_REDIS mode, we show local updates if ((await redis.getIntegrationMode()) === REDIS_INTEGRATION_MODE.NO_REDIS) { - return {}; + remoteStateScopedValues = this.__stateScopedValues ?? {}; + } else { + remoteStateScopedValues = await redis.hashGetAllObjects(this.__redisKey); } - const remoteStateScopedValues = await redis.hashGetAllObjects(this.__redisKey); if (!remoteStateScopedValues) { return null; } diff --git a/test/cds-service/__snapshots__/featureToggles.service.test.js.snap b/test/cds-service/__snapshots__/featureToggles.service.test.js.snap index b3d7a5c..5a8bf1b 100644 --- a/test/cds-service/__snapshots__/featureToggles.service.test.js.snap +++ b/test/cds-service/__snapshots__/featureToggles.service.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`cds-service endpoints state response 1`] = ` +exports[`cds-service endpoints state response no change 1`] = ` { "test/feature_a": { "config": { @@ -99,3 +99,108 @@ exports[`cds-service endpoints state response 1`] = ` }, } `; + +exports[`cds-service endpoints state response with changes 1`] = ` +{ + "test/feature_a": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "boolean", + }, + "fallbackValue": false, + }, + "test/feature_aa": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "boolean", + "VALIDATIONS": [ + { + "scopes": [ + "tenant", + "user", + ], + }, + ], + }, + "fallbackValue": false, + }, + "test/feature_b": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "number", + }, + "fallbackValue": 1, + "rootValue": 2, + "scopedValues": { + "tenant::a": 20, + "tenant::b": 30, + }, + }, + "test/feature_c": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "string", + }, + "fallbackValue": "best", + }, + "test/feature_d": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "boolean", + "VALIDATIONS": [ + { + "regex": "^(?:true)$", + }, + ], + }, + "fallbackValue": true, + }, + "test/feature_e": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "number", + "VALIDATIONS": [ + { + "scopes": [ + "component", + "layer", + "tenant", + ], + }, + { + "regex": "^\\d{1}$", + }, + ], + }, + "fallbackValue": 5, + }, + "test/feature_f": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "string", + "VALIDATIONS": [ + { + "regex": "^(?:best|worst)$", + }, + ], + }, + "fallbackValue": "best", + }, + "test/feature_g": { + "config": { + "ACTIVE": false, + "SOURCE": "RUNTIME", + "TYPE": "string", + }, + "fallbackValue": "activeTest", + }, + "test/feature_h": { + "config": { + "APP_URL": "\\.cfapps\\.sap\\.hana\\.ondemand\\.com$", + "SOURCE": "RUNTIME", + "TYPE": "string", + }, + "fallbackValue": "appUrlTest", + }, +} +`; diff --git a/test/cds-service/featureToggles.service.test.js b/test/cds-service/featureToggles.service.test.js index ce707ee..9277cf7 100644 --- a/test/cds-service/featureToggles.service.test.js +++ b/test/cds-service/featureToggles.service.test.js @@ -6,9 +6,32 @@ const toggles = require("../../src"); const { FEATURE, mockConfig: config } = require("../__common__/mockdata"); const server = cds.test("test/cds-test-project"); +const systemCall = { validateStatus: () => true, auth: { username: "system", password: "system" } }; describe("cds-service", () => { - beforeAll(async () => { + const featureBChanges = [ + { + key: FEATURE.B, + value: 2, + }, + { + key: FEATURE.B, + value: 20, + scope: { + tenant: "a", + }, + }, + { + key: FEATURE.B, + value: 30, + scope: { + tenant: "b", + }, + }, + ]; + + beforeEach(async () => { + toggles._reset(); await toggles.initializeFeatures({ config }); }); afterEach(() => { @@ -16,49 +39,52 @@ describe("cds-service", () => { }); describe("endpoints", () => { - test.each(["/rest/feature/state"])("%s GET is unauthorized", async (endpoint) => { - const response = await server.get(endpoint, { validateStatus: () => true }); - expect(response.status).toBe(401); - }); - - test.each(["/rest/feature/redisRead", "/rest/feature/redisUpdate", "/rest/feature/redisSendCommand"])( - "%s POST is unauthorized", - async (endpoint) => { - const response = await server.post(endpoint, {}, { validateStatus: () => true }); - expect(response.status).toBe(401); - } - ); - test.each(["/rest/feature/redisSendCommand"])("%s is forbidden for system", async (endpoint) => { - const response = await server.post( - endpoint, - {}, - { - validateStatus: () => true, - auth: { username: "system", password: "system" }, - } - ); + const response = await server.post(endpoint, {}, systemCall); expect(response.status).toBe(403); }); - test("state response", async () => { - const response = await server.get("/rest/feature/state", { - auth: { username: "system", password: "system" }, - }); + test("state response no change", async () => { + const response = await server.get("/rest/feature/state", systemCall); expect(response.status).toBe(200); expect(response.data).toMatchSnapshot(); }); - test("redisRead response", async () => { - const response = await server.post( - "/rest/feature/redisRead", - {}, - { auth: { username: "system", password: "system" } } - ); + test("state response with changes", async () => { + await server.post("/rest/feature/redisUpdate", featureBChanges, systemCall); + const response = await server.get("/rest/feature/state", systemCall); + expect(response.status).toBe(200); + expect(response.data).toMatchSnapshot(); + }); + + test("redisRead response no change", async () => { + const response = await server.post("/rest/feature/redisRead", {}, systemCall); expect(response.status).toBe(200); expect(response.data).toMatchInlineSnapshot(`{}`); }); + test("redisRead response with changes", async () => { + await server.post("/rest/feature/redisUpdate", featureBChanges, systemCall); + const response = await server.post("/rest/feature/redisRead", {}, systemCall); + expect(response.status).toBe(200); + expect(response.data).toMatchInlineSnapshot(` + { + "test/feature_b": { + "config": { + "SOURCE": "RUNTIME", + "TYPE": "number", + }, + "fallbackValue": 1, + "rootValue": 2, + "scopedValues": { + "tenant::a": 20, + "tenant::b": 30, + }, + }, + } + `); + }); + test("redisUpdate response success", async () => { const response = await server.post( "/rest/feature/redisUpdate", @@ -66,9 +92,7 @@ describe("cds-service", () => { key: FEATURE.A, value: true, }, - { - auth: { username: "system", password: "system" }, - } + systemCall ); expect(response.status).toBe(204); expect(response.data).toMatchInlineSnapshot(`""`); @@ -81,10 +105,7 @@ describe("cds-service", () => { key: FEATURE.A, value: 100, }, - { - validateStatus: () => true, - auth: { username: "system", password: "system" }, - } + systemCall ); expect(response.status).toBe(422); expect(response.data).toMatchInlineSnapshot(` diff --git a/test/uniqueName.test.js b/test/uniqueName.test.js index 286f692..f5167a8 100644 --- a/test/uniqueName.test.js +++ b/test/uniqueName.test.js @@ -12,7 +12,7 @@ describe("singleton", () => { const toggles = require("../src/"); const FeatureToggles = toggles.constructor; - expect(FeatureToggles._getInstanceUniqueName()).toMatchInlineSnapshot(`"test_app_name"`); + expect(FeatureToggles._getDefaultUniqueName()).toMatchInlineSnapshot(`"test_app_name"`); Reflect.deleteProperty(process.env, "VCAP_APPLICATION"); }); @@ -23,7 +23,7 @@ describe("singleton", () => { const toggles = require("../src/"); const FeatureToggles = toggles.constructor; - expect(FeatureToggles._getInstanceUniqueName()).toMatchInlineSnapshot(`"test_unique_name"`); + expect(FeatureToggles._getDefaultUniqueName()).toMatchInlineSnapshot(`"test_unique_name"`); Reflect.deleteProperty(process.env, "VCAP_APPLICATION"); Reflect.deleteProperty(process.env, ENV.UNIQUE_NAME);