Skip to content

Commit

Permalink
feat: add segment limits (#1469)
Browse files Browse the repository at this point in the history
* feat: add segment limits

* refactor: move segment limits to constants
  • Loading branch information
olav committed Apr 1, 2022
1 parent cf06b56 commit 1da3878
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 0 deletions.
40 changes: 40 additions & 0 deletions src/lib/services/segment-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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({
Expand All @@ -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);

Expand All @@ -97,11 +106,42 @@ 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);
}

// Used by unleash-enterprise.
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`,
);
}
}
}
2 changes: 2 additions & 0 deletions src/lib/util/segments.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const SEGMENT_VALUES_LIMIT = 100;
export const STRATEGY_SEGMENTS_LIMIT = 5;
71 changes: 71 additions & 0 deletions src/test/e2e/api/client/segment.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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`);
});

0 comments on commit 1da3878

Please sign in to comment.