Skip to content

Commit 6ee1692

Browse files
rlindner81rlindner81
authored
Fb/allowed scopes (#11)
* better wording in jsdocs * allowed scoping feature TODO * allowed scopes impl 1 * allowed scopes impl 2 * allowed scopes impl 3 * first test * second test * finished todo * changes from new feature_AA * fix tests
1 parent 7e39b46 commit 6ee1692

File tree

7 files changed

+157
-18
lines changed

7 files changed

+157
-18
lines changed

‎example-cap-server/srv/feature/features.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
type: number
55
fallbackValue: 0
66
validation: '^\d+$'
7+
allowedScopes: [user, tenant]
78

89
# info: memory statistics logging interval (in milliseconds); 0 means disabled
910
/memory/logInterval:

‎src/env.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ class CfEnv {
4747
}, {});
4848
}
4949

50+
static getInstance() {
51+
if (!CfEnv.__instance) {
52+
CfEnv.__instance = new CfEnv();
53+
}
54+
return CfEnv.__instance;
55+
}
56+
5057
cfApp() {
5158
return this.__cfApp;
5259
}
@@ -71,5 +78,5 @@ class CfEnv {
7178
module.exports = {
7279
isLocal,
7380
isOnCF,
74-
cfEnv: new CfEnv(),
81+
cfEnv: CfEnv.getInstance(),
7582
};

‎src/featureToggles.js‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const CONFIG_KEY = Object.freeze({
4747
VALIDATION_REG_EXP: "VALIDATION_REG_EXP",
4848
APP_URL: "APP_URL",
4949
APP_URL_ACTIVE: "APP_URL_ACTIVE",
50+
ALLOWED_SCOPES: "ALLOWED_SCOPES",
51+
ALLOWED_SCOPES_CHECK_MAP: "ALLOWED_SCOPES_CHECK_MAP",
5052
});
5153

5254
const CONFIG_INFO_KEY = {
@@ -55,6 +57,7 @@ const CONFIG_INFO_KEY = {
5557
[CONFIG_KEY.VALIDATION]: true,
5658
[CONFIG_KEY.APP_URL]: true,
5759
[CONFIG_KEY.APP_URL_ACTIVE]: true,
60+
[CONFIG_KEY.ALLOWED_SCOPES]: true,
5861
};
5962

6063
const COMPONENT_NAME = "/FeatureToggles";
@@ -123,7 +126,7 @@ class FeatureToggles {
123126
const { uris: cfAppUris } = cfEnv.cfApp();
124127

125128
const configEntries = Object.entries(config);
126-
for (const [featureKey, { type, active, appUrl, validation, fallbackValue }] of configEntries) {
129+
for (const [featureKey, { type, active, appUrl, validation, fallbackValue, allowedScopes }] of configEntries) {
127130
this.__featureKeys.push(featureKey);
128131
this.__fallbackValues[featureKey] = fallbackValue;
129132
this.__config[featureKey] = {};
@@ -149,6 +152,14 @@ class FeatureToggles {
149152
!Array.isArray(cfAppUris) ||
150153
cfAppUris.reduce((accumulator, cfAppUri) => accumulator && appUrlRegex.test(cfAppUri), true);
151154
}
155+
156+
if (Array.isArray(allowedScopes)) {
157+
this.__config[featureKey][CONFIG_KEY.ALLOWED_SCOPES] = allowedScopes;
158+
this.__config[featureKey][CONFIG_KEY.ALLOWED_SCOPES_CHECK_MAP] = allowedScopes.reduce((acc, scope) => {
159+
acc[scope] = true;
160+
return acc;
161+
}, {});
162+
}
152163
}
153164

154165
this.__isConfigProcessed = true;
@@ -259,7 +270,7 @@ class FeatureToggles {
259270
}
260271

261272
// NOTE: this function is used during initialization, so we cannot check this.__isInitialized
262-
async _validateFeatureValue(featureKey, value, scopeKey) {
273+
async _validateFeatureValue(featureKey, value, scopeMap, scopeKey) {
263274
if (!this.__isConfigProcessed) {
264275
return [{ errorMessage: "not initialized" }];
265276
}
@@ -272,6 +283,22 @@ class FeatureToggles {
272283
return [{ featureKey, scopeKey, errorMessage: "scopeKey is not valid" }];
273284
}
274285

286+
const allowedScopesCheckMap = this.__config[featureKey][CONFIG_KEY.ALLOWED_SCOPES_CHECK_MAP];
287+
if (allowedScopesCheckMap && scopeKey !== undefined && scopeKey !== SCOPE_ROOT_KEY) {
288+
const actualScopes = Object.keys(scopeMap);
289+
const mismatch = actualScopes.find((scope) => !allowedScopesCheckMap[scope]);
290+
if (mismatch !== undefined) {
291+
return [
292+
{
293+
featureKey,
294+
scopeKey,
295+
errorMessage: 'scope "{0}" is not allowed',
296+
errorMessageValues: [mismatch],
297+
},
298+
];
299+
}
300+
}
301+
275302
// NOTE: value === null is our way of encoding featureKey resetting changes, so it is always allowed
276303
if (value === null) {
277304
return [];
@@ -287,7 +314,7 @@ class FeatureToggles {
287314
return [
288315
{
289316
featureKey,
290-
errorMessage: "feature key is not active because app url does not match regular expression {1}",
317+
errorMessage: "feature key is not active because app url does not match regular expression {0}",
291318
errorMessageValues: [this.__config[featureKey][CONFIG_KEY.APP_URL]],
292319
},
293320
];
@@ -405,11 +432,9 @@ class FeatureToggles {
405432
* @returns {Promise<Array<ValidationError>>} validation errors if any are found or an empty array otherwise
406433
*/
407434
async validateFeatureValue(featureKey, value, scopeMap = undefined) {
408-
return await this._validateFeatureValue(
409-
featureKey,
410-
value,
411-
scopeMap !== undefined ? FeatureToggles.getScopeKey(scopeMap) : scopeMap
412-
);
435+
return scopeMap === undefined
436+
? await this._validateFeatureValue(featureKey, value)
437+
: await this._validateFeatureValue(featureKey, value, scopeMap, FeatureToggles.getScopeKey(scopeMap));
413438
}
414439

415440
/**
@@ -436,7 +461,12 @@ class FeatureToggles {
436461
let validatedStateScopedValues = {};
437462

438463
for (const [scopeKey, value] of Object.entries(scopedValues)) {
439-
const entryValidationErrors = await this._validateFeatureValue(featureKey, value, scopeKey);
464+
const entryValidationErrors = await this._validateFeatureValue(
465+
featureKey,
466+
value,
467+
FeatureToggles.getScopeMap(scopeKey),
468+
scopeKey
469+
);
440470
let updateValue = value;
441471
if (Array.isArray(entryValidationErrors) && entryValidationErrors.length > 0) {
442472
validationErrors = validationErrors.concat(entryValidationErrors);
@@ -817,7 +847,7 @@ class FeatureToggles {
817847
* const FEATURE_VALUE_KEY = "/server/part_x/feature_y"
818848
* ...
819849
* const result = getFeatureValue(FEATURE_VALUE_KEY);
820-
* const resultForTenant = getFeatureValue(FEATURE_VALUE_KEY, { tenantId: "tenant123" });
850+
* const resultForTenant = getFeatureValue(FEATURE_VALUE_KEY, { tenant: "tenant123" });
821851
*
822852
* @param {string} featureKey valid feature key
823853
* @param {Map<string, string>} [scopeMap] optional scope restrictions
@@ -1025,7 +1055,7 @@ class FeatureToggles {
10251055
scopeMap
10261056
);
10271057

1028-
const validationErrors = await this._validateFeatureValue(featureKey, newValue, scopeKey);
1058+
const validationErrors = await this._validateFeatureValue(featureKey, newValue, scopeMap, scopeKey);
10291059
if (Array.isArray(validationErrors) && validationErrors.length > 0) {
10301060
logger.warning(
10311061
new VError(
@@ -1072,7 +1102,7 @@ class FeatureToggles {
10721102

10731103
async _changeRemoteFeatureValue(featureKey, newValue, scopeMap, options) {
10741104
const scopeKey = FeatureToggles.getScopeKey(scopeMap);
1075-
const validationErrors = await this._validateFeatureValue(featureKey, newValue, scopeKey);
1105+
const validationErrors = await this._validateFeatureValue(featureKey, newValue, scopeMap, scopeKey);
10761106
if (Array.isArray(validationErrors) && validationErrors.length > 0) {
10771107
return validationErrors;
10781108
}

‎test/__snapshots__/featureToggles.test.js.snap‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
exports[`feature toggles test basic apis _changeRemoteFeatureValues 2`] = `
44
{
55
"test/feature_a": {},
6+
"test/feature_aa": {},
67
"test/feature_b": {},
78
"test/feature_c": {},
89
"test/feature_d": {},
@@ -16,6 +17,7 @@ exports[`feature toggles test basic apis _changeRemoteFeatureValues 2`] = `
1617
exports[`feature toggles test basic apis _changeRemoteFeatureValues 3`] = `
1718
{
1819
"test/feature_a": {},
20+
"test/feature_aa": {},
1921
"test/feature_b": {},
2022
"test/feature_c": {
2123
"rootValue": "new_a",
@@ -36,6 +38,16 @@ exports[`feature toggles test basic apis getFeaturesInfos 1`] = `
3638
},
3739
"fallbackValue": false,
3840
},
41+
"test/feature_aa": {
42+
"config": {
43+
"ALLOWED_SCOPES": [
44+
"tenant",
45+
"user",
46+
],
47+
"TYPE": "boolean",
48+
},
49+
"fallbackValue": false,
50+
},
3951
"test/feature_b": {
4052
"config": {
4153
"TYPE": "number",
@@ -92,6 +104,17 @@ exports[`feature toggles test basic apis initializeFeatureToggles 1`] = `
92104
"test/feature_a": {
93105
"TYPE": "boolean",
94106
},
107+
"test/feature_aa": {
108+
"ALLOWED_SCOPES": [
109+
"tenant",
110+
"user",
111+
],
112+
"ALLOWED_SCOPES_CHECK_MAP": {
113+
"tenant": true,
114+
"user": true,
115+
},
116+
"TYPE": "boolean",
117+
},
95118
"test/feature_b": {
96119
"TYPE": "number",
97120
},

‎test/featureToggles.test.js‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const featureTogglesModule = require("../src/featureToggles");
66
const { FeatureToggles, readConfigFromFile, SCOPE_ROOT_KEY } = featureTogglesModule;
77
const { CONFIG_KEY, CONFIG_INFO_KEY } = featureTogglesModule._;
88

9+
const { cfEnv } = require("../src/env");
910
const { FEATURE, mockConfig, redisKey, redisChannel } = require("./mockdata");
1011

1112
const { readFile: readFileSpy } = require("fs");
@@ -45,7 +46,7 @@ describe("feature toggles test", () => {
4546

4647
describe("enums", () => {
4748
it("config info consistency", () => {
48-
const internalKeys = [CONFIG_KEY.VALIDATION_REG_EXP];
49+
const internalKeys = [CONFIG_KEY.VALIDATION_REG_EXP, CONFIG_KEY.ALLOWED_SCOPES_CHECK_MAP];
4950
const configKeysCheck = [].concat(Object.keys(CONFIG_INFO_KEY), internalKeys).sort();
5051
const configKeys = Object.values(CONFIG_KEY).sort();
5152

@@ -353,6 +354,12 @@ describe("feature toggles test", () => {
353354
const inputArgsList = [
354355
[FEATURE.A, 1, { a: 1 }],
355356
[FEATURE.A, 1, "a::1"],
357+
[FEATURE.AA, true],
358+
[FEATURE.AA, true, { tenant: "t1", user: "u1" }],
359+
[FEATURE.AA, true, { tenant: "t1" }],
360+
[FEATURE.AA, true, { user: "u1" }],
361+
[FEATURE.AA, true, { usr: "u1" }],
362+
[FEATURE.AA, true, { Tenant: "t1" }],
356363
];
357364

358365
let i = 0;
@@ -384,6 +391,34 @@ describe("feature toggles test", () => {
384391
},
385392
]
386393
`);
394+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`[]`);
395+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`[]`);
396+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`[]`);
397+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`[]`);
398+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`
399+
[
400+
{
401+
"errorMessage": "scope "{0}" is not allowed",
402+
"errorMessageValues": [
403+
"usr",
404+
],
405+
"featureKey": "test/feature_aa",
406+
"scopeKey": "usr::u1",
407+
},
408+
]
409+
`);
410+
expect(await featureToggles.validateFeatureValue(...inputArgsList[i++])).toMatchInlineSnapshot(`
411+
[
412+
{
413+
"errorMessage": "scope "{0}" is not allowed",
414+
"errorMessageValues": [
415+
"Tenant",
416+
],
417+
"featureKey": "test/feature_aa",
418+
"scopeKey": "Tenant::t1",
419+
},
420+
]
421+
`);
387422
expect(i).toBe(inputArgsList.length);
388423

389424
expect(loggerSpy.warning).not.toHaveBeenCalled();
@@ -462,6 +497,7 @@ describe("feature toggles test", () => {
462497
it("getFeatureValue", async () => {
463498
const mockFeatureValuesEntries = [
464499
[[FEATURE.A], { [SCOPE_ROOT_KEY]: true }],
500+
[[FEATURE.AA], { [SCOPE_ROOT_KEY]: true }],
465501
[[FEATURE.B], { [SCOPE_ROOT_KEY]: 0 }],
466502
[[FEATURE.C], { [SCOPE_ROOT_KEY]: "cvalue" }],
467503
];
@@ -471,7 +507,7 @@ describe("feature toggles test", () => {
471507
["e", "bvalue"],
472508
["f", "cvalue"],
473509
];
474-
for (let i = 0; i < 3; i++) {
510+
for (let i = 0; i < mockFeatureValuesEntries.length; i++) {
475511
redisWrapperMock.watchedHashGetSetObject.mockImplementationOnce((key, field) => mockFeatureValues[field]);
476512
}
477513
await featureToggles.initializeFeatures({ config: mockConfig });
@@ -492,6 +528,7 @@ describe("feature toggles test", () => {
492528
const testScopeKey = FeatureToggles.getScopeKey(testScopeMap);
493529
const mockFeatureValuesEntries = [
494530
[[FEATURE.A], { [SCOPE_ROOT_KEY]: true, [testScopeKey]: false }],
531+
[[FEATURE.AA], { [SCOPE_ROOT_KEY]: true, [testScopeKey]: false }],
495532
[[FEATURE.B], { [SCOPE_ROOT_KEY]: 0, [testScopeKey]: 10 }],
496533
[[FEATURE.C], { [SCOPE_ROOT_KEY]: "cvalue", [testScopeKey]: "" }],
497534
];
@@ -501,7 +538,7 @@ describe("feature toggles test", () => {
501538
["e", "bvalue"],
502539
["f", "cvalue"],
503540
];
504-
for (let i = 0; i < 3; i++) {
541+
for (let i = 0; i < mockFeatureValuesEntries.length; i++) {
505542
redisWrapperMock.watchedHashGetSetObject.mockImplementationOnce((key, field) => mockFeatureValues[field]);
506543
}
507544
await featureToggles.initializeFeatures({ config: mockConfig });
@@ -775,6 +812,41 @@ describe("feature toggles test", () => {
775812
expect(featureToggles.getFeatureValue(FEATURE.G)).toBe(oldValue);
776813
});
777814

815+
it("FeatureValueValidation and appUrl working", async () => {
816+
jest.spyOn(cfEnv, "cfApp").mockReturnValueOnce({ uris: ["https://it.cfapps.sap.hana.ondemand.com"] });
817+
const newValue = "newValue";
818+
await featureToggles.initializeFeatures({ config: mockConfig });
819+
const featureConfig = featureToggles.getFeatureInfo(FEATURE.H).config;
820+
821+
expect(featureConfig.APP_URL).toMatchInlineSnapshot(`"\\.cfapps\\.sap\\.hana\\.ondemand\\.com$"`);
822+
expect(featureConfig.APP_URL_ACTIVE).toBe(true);
823+
expect(await featureToggles.changeFeatureValue(FEATURE.H, newValue)).toBeUndefined();
824+
expect(featureToggles.getFeatureValue(FEATURE.H)).toBe(newValue);
825+
});
826+
827+
it("FeatureValueValidation and appUrl failing", async () => {
828+
jest.spyOn(cfEnv, "cfApp").mockReturnValueOnce({ uris: ["https://not-it.com"] });
829+
const newValue = "newValue";
830+
await featureToggles.initializeFeatures({ config: mockConfig });
831+
const oldValue = featureToggles.getFeatureValue(FEATURE.H);
832+
const featureConfig = featureToggles.getFeatureInfo(FEATURE.H).config;
833+
834+
expect(featureConfig.APP_URL).toMatchInlineSnapshot(`"\\.cfapps\\.sap\\.hana\\.ondemand\\.com$"`);
835+
expect(featureConfig.APP_URL_ACTIVE).toBe(false);
836+
expect(await featureToggles.changeFeatureValue(FEATURE.H, newValue)).toMatchInlineSnapshot(`
837+
[
838+
{
839+
"errorMessage": "feature key is not active because app url does not match regular expression {0}",
840+
"errorMessageValues": [
841+
"\\.cfapps\\.sap\\.hana\\.ondemand\\.com$",
842+
],
843+
"featureKey": "test/feature_h",
844+
},
845+
]
846+
`);
847+
expect(featureToggles.getFeatureValue(FEATURE.H)).toBe(oldValue);
848+
});
849+
778850
it("validateInput throws error", async () => {
779851
const error = new Error("bad validator");
780852
const validator = jest.fn().mockRejectedValue(error);

‎test/logger.test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe("logger test", () => {
7474
expect(consoleSpy.info.mock.calls.map(cleanupReadableLogCalls)).toMatchInlineSnapshot(`
7575
[
7676
[
77-
"88:88:88.888 | info | Testing | finished initialization with 8 feature toggles with CF_REDIS",
77+
"88:88:88.888 | info | Testing | finished initialization with 9 feature toggles with CF_REDIS",
7878
],
7979
]
8080
`);
@@ -121,7 +121,7 @@ describe("logger test", () => {
121121
"{"logger":"nodejs-logger","type":"log","msg":"FeatureTogglesError: found invalid fallback values during initialization","level":"warn","stacktrace":["FeatureTogglesError: found invalid fallback values during initialization"],"layer":"Testing","errInfo":"{\\"validationErrors\\":\\"[{\\\\\\"featureKey\\\\\\":\\\\\\"test/feature_b\\\\\\",\\\\\\"errorMessage\\\\\\":\\\\\\"registered validator \\\\\\\\\\\\\\"{0}\\\\\\\\\\\\\\" failed for value \\\\\\\\\\\\\\"{1}\\\\\\\\\\\\\\" with error {2}\\\\\\",\\\\\\"errorMessageValues\\\\\\":[\\\\\\"mockConstructor\\\\\\",1,\\\\\\"bad validator\\\\\\"]}]\\"}"}",
122122
],
123123
[
124-
"{"logger":"nodejs-logger","type":"log","msg":"finished initialization with 8 feature toggles with CF_REDIS","level":"info","layer":"Testing"}",
124+
"{"logger":"nodejs-logger","type":"log","msg":"finished initialization with 9 feature toggles with CF_REDIS","level":"info","layer":"Testing"}",
125125
],
126126
]
127127
`);

‎test/mockdata.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const redisChannel = "feature-channel";
55

66
const FEATURE = {
77
A: "test/feature_a",
8+
AA: "test/feature_aa",
89
B: "test/feature_b",
910
C: "test/feature_c",
1011
D: "test/feature_d",
@@ -19,6 +20,11 @@ const mockConfig = {
1920
fallbackValue: false,
2021
type: "boolean",
2122
},
23+
[FEATURE.AA]: {
24+
fallbackValue: false,
25+
type: "boolean",
26+
allowedScopes: ["tenant", "user"],
27+
},
2228
[FEATURE.B]: {
2329
fallbackValue: 1,
2430
type: "number",

0 commit comments

Comments
 (0)