Skip to content

Commit

Permalink
feat: allow you to gradually scale back constraint usage (#7622)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thomasheartman authored Jul 19, 2024
1 parent a0ba44d commit 3db1159
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -219,5 +219,10 @@ export const createFakeFeatureToggleService = (config: IUnleashConfig) => {
dependentFeaturesService,
featureLifecycleReadModel,
);
return { featureToggleService, featureToggleStore, projectStore };
return {
featureToggleService,
featureToggleStore,
projectStore,
featureStrategiesStore,
};
};
22 changes: 17 additions & 5 deletions src/lib/features/feature-toggle/feature-toggle-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,19 +390,25 @@ 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,
});
}

const constraintValuesLimit = this.resourceLimits.constraintValues;
const constraintOverLimit = updatedConstrains.find(
const constraintOverLimit = constraints.updated.find(
(constraint) =>
Array.isArray(constraint.values) &&
constraint.values?.length > constraintValuesLimit,
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 } =
Expand Down

0 comments on commit 3db1159

Please sign in to comment.