From 7e0874df63db436a66aac4a52c67c39736fe9e02 Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Tue, 21 May 2024 10:55:32 +0800 Subject: [PATCH 1/2] fix evaluation: enabled default to false --- src/featureManager.ts | 4 +-- test/noFilters.test.ts | 70 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 test/noFilters.test.ts diff --git a/src/featureManager.ts b/src/featureManager.ts index 99b7700..81907f5 100644 --- a/src/featureManager.ts +++ b/src/featureManager.ts @@ -36,8 +36,8 @@ export class FeatureManager { return false; } - if (featureFlag.enabled === false) { - // If the feature is explicitly disabled, then it is disabled. + if (featureFlag.enabled !== true) { + // If the feature is not explicitly enabled, then it is disabled by default. return false; } diff --git a/test/noFilters.test.ts b/test/noFilters.test.ts new file mode 100644 index 0000000..4ff9568 --- /dev/null +++ b/test/noFilters.test.ts @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +chai.use(chaiAsPromised); +const expect = chai.expect; + +import { FeatureManager, ConfigurationObjectFeatureFlagProvider } from "./exportedApi"; + +const featureFlagsDataObject = { + "feature_management": { + "feature_flags": [ + { + "id": "BooleanTrue", + "description": "A feature flag with no Filters, that returns true.", + "enabled": true, + "conditions": { + "client_filters": [] + } + }, + { + "id": "BooleanFalse", + "description": "A feature flag with no Filters, that returns false.", + "enabled": false, + "conditions": { + "client_filters": [] + } + }, + { + "id": "InvalidEnabled", + "description": "A feature flag with an invalid 'enabled' value, that returns false.", + "enabled": "invalid", + "conditions": { + "client_filters": [] + } + }, + { + "id": "Minimal", + "enabled": true + }, + { + "id": "NoEnabled" + }, + { + "id": "EmptyConditions", + "description": "A feature flag with no values in conditions, that returns true.", + "enabled": true, + "conditions": { + } + } + ] + } +}; + +describe("feature flags with no filters", () => { + it("should validate feature flags without filters", () => { + const provider = new ConfigurationObjectFeatureFlagProvider(featureFlagsDataObject); + const featureManager = new FeatureManager(provider); + + return Promise.all([ + expect(featureManager.isEnabled("BooleanTrue")).eventually.eq(true), + expect(featureManager.isEnabled("BooleanFalse")).eventually.eq(false), + expect(featureManager.isEnabled("InvalidEnabled")).eventually.eq(false), + expect(featureManager.isEnabled("Minimal")).eventually.eq(true), + expect(featureManager.isEnabled("NoEnabled")).eventually.eq(false), + expect(featureManager.isEnabled("EmptyConditions")).eventually.eq(true) + ]); + }); +}); From 1de025d24419fa71a1be2548347d38e407ac47fd Mon Sep 17 00:00:00 2001 From: Yan Zhang Date: Wed, 22 May 2024 09:56:25 +0800 Subject: [PATCH 2/2] chore: Add feature flag format validation in FeatureManager --- src/featureManager.ts | 9 +++++++++ test/noFilters.test.ts | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/featureManager.ts b/src/featureManager.ts index 81907f5..d113f74 100644 --- a/src/featureManager.ts +++ b/src/featureManager.ts @@ -36,6 +36,9 @@ export class FeatureManager { return false; } + // Ensure that the feature flag is in the correct format. Feature providers should validate the feature flags, but we do it here as a safeguard. + validateFeatureFlagFormat(featureFlag); + if (featureFlag.enabled !== true) { // If the feature is not explicitly enabled, then it is disabled by default. return false; @@ -77,3 +80,9 @@ export class FeatureManager { interface FeatureManagerOptions { customFilters?: IFeatureFilter[]; } + +function validateFeatureFlagFormat(featureFlag: any): void { + if (featureFlag.enabled !== undefined && typeof featureFlag.enabled !== "boolean") { + throw new Error(`Feature flag ${featureFlag.id} has an invalid 'enabled' value.`); + } +} diff --git a/test/noFilters.test.ts b/test/noFilters.test.ts index 4ff9568..a303748 100644 --- a/test/noFilters.test.ts +++ b/test/noFilters.test.ts @@ -29,7 +29,7 @@ const featureFlagsDataObject = { }, { "id": "InvalidEnabled", - "description": "A feature flag with an invalid 'enabled' value, that returns false.", + "description": "A feature flag with an invalid 'enabled' value, that throws an exception.", "enabled": "invalid", "conditions": { "client_filters": [] @@ -61,7 +61,7 @@ describe("feature flags with no filters", () => { return Promise.all([ expect(featureManager.isEnabled("BooleanTrue")).eventually.eq(true), expect(featureManager.isEnabled("BooleanFalse")).eventually.eq(false), - expect(featureManager.isEnabled("InvalidEnabled")).eventually.eq(false), + expect(featureManager.isEnabled("InvalidEnabled")).eventually.rejectedWith("Feature flag InvalidEnabled has an invalid 'enabled' value."), expect(featureManager.isEnabled("Minimal")).eventually.eq(true), expect(featureManager.isEnabled("NoEnabled")).eventually.eq(false), expect(featureManager.isEnabled("EmptyConditions")).eventually.eq(true)