fix: reject duplicate segment names (#1475)

* fix: reject duplicate segment names

* refactor: remove unnecessary comment

* refactor: improve validateName logic with existsByName

* fix: removed unused NotFoundError import

src/lib/db/segment-store.ts

@@ -170,6 +170,15 @@ export default class SegmentStore implements ISegmentStore {
         }));
     }
 
+    async existsByName(name: string): Promise<boolean> {
+        const rows: ISegmentRow[] = await this.db
+            .select(this.prefixColumns())
+            .from(T.segments)
+            .where({ name });
+
+        return Boolean(rows[0]);
+    }
+
     prefixColumns(): string[] {
         return COLUMNS.map((c) => `${T.segments}.${c}`);
     }

src/lib/services/segment-service.ts

@@ -2,6 +2,7 @@ import { IUnleashConfig } from '../types/option';
 import { IEventStore } from '../types/stores/event-store';
 import { IUnleashStores } from '../types';
 import { Logger } from '../logger';
+import NameExistsError from '../error/name-exists-error';
 import { ISegmentStore } from '../types/stores/segment-store';
 import { IFeatureStrategy, ISegment } from '../types/model';
 import { segmentSchema } from './segment-schema';
@@ -69,6 +70,7 @@ export class SegmentService {
     async create(data: unknown, user: User): Promise<void> {
         const input = await segmentSchema.validateAsync(data);
         this.validateSegmentValuesLimit(input);
+        await this.validateName(input.name);
 
         const segment = await this.segmentStore.create(input, user);
 
@@ -82,8 +84,12 @@ 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);
+
+        if (preData.name !== input.name) {
+            await this.validateName(input.name);
+        }
+
         const segment = await this.segmentStore.update(id, input);
 
         await this.eventStore.store({
@@ -115,6 +121,16 @@ export class SegmentService {
         await this.segmentStore.removeFromStrategy(id, strategyId);
     }
 
+    async validateName(name: string): Promise<void> {
+        if (!name) {
+            throw new BadDataError('Segment name cannot be empty');
+        }
+
+        if (await this.segmentStore.existsByName(name)) {
+            throw new NameExistsError('Segment name already exists');
+        }
+    }
+
     private async validateStrategySegmentLimit(
         strategyId: string,
     ): Promise<void> {

src/lib/types/stores/segment-store.ts

@@ -23,4 +23,6 @@ export interface ISegmentStore extends Store<ISegment, number> {
     removeFromStrategy(id: number, strategyId: string): Promise<void>;
 
     getAllFeatureStrategySegments(): Promise<IFeatureStrategySegment[]>;
+
+    existsByName(name: string): Promise<boolean>;
 }

src/migrations/20220405103233-add-segments-name-index.js

@@ -0,0 +1,19 @@
+'use strict';
+
+exports.up = function (db, cb) {
+    db.runSql(
+        `
+        create index segments_name_index on segments (name);
+        `,
+        cb,
+    );
+};
+
+exports.down = function (db, cb) {
+    db.runSql(
+        `
+        drop index segments_name_index;
+        `,
+        cb,
+    );
+};

src/test/fixtures/fake-segment-store.ts

@@ -50,5 +50,9 @@ export default class FakeSegmentStore implements ISegmentStore {
         return [];
     }
 
+    async existsByName(): Promise<boolean> {
+        throw new Error('Method not implemented.');
+    }
+
     destroy(): void {}
 }