diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index ce72c728ce..04308a0d82 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -64,6 +64,7 @@ Object { "file": undefined, "keepExisting": false, }, + "inlineSegmentConstraints": true, "listen": Object { "host": undefined, "port": 4242, @@ -71,6 +72,7 @@ Object { "preHook": undefined, "preRouterHook": undefined, "secureHeaders": false, + "segmentValuesLimit": 100, "server": Object { "baseUriPath": "", "cdnPrefix": undefined, @@ -90,6 +92,7 @@ Object { "db": true, "ttlHours": 48, }, + "strategySegmentsLimit": 5, "ui": Object { "flags": Object { "E": true, diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index 863be47de9..10c8761720 100644 --- a/src/lib/create-config.ts +++ b/src/lib/create-config.ts @@ -28,6 +28,12 @@ import { mapLegacyToken, validateApiToken, } from './types/models/api-token'; +import { parseEnvVarBoolean, parseEnvVarNumber } from './util/env'; +import { IExperimentalOptions } from './experimental'; +import { + DEFAULT_SEGMENT_VALUES_LIMIT, + DEFAULT_STRATEGY_SEGMENTS_LIMIT, +} from './util/segments'; const safeToUpper = (s: string) => (s ? s.toUpperCase() : s); @@ -38,33 +44,12 @@ export function authTypeFromString( return IAuthType[safeToUpper(s)] || defaultType; } -function safeNumber(envVar, defaultVal): number { - if (envVar) { - try { - return Number.parseInt(envVar, 10); - } catch (err) { - return defaultVal; - } - } else { - return defaultVal; - } -} - -function safeBoolean(envVar: string, defaultVal: boolean): boolean { - if (envVar) { - return envVar === 'true' || envVar === '1' || envVar === 't'; - } - return defaultVal; -} - function mergeAll(objects: Partial[]): T { return merge.all(objects.filter((i) => i)); } -function loadExperimental(options: IUnleashOptions): any { - const experimental = options.experimental || {}; - - return experimental; +function loadExperimental(options: IUnleashOptions): IExperimentalOptions { + return options.experimental || {}; } function loadUI(options: IUnleashOptions): IUIConfig { @@ -81,7 +66,7 @@ const defaultDbOptions: IDBOption = { user: process.env.DATABASE_USERNAME, password: process.env.DATABASE_PASSWORD, host: process.env.DATABASE_HOST, - port: safeNumber(process.env.DATABASE_PORT, 5432), + port: parseEnvVarNumber(process.env.DATABASE_PORT, 5432), database: process.env.DATABASE_NAME || 'unleash', ssl: process.env.DATABASE_SSL != null @@ -91,9 +76,9 @@ const defaultDbOptions: IDBOption = { version: process.env.DATABASE_VERSION, acquireConnectionTimeout: secondsToMilliseconds(30), pool: { - min: safeNumber(process.env.DATABASE_POOL_MIN, 0), - max: safeNumber(process.env.DATABASE_POOL_MAX, 4), - idleTimeoutMillis: safeNumber( + min: parseEnvVarNumber(process.env.DATABASE_POOL_MIN, 0), + max: parseEnvVarNumber(process.env.DATABASE_POOL_MAX, 4), + idleTimeoutMillis: parseEnvVarNumber( process.env.DATABASE_POOL_IDLE_TIMEOUT_MS, secondsToMilliseconds(30), ), @@ -105,14 +90,14 @@ const defaultDbOptions: IDBOption = { }; const defaultSessionOption: ISessionOption = { - ttlHours: safeNumber(process.env.SESSION_TTL_HOURS, 48), + ttlHours: parseEnvVarNumber(process.env.SESSION_TTL_HOURS, 48), db: true, }; const defaultServerOption: IServerOption = { pipe: undefined, host: process.env.HTTP_HOST, - port: safeNumber(process.env.HTTP_PORT || process.env.PORT, 4242), + port: parseEnvVarNumber(process.env.HTTP_PORT || process.env.PORT, 4242), baseUriPath: formatBaseUri(process.env.BASE_URI_PATH), cdnPrefix: process.env.CDN_PREFIX, unleashUrl: process.env.UNLEASH_URL || 'http://localhost:4242', @@ -120,11 +105,11 @@ const defaultServerOption: IServerOption = { keepAliveTimeout: minutesToMilliseconds(1), headersTimeout: secondsToMilliseconds(61), enableRequestLogger: false, - gracefulShutdownEnable: safeBoolean( + gracefulShutdownEnable: parseEnvVarBoolean( process.env.GRACEFUL_SHUTDOWN_ENABLE, true, ), - gracefulShutdownTimeout: safeNumber( + gracefulShutdownTimeout: parseEnvVarNumber( process.env.GRACEFUL_SHUTDOWN_TIMEOUT, secondsToMilliseconds(1), ), @@ -133,11 +118,11 @@ const defaultServerOption: IServerOption = { const defaultVersionOption: IVersionOption = { url: process.env.UNLEASH_VERSION_URL || 'https://version.unleash.run', - enable: safeBoolean(process.env.CHECK_VERSION, true), + enable: parseEnvVarBoolean(process.env.CHECK_VERSION, true), }; const defaultAuthentication: IAuthOption = { - enableApiToken: safeBoolean(process.env.AUTH_ENABLE_API_TOKEN, true), + enableApiToken: parseEnvVarBoolean(process.env.AUTH_ENABLE_API_TOKEN, true), type: authTypeFromString(process.env.AUTH_TYPE), customAuthHandler: defaultCustomAuthDenyAll, createAdminUser: true, @@ -146,14 +131,17 @@ const defaultAuthentication: IAuthOption = { const defaultImport: IImportOption = { file: process.env.IMPORT_FILE, - dropBeforeImport: safeBoolean(process.env.IMPORT_DROP_BEFORE_IMPORT, false), - keepExisting: safeBoolean(process.env.IMPORT_KEEP_EXISTING, false), + dropBeforeImport: parseEnvVarBoolean( + process.env.IMPORT_DROP_BEFORE_IMPORT, + false, + ), + keepExisting: parseEnvVarBoolean(process.env.IMPORT_KEEP_EXISTING, false), }; const defaultEmail: IEmailOption = { host: process.env.EMAIL_HOST, - secure: safeBoolean(process.env.EMAIL_SECURE, false), - port: safeNumber(process.env.EMAIL_PORT, 587), + secure: parseEnvVarBoolean(process.env.EMAIL_SECURE, false), + port: parseEnvVarNumber(process.env.EMAIL_PORT, 587), sender: process.env.EMAIL_SENDER || 'noreply@unleash-hosted.com', smtpuser: process.env.EMAIL_USER, smtppass: process.env.EMAIL_PASSWORD, @@ -342,19 +330,35 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { } const secureHeaders = - options.secureHeaders || safeBoolean(process.env.SECURE_HEADERS, false); + options.secureHeaders || + parseEnvVarBoolean(process.env.SECURE_HEADERS, false); const enableOAS = - options.enableOAS || safeBoolean(process.env.ENABLE_OAS, false); + options.enableOAS || parseEnvVarBoolean(process.env.ENABLE_OAS, false); const disableLegacyFeaturesApi = options.disableLegacyFeaturesApi || - safeBoolean(process.env.DISABLE_LEGACY_FEATURES_API, false); + parseEnvVarBoolean(process.env.DISABLE_LEGACY_FEATURES_API, false); const additionalCspAllowedDomains: ICspDomainConfig = parseCspConfig(options.additionalCspAllowedDomains) || parseCspEnvironmentVariables(); + const inlineSegmentConstraints = + typeof options.inlineSegmentConstraints === 'boolean' + ? options.inlineSegmentConstraints + : true; + + const segmentValuesLimit = parseEnvVarNumber( + process.env.UNLEASH_SEGMENT_VALUES_LIMIT, + DEFAULT_SEGMENT_VALUES_LIMIT, + ); + + const strategySegmentsLimit = parseEnvVarNumber( + process.env.UNLEASH_STRATEGY_SEGMENTS_LIMIT, + DEFAULT_STRATEGY_SEGMENTS_LIMIT, + ); + return { db, session, @@ -377,6 +381,9 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { eventBus: new EventEmitter(), environmentEnableOverrides, additionalCspAllowedDomains, + inlineSegmentConstraints, + segmentValuesLimit, + strategySegmentsLimit, }; } diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 731a7872a4..8281a24c07 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -10,7 +10,6 @@ import { import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client-store'; import { DEFAULT_ENV } from '../util/constants'; import { PartialDeep } from '../types/partial'; -import { IExperimentalOptions } from '../experimental'; import EventEmitter from 'events'; export interface FeaturesTable { @@ -31,7 +30,7 @@ export default class FeatureToggleClientStore private logger: Logger; - private experimental: IExperimentalOptions; + private inlineSegmentConstraints: boolean; private timer: Function; @@ -39,11 +38,11 @@ export default class FeatureToggleClientStore db: Knex, eventBus: EventEmitter, getLogger: LogProvider, - experimental: IExperimentalOptions, + inlineSegmentConstraints: boolean, ) { this.db = db; this.logger = getLogger('feature-toggle-client-store.ts'); - this.experimental = experimental; + this.inlineSegmentConstraints = inlineSegmentConstraints; this.timer = (action) => metricsHelper.wrapTimer(eventBus, DB_TIME, { store: 'feature-toggle', @@ -59,9 +58,6 @@ export default class FeatureToggleClientStore const environment = featureQuery?.environment || DEFAULT_ENV; const stopTimer = this.timer('getFeatureAdmin'); - const { inlineSegmentConstraints = false } = - this.experimental?.segments ?? {}; - let selectColumns = [ 'features.name as name', 'features.description as description', @@ -143,9 +139,9 @@ export default class FeatureToggleClientStore FeatureToggleClientStore.rowToStrategy(r), ); } - if (inlineSegmentConstraints && r.segment_id) { + if (this.inlineSegmentConstraints && r.segment_id) { this.addSegmentToStrategy(feature, r); - } else if (!inlineSegmentConstraints && r.segment_id) { + } else if (!this.inlineSegmentConstraints && r.segment_id) { this.addSegmentIdsToStrategy(feature, r); } feature.impressionData = r.impression_data; diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 07e7b1106b..5d3ec09dc1 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -70,7 +70,7 @@ export const createStores = ( db, eventBus, getLogger, - config.experimental, + config.inlineSegmentConstraints, ), environmentStore: new EnvironmentStore(db, eventBus, getLogger), featureTagStore: new FeatureTagStore(db, eventBus, getLogger), diff --git a/src/lib/experimental.ts b/src/lib/experimental.ts index 9c258f415d..f0c018e1b4 100644 --- a/src/lib/experimental.ts +++ b/src/lib/experimental.ts @@ -1,24 +1,9 @@ export interface IExperimentalOptions { metricsV2?: IExperimentalToggle; clientFeatureMemoize?: IExperimentalToggle; - segments?: IExperimentalSegments; anonymiseEventLog?: boolean; } export interface IExperimentalToggle { enabled: boolean; } - -export interface IExperimentalSegments { - enableSegmentsClientApi: boolean; - enableSegmentsAdminApi: boolean; - inlineSegmentConstraints: boolean; -} - -export const experimentalSegmentsConfig = (): IExperimentalSegments => { - return { - enableSegmentsAdminApi: true, - enableSegmentsClientApi: true, - inlineSegmentConstraints: true, - }; -}; diff --git a/src/lib/routes/admin-api/config.test.ts b/src/lib/routes/admin-api/config.test.ts index 1d9b4c538c..16849a5ab5 100644 --- a/src/lib/routes/admin-api/config.test.ts +++ b/src/lib/routes/admin-api/config.test.ts @@ -4,6 +4,10 @@ import { createTestConfig } from '../../../test/config/test-config'; import createStores from '../../../test/fixtures/store'; import getApp from '../../app'; import { createServices } from '../../services'; +import { + DEFAULT_SEGMENT_VALUES_LIMIT, + DEFAULT_STRATEGY_SEGMENTS_LIMIT, +} from '../../util/segments'; const uiConfig = { headerBackground: 'red', @@ -46,14 +50,15 @@ beforeEach(async () => { afterEach(() => { destroy(); }); -test('should get ui config', () => { - expect.assertions(2); - return request + +test('should get ui config', async () => { + const { body } = await request .get(`${base}/api/admin/ui-config`) .expect('Content-Type', /json/) - .expect(200) - .expect((res) => { - expect(res.body.slogan === 'hello').toBe(true); - expect(res.body.headerBackground === 'red').toBe(true); - }); + .expect(200); + + expect(body.slogan).toEqual('hello'); + expect(body.headerBackground).toEqual('red'); + expect(body.segmentValuesLimit).toEqual(DEFAULT_SEGMENT_VALUES_LIMIT); + expect(body.strategySegmentsLimit).toEqual(DEFAULT_STRATEGY_SEGMENTS_LIMIT); }); diff --git a/src/lib/routes/admin-api/config.ts b/src/lib/routes/admin-api/config.ts index 16298da06a..f5be01b034 100644 --- a/src/lib/routes/admin-api/config.ts +++ b/src/lib/routes/admin-api/config.ts @@ -1,22 +1,35 @@ import { Request, Response } from 'express'; import { IUnleashServices } from '../../types/services'; -import { IAuthType, IUnleashConfig } from '../../types/option'; +import { IAuthType, IUIConfig, IUnleashConfig } from '../../types/option'; import version from '../../util/version'; - import Controller from '../controller'; -import VersionService from '../../services/version-service'; +import VersionService, { IVersionHolder } from '../../services/version-service'; import SettingService from '../../services/setting-service'; import { simpleAuthKey, SimpleAuthSettings, } from '../../types/settings/simple-auth-settings'; +interface IUIConfigResponse extends IUIConfig { + version: string; + unleashUrl: string; + baseUriPath: string; + authenticationType?: IAuthType; + versionInfo: IVersionHolder; + disablePasswordAuth: boolean; + segmentValuesLimit: number; + strategySegmentsLimit: number; +} + class ConfigController extends Controller { private versionService: VersionService; private settingService: SettingService; - private uiConfig: any; + private uiConfig: Omit< + IUIConfigResponse, + 'versionInfo' | 'disablePasswordAuth' + >; constructor( config: IUnleashConfig, @@ -32,15 +45,20 @@ class ConfigController extends Controller { config.authentication && config.authentication.type; this.uiConfig = { ...config.ui, - authenticationType, + version, unleashUrl: config.server.unleashUrl, baseUriPath: config.server.baseUriPath, - version, + authenticationType, + segmentValuesLimit: config.segmentValuesLimit, + strategySegmentsLimit: config.strategySegmentsLimit, }; this.get('/', this.getUIConfig); } - async getUIConfig(req: Request, res: Response): Promise { + async getUIConfig( + req: Request, + res: Response, + ): Promise { const config = this.uiConfig; const simpleAuthSettings = await this.settingService.get(simpleAuthKey); diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index d49a94f962..dea5e8da3e 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -46,10 +46,10 @@ export default class FeatureController extends Controller { this.featureToggleServiceV2 = featureToggleServiceV2; this.segmentService = segmentService; this.logger = config.getLogger('client-api/feature.js'); + this.useGlobalSegments = !this.config.inlineSegmentConstraints; + this.get('/', this.getAll); this.get('/:featureName', this.getFeatureToggle); - this.useGlobalSegments = - experimental && !experimental?.segments?.inlineSegmentConstraints; if (experimental && experimental.clientFeatureMemoize) { // @ts-ignore diff --git a/src/lib/routes/client-api/index.ts b/src/lib/routes/client-api/index.ts index aaa85eed85..7265ce289f 100644 --- a/src/lib/routes/client-api/index.ts +++ b/src/lib/routes/client-api/index.ts @@ -5,7 +5,6 @@ import MetricsController from './metrics'; import RegisterController from './register'; import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types'; -import { SegmentsController } from './segments'; const apiDef = require('./api-def.json'); @@ -17,13 +16,6 @@ export default class ClientApi extends Controller { this.use('/features', new FeatureController(services, config).router); this.use('/metrics', new MetricsController(services, config).router); this.use('/register', new RegisterController(services, config).router); - - if (config.experimental?.segments?.enableSegmentsClientApi) { - this.use( - '/segments', - new SegmentsController(services, config).router, - ); - } } index(req: Request, res: Response): void { diff --git a/src/lib/routes/client-api/segments.ts b/src/lib/routes/client-api/segments.ts deleted file mode 100644 index aed95ab0f4..0000000000 --- a/src/lib/routes/client-api/segments.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Response } from 'express'; -import Controller from '../controller'; -import { IUnleashConfig } from '../../types/option'; -import { IUnleashServices } from '../../types'; -import { Logger } from '../../logger'; -import { SegmentService } from '../../services/segment-service'; -import { IAuthRequest } from '../unleash-types'; - -export class SegmentsController extends Controller { - private logger: Logger; - - private segmentService: SegmentService; - - constructor( - { segmentService }: Pick, - config: IUnleashConfig, - ) { - super(config); - this.logger = config.getLogger('/client-api/segments.ts'); - this.segmentService = segmentService; - - this.get('/active', this.getActive); - } - - async getActive(req: IAuthRequest, res: Response): Promise { - const segments = await this.segmentService.getActive(); - res.json({ segments }); - } -} diff --git a/src/lib/services/segment-service.ts b/src/lib/services/segment-service.ts index bb2748fd7d..880654e486 100644 --- a/src/lib/services/segment-service.ts +++ b/src/lib/services/segment-service.ts @@ -14,10 +14,6 @@ import { 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; @@ -28,6 +24,8 @@ export class SegmentService { private eventStore: IEventStore; + private config: IUnleashConfig; + constructor( { segmentStore, @@ -37,12 +35,13 @@ export class SegmentService { IUnleashStores, 'segmentStore' | 'featureStrategiesStore' | 'eventStore' >, - { getLogger }: Pick, + config: IUnleashConfig, ) { this.segmentStore = segmentStore; this.featureStrategiesStore = featureStrategiesStore; this.eventStore = eventStore; - this.logger = getLogger('services/segment-service.ts'); + this.logger = config.getLogger('services/segment-service.ts'); + this.config = config; } async get(id: number): Promise { @@ -69,7 +68,7 @@ export class SegmentService { async create(data: unknown, user: User): Promise { const input = await segmentSchema.validateAsync(data); - SegmentService.validateSegmentValuesLimit(input); + this.validateSegmentValuesLimit(input); await this.validateName(input.name); const segment = await this.segmentStore.create(input, user); @@ -83,7 +82,7 @@ export class SegmentService { async update(id: number, data: unknown, user: User): Promise { const input = await segmentSchema.validateAsync(data); - SegmentService.validateSegmentValuesLimit(input); + this.validateSegmentValuesLimit(input); const preData = await this.segmentStore.get(id); if (preData.name !== input.name) { @@ -134,31 +133,28 @@ export class SegmentService { private async validateStrategySegmentLimit( strategyId: string, ): Promise { - const limit = STRATEGY_SEGMENTS_LIMIT; + const { strategySegmentsLimit } = this.config; - if (typeof limit === 'undefined') { - return; - } - - if ((await this.getByStrategy(strategyId)).length >= limit) { + if ( + (await this.getByStrategy(strategyId)).length >= + strategySegmentsLimit + ) { throw new BadDataError( - `Strategies may not have more than ${limit} segments`, + `Strategies may not have more than ${strategySegmentsLimit} segments`, ); } } - private static validateSegmentValuesLimit( - segment: Omit, - ): void { - const limit = SEGMENT_VALUES_LIMIT; + private validateSegmentValuesLimit(segment: Omit): void { + const { segmentValuesLimit } = this.config; const valuesCount = segment.constraints .flatMap((constraint) => constraint.values?.length ?? 0) .reduce((acc, length) => acc + length, 0); - if (valuesCount > limit) { + if (valuesCount > segmentValuesLimit) { throw new BadDataError( - `Segments may not have more than ${limit} values`, + `Segments may not have more than ${segmentValuesLimit} values`, ); } } diff --git a/src/lib/types/option.ts b/src/lib/types/option.ts index fd42168e07..7f83f067f5 100644 --- a/src/lib/types/option.ts +++ b/src/lib/types/option.ts @@ -104,6 +104,7 @@ export interface IUnleashOptions { eventHook?: EventHook; enterpriseVersion?: string; disableLegacyFeaturesApi?: boolean; + inlineSegmentConstraints?: boolean; } export interface IEmailOption { @@ -176,4 +177,7 @@ export interface IUnleashConfig { eventBus: EventEmitter; disableLegacyFeaturesApi?: boolean; environmentEnableOverrides?: string[]; + inlineSegmentConstraints: boolean; + segmentValuesLimit: number; + strategySegmentsLimit: number; } diff --git a/src/lib/util/env.test.ts b/src/lib/util/env.test.ts new file mode 100644 index 0000000000..13cc809ad1 --- /dev/null +++ b/src/lib/util/env.test.ts @@ -0,0 +1,25 @@ +import { parseEnvVarBoolean, parseEnvVarNumber } from './env'; + +test('parseEnvVarNumber', () => { + expect(parseEnvVarNumber('', 1)).toEqual(1); + expect(parseEnvVarNumber('', 2)).toEqual(2); + expect(parseEnvVarNumber(' ', 1)).toEqual(1); + expect(parseEnvVarNumber('a', 1)).toEqual(1); + expect(parseEnvVarNumber('1', 1)).toEqual(1); + expect(parseEnvVarNumber('2', 1)).toEqual(2); + expect(parseEnvVarNumber('2e2', 1)).toEqual(2); + expect(parseEnvVarNumber('-1', 1)).toEqual(-1); +}); + +test('parseEnvVarBoolean', () => { + expect(parseEnvVarBoolean('', true)).toEqual(true); + expect(parseEnvVarBoolean('', false)).toEqual(false); + expect(parseEnvVarBoolean(' ', false)).toEqual(false); + expect(parseEnvVarBoolean('1', false)).toEqual(true); + expect(parseEnvVarBoolean('2', false)).toEqual(false); + expect(parseEnvVarBoolean('t', false)).toEqual(true); + expect(parseEnvVarBoolean('f', false)).toEqual(false); + expect(parseEnvVarBoolean('true', false)).toEqual(true); + expect(parseEnvVarBoolean('false', false)).toEqual(false); + expect(parseEnvVarBoolean('test', false)).toEqual(false); +}); diff --git a/src/lib/util/env.ts b/src/lib/util/env.ts new file mode 100644 index 0000000000..b8b603ae66 --- /dev/null +++ b/src/lib/util/env.ts @@ -0,0 +1,20 @@ +export function parseEnvVarNumber(envVar: string, defaultVal: number): number { + const parsed = Number.parseInt(envVar, 10); + + if (Number.isNaN(parsed)) { + return defaultVal; + } + + return parsed; +} + +export function parseEnvVarBoolean( + envVar: string, + defaultVal: boolean, +): boolean { + if (envVar) { + return envVar === 'true' || envVar === '1' || envVar === 't'; + } + + return defaultVal; +} diff --git a/src/lib/util/segments.ts b/src/lib/util/segments.ts index 9837119c2a..d8831f073d 100644 --- a/src/lib/util/segments.ts +++ b/src/lib/util/segments.ts @@ -1,2 +1,2 @@ -export const SEGMENT_VALUES_LIMIT = 100; -export const STRATEGY_SEGMENTS_LIMIT = 5; +export const DEFAULT_SEGMENT_VALUES_LIMIT = 100; +export const DEFAULT_STRATEGY_SEGMENTS_LIMIT = 5; diff --git a/src/server-dev.ts b/src/server-dev.ts index b15ae7d4d1..a331426075 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -2,7 +2,6 @@ import { start } from './lib/server-impl'; import { createConfig } from './lib/create-config'; import { LogLevel } from './lib/logger'; import { ApiTokenType } from './lib/types/models/api-token'; -import { experimentalSegmentsConfig } from './lib/experimental'; process.nextTick(async () => { try { @@ -33,7 +32,6 @@ process.nextTick(async () => { }, experimental: { metricsV2: { enabled: true }, - segments: experimentalSegmentsConfig(), anonymiseEventLog: false, }, authentication: { diff --git a/src/test/config/test-config.ts b/src/test/config/test-config.ts index da029dd9e4..02b41a027e 100644 --- a/src/test/config/test-config.ts +++ b/src/test/config/test-config.ts @@ -5,9 +5,7 @@ import { IUnleashOptions, } from '../../lib/types/option'; import getLogger from '../fixtures/no-logger'; - import { createConfig } from '../../lib/create-config'; -import { experimentalSegmentsConfig } from '../../lib/experimental'; function mergeAll(objects: Partial[]): T { return merge.all(objects.filter((i) => i)); @@ -19,7 +17,6 @@ export function createTestConfig(config?: IUnleashOptions): IUnleashConfig { authentication: { type: IAuthType.NONE, createAdminUser: false }, server: { secret: 'really-secret' }, session: { db: false }, - experimental: { segments: experimentalSegmentsConfig() }, versionCheck: { enable: false }, enableOAS: true, }; diff --git a/src/test/e2e/api/client/global.segment.e2e.test.ts b/src/test/e2e/api/client/global.segment.e2e.test.ts index 4d6fe00c19..a9af13cac5 100644 --- a/src/test/e2e/api/client/global.segment.e2e.test.ts +++ b/src/test/e2e/api/client/global.segment.e2e.test.ts @@ -96,21 +96,9 @@ const createTestSegments = async (): Promise => { }; beforeAll(async () => { - const experimentalConfig = { - segments: { - enableSegmentsAdminApi: true, - enableSegmentsClientApi: true, - inlineSegmentConstraints: false, - }, - }; - - db = await dbInit('global_segments', getLogger, { - experimental: experimentalConfig, - }); - - app = await setupAppWithCustomConfig(db.stores, { - experimental: experimentalConfig, - }); + const config = { inlineSegmentConstraints: false }; + db = await dbInit('global_segments', getLogger, config); + app = await setupAppWithCustomConfig(db.stores, config); }); afterAll(async () => { @@ -143,6 +131,8 @@ test('should send all segments that are in use by feature', async () => { const clientFeatures = await fetchClientResponse(); const globalSegments = clientFeatures.segments; + expect(globalSegments).toHaveLength(2); + const globalSegmentIds = globalSegments.map((segment) => segment.id); const allSegmentIds = clientFeatures.features .map((feat) => feat.strategies.map((strategy) => strategy.segments)) @@ -150,6 +140,5 @@ test('should send all segments that are in use by feature', async () => { .flat() .filter((x) => !!x); const toggleSegmentIds = [...new Set(allSegmentIds)]; - expect(globalSegmentIds).toEqual(toggleSegmentIds); }); diff --git a/src/test/e2e/api/client/segment.e2e.test.ts b/src/test/e2e/api/client/segment.e2e.test.ts index 921fe7298f..c9a43ced08 100644 --- a/src/test/e2e/api/client/segment.e2e.test.ts +++ b/src/test/e2e/api/client/segment.e2e.test.ts @@ -1,7 +1,6 @@ import dbInit, { ITestDb } from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; import { IUnleashTest, setupApp } from '../../helpers/test-helper'; -import { collectIds } from '../../../../lib/util/collect-ids'; import { IConstraint, IFeatureToggleClient, @@ -10,8 +9,8 @@ import { import { randomId } from '../../../../lib/util/random-id'; import User from '../../../../lib/types/user'; import { - SEGMENT_VALUES_LIMIT, - STRATEGY_SEGMENTS_LIMIT, + DEFAULT_SEGMENT_VALUES_LIMIT, + DEFAULT_STRATEGY_SEGMENTS_LIMIT, } from '../../../../lib/util/segments'; let db: ITestDb; @@ -45,13 +44,6 @@ const fetchGlobalSegments = (): Promise => { .then((res) => res.body.segments); }; -const fetchClientSegmentsActive = (): Promise => { - return app.request - .get('/api/client/segments/active') - .expect(200) - .then((res) => res.body.segments); -}; - const createSegment = (postData: object): Promise => { const user = { email: 'test@example.com' } as User; return app.services.segmentService.create(postData, user); @@ -141,52 +133,28 @@ test('should add segments to features as constraints', async () => { expect(uniqueValues.length).toEqual(3); }); -test('should list active segments', async () => { - const constraints = mockConstraints(); - await createSegment({ name: 'S1', constraints }); - await createSegment({ name: 'S2', constraints }); - await createSegment({ name: 'S3', constraints }); - await createFeatureToggle(mockFeatureToggle()); - await createFeatureToggle(mockFeatureToggle()); - await createFeatureToggle(mockFeatureToggle()); - const [feature1, feature2] = await fetchFeatures(); - const [segment1, segment2] = await fetchSegments(); - - await addSegmentToStrategy(segment1.id, feature1.strategies[0].id); - await addSegmentToStrategy(segment2.id, feature1.strategies[0].id); - await addSegmentToStrategy(segment2.id, feature2.strategies[0].id); - - const clientSegments = await fetchClientSegmentsActive(); - - expect(collectIds(clientSegments)).toEqual( - 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), + values: mockConstraintValues(DEFAULT_SEGMENT_VALUES_LIMIT + 1), }, ]; await expect( createSegment({ name: randomId(), constraints }), - ).rejects.toThrow(`Segments may not have more than ${limit} values`); + ).rejects.toThrow( + `Segments may not have more than ${DEFAULT_SEGMENT_VALUES_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), + values: mockConstraintValues(DEFAULT_SEGMENT_VALUES_LIMIT), }, { contextName: randomId(), @@ -197,12 +165,12 @@ test('should validate segment constraint values limit with multiple constraints' await expect( createSegment({ name: randomId(), constraints }), - ).rejects.toThrow(`Segments may not have more than ${limit} values`); + ).rejects.toThrow( + `Segments may not have more than ${DEFAULT_SEGMENT_VALUES_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: [] }); @@ -221,7 +189,9 @@ test('should validate feature strategy segment limit', async () => { await expect( addSegmentToStrategy(segments[5].id, feature1.strategies[0].id), - ).rejects.toThrow(`Strategies may not have more than ${limit} segments`); + ).rejects.toThrow( + `Strategies may not have more than ${DEFAULT_STRATEGY_SEGMENTS_LIMIT} segments`, + ); }); test('should not return segments in base of toggle response if inline is enabled', async () => {