From 200792e434833dd5ba0d3e61b628f53b9a3a41a8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 28 Oct 2020 22:04:17 +0100 Subject: [PATCH] core-api: refactor feature flags user flags access --- .../src/apis/definitions/FeatureFlagsApi.ts | 41 +++-- packages/core-api/src/app/App.tsx | 4 +- .../core-api/src/app/FeatureFlags.test.tsx | 143 +++++++----------- packages/core-api/src/app/FeatureFlags.tsx | 140 ++++++----------- packages/core-api/src/app/index.ts | 1 - packages/core-api/src/plugin/Plugin.tsx | 3 - .../CostInsightsPage/CostInsightsPage.tsx | 6 +- .../components/FeatureFlags/FeatureFlags.tsx | 26 ++-- 8 files changed, 156 insertions(+), 208 deletions(-) diff --git a/packages/core-api/src/apis/definitions/FeatureFlagsApi.ts b/packages/core-api/src/apis/definitions/FeatureFlagsApi.ts index 76a1e4f9ba..243af562b1 100644 --- a/packages/core-api/src/apis/definitions/FeatureFlagsApi.ts +++ b/packages/core-api/src/apis/definitions/FeatureFlagsApi.ts @@ -15,7 +15,6 @@ */ import { ApiRef, createApiRef } from '../system'; -import { UserFlags } from '../../app/FeatureFlags'; /** * The feature flags API is used to toggle functionality to users across plugins and Backstage. @@ -29,16 +28,35 @@ import { UserFlags } from '../../app/FeatureFlags'; * to enable and disable feature flags, this API acts as another way to enable/disable. */ -export enum FeatureFlagState { - Off = 0, - On = 1, -} - -export interface FeatureFlag { +export type FeatureFlag = { name: string; pluginId: string; +}; + +export enum FeatureFlagState { + None = 0, + Active = 1, } +/** + * Options to use when saving feature flags. + */ +export type FeatureFlagsSaveOptions = { + /** + * The new feature flag states to save. + */ + states: Record; + + /** + * Whether the saves states should be merged into the existing ones, or replace them. + * + * Defaults to false. + */ + merge?: boolean; +}; + +export type UserFlags = {}; + export interface FeatureFlagsApi { /** * Registers a new feature flag. Once a feature flag has been registered it @@ -52,9 +70,14 @@ export interface FeatureFlagsApi { getRegisteredFlags(): FeatureFlag[]; /** - * Get a list of all feature flags from the current user. + * Whether the feature flag with the given name is currently activated for the user. */ - getFlags(): UserFlags; + isActive(name: string): boolean; + + /** + * Save the user's choice of feature flag states. + */ + save(options: FeatureFlagsSaveOptions): void; } export const featureFlagsApiRef: ApiRef = createApiRef({ diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index 45496b9b8a..91b55a7d2e 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -54,7 +54,7 @@ import { import { useAsync } from 'react-use'; import { AppIdentity } from './AppIdentity'; import { ApiResolver, ApiFactoryRegistry } from '../apis/system'; -import { FeatureFlags } from './FeatureFlags'; +import { LocalStorageFeatureFlags } from './FeatureFlags'; type FullAppOptions = { apis: Iterable; @@ -320,7 +320,7 @@ export class PrivateAppImpl implements BackstageApp { registry.register('default', { api: featureFlagsApiRef, deps: {}, - factory: () => new FeatureFlags(), + factory: () => new LocalStorageFeatureFlags(), }); for (const factory of this.defaultApis) { registry.register('default', factory); diff --git a/packages/core-api/src/app/FeatureFlags.test.tsx b/packages/core-api/src/app/FeatureFlags.test.tsx index c04e60b069..02641d6d18 100644 --- a/packages/core-api/src/app/FeatureFlags.test.tsx +++ b/packages/core-api/src/app/FeatureFlags.test.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { FeatureFlags as FeatureFlagsImpl } from './FeatureFlags'; +import { LocalStorageFeatureFlags } from './FeatureFlags'; import { FeatureFlagState, FeatureFlagsApi } from '../apis/definitions'; describe('FeatureFlags', () => { @@ -22,62 +22,44 @@ describe('FeatureFlags', () => { window.localStorage.clear(); }); - describe('#getFlags', () => { + describe('getFlags', () => { let featureFlags: FeatureFlagsApi; beforeEach(() => { - featureFlags = new FeatureFlagsImpl(); + featureFlags = new LocalStorageFeatureFlags(); }); it('returns no flags', () => { - expect(featureFlags.getFlags().toObject()).toMatchObject({}); + expect(featureFlags.getRegisteredFlags()).toEqual([]); }); - it('returns the correct flags', () => { + it('loads flags from local storage', () => { window.localStorage.setItem( 'featureFlags', JSON.stringify({ 'feature-flag-one': 1, 'feature-flag-two': 1, 'feature-flag-three': 0, + 'feature-flag-four': 2, + 'feature-flag-five': 'not-valid', }), ); - featureFlags = new FeatureFlagsImpl(); - expect(featureFlags.getFlags().toObject()).toMatchObject({ - 'feature-flag-one': FeatureFlagState.On, - 'feature-flag-two': FeatureFlagState.On, - 'feature-flag-three': FeatureFlagState.Off, - }); - }); - - it('gets the correct values', () => { - window.localStorage.setItem( - 'featureFlags', - JSON.stringify({ - 'feature-flag-one': 1, - 'feature-flag-two': 0, - }), - ); - - featureFlags = new FeatureFlagsImpl(); - - expect(featureFlags.getFlags().get('feature-flag-one')).toEqual( - FeatureFlagState.On, - ); - expect(featureFlags.getFlags().get('feature-flag-two')).toEqual( - FeatureFlagState.Off, - ); - expect(featureFlags.getFlags().get('feature-flag-three')).toEqual( - FeatureFlagState.Off, - ); + expect(featureFlags.isActive('feature-flag-one')).toBe(true); + expect(featureFlags.isActive('feature-flag-two')).toBe(true); + expect(featureFlags.isActive('feature-flag-three')).toBe(false); + expect(featureFlags.isActive('feature-flag-four')).toBe(false); + expect(featureFlags.isActive('feature-flag-five')).toBe(false); }); it('sets the correct values', () => { - const flags = featureFlags.getFlags(); - flags.set('feature-flag-zero', FeatureFlagState.On); + featureFlags.save({ + states: { + 'feature-flag-zero': FeatureFlagState.Active, + }, + }); - expect(flags.get('feature-flag-zero')).toEqual(FeatureFlagState.On); + expect(featureFlags.isActive('feature-flag-zero')).toBe(true); expect(window.localStorage.getItem('featureFlags')).toEqual( '{"feature-flag-zero":1}', ); @@ -89,16 +71,20 @@ describe('FeatureFlags', () => { JSON.stringify({ 'feature-flag-one': 1, 'feature-flag-two': 0, + 'feature-flag-tree': 1, + 'feature-flag-four': 0, }), ); - featureFlags = new FeatureFlagsImpl(); - const flags = featureFlags.getFlags(); - flags.delete('feature-flag-one'); + featureFlags.save({ + states: { + 'feature-flag-one': FeatureFlagState.None, + 'feature-flag-two': FeatureFlagState.Active, + }, + }); - expect(flags.get('feature-flag-one')).toEqual(FeatureFlagState.Off); expect(window.localStorage.getItem('featureFlags')).toEqual( - '{"feature-flag-two":0}', + '{"feature-flag-two":1}', ); }); @@ -112,19 +98,25 @@ describe('FeatureFlags', () => { }), ); - const flags = featureFlags.getFlags(); - flags.clear(); + expect(featureFlags.isActive('feature-flag-one')).toBe(true); + expect(featureFlags.isActive('feature-flag-two')).toBe(true); + expect(featureFlags.isActive('feature-flag-three')).toBe(false); + + featureFlags.save({ states: {} }); + + expect(featureFlags.isActive('feature-flag-one')).toBe(false); + expect(featureFlags.isActive('feature-flag-two')).toBe(false); + expect(featureFlags.isActive('feature-flag-three')).toBe(false); - expect(flags.toObject()).toEqual({}); expect(window.localStorage.getItem('featureFlags')).toEqual('{}'); }); }); - describe('#getRegisteredFlags', () => { + describe('getRegisteredFlags', () => { let featureFlags: FeatureFlagsApi; beforeEach(() => { - featureFlags = new FeatureFlagsImpl(); + featureFlags = new LocalStorageFeatureFlags(); featureFlags.registerFlag({ name: 'registered-flag-1', pluginId: 'plugin-one', @@ -140,7 +132,7 @@ describe('FeatureFlags', () => { }); it('should return an empty list', () => { - featureFlags = new FeatureFlagsImpl(); + featureFlags = new LocalStorageFeatureFlags(); expect(featureFlags.getRegisteredFlags()).toEqual([]); }); @@ -152,6 +144,25 @@ describe('FeatureFlags', () => { ]); }); + it('should provide a copy of the list of flags', () => { + const flags = featureFlags.getRegisteredFlags(); + expect(flags).toEqual([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + ]); + flags.splice(2, 1); + expect(flags).toEqual([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + ]); + expect(featureFlags.getRegisteredFlags()).toEqual([ + { name: 'registered-flag-1', pluginId: 'plugin-one' }, + { name: 'registered-flag-2', pluginId: 'plugin-one' }, + { name: 'registered-flag-3', pluginId: 'plugin-two' }, + ]); + }); + it('should get the correct values', () => { const getByName = (name: string) => featureFlags.getRegisteredFlags().find(flag => flag.name === name); @@ -171,44 +182,6 @@ describe('FeatureFlags', () => { }); }); - it('should append the correct value', () => { - const flags = featureFlags.getRegisteredFlags(); - - flags.push({ - name: 'registered-flag-4', - pluginId: 'plugin-three', - }); - - expect(flags).toEqual([ - { name: 'registered-flag-1', pluginId: 'plugin-one' }, - { name: 'registered-flag-2', pluginId: 'plugin-one' }, - { name: 'registered-flag-3', pluginId: 'plugin-two' }, - { name: 'registered-flag-4', pluginId: 'plugin-three' }, - ]); - }); - - it('should concat the correct values', () => { - const flags = featureFlags.getRegisteredFlags(); - const concatValues = flags.concat([ - { - name: 'registered-flag-4', - pluginId: 'plugin-three', - }, - { - name: 'registered-flag-5', - pluginId: 'plugin-four', - }, - ]); - - expect(concatValues).toMatchObject([ - { name: 'registered-flag-1', pluginId: 'plugin-one' }, - { name: 'registered-flag-2', pluginId: 'plugin-one' }, - { name: 'registered-flag-3', pluginId: 'plugin-two' }, - { name: 'registered-flag-4', pluginId: 'plugin-three' }, - { name: 'registered-flag-5', pluginId: 'plugin-four' }, - ]); - }); - it('throws an error if length is less than three characters', () => { expect(() => featureFlags.registerFlag({ diff --git a/packages/core-api/src/app/FeatureFlags.tsx b/packages/core-api/src/app/FeatureFlags.tsx index 0f794d8c44..73215481b7 100644 --- a/packages/core-api/src/app/FeatureFlags.tsx +++ b/packages/core-api/src/app/FeatureFlags.tsx @@ -18,19 +18,9 @@ import { FeatureFlagState, FeatureFlagsApi, FeatureFlag, + FeatureFlagsSaveOptions, } from '../apis/definitions'; -/** - * Helper method for validating compatibility and flag name. - */ -export function validateBrowserCompat(): void { - if (!('localStorage' in window)) { - throw new Error( - 'Feature Flags are not supported on browsers without the Local Storage API', - ); - } -} - export function validateFlagName(name: string): void { if (name.length < 3) { throw new Error( @@ -52,86 +42,12 @@ export function validateFlagName(name: string): void { } } -/** - * The UserFlags class. - * - * This acts as a data structure for the user's feature flags. You - * can use this to retrieve, add, edit, delete, clear and save the user's - * feature flags to the local browser for persisted storage. - */ -export class UserFlags extends Map { - static load(): UserFlags { - validateBrowserCompat(); - - try { - const jsonString = window.localStorage.getItem('featureFlags') as string; - const json = JSON.parse(jsonString); - return new this(Object.entries(json)); - } catch (err) { - return new this([]); - } - } - - get(name: string): FeatureFlagState { - return super.get(name) || FeatureFlagState.Off; - } - - set(name: string, state: FeatureFlagState): this { - validateFlagName(name); - const output = super.set(name, state); - this.save(); - return output; - } - - toggle(name: string): FeatureFlagState { - if (super.get(name) === FeatureFlagState.On) { - super.set(name, FeatureFlagState.Off); - } else { - super.set(name, FeatureFlagState.On); - } - return super.get(name) || FeatureFlagState.Off; - } - - delete(name: string): boolean { - const output = super.delete(name); - this.save(); - return output; - } - - clear(): void { - super.clear(); - this.save(); - } - - save(): void { - window.localStorage.setItem( - 'featureFlags', - JSON.stringify(this.toObject()), - ); - } - - toObject() { - return Array.from(this.entries()).reduce( - (obj, [key, value]) => ({ ...obj, [key]: value }), - {}, - ); - } - - toJSON() { - return JSON.stringify(this.toObject()); - } - - toString() { - return this.toJSON(); - } -} - /** * Create the FeatureFlags implementation based on the API. */ -export class FeatureFlags implements FeatureFlagsApi { +export class LocalStorageFeatureFlags implements FeatureFlagsApi { private registeredFeatureFlags: FeatureFlag[] = []; - private userFlags: UserFlags | undefined; + private flags?: Map; registerFlag(flag: FeatureFlag) { validateFlagName(flag.name); @@ -142,8 +58,52 @@ export class FeatureFlags implements FeatureFlagsApi { return this.registeredFeatureFlags.slice(); } - getFlags(): UserFlags { - if (!this.userFlags) this.userFlags = UserFlags.load(); - return this.userFlags; + isActive(name: string): boolean { + if (!this.flags) { + this.flags = this.load(); + } + return this.flags.get(name) === FeatureFlagState.Active; + } + + save(options: FeatureFlagsSaveOptions): void { + if (!this.flags) { + this.flags = this.load(); + } + if (!options.merge) { + this.flags.clear(); + } + for (const [name, state] of Object.entries(options.states)) { + this.flags.set(name, state); + } + + const enabled = Array.from(this.flags.entries()).filter( + ([, state]) => state === FeatureFlagState.Active, + ); + window.localStorage.setItem( + 'featureFlags', + JSON.stringify(Object.fromEntries(enabled)), + ); + } + + private load(): Map { + try { + const jsonStr = window.localStorage.getItem('featureFlags'); + if (!jsonStr) { + return new Map(); + } + const json = JSON.parse(jsonStr) as unknown; + if (typeof json !== 'object' || json === null || Array.isArray(json)) { + return new Map(); + } + + const entries = Object.entries(json).filter(([name, value]) => { + validateFlagName(name); + return value === FeatureFlagState.Active; + }); + + return new Map(entries); + } catch { + return new Map(); + } } } diff --git a/packages/core-api/src/app/index.ts b/packages/core-api/src/app/index.ts index 003b71e353..17610ea3ee 100644 --- a/packages/core-api/src/app/index.ts +++ b/packages/core-api/src/app/index.ts @@ -14,6 +14,5 @@ * limitations under the License. */ -export { FeatureFlags } from './FeatureFlags'; export { useApp } from './AppContext'; export * from './types'; diff --git a/packages/core-api/src/plugin/Plugin.tsx b/packages/core-api/src/plugin/Plugin.tsx index 866d430070..d69bb7e721 100644 --- a/packages/core-api/src/plugin/Plugin.tsx +++ b/packages/core-api/src/plugin/Plugin.tsx @@ -15,7 +15,6 @@ */ import { PluginConfig, PluginOutput, BackstagePlugin } from './types'; -import { validateBrowserCompat, validateFlagName } from '../app/FeatureFlags'; import { AnyApiFactory } from '../apis'; export class PluginImpl { @@ -57,8 +56,6 @@ export class PluginImpl { }, featureFlags: { register(name) { - validateBrowserCompat(); - validateFlagName(name); outputs.push({ type: 'feature-flag', name }); }, }, diff --git a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx index 638f6fef00..52f70d2756 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx @@ -55,9 +55,7 @@ import { useSubtleTypographyStyles } from '../../utils/styles'; export const CostInsightsPage = () => { const classes = useSubtleTypographyStyles(); - const flags = useApi(featureFlagsApiRef).getFlags(); - // There is not currently a UI to set feature flags - // flags.set('cost-insights-currencies', FeatureFlagState.On); + const featureFlags = useApi(featureFlagsApiRef); const client = useApi(costInsightsApiRef); const config = useConfig(); const groups = useGroups(); @@ -211,7 +209,7 @@ export const CostInsightsPage = () => { - {!!flags.get('cost-insights-currencies') && ( + {featureFlags.isActive('cost-insights-currencies') && ( { const featureFlagsApi = useApi(featureFlagsApiRef); const featureFlags = featureFlagsApi.getRegisteredFlags(); - const initialFlagState = featureFlags.reduce( - (result, featureFlag: FeatureFlag) => { - const state = featureFlagsApi.getFlags().get(featureFlag.name); - result[featureFlag.name] = state; - return result; - }, - {} as Record, + const initialFlagState = Object.fromEntries( + featureFlags.map(({ name }) => [name, featureFlagsApi.isActive(name)]), ); - const [state, setState] = useState>( - initialFlagState, - ); + const [state, setState] = useState>(initialFlagState); const toggleFlag = useCallback( (flagName: string) => { - const newState = featureFlagsApi.getFlags().toggle(flagName); + const newState = featureFlagsApi.isActive(flagName) + ? FeatureFlagState.None + : FeatureFlagState.Active; + + featureFlagsApi.save({ + states: { [flagName]: newState }, + merge: true, + }); setState(prevState => ({ ...prevState, - [flagName]: newState, + [flagName]: newState === FeatureFlagState.Active, })); - featureFlagsApi.getFlags().save(); }, [featureFlagsApi], );