fix: clone segments when cloning a toggle (#1678)

* refactor: merge segment test files

* fix: clone segments when cloning a toggle

src/lib/services/feature-toggle-service.ts

@@ -41,7 +41,6 @@ import {
     FeatureToggleLegacy,
     FeatureToggleWithEnvironment,
     IConstraint,
-    IEnvironmentDetail,
     IFeatureEnvironmentInfo,
     IFeatureOverview,
     IFeatureStrategy,
@@ -70,6 +69,7 @@ import {
 } from '../util/validators/constraint-types';
 import { IContextFieldStore } from 'lib/types/stores/context-field-store';
 import { Saved, Unsaved } from '../types/saved';
+import { SegmentService } from './segment-service';
 
 interface IFeatureContext {
     featureName: string;
@@ -103,6 +103,8 @@ class FeatureToggleService {
 
     private contextFieldStore: IContextFieldStore;
 
+    private segmentService: SegmentService;
+
     constructor(
         {
             featureStrategiesStore,
@@ -125,6 +127,7 @@ class FeatureToggleService {
             | 'contextFieldStore'
         >,
         { getLogger }: Pick<IUnleashConfig, 'getLogger'>,
+        segmentService: SegmentService,
     ) {
         this.logger = getLogger('services/feature-toggle-service.ts');
         this.featureStrategiesStore = featureStrategiesStore;
@@ -135,6 +138,7 @@ class FeatureToggleService {
         this.eventStore = eventStore;
         this.featureEnvironmentStore = featureEnvironmentStore;
         this.contextFieldStore = contextFieldStore;
+        this.segmentService = segmentService;
     }
 
     async validateFeatureContext({
@@ -618,37 +622,29 @@ class FeatureToggleService {
             );
 
         const newToggle = { ...cToggle, name: newFeatureName };
-
-        // Create feature toggle
         const created = await this.createFeatureToggle(
             projectId,
             newToggle,
             userName,
         );
 
-        const createStrategies = [];
+        const tasks = newToggle.environments.flatMap((e) =>
-        newToggle.environments.forEach((e: IEnvironmentDetail) =>
+            e.strategies.map((s) => {
-            e.strategies.forEach((s: IStrategyConfig) => {
                 if (replaceGroupId && s.parameters.hasOwnProperty('groupId')) {
                     s.parameters.groupId = newFeatureName;
                 }
-                delete s.id;
+                const context = {
-                createStrategies.push(
+                    projectId,
-                    this.createStrategy(
+                    featureName: newFeatureName,
-                        s,
+                    environment: e.name,
-                        {
+                };
-                            projectId,
+                return this.createStrategy(s, context, userName).then((s2) =>
-                            featureName: newFeatureName,
+                    this.segmentService.cloneStrategySegments(s.id, s2.id),
-                            environment: e.name,
-                        },
-                        userName,
-                    ),
                 );
             }),
         );
 
-        // Create strategies
+        await Promise.allSettled(tasks);
-        await Promise.allSettled(createStrategies);
         return created;
     }
 

src/lib/services/index.ts

@@ -62,7 +62,12 @@ export const createServices = (
     const versionService = new VersionService(stores, config);
     const healthService = new HealthService(stores, config);
     const userFeedbackService = new UserFeedbackService(stores, config);
-    const featureToggleServiceV2 = new FeatureToggleService(stores, config);
+    const segmentService = new SegmentService(stores, config);
+    const featureToggleServiceV2 = new FeatureToggleService(
+        stores,
+        config,
+        segmentService,
+    );
     const environmentService = new EnvironmentService(stores, config);
     const featureTagService = new FeatureTagService(stores, config);
     const projectHealthService = new ProjectHealthService(
@@ -77,7 +82,6 @@ export const createServices = (
         featureToggleServiceV2,
     );
     const userSplashService = new UserSplashService(stores, config);
-    const segmentService = new SegmentService(stores, config);
     const openApiService = new OpenApiService(config);
     const clientSpecService = new ClientSpecService(config);
 

src/lib/services/segment-service.ts

@@ -109,6 +109,24 @@ export class SegmentService {
         });
     }
 
+    async cloneStrategySegments(
+        sourceStrategyId: string,
+        targetStrategyId: string,
+    ): Promise<void> {
+        const sourceStrategySegments = await this.getByStrategy(
+            sourceStrategyId,
+        );
+
+        await Promise.all(
+            sourceStrategySegments.map((sourceStrategySegment) => {
+                return this.addToStrategy(
+                    sourceStrategySegment.id,
+                    targetStrategyId,
+                );
+            }),
+        );
+    }
+
     // Used by unleash-enterprise.
     async addToStrategy(id: number, strategyId: string): Promise<void> {
         await this.validateStrategySegmentLimit(strategyId);

src/test/e2e/api/client/global.segment.e2e.test.ts

@@ -1,145 +0,0 @@
-import dbInit, { ITestDb } from '../../helpers/database-init';
-import getLogger from '../../../fixtures/no-logger';
-import {
-    IUnleashTest,
-    setupAppWithCustomConfig,
-} from '../../helpers/test-helper';
-import {
-    IConstraint,
-    IFeatureToggleClient,
-    ISegment,
-} from '../../../../lib/types/model';
-import { randomId } from '../../../../lib/util/random-id';
-import User from '../../../../lib/types/user';
-
-let db: ITestDb;
-let app: IUnleashTest;
-
-const FEATURES_ADMIN_BASE_PATH = '/api/admin/features';
-const FEATURES_CLIENT_BASE_PATH = '/api/client/features';
-
-interface ApiResponse {
-    features: IFeatureToggleClient[];
-    version: number;
-    segments: ISegment[];
-}
-
-const fetchSegments = (): Promise<ISegment[]> => {
-    return app.services.segmentService.getAll();
-};
-
-const fetchFeatures = (): Promise<IFeatureToggleClient[]> => {
-    return app.request
-        .get(FEATURES_ADMIN_BASE_PATH)
-        .expect(200)
-        .then((res) => res.body.features);
-};
-
-const fetchClientResponse = (): Promise<ApiResponse> => {
-    return app.request
-        .get(FEATURES_CLIENT_BASE_PATH)
-        .set('Unleash-Client-Spec', '4.2.0')
-        .expect(200)
-        .then((res) => res.body);
-};
-
-const createSegment = (postData: object): Promise<unknown> => {
-    const user = { email: 'test@example.com' } as User;
-    return app.services.segmentService.create(postData, user);
-};
-
-const createFeatureToggle = (
-    postData: object,
-    expectStatusCode = 201,
-): Promise<unknown> => {
-    return app.request
-        .post(FEATURES_ADMIN_BASE_PATH)
-        .send(postData)
-        .expect(expectStatusCode);
-};
-
-const addSegmentToStrategy = (
-    segmentId: number,
-    strategyId: string,
-): Promise<unknown> => {
-    return app.services.segmentService.addToStrategy(segmentId, strategyId);
-};
-
-const mockFeatureToggle = (): object => {
-    return {
-        name: randomId(),
-        strategies: [{ name: randomId(), constraints: [], parameters: {} }],
-    };
-};
-
-const mockConstraints = (): IConstraint[] => {
-    return Array.from({ length: 5 }).map(() => ({
-        values: ['x', 'y', 'z'],
-        operator: 'IN',
-        contextName: 'a',
-    }));
-};
-
-const createTestSegments = async (): Promise<void> => {
-    const constraints = mockConstraints();
-    await createSegment({ name: 'S1', constraints });
-    await createSegment({ name: 'S2', constraints });
-    await createSegment({ name: 'S3', constraints });
-    await createFeatureToggle(mockFeatureToggle());
-    await createFeatureToggle(mockFeatureToggle());
-    await createFeatureToggle(mockFeatureToggle());
-    const [feature1, feature2] = await fetchFeatures();
-    const [segment1, segment2] = await fetchSegments();
-
-    await addSegmentToStrategy(segment1.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature2.strategies[0].id);
-};
-
-beforeAll(async () => {
-    const config = { inlineSegmentConstraints: false };
-    db = await dbInit('global_segments', getLogger, config);
-    app = await setupAppWithCustomConfig(db.stores, config);
-});
-
-afterAll(async () => {
-    await app.destroy();
-    await db.destroy();
-});
-
-afterEach(async () => {
-    await db.stores.segmentStore.deleteAll();
-    await db.stores.featureToggleStore.deleteAll();
-});
-
-test('should return segments in base of toggle response if inline is disabled', async () => {
-    await createTestSegments();
-
-    const clientFeatures = await fetchClientResponse();
-    expect(clientFeatures.segments.length).toBeDefined();
-});
-
-test('should only send segments that are in use', async () => {
-    await createTestSegments();
-
-    const clientFeatures = await fetchClientResponse();
-    //3 segments were created in createTestSegments, only 2 are in use
-    expect(clientFeatures.segments.length).toEqual(2);
-});
-
-test('should send all segments that are in use by feature', async () => {
-    await createTestSegments();
-
-    const clientFeatures = await fetchClientResponse();
-    const globalSegments = clientFeatures.segments;
-    expect(globalSegments).toHaveLength(2);
-
-    const globalSegmentIds = globalSegments.map((segment) => segment.id);
-    const allSegmentIds = clientFeatures.features
-        .map((feat) => feat.strategies.map((strategy) => strategy.segments))
-        .flat()
-        .flat()
-        .filter((x) => !!x);
-    const toggleSegmentIds = [...new Set(allSegmentIds)];
-    expect(globalSegmentIds).toEqual(toggleSegmentIds);
-});

src/test/e2e/api/client/segment.e2e.test.ts

@@ -81,6 +81,36 @@ const mockConstraintValues = (length: number): string[] => {
     });
 };
 
+const fetchClientResponse = (): Promise<{
+    features: IFeatureToggleClient[];
+    version: number;
+    segments: ISegment[];
+}> => {
+    return app.request
+        .get(FEATURES_CLIENT_BASE_PATH)
+        .set('Unleash-Client-Spec', '4.2.0')
+        .expect(200)
+        .then((res) => res.body);
+};
+
+const createTestSegments = async () => {
+    const constraints = mockConstraints();
+
+    await createSegment({ name: 'S1', constraints });
+    await createSegment({ name: 'S2', constraints });
+    await createSegment({ name: 'S3', constraints });
+
+    await createFeatureToggle(mockFeatureToggle());
+    await createFeatureToggle(mockFeatureToggle());
+    await createFeatureToggle(mockFeatureToggle());
+
+    const [feature1, feature2] = await fetchFeatures();
+    const [segment1, segment2] = await fetchSegments();
+    await addSegmentToStrategy(segment1.id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segment2.id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segment2.id, feature2.strategies[0].id);
+};
+
 beforeAll(async () => {
     db = await dbInit('segments', getLogger);
     app = await setupApp(db.stores);
@@ -97,37 +127,6 @@ afterEach(async () => {
     await db.stores.eventStore.deleteAll();
 });
 
-test('should inline segment constraints into features by default', async () => {
-    const constraints = mockConstraints();
-    await createSegment({ name: 'S1', constraints });
-    await createSegment({ name: 'S2', constraints });
-    await createSegment({ name: 'S3', constraints });
-    await createFeatureToggle(mockFeatureToggle());
-    await createFeatureToggle(mockFeatureToggle());
-    await createFeatureToggle(mockFeatureToggle());
-    const [feature1, feature2, feature3] = await fetchFeatures();
-    const [segment1, segment2, segment3] = await fetchSegments();
-
-    await addSegmentToStrategy(segment1.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature2.strategies[0].id);
-    await addSegmentToStrategy(segment3.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment3.id, feature2.strategies[0].id);
-    await addSegmentToStrategy(segment3.id, feature3.strategies[0].id);
-
-    const clientFeatures = await fetchClientFeatures();
-    const clientStrategies = clientFeatures.flatMap((f) => f.strategies);
-    const clientConstraints = clientStrategies.flatMap((s) => s.constraints);
-    const clientValues = clientConstraints.flatMap((c) => c.values);
-    const uniqueValues = [...new Set(clientValues)];
-
-    expect(clientFeatures.length).toEqual(3);
-    expect(clientStrategies.length).toEqual(3);
-    expect(clientConstraints.length).toEqual(5 * 6);
-    expect(clientValues.length).toEqual(5 * 6 * 3);
-    expect(uniqueValues.length).toEqual(3);
-});
-
 test('should validate segment constraint values limit', async () => {
     const constraints: IConstraint[] = [
         {
@@ -189,22 +188,77 @@ test('should validate feature strategy segment limit', async () => {
     );
 });
 
-test('should only return segments to clients with the segments capability', async () => {
+test('should clone feature strategy segments', async () => {
     const constraints = mockConstraints();
     await createSegment({ name: 'S1', constraints });
-    await createSegment({ name: 'S2', constraints });
-    await createSegment({ name: 'S3', constraints });
-    await createFeatureToggle(mockFeatureToggle());
     await createFeatureToggle(mockFeatureToggle());
     await createFeatureToggle(mockFeatureToggle());
+
     const [feature1, feature2] = await fetchFeatures();
+    const strategy1 = feature1.strategies[0].id;
+    const strategy2 = feature2.strategies[0].id;
+    const [segment1] = await fetchSegments();
+    await addSegmentToStrategy(segment1.id, feature1.strategies[0].id);
+
+    let segments1 = await app.services.segmentService.getByStrategy(strategy1);
+    let segments2 = await app.services.segmentService.getByStrategy(strategy2);
+    expect(collectIds(segments1)).toEqual([segment1.id]);
+    expect(collectIds(segments2)).toEqual([]);
+
+    await app.services.segmentService.cloneStrategySegments(
+        strategy1,
+        strategy2,
+    );
+
+    segments1 = await app.services.segmentService.getByStrategy(strategy1);
+    segments2 = await app.services.segmentService.getByStrategy(strategy2);
+    expect(collectIds(segments1)).toEqual([segment1.id]);
+    expect(collectIds(segments2)).toEqual([segment1.id]);
+});
+
+test('should store segment-created and segment-deleted events', async () => {
+    const constraints = mockConstraints();
+    const user = new User({ id: 1, email: 'test@example.com' });
+
+    await createSegment({ name: 'S1', constraints });
+    const [segment1] = await fetchSegments();
+    await app.services.segmentService.delete(segment1.id, user);
+    const events = await db.stores.eventStore.getEvents();
+
+    expect(events[0].type).toEqual('segment-deleted');
+    expect(events[0].data.id).toEqual(segment1.id);
+    expect(events[1].type).toEqual('segment-created');
+    expect(events[1].data.id).toEqual(segment1.id);
+});
+
+test('should inline segment constraints into features by default', async () => {
+    await createTestSegments();
+
+    const [feature1, feature2, feature3] = await fetchFeatures();
+    const [, , segment3] = await fetchSegments();
+    await addSegmentToStrategy(segment3.id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segment3.id, feature2.strategies[0].id);
+    await addSegmentToStrategy(segment3.id, feature3.strategies[0].id);
+
+    const clientFeatures = await fetchClientFeatures();
+    const clientStrategies = clientFeatures.flatMap((f) => f.strategies);
+    const clientConstraints = clientStrategies.flatMap((s) => s.constraints);
+    const clientValues = clientConstraints.flatMap((c) => c.values);
+    const uniqueValues = [...new Set(clientValues)];
+
+    expect(clientFeatures.length).toEqual(3);
+    expect(clientStrategies.length).toEqual(3);
+    expect(clientConstraints.length).toEqual(5 * 6);
+    expect(clientValues.length).toEqual(5 * 6 * 3);
+    expect(uniqueValues.length).toEqual(3);
+});
+
+test('should only return segments to clients that support the spec', async () => {
+    await createTestSegments();
+
     const [segment1, segment2] = await fetchSegments();
     const segmentIds = collectIds([segment1, segment2]);
 
-    await addSegmentToStrategy(segment1.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature1.strategies[0].id);
-    await addSegmentToStrategy(segment2.id, feature2.strategies[0].id);
-
     const unknownClientResponse = await app.request
         .get(FEATURES_CLIENT_BASE_PATH)
         .expect(200)
@@ -227,17 +281,33 @@ test('should only return segments to clients with the segments capability', asyn
     expect(supportedClientConstraints.length).toEqual(0);
 });
 
-test('should store segment-created and segment-deleted events', async () => {
+test('should return segments in base of toggle response if inline is disabled', async () => {
-    const constraints = mockConstraints();
+    await createTestSegments();
-    const user = new User({ id: 1, email: 'test@example.com' });
 
-    await createSegment({ name: 'S1', constraints });
+    const clientFeatures = await fetchClientResponse();
-    const [segment1] = await fetchSegments();
+    expect(clientFeatures.segments.length).toBeDefined();
-    await app.services.segmentService.delete(segment1.id, user);
+});
-    const events = await db.stores.eventStore.getEvents();
+
-
+test('should only send segments that are in use', async () => {
-    expect(events[0].type).toEqual('segment-deleted');
+    await createTestSegments();
-    expect(events[0].data.id).toEqual(segment1.id);
+
-    expect(events[1].type).toEqual('segment-created');
+    const clientFeatures = await fetchClientResponse();
-    expect(events[1].data.id).toEqual(segment1.id);
+    expect(clientFeatures.segments.length).toEqual(2);
+});
+
+test('should send all segments that are in use by feature', async () => {
+    await createTestSegments();
+
+    const clientFeatures = await fetchClientResponse();
+    const globalSegments = clientFeatures.segments;
+    expect(globalSegments).toHaveLength(2);
+
+    const globalSegmentIds = globalSegments.map((segment) => segment.id);
+    const allSegmentIds = clientFeatures.features
+        .map((feat) => feat.strategies.map((strategy) => strategy.segments))
+        .flat()
+        .flat()
+        .filter((x) => !!x);
+    const toggleSegmentIds = [...new Set(allSegmentIds)];
+    expect(globalSegmentIds).toEqual(toggleSegmentIds);
 });

src/test/e2e/services/access-service.e2e.test.ts

@@ -14,6 +14,7 @@ import FeatureToggleService from '../../../lib/services/feature-toggle-service';
 import ProjectService from '../../../lib/services/project-service';
 import { createTestConfig } from '../../config/test-config';
 import { DEFAULT_PROJECT } from '../../../lib/types/project';
+import { SegmentService } from '../../../lib/services/segment-service';
 
 let db: ITestDb;
 let stores: IUnleashStores;
@@ -212,7 +213,11 @@ beforeAll(async () => {
     editorRole = roles.find((r) => r.name === RoleName.EDITOR);
     adminRole = roles.find((r) => r.name === RoleName.ADMIN);
     readRole = roles.find((r) => r.name === RoleName.VIEWER);
-    featureToggleService = new FeatureToggleService(stores, config);
+    featureToggleService = new FeatureToggleService(
+        stores,
+        config,
+        new SegmentService(stores, config),
+    );
     projectService = new ProjectService(
         stores,
         config,

src/test/e2e/services/api-token-service.e2e.test.ts

@@ -8,6 +8,7 @@ import { addDays, subDays } from 'date-fns';
 import ProjectService from '../../../lib/services/project-service';
 import FeatureToggleService from '../../../lib/services/feature-toggle-service';
 import { AccessService } from '../../../lib/services/access-service';
+import { SegmentService } from '../../../lib/services/segment-service';
 
 let db;
 let stores;
@@ -21,7 +22,11 @@ beforeAll(async () => {
     db = await dbInit('api_token_service_serial', getLogger);
     stores = db.stores;
     const accessService = new AccessService(stores, config);
-    const featureToggleService = new FeatureToggleService(stores, config);
+    const featureToggleService = new FeatureToggleService(
+        stores,
+        config,
+        new SegmentService(stores, config),
+    );
     const project = {
         id: 'test-project',
         name: 'Test Project',

src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts

@@ -3,6 +3,7 @@ import { createTestConfig } from '../../config/test-config';
 import dbInit from '../helpers/database-init';
 import { DEFAULT_ENV } from '../../../lib/util/constants';
 import { StrategySchema } from '../../../lib/openapi/spec/strategy-schema';
+import { SegmentService } from '../../../lib/services/segment-service';
 
 let stores;
 let db;
@@ -15,7 +16,11 @@ beforeAll(async () => {
         config.getLogger,
     );
     stores = db.stores;
-    service = new FeatureToggleService(stores, config);
+    service = new FeatureToggleService(
+        stores,
+        config,
+        new SegmentService(stores, config),
+    );
 });
 
 afterAll(async () => {

src/test/e2e/services/project-health-service.e2e.test.ts

@@ -7,6 +7,7 @@ import ProjectHealthService from '../../../lib/services/project-health-service';
 import { createTestConfig } from '../../config/test-config';
 import { IUnleashStores } from '../../../lib/types';
 import { IUser } from '../../../lib/server-impl';
+import { SegmentService } from '../../../lib/services/segment-service';
 
 let stores: IUnleashStores;
 let db: ITestDb;
@@ -25,7 +26,11 @@ beforeAll(async () => {
         email: 'test@getunleash.io',
     });
     accessService = new AccessService(stores, config);
-    featureToggleService = new FeatureToggleService(stores, config);
+    featureToggleService = new FeatureToggleService(
+        stores,
+        config,
+        new SegmentService(stores, config),
+    );
     projectService = new ProjectService(
         stores,
         config,

src/test/e2e/services/project-service.e2e.test.ts

@@ -9,6 +9,7 @@ import { RoleName } from '../../../lib/types/model';
 import { randomId } from '../../../lib/util/random-id';
 import EnvironmentService from '../../../lib/services/environment-service';
 import IncompatibleProjectError from '../../../lib/error/incompatible-project-error';
+import { SegmentService } from '../../../lib/services/segment-service';
 
 let stores;
 let db: ITestDb;
@@ -32,7 +33,11 @@ beforeAll(async () => {
         experimental: { environments: { enabled: true } },
     });
     accessService = new AccessService(stores, config);
-    featureToggleService = new FeatureToggleService(stores, config);
+    featureToggleService = new FeatureToggleService(
+        stores,
+        config,
+        new SegmentService(stores, config),
+    );
     environmentService = new EnvironmentService(stores, config);
     projectService = new ProjectService(
         stores,