From 3db1159304b04d9a74e2323d7273e26ff5fee1b9 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 19 Jul 2024 10:40:45 +0200 Subject: [PATCH] feat: allow you to gradually scale back constraint usage (#7622) This PR updates the limit validation for constraint numbers on a single strategy. In cases where you're already above the limit, it allows you to still update the strategy as long as you don't add any **new** constraints (that is: the number of constraints doesn't increase). A discussion point: I've only tested this with unit tests of the method directly. I haven't tested that the right parameters are passed in from calling functions. The main reason being that that would involve updating the fake strategy and feature stores to sync their flag lists (or just checking that the thrown error isn't a limit exceeded error), because right now the fake strategy store throws an error when it doesn't find the flag I want to update. --- .../createFeatureToggleService.ts | 7 +- .../feature-toggle/feature-toggle-service.ts | 22 ++++-- .../feature-toggle-service.limit.test.ts | 67 +++++++++++++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) diff --git a/src/lib/features/feature-toggle/createFeatureToggleService.ts b/src/lib/features/feature-toggle/createFeatureToggleService.ts index ed5d2fa5fe..ba5c82f38f 100644 --- a/src/lib/features/feature-toggle/createFeatureToggleService.ts +++ b/src/lib/features/feature-toggle/createFeatureToggleService.ts @@ -219,5 +219,10 @@ export const createFakeFeatureToggleService = (config: IUnleashConfig) => { dependentFeaturesService, featureLifecycleReadModel, ); - return { featureToggleService, featureToggleStore, projectStore }; + return { + featureToggleService, + featureToggleStore, + projectStore, + featureStrategiesStore, + }; }; diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index fe5d2d304e..3ebb9e8121 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -390,11 +390,17 @@ class FeatureToggleService { } } - validateConstraintsLimit(updatedConstrains: IConstraint[]) { + private validateConstraintsLimit(constraints: { + updated: IConstraint[]; + existing: IConstraint[]; + }) { if (!this.flagResolver.isEnabled('resourceLimits')) return; const constraintsLimit = this.resourceLimits.constraints; - if (updatedConstrains.length > constraintsLimit) { + if ( + constraints.updated.length > constraintsLimit && + constraints.updated.length > constraints.existing.length + ) { throwExceedsLimitError(this.eventBus, { resource: `constraints`, limit: constraintsLimit, @@ -402,7 +408,7 @@ class FeatureToggleService { } const constraintValuesLimit = this.resourceLimits.constraintValues; - const constraintOverLimit = updatedConstrains.find( + const constraintOverLimit = constraints.updated.find( (constraint) => Array.isArray(constraint.values) && constraint.values?.length > constraintValuesLimit, @@ -661,7 +667,10 @@ class FeatureToggleService { strategyConfig.constraints && strategyConfig.constraints.length > 0 ) { - this.validateConstraintsLimit(strategyConfig.constraints); + this.validateConstraintsLimit({ + updated: strategyConfig.constraints, + existing: [], + }); strategyConfig.constraints = await this.validateConstraints( strategyConfig.constraints, ); @@ -820,7 +829,10 @@ class FeatureToggleService { if (existingStrategy.id === id) { if (updates.constraints && updates.constraints.length > 0) { - this.validateConstraintsLimit(updates.constraints); + this.validateConstraintsLimit({ + updated: updates.constraints, + existing: existingStrategy.constraints, + }); updates.constraints = await this.validateConstraints( updates.constraints, ); diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts index a3ee0a3f37..2e22b25506 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts @@ -8,6 +8,7 @@ import type { IUser, } from '../../../types'; import getLogger from '../../../../test/fixtures/no-logger'; +import { ExceedsLimitError } from '../../../error/exceeds-limit-error'; const alwaysOnFlagResolver = { isEnabled() { @@ -91,6 +92,72 @@ describe('Strategy limits', () => { ); }); + test('Should not throw limit exceeded errors if the new number of constraints is less than or equal to the previous number', async () => { + const LIMIT = 1; + const { featureToggleService, featureStrategiesStore } = + createFakeFeatureToggleService({ + getLogger, + flagResolver: alwaysOnFlagResolver, + resourceLimits: { + constraints: LIMIT, + }, + } as unknown as IUnleashConfig); + + const constraints: IConstraint[] = [ + { + values: ['1'], + operator: 'IN', + contextName: 'appName', + }, + { + values: ['2'], + operator: 'IN', + contextName: 'appName', + }, + { + values: ['3'], + operator: 'IN', + contextName: 'appName', + }, + ]; + + const flagName = 'feature'; + + await featureStrategiesStore.createFeature({ + name: flagName, + createdByUserId: 1, + }); + + const strategy = await featureStrategiesStore.createStrategyFeatureEnv({ + parameters: {}, + strategyName: 'default', + featureName: flagName, + constraints: constraints, + projectId: 'default', + environment: 'default', + }); + + const updateStrategy = (newConstraints) => + featureToggleService.unprotectedUpdateStrategy( + strategy.id, + { + constraints: newConstraints, + }, + { projectId: 'default', featureName: 'feature' } as any, + {} as IAuditUser, + ); + + // check that you can save the same amount of constraints + await updateStrategy(constraints); + // check that you can save fewer constraints but still over the limit + await updateStrategy(constraints.slice(0, 2)); + + // check that you can't save more constraints + await expect(async () => + updateStrategy([...constraints, ...constraints]), + ).rejects.toThrow(new ExceedsLimitError('constraints', LIMIT)); + }); + test('Should not allow to exceed constraint values limit', async () => { const LIMIT = 3; const { featureToggleService, featureToggleStore } =