feat: add segment limits (#1469)

* feat: add segment limits

* refactor: move segment limits to constants

src/lib/services/segment-service.ts

@@ -12,6 +12,11 @@ import {
 } from '../types/events';
 import User from '../types/user';
 import { IFeatureStrategiesStore } from '../types/stores/feature-strategies-store';
+import BadDataError from '../error/bad-data-error';
+import {
+    SEGMENT_VALUES_LIMIT,
+    STRATEGY_SEGMENTS_LIMIT,
+} from '../util/segments';
 
 export class SegmentService {
     private logger: Logger;
@@ -63,6 +68,8 @@ export class SegmentService {
 
     async create(data: unknown, user: User): Promise<void> {
         const input = await segmentSchema.validateAsync(data);
+        this.validateSegmentValuesLimit(input);
+
         const segment = await this.segmentStore.create(input, user);
 
         await this.eventStore.store({
@@ -74,6 +81,8 @@ export class SegmentService {
 
     async update(id: number, data: unknown, user: User): Promise<void> {
         const input = await segmentSchema.validateAsync(data);
+        this.validateSegmentValuesLimit(input);
+
         const preData = await this.segmentStore.get(id);
         const segment = await this.segmentStore.update(id, input);
 
@@ -97,6 +106,7 @@ export class SegmentService {
 
     // Used by unleash-enterprise.
     async addToStrategy(id: number, strategyId: string): Promise<void> {
+        await this.validateStrategySegmentLimit(strategyId);
         await this.segmentStore.addToStrategy(id, strategyId);
     }
 
@@ -104,4 +114,34 @@ export class SegmentService {
     async removeFromStrategy(id: number, strategyId: string): Promise<void> {
         await this.segmentStore.removeFromStrategy(id, strategyId);
     }
+
+    private async validateStrategySegmentLimit(
+        strategyId: string,
+    ): Promise<void> {
+        const limit = STRATEGY_SEGMENTS_LIMIT;
+
+        if (typeof limit === 'undefined') {
+            return;
+        }
+
+        if ((await this.getByStrategy(strategyId)).length >= limit) {
+            throw new BadDataError(
+                `Strategies may not have more than ${limit} segments`,
+            );
+        }
+    }
+
+    private validateSegmentValuesLimit(segment: Omit<ISegment, 'id'>): void {
+        const limit = SEGMENT_VALUES_LIMIT;
+
+        const valuesCount = segment.constraints
+            .flatMap((constraint) => constraint.values?.length ?? 0)
+            .reduce((acc, length) => acc + length, 0);
+
+        if (valuesCount > limit) {
+            throw new BadDataError(
+                `Segments may not have more than ${limit} values`,
+            );
+        }
+    }
 }

src/lib/util/segments.ts

@@ -0,0 +1,2 @@
+export const SEGMENT_VALUES_LIMIT = 100;
+export const STRATEGY_SEGMENTS_LIMIT = 5;

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

@@ -9,6 +9,10 @@ import {
 } from '../../../../lib/types/model';
 import { randomId } from '../../../../lib/util/random-id';
 import User from '../../../../lib/types/user';
+import {
+    SEGMENT_VALUES_LIMIT,
+    STRATEGY_SEGMENTS_LIMIT,
+} from '../../../../lib/util/segments';
 
 let db: ITestDb;
 let app: IUnleashTest;
@@ -78,6 +82,12 @@ const mockConstraints = (): IConstraint[] => {
     }));
 };
 
+const mockConstraintValues = (length: number): string[] => {
+    return Array.from({ length }).map(() => {
+        return randomId();
+    });
+};
+
 beforeAll(async () => {
     db = await dbInit('segments', getLogger);
     app = await setupApp(db.stores);
@@ -145,3 +155,64 @@ test('should list active segments', async () => {
         collectIds([segment1, segment2]),
     );
 });
+
+test('should validate segment constraint values limit', async () => {
+    const limit = SEGMENT_VALUES_LIMIT;
+
+    const constraints: IConstraint[] = [
+        {
+            contextName: randomId(),
+            operator: 'IN',
+            values: mockConstraintValues(limit + 1),
+        },
+    ];
+
+    await expect(
+        createSegment({ name: randomId(), constraints }),
+    ).rejects.toThrow(`Segments may not have more than ${limit} values`);
+});
+
+test('should validate segment constraint values limit with multiple constraints', async () => {
+    const limit = SEGMENT_VALUES_LIMIT;
+
+    const constraints: IConstraint[] = [
+        {
+            contextName: randomId(),
+            operator: 'IN',
+            values: mockConstraintValues(limit),
+        },
+        {
+            contextName: randomId(),
+            operator: 'IN',
+            values: mockConstraintValues(1),
+        },
+    ];
+
+    await expect(
+        createSegment({ name: randomId(), constraints }),
+    ).rejects.toThrow(`Segments may not have more than ${limit} values`);
+});
+
+test('should validate feature strategy segment limit', async () => {
+    const limit = STRATEGY_SEGMENTS_LIMIT;
+
+    await createSegment({ name: 'S1', constraints: [] });
+    await createSegment({ name: 'S2', constraints: [] });
+    await createSegment({ name: 'S3', constraints: [] });
+    await createSegment({ name: 'S4', constraints: [] });
+    await createSegment({ name: 'S5', constraints: [] });
+    await createSegment({ name: 'S6', constraints: [] });
+    await createFeatureToggle(mockFeatureToggle());
+    const [feature1] = await fetchFeatures();
+    const segments = await fetchSegments();
+
+    await addSegmentToStrategy(segments[0].id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segments[1].id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segments[2].id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segments[3].id, feature1.strategies[0].id);
+    await addSegmentToStrategy(segments[4].id, feature1.strategies[0].id);
+
+    await expect(
+        addSegmentToStrategy(segments[5].id, feature1.strategies[0].id),
+    ).rejects.toThrow(`Strategies may not have more than ${limit} segments`);
+});