1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-08-13 13:48:59 +02:00

fix: stickiness should be preserved on strategy updates

This commit is contained in:
Gastón Fournier 2025-07-30 11:21:56 +02:00
parent 32996460df
commit 7456ae75f3
No known key found for this signature in database
GPG Key ID: AF45428626E17A8E
6 changed files with 168 additions and 111 deletions

View File

@ -343,4 +343,8 @@ export default class FakeFeatureStrategiesStore
getCustomStrategiesInUseCount(): Promise<number> {
return Promise.resolve(3);
}
getDefaultStickiness(_projectId: string): Promise<string> {
return Promise.resolve('default');
}
}

View File

@ -370,16 +370,6 @@ export class FeatureToggleService {
'You can not change the featureName for an activation strategy.',
);
}
if (
existingStrategy.parameters &&
'stickiness' in existingStrategy.parameters &&
existingStrategy.parameters.stickiness === ''
) {
throw new InvalidOperationError(
'You can not have an empty string for stickiness.',
);
}
}
async validateProjectCanAccessSegments(
@ -680,6 +670,59 @@ export class FeatureToggleService {
);
}
async standardizeStrategyConfig(
projectId: string,
strategyConfig: Unsaved<IStrategyConfig>,
existing?: IFeatureStrategy,
): Promise<
{ strategyName: string } & Pick<
Partial<IFeatureStrategy>,
| 'title'
| 'disabled'
| 'variants'
| 'sortOrder'
| 'constraints'
| 'parameters'
>
> {
const { name, title, disabled, sortOrder } = strategyConfig;
let { constraints, parameters, variants } = strategyConfig;
if (constraints && constraints.length > 0) {
this.validateConstraintsLimit({
updated: constraints,
existing: existing?.constraints ?? [],
});
constraints = await this.validateConstraints(constraints);
}
if (
parameters &&
(!('stickiness' in parameters) ||
('stickiness' in parameters && parameters.stickiness === ''))
) {
parameters.stickiness =
existing?.parameters?.stickiness ||
(await this.featureStrategiesStore.getDefaultStickiness(
projectId,
));
}
if (variants && variants.length > 0) {
await variantsArraySchema.validateAsync(variants);
const fixedVariants = this.fixVariantWeights(variants);
variants = fixedVariants;
}
return {
strategyName: name,
title,
disabled,
sortOrder,
constraints,
variants,
parameters,
};
}
async unprotectedCreateStrategy(
strategyConfig: Unsaved<IStrategyConfig>,
context: IFeatureStrategyContext,
@ -694,34 +737,10 @@ export class FeatureToggleService {
strategyConfig.segments,
);
if (
strategyConfig.constraints &&
strategyConfig.constraints.length > 0
) {
this.validateConstraintsLimit({
updated: strategyConfig.constraints,
existing: [],
});
strategyConfig.constraints = await this.validateConstraints(
strategyConfig.constraints,
);
}
if (
strategyConfig.parameters &&
'stickiness' in strategyConfig.parameters &&
strategyConfig.parameters.stickiness === ''
) {
strategyConfig.parameters.stickiness = 'default';
}
if (strategyConfig.variants && strategyConfig.variants.length > 0) {
await variantsArraySchema.validateAsync(strategyConfig.variants);
const fixedVariants = this.fixVariantWeights(
strategyConfig.variants,
);
strategyConfig.variants = fixedVariants;
}
const standardizedConfig = await this.standardizeStrategyConfig(
projectId,
strategyConfig,
);
await this.validateStrategyLimit({
featureName,
@ -732,13 +751,10 @@ export class FeatureToggleService {
try {
const newFeatureStrategy =
await this.featureStrategiesStore.createStrategyFeatureEnv({
strategyName: strategyConfig.name,
title: strategyConfig.title,
disabled: strategyConfig.disabled,
constraints: strategyConfig.constraints || [],
variants: strategyConfig.variants || [],
parameters: strategyConfig.parameters || {},
sortOrder: strategyConfig.sortOrder,
...standardizedConfig,
constraints: standardizedConfig.constraints || [],
variants: standardizedConfig.variants || [],
parameters: standardizedConfig.parameters || {},
projectId,
featureName,
environment,
@ -864,25 +880,14 @@ export class FeatureToggleService {
const existingSegments = await this.segmentService.getByStrategy(id);
if (existingStrategy.id === id) {
if (updates.constraints && updates.constraints.length > 0) {
this.validateConstraintsLimit({
updated: updates.constraints,
existing: existingStrategy.constraints,
});
updates.constraints = await this.validateConstraints(
updates.constraints,
);
}
if (updates.variants && updates.variants.length > 0) {
await variantsArraySchema.validateAsync(updates.variants);
const fixedVariants = this.fixVariantWeights(updates.variants);
updates.variants = fixedVariants;
}
const standardizedUpdates = await this.standardizeStrategyConfig(
projectId,
{ ...updates, name: updates.name! },
existingStrategy,
);
const strategy = await this.featureStrategiesStore.updateStrategy(
id,
updates,
standardizedUpdates,
);
if (updates.segments && Array.isArray(updates.segments)) {

View File

@ -248,7 +248,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
});
return Number.isInteger(max) ? max + 1 : 0;
}
private async getDefaultStickiness(projectName: string): Promise<string> {
async getDefaultStickiness(projectName: string): Promise<string> {
const defaultFromDb = await this.db(T.projectSettings)
.select('default_stickiness')
.where('project', projectName)
@ -264,9 +264,9 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
strategyConfig.featureName,
strategyConfig.environment,
));
const stickiness = await this.getDefaultStickiness(
strategyConfig.projectId,
);
const stickiness =
strategyConfig.parameters?.stickiness ??
(await this.getDefaultStickiness(strategyConfig.projectId));
strategyConfig.parameters = parametersWithDefaults(
strategyConfig,
stickiness,

View File

@ -141,7 +141,11 @@ test('Should be able to update existing strategy configuration', async () => {
TEST_AUDIT_USER,
);
expect(createdConfig.id).toEqual(updatedConfig.id);
expect(updatedConfig.parameters).toEqual({ b2b: 'true' });
expect(updatedConfig.parameters).toEqual({
b2b: 'true',
// make sure stickiness is preserved
stickiness: createdConfig.parameters?.stickiness,
});
});
test('Should be able to get strategy by id', async () => {
@ -758,34 +762,79 @@ test('Should return last seen at per environment', async () => {
expect(featureToggle.lastSeenAt).toEqual(new Date(lastSeenAtStoreDate));
});
test('Should return "default" for stickiness when creating a flexibleRollout strategy with empty stickiness', async () => {
const strategy = {
name: 'flexibleRollout',
parameters: {
rollout: '100',
stickiness: '',
},
constraints: [],
};
const feature = {
name: 'test-feature-stickiness-1',
description: 'the #1 feature',
};
const projectId = 'default';
test.each([
['empty stickiness', { rollout: '100', stickiness: '' }],
['undefined stickiness', { rollout: '100' }],
['undefined parameters', undefined],
])(
'Should set project default stickiness when creating a flexibleRollout strategy with %s',
async (description, parameters) => {
const strategy = {
name: 'flexibleRollout',
parameters,
constraints: [],
};
const feature = {
name: `test-feature-create-${description.replaceAll(' ', '-')}`,
};
const projectId = 'default';
const defaultStickiness = `not-default-${description.replaceAll(' ', '-')}`;
const project = await stores.projectStore.update({
id: projectId,
name: 'stickiness-project-test',
defaultStickiness,
});
const context = {
projectId,
featureName: feature.name,
environment: DEFAULT_ENV,
};
await service.createFeatureToggle(projectId, feature, TEST_AUDIT_USER);
await service.createStrategy(
strategy,
{ projectId, featureName: feature.name, environment: DEFAULT_ENV },
TEST_AUDIT_USER,
);
await service.createFeatureToggle(projectId, feature, TEST_AUDIT_USER);
const createdStrategy = await service.createStrategy(
strategy,
context,
TEST_AUDIT_USER,
);
const featureDB = await service.getFeature({ featureName: feature.name });
const featureDB = await service.getFeature({
featureName: feature.name,
});
expect(featureDB.environments[0]).toMatchObject({
strategies: [{ parameters: { stickiness: 'default' } }],
});
});
expect(featureDB.environments[0]).toMatchObject({
strategies: [
{
parameters: {
...parameters,
stickiness: defaultStickiness,
},
},
],
});
// Verify that updating the strategy with same data is idempotent
await service.updateStrategy(
createdStrategy.id,
strategy,
context,
TEST_AUDIT_USER,
);
const featureDBAfterUpdate = await service.getFeature({
featureName: feature.name,
});
expect(featureDBAfterUpdate.environments[0]).toMatchObject({
strategies: [
{
parameters: {
...parameters,
stickiness: defaultStickiness,
},
},
],
});
},
);
test('Should not allow to add flags to archived projects', async () => {
const project = await stores.projectStore.create({

View File

@ -124,4 +124,6 @@ export interface IFeatureStrategiesStore
): Promise<IFeatureStrategy[]>;
getCustomStrategiesInUseCount(): Promise<number>;
getDefaultStickiness(projectId: string): Promise<string>;
}

View File

@ -13,25 +13,6 @@ import type {
import type { Store } from '../../types/stores/store.js';
import type { CreateFeatureStrategySchema } from '../../openapi/index.js';
export interface IProjectInsert {
id: string;
name: string;
description?: string;
updatedAt?: Date;
changeRequestsEnabled?: boolean;
mode?: ProjectMode;
featureLimit?: number;
featureNaming?: IFeatureNaming;
linkTemplates?: IProjectLinkTemplate[];
}
export interface IProjectEnterpriseSettingsUpdate {
id: string;
mode?: ProjectMode;
featureNaming?: IFeatureNaming;
linkTemplates?: IProjectLinkTemplate[];
}
export interface IProjectSettings {
mode: ProjectMode;
defaultStickiness: string;
@ -42,6 +23,22 @@ export interface IProjectSettings {
linkTemplates?: IProjectLinkTemplate[];
}
export interface IProjectInsert extends Partial<IProjectSettings> {
id: string;
name: string;
description?: string;
updatedAt?: Date;
changeRequestsEnabled?: boolean;
featureNaming?: IFeatureNaming;
}
export interface IProjectEnterpriseSettingsUpdate {
id: string;
mode?: ProjectMode;
featureNaming?: IFeatureNaming;
linkTemplates?: IProjectLinkTemplate[];
}
export interface IProjectHealthUpdate {
id: string;
health: number;