From 794327f8e00e0200f8ad1261d8d3d61370006d3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nuno=20G=C3=B3is?= <github@nunogois.com>
Date: Wed, 6 Apr 2022 14:01:50 +0100
Subject: [PATCH] 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                   |  9 +++++++++
 src/lib/services/segment-service.ts           | 18 +++++++++++++++++-
 src/lib/types/stores/segment-store.ts         |  2 ++
 .../20220405103233-add-segments-name-index.js | 19 +++++++++++++++++++
 src/test/fixtures/fake-segment-store.ts       |  4 ++++
 5 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 src/migrations/20220405103233-add-segments-name-index.js

diff --git a/src/lib/db/segment-store.ts b/src/lib/db/segment-store.ts
index cfc0231342..8cab85b51a 100644
--- a/src/lib/db/segment-store.ts
+++ b/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}`);
     }
diff --git a/src/lib/services/segment-service.ts b/src/lib/services/segment-service.ts
index 8df4634daa..79c1235565 100644
--- a/src/lib/services/segment-service.ts
+++ b/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> {
diff --git a/src/lib/types/stores/segment-store.ts b/src/lib/types/stores/segment-store.ts
index 79da738a9d..aa56aec213 100644
--- a/src/lib/types/stores/segment-store.ts
+++ b/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>;
 }
diff --git a/src/migrations/20220405103233-add-segments-name-index.js b/src/migrations/20220405103233-add-segments-name-index.js
new file mode 100644
index 0000000000..672d6552bf
--- /dev/null
+++ b/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,
+    );
+};
diff --git a/src/test/fixtures/fake-segment-store.ts b/src/test/fixtures/fake-segment-store.ts
index 644210282a..ee810e2f34 100644
--- a/src/test/fixtures/fake-segment-store.ts
+++ b/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 {}
 }