chore: refactor segments to stop depending on the implementation (#3315)

## About the changes
- Introducing ISegmentService interface to decouple from the actual
implementation
- Moving UpsertSegmentSchema to OSS to be able to use types
- Added comments where our code is coupled with segments just to
highlight and have a conversation about some use cases if needed, but
they can be removed before merging
- Removed segment service from some project features as it was not used

src/lib/features/export-import-toggles/export-import.e2e.test.ts

@@ -20,6 +20,7 @@ import { DEFAULT_ENV } from '../../util';
 import {
     ContextFieldSchema,
     ImportTogglesSchema,
+    UpsertSegmentSchema,
     VariantsSchema,
 } from '../../openapi';
 import User from '../../types/user';
@@ -117,7 +118,7 @@ const createProject = async () => {
         .expect(200);
 };
 
-const createSegment = (postData: object): Promise<ISegment> => {
+const createSegment = (postData: UpsertSegmentSchema): Promise<ISegment> => {
     return app.services.segmentService.create(postData, {
         email: 'test@example.com',
     });

src/lib/openapi/index.ts

@@ -117,6 +117,7 @@ import {
     updateTagTypeSchema,
     updateUserSchema,
     upsertContextFieldSchema,
+    upsertSegmentSchema,
     upsertStrategySchema,
     userSchema,
     usersGroupsBaseSchema,
@@ -272,6 +273,7 @@ export const schemas = {
     updateUserSchema,
     updateTagsSchema,
     upsertContextFieldSchema,
+    upsertSegmentSchema,
     upsertStrategySchema,
     userSchema,
     usersGroupsBaseSchema,

src/lib/openapi/spec/index.ts

@@ -131,4 +131,5 @@ export * from './import-toggles-validate-schema';
 export * from './import-toggles-schema';
 export * from './stickiness-schema';
 export * from './tags-bulk-add-schema';
+export * from './upsert-segment-schema';
 export * from './batch-features-schema';

src/lib/openapi/spec/upsert-segment-schema.ts

@@ -0,0 +1,34 @@
+import { FromSchema } from 'json-schema-to-ts';
+import { constraintSchema } from './constraint-schema';
+
+export const upsertSegmentSchema = {
+    $id: '#/components/schemas/upsertSegmentSchema',
+    type: 'object',
+    required: ['name', 'constraints'],
+    properties: {
+        name: {
+            type: 'string',
+        },
+        description: {
+            type: 'string',
+            nullable: true,
+        },
+        project: {
+            type: 'string',
+            nullable: true,
+        },
+        constraints: {
+            type: 'array',
+            items: {
+                $ref: '#/components/schemas/constraintSchema',
+            },
+        },
+    },
+    components: {
+        schemas: {
+            constraintSchema,
+        },
+    },
+} as const;
+
+export type UpsertSegmentSchema = FromSchema<typeof upsertSegmentSchema>;

src/lib/proxy/proxy-repository.ts

@@ -139,7 +139,7 @@ export class ProxyRepository
 
     private async segmentsForToken(): Promise<Segment[]> {
         return mapSegmentsForClient(
-            await this.services.segmentService.getAll(),
+            await this.services.segmentService.getAll(), // TODO coupled with enterprise feature
         );
     }
 

src/lib/routes/admin-api/project/project-features.ts

@@ -39,11 +39,7 @@ import {
     UpdateFeatureSchema,
     UpdateFeatureStrategySchema,
 } from '../../../openapi';
-import {
-    OpenApiService,
-    SegmentService,
-    FeatureToggleService,
-} from '../../../services';
+import { OpenApiService, FeatureToggleService } from '../../../services';
 import { querySchema } from '../../../schema/feature-schema';
 import NotFoundError from '../../../error/notfound-error';
 import { BatchStaleSchema } from '../../../openapi/spec/batch-stale-schema';
@@ -97,24 +93,17 @@ export default class ProjectFeaturesController extends Controller {
 
     private openApiService: OpenApiService;
 
-    private segmentService: SegmentService;
-
     private flagResolver: IFlagResolver;
 
     private readonly logger: Logger;
 
     constructor(
         config: IUnleashConfig,
-        {
-            featureToggleServiceV2,
-            openApiService,
-            segmentService,
-        }: ProjectFeaturesServices,
+        { featureToggleServiceV2, openApiService }: ProjectFeaturesServices,
     ) {
         super(config);
         this.featureService = featureToggleServiceV2;
         this.openApiService = openApiService;
-        this.segmentService = segmentService;
         this.flagResolver = config.flagResolver;
         this.logger = config.getLogger('/admin-api/project/features.ts');
 

src/lib/routes/client-api/feature.ts

@@ -10,7 +10,6 @@ import NotFoundError from '../../error/notfound-error';
 import { IAuthRequest } from '../unleash-types';
 import ApiUser from '../../types/api-user';
 import { ALL, isAllProjects } from '../../types/models/api-token';
-import { SegmentService } from '../../services/segment-service';
 import { FeatureConfigurationClient } from '../../types/stores/feature-strategies-store';
 import { ClientSpecService } from '../../services/client-spec-service';
 import { OpenApiService } from '../../services/openapi-service';
@@ -25,6 +24,7 @@ import {
     clientFeaturesSchema,
     ClientFeaturesSchema,
 } from '../../openapi/spec/client-features-schema';
+import { ISegmentService } from 'lib/segments/segment-service-interface';
 
 const version = 2;
 
@@ -38,7 +38,7 @@ export default class FeatureController extends Controller {
 
     private featureToggleServiceV2: FeatureToggleService;
 
-    private segmentService: SegmentService;
+    private segmentService: ISegmentService;
 
     private clientSpecService: ClientSpecService;
 

src/lib/segments/segment-service-interface.ts

@@ -0,0 +1,29 @@
+import { UpsertSegmentSchema } from 'lib/openapi';
+import { ISegment, IUser } from 'lib/types';
+
+export interface ISegmentService {
+    updateStrategySegments: (
+        strategyId: string,
+        segmentIds: number[],
+    ) => Promise<void>;
+
+    addToStrategy(id: number, strategyId: string): Promise<void>;
+
+    getByStrategy(strategyId: string): Promise<ISegment[]>;
+
+    getActive(): Promise<ISegment[]>;
+
+    getAll(): Promise<ISegment[]>;
+
+    create(
+        data: UpsertSegmentSchema,
+        user: Partial<Pick<IUser, 'username' | 'email'>>,
+    ): Promise<ISegment>;
+
+    delete(id: number, user: IUser): Promise<void>;
+
+    cloneStrategySegments(
+        sourceStrategyId: string,
+        targetStrategyId: string,
+    ): Promise<void>;
+}

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

@@ -71,7 +71,6 @@ 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';
 import { SetStrategySortOrderSchema } from 'lib/openapi/spec/set-strategy-sort-order-schema';
 import { getDefaultStrategy } from '../util/feature-evaluator/helpers';
 import { AccessService } from './access-service';
@@ -83,6 +82,7 @@ import {
 import NoAccessError from '../error/no-access-error';
 import { IFeatureProjectUserParams } from '../routes/admin-api/project/project-features';
 import { unique } from '../util/unique';
+import { ISegmentService } from 'lib/segments/segment-service-interface';
 
 interface IFeatureContext {
     featureName: string;
@@ -124,7 +124,7 @@ class FeatureToggleService {
 
     private contextFieldStore: IContextFieldStore;
 
-    private segmentService: SegmentService;
+    private segmentService: ISegmentService;
 
     private accessService: AccessService;
 
@@ -155,7 +155,7 @@ class FeatureToggleService {
             getLogger,
             flagResolver,
         }: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
-        segmentService: SegmentService,
+        segmentService: ISegmentService,
         accessService: AccessService,
     ) {
         this.logger = getLogger('services/feature-toggle-service.ts');
@@ -375,7 +375,10 @@ class FeatureToggleService {
         const { featureName, projectId, environment } = context;
         await this.validateFeatureContext(context);
 
-        if (strategyConfig.constraints?.length > 0) {
+        if (
+            strategyConfig.constraints &&
+            strategyConfig.constraints.length > 0
+        ) {
             strategyConfig.constraints = await this.validateConstraints(
                 strategyConfig.constraints,
             );
@@ -406,7 +409,7 @@ class FeatureToggleService {
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
             const segments = await this.segmentService.getByStrategy(
                 newFeatureStrategy.id,
-            );
+            ); // TODO coupled with enterprise feature
             const strategy = this.featureStrategyToPublic(
                 newFeatureStrategy,
                 segments,
@@ -468,7 +471,7 @@ class FeatureToggleService {
         this.validateFeatureStrategyContext(existingStrategy, context);
 
         if (existingStrategy.id === id) {
-            if (updates.constraints?.length > 0) {
+            if (updates.constraints && updates.constraints.length > 0) {
                 updates.constraints = await this.validateConstraints(
                     updates.constraints,
                 );
@@ -488,7 +491,7 @@ class FeatureToggleService {
 
             const segments = await this.segmentService.getByStrategy(
                 strategy.id,
-            );
+            ); // TODO coupled with enterprise feature
 
             // Store event!
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
@@ -534,7 +537,7 @@ class FeatureToggleService {
             const tags = await this.tagStore.getAllTagsForFeature(featureName);
             const segments = await this.segmentService.getByStrategy(
                 strategy.id,
-            );
+            ); // TODO coupled with enterprise feature
             const data = this.featureStrategyToPublic(strategy, segments);
             const preData = this.featureStrategyToPublic(
                 existingStrategy,
@@ -633,7 +636,7 @@ class FeatureToggleService {
                 const segments =
                     (await this.segmentService.getByStrategy(strat.id)).map(
                         (segment) => segment.id,
-                    ) ?? [];
+                    ) ?? []; // TODO coupled with enterprise feature
                 result.push({
                     id: strat.id,
                     name: strat.strategyName,
@@ -950,7 +953,7 @@ class FeatureToggleService {
             strategyId,
         );
 
-        const segments = await this.segmentService.getByStrategy(strategyId);
+        const segments = await this.segmentService.getByStrategy(strategyId); // TODO coupled with enterprise feature
         let result: Saved<IStrategyConfig> = {
             id: strategy.id,
             name: strategy.strategyName,

src/lib/services/index.ts

@@ -138,7 +138,7 @@ export const createServices = (
     const versionService = new VersionService(stores, config);
     const healthService = new HealthService(stores, config);
     const userFeedbackService = new UserFeedbackService(stores, config);
-    const segmentService = new SegmentService(stores, config);
+    const segmentService = new SegmentService(stores, config); // TODO coupled with enterprise feature
     const featureToggleServiceV2 = new FeatureToggleService(
         stores,
         config,

src/lib/services/instance-stats-service.ts

@@ -193,7 +193,7 @@ export class InstanceStatsService {
             this.groupStore.count(),
             this.roleStore.count(),
             this.environmentStore.count(),
-            this.segmentStore.count(),
+            this.segmentStore.count(), // TODO coupled with enterprise feature
             this.strategyStore.count(),
             this.hasSAML(),
             this.hasOIDC(),

src/lib/services/playground-service.ts

@@ -8,14 +8,14 @@ import { IUnleashConfig } from 'lib/types';
 import { offlineUnleashClient } from '../util/offline-unleash-client';
 import { FeatureInterface } from 'lib/util/feature-evaluator/feature';
 import { FeatureStrategiesEvaluationResult } from 'lib/util/feature-evaluator/client';
-import { SegmentService } from './segment-service';
+import { ISegmentService } from 'lib/segments/segment-service-interface';
 
 export class PlaygroundService {
     private readonly logger: Logger;
 
     private readonly featureToggleService: FeatureToggleService;
 
-    private readonly segmentService: SegmentService;
+    private readonly segmentService: ISegmentService;
 
     constructor(
         config: IUnleashConfig,

src/lib/services/segment-service.ts

@@ -14,8 +14,9 @@ import {
 import User from '../types/user';
 import { IFeatureStrategiesStore } from '../types/stores/feature-strategies-store';
 import BadDataError from '../error/bad-data-error';
+import { ISegmentService } from '../segments/segment-service-interface';
 
-export class SegmentService {
+export class SegmentService implements ISegmentService {
     private logger: Logger;
 
     private segmentStore: ISegmentStore;

src/lib/services/state-service.test.ts

@@ -20,7 +20,6 @@ function getSetup() {
     return {
         stateService: new StateService(stores, {
             getLogger,
-            flagResolver: { isEnabled: () => true, getAll: () => ({}) },
         }),
         stores,
     };
@@ -65,10 +64,6 @@ async function setupV3VariantsCompatibilityScenario(
     return {
         stateService: new StateService(stores, {
             getLogger,
-            flagResolver: {
-                isEnabled: () => true,
-                getAll: () => ({}),
-            },
         }),
         stores,
     };
@@ -579,7 +574,6 @@ test('exporting to new format works', async () => {
     const stores = createStores();
     const stateService = new StateService(stores, {
         getLogger,
-        flagResolver: { isEnabled: () => true, getAll: () => ({}) },
     });
     await stores.projectStore.create({
         id: 'fancy',

src/lib/services/state-service.ts

@@ -50,7 +50,6 @@ import { DEFAULT_ENV } from '../util/constants';
 import { GLOBAL_ENV } from '../types/environment';
 import { ISegmentStore } from '../types/stores/segment-store';
 import { PartialSome } from '../types/partial';
-import { IFlagResolver } from 'lib/types';
 
 export interface IBackupOption {
     includeFeatureToggles: boolean;
@@ -93,14 +92,9 @@ export default class StateService {
 
     private segmentStore: ISegmentStore;
 
-    private flagResolver: IFlagResolver;
-
     constructor(
         stores: IUnleashStores,
-        {
-            getLogger,
-            flagResolver,
-        }: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
+        { getLogger }: Pick<IUnleashConfig, 'getLogger'>,
     ) {
         this.eventStore = stores.eventStore;
         this.toggleStore = stores.featureToggleStore;
@@ -113,7 +107,6 @@ export default class StateService {
         this.featureTagStore = stores.featureTagStore;
         this.environmentStore = stores.environmentStore;
         this.segmentStore = stores.segmentStore;
-        this.flagResolver = flagResolver;
         this.logger = getLogger('services/state-service.js');
     }
 

src/lib/types/services.ts

@@ -24,7 +24,6 @@ import FeatureTagService from '../services/feature-tag-service';
 import ProjectHealthService from '../services/project-health-service';
 import ClientMetricsServiceV2 from '../services/client-metrics/metrics-service-v2';
 import UserSplashService from '../services/user-splash-service';
-import { SegmentService } from '../services/segment-service';
 import { OpenApiService } from '../services/openapi-service';
 import { ClientSpecService } from '../services/client-spec-service';
 import { PlaygroundService } from 'lib/services/playground-service';
@@ -41,6 +40,7 @@ import { AccountService } from '../services/account-service';
 import { SchedulerService } from '../services/scheduler-service';
 import { Knex } from 'knex';
 import ExportImportService from '../features/export-import-toggles/export-import-service';
+import { ISegmentService } from 'lib/segments/segment-service-interface';
 
 export interface IUnleashServices {
     accessService: AccessService;
@@ -76,7 +76,7 @@ export interface IUnleashServices {
     userService: UserService;
     versionService: VersionService;
     userSplashService: UserSplashService;
-    segmentService: SegmentService;
+    segmentService: ISegmentService;
     openApiService: OpenApiService;
     clientSpecService: ClientSpecService;
     patService: PatService;

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

@@ -14,6 +14,7 @@ import {
 } from '../../../../lib/util/segments';
 import { collectIds } from '../../../../lib/util/collect-ids';
 import { arraysHaveSameItems } from '../../../../lib/util/arraysHaveSameItems';
+import { UpsertSegmentSchema } from 'lib/openapi';
 
 let db: ITestDb;
 let app: IUnleashTest;
@@ -39,7 +40,7 @@ const fetchClientFeatures = (): Promise<IFeatureToggleClient[]> => {
         .then((res) => res.body.features);
 };
 
-const createSegment = (postData: object): Promise<unknown> => {
+const createSegment = (postData: UpsertSegmentSchema): Promise<unknown> => {
     return app.services.segmentService.create(postData, {
         email: 'test@example.com',
     });

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -3993,6 +3993,32 @@ Stats are divided into current and previous **windows**.
         ],
         "type": "object",
       },
+      "upsertSegmentSchema": {
+        "properties": {
+          "constraints": {
+            "items": {
+              "$ref": "#/components/schemas/constraintSchema",
+            },
+            "type": "array",
+          },
+          "description": {
+            "nullable": true,
+            "type": "string",
+          },
+          "name": {
+            "type": "string",
+          },
+          "project": {
+            "nullable": true,
+            "type": "string",
+          },
+        },
+        "required": [
+          "name",
+          "constraints",
+        ],
+        "type": "object",
+      },
       "upsertStrategySchema": {
         "properties": {
           "description": {

src/test/e2e/seed/segment.seed.ts

@@ -9,6 +9,7 @@ import {
     ISegment,
 } from '../../../lib/types/model';
 import { IUnleashTest, setupApp } from '../helpers/test-helper';
+import { UpsertSegmentSchema } from 'lib/openapi';
 
 interface ISeedSegmentSpec {
     featuresCount: number;
@@ -41,7 +42,7 @@ const fetchFeatures = (app: IUnleashTest): Promise<IFeatureToggleClient[]> => {
 
 const createSegment = (
     app: IUnleashTest,
-    postData: object,
+    postData: UpsertSegmentSchema,
 ): Promise<unknown> => {
     const user = { email: 'test@example.com' } as User;
     return app.services.segmentService.create(postData, user);
@@ -86,7 +87,7 @@ const seedConstraints = (spec: ISeedSegmentSpec): IConstraint[] => {
     }));
 };
 
-const seedSegments = (spec: ISeedSegmentSpec): Partial<ISegment>[] => {
+const seedSegments = (spec: ISeedSegmentSpec): UpsertSegmentSchema[] => {
     return Array.from({ length: spec.segmentsPerFeature }).map((v, i) => {
         return {
             name: `${seedSchema}_segment_${i}`,

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

@@ -11,11 +11,12 @@ import { GroupService } from '../../../lib/services/group-service';
 import EnvironmentService from '../../../lib/services/environment-service';
 import { NoAccessError } from '../../../lib/error';
 import { SKIP_CHANGE_REQUEST } from '../../../lib/types';
+import { ISegmentService } from '../../../lib/segments/segment-service-interface';
 
 let stores;
 let db;
 let service: FeatureToggleService;
-let segmentService: SegmentService;
+let segmentService: ISegmentService;
 let environmentService: EnvironmentService;
 let unleashConfig;
 

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

@@ -20,12 +20,13 @@ import { playgroundStrategyEvaluation } from '../../../lib/openapi/spec/playgrou
 import { PlaygroundSegmentSchema } from 'lib/openapi/spec/playground-segment-schema';
 import { GroupService } from '../../../lib/services/group-service';
 import { AccessService } from '../../../lib/services/access-service';
+import { ISegmentService } from '../../../lib/segments/segment-service-interface';
 
 let stores: IUnleashStores;
 let db: ITestDb;
 let service: PlaygroundService;
 let featureToggleService: FeatureToggleService;
-let segmentService: SegmentService;
+let segmentService: ISegmentService;
 
 beforeAll(async () => {
     const config = createTestConfig();
@@ -147,7 +148,7 @@ export const seedDatabaseForPlaygroundTest = async (
                                 featureName: feature.name,
                                 environment,
                                 strategyName: strategy.name,
-                                projectId: feature.project,
+                                projectId: feature.project!,
                             },
                         );
 
@@ -156,7 +157,7 @@ export const seedDatabaseForPlaygroundTest = async (
                                 strategySegments.map((segmentId) =>
                                     database.stores.segmentStore.addToStrategy(
                                         segmentId,
-                                        strategy.id,
+                                        strategy.id!,
                                     ),
                                 ),
                             );