diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 09957dba7a..731a7872a4 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -78,16 +78,10 @@ export default class FeatureToggleClientStore 'fs.strategy_name as strategy_name', 'fs.parameters as parameters', 'fs.constraints as constraints', + 'segments.id as segment_id', + 'segments.constraints as segment_constraints', ]; - if (inlineSegmentConstraints) { - selectColumns = [ - ...selectColumns, - 'segments.id as segment_id', - 'segments.constraints as segment_constraints', - ]; - } - let query = this.db('features') .select(selectColumns) .fullOuterJoin( @@ -105,17 +99,13 @@ export default class FeatureToggleClientStore .as('fe'), 'fe.feature_name', 'features.name', - ); - - if (inlineSegmentConstraints) { - query = query - .fullOuterJoin( - 'feature_strategy_segment as fss', - `fss.feature_strategy_id`, - `fs.id`, - ) - .fullOuterJoin('segments', `segments.id`, `fss.segment_id`); - } + ) + .fullOuterJoin( + 'feature_strategy_segment as fss', + `fss.feature_strategy_id`, + `fs.id`, + ) + .fullOuterJoin('segments', `segments.id`, `fss.segment_id`); query = query.where({ archived, @@ -155,6 +145,8 @@ export default class FeatureToggleClientStore } if (inlineSegmentConstraints && r.segment_id) { this.addSegmentToStrategy(feature, r); + } else if (!inlineSegmentConstraints && r.segment_id) { + this.addSegmentIdsToStrategy(feature, r); } feature.impressionData = r.impression_data; feature.enabled = !!r.enabled; @@ -220,6 +212,22 @@ export default class FeatureToggleClientStore ?.constraints.push(...row.segment_constraints); } + private addSegmentIdsToStrategy( + feature: PartialDeep, + row: Record, + ) { + const strategy = feature.strategies.find( + (s) => s.id === row.strategy_id, + ); + if (!strategy) { + return; + } + if (!strategy.segments) { + strategy.segments = []; + } + strategy.segments.push(row.segment_id); + } + async getClient( featureQuery?: IFeatureToggleQuery, ): Promise { diff --git a/src/lib/routes/client-api/feature.test.ts b/src/lib/routes/client-api/feature.test.ts index 728cff5fbe..e34f0469fe 100644 --- a/src/lib/routes/client-api/feature.test.ts +++ b/src/lib/routes/client-api/feature.test.ts @@ -60,13 +60,19 @@ test('should get empty getFeatures via client', () => { test('if caching is enabled should memoize', async () => { const getClientFeatures = jest.fn().mockReturnValue([]); + const getActive = jest.fn().mockReturnValue([]); const featureToggleServiceV2 = { getClientFeatures, }; + + const segmentService = { + getActive, + }; + const controller = new FeatureController( // @ts-ignore - { featureToggleServiceV2 }, + { featureToggleServiceV2, segmentService }, { getLogger, experimental: { @@ -87,12 +93,19 @@ test('if caching is enabled should memoize', async () => { test('if caching is not enabled all calls goes to service', async () => { const getClientFeatures = jest.fn().mockReturnValue([]); + const getActive = jest.fn().mockReturnValue([]); + const featureToggleServiceV2 = { getClientFeatures, }; + + const segmentService = { + getActive, + }; + const controller = new FeatureController( // @ts-ignore - { featureToggleServiceV2 }, + { featureToggleServiceV2, segmentService }, { getLogger, experimental: { diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index 8274982df0..d49a94f962 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -6,11 +6,13 @@ import { IUnleashConfig } from '../../types/option'; import FeatureToggleService from '../../services/feature-toggle-service'; import { Logger } from '../../logger'; import { querySchema } from '../../schema/feature-schema'; -import { IFeatureToggleQuery } from '../../types/model'; +import { IFeatureToggleQuery, ISegment } from '../../types/model'; import NotFoundError from '../../error/notfound-error'; import { IAuthRequest } from '../unleash-types'; import ApiUser from '../../types/api-user'; import { ALL, isAllProjects } from '../../types/models/api-token'; +import { SegmentService } from '../../services/segment-service'; +import { FeatureConfigurationClient } from '../../types/stores/feature-strategies-store'; const version = 2; @@ -24,27 +26,36 @@ export default class FeatureController extends Controller { private featureToggleServiceV2: FeatureToggleService; + private segmentService: SegmentService; + private readonly cache: boolean; private cachedFeatures: any; + private useGlobalSegments: boolean; + constructor( { featureToggleServiceV2, - }: Pick, + segmentService, + }: Pick, config: IUnleashConfig, ) { super(config); const { experimental } = config; this.featureToggleServiceV2 = featureToggleServiceV2; + this.segmentService = segmentService; this.logger = config.getLogger('client-api/feature.js'); this.get('/', this.getAll); this.get('/:featureName', this.getFeatureToggle); + this.useGlobalSegments = + experimental && !experimental?.segments?.inlineSegmentConstraints; + if (experimental && experimental.clientFeatureMemoize) { // @ts-ignore this.cache = experimental.clientFeatureMemoize.enabled; this.cachedFeatures = memoizee( - (query) => this.featureToggleServiceV2.getClientFeatures(query), + (query) => this.resolveFeaturesAndSegments(query), { promise: true, // @ts-ignore @@ -58,6 +69,23 @@ export default class FeatureController extends Controller { } } + private async resolveSegments() { + if (this.useGlobalSegments) { + return this.segmentService.getActive(); + } + return Promise.resolve([]); + } + + private async resolveFeaturesAndSegments( + query?: IFeatureToggleQuery, + ): Promise<[FeatureConfigurationClient[], ISegment[]]> { + let segments = this.resolveSegments(); + return Promise.all([ + this.featureToggleServiceV2.getClientFeatures(query), + segments, + ]); + } + private async resolveQuery( req: IAuthRequest, ): Promise { @@ -110,15 +138,30 @@ export default class FeatureController extends Controller { async getAll(req: IAuthRequest, res: Response): Promise { const featureQuery = await this.resolveQuery(req); - let features; + let features, segments; if (this.cache) { - features = await this.cachedFeatures(featureQuery); + [features, segments] = await this.cachedFeatures(featureQuery); } else { features = await this.featureToggleServiceV2.getClientFeatures( featureQuery, ); + segments = await this.resolveSegments(); + } + + const response = { + version, + features, + query: featureQuery, + }; + + if (this.useGlobalSegments) { + res.json({ + ...response, + segments, + }); + } else { + res.json(response); } - res.json({ version, features, query: featureQuery }); } async getFeatureToggle(req: IAuthRequest, res: Response): Promise { diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index 223b6b3920..c772188b4c 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -23,6 +23,7 @@ export interface IStrategyConfig { id?: string; name: string; constraints?: IConstraint[]; + segments?: number[]; parameters?: { [key: string]: string }; sortOrder?: number; } diff --git a/src/test/e2e/api/client/global.segment.e2e.test.ts b/src/test/e2e/api/client/global.segment.e2e.test.ts new file mode 100644 index 0000000000..4d6fe00c19 --- /dev/null +++ b/src/test/e2e/api/client/global.segment.e2e.test.ts @@ -0,0 +1,155 @@ +import dbInit, { ITestDb } from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import { + IUnleashTest, + setupAppWithCustomConfig, +} from '../../helpers/test-helper'; +import { + IConstraint, + IFeatureToggleClient, + ISegment, +} from '../../../../lib/types/model'; +import { randomId } from '../../../../lib/util/random-id'; +import User from '../../../../lib/types/user'; + +let db: ITestDb; +let app: IUnleashTest; + +const FEATURES_ADMIN_BASE_PATH = '/api/admin/features'; +const FEATURES_CLIENT_BASE_PATH = '/api/client/features'; + +interface ApiResponse { + features: IFeatureToggleClient[]; + version: number; + segments: ISegment[]; +} + +const fetchSegments = (): Promise => { + return app.services.segmentService.getAll(); +}; + +const fetchFeatures = (): Promise => { + return app.request + .get(FEATURES_ADMIN_BASE_PATH) + .expect(200) + .then((res) => res.body.features); +}; + +const fetchClientResponse = (): Promise => { + return app.request + .get(FEATURES_CLIENT_BASE_PATH) + .expect(200) + .then((res) => res.body); +}; + +const createSegment = (postData: object): Promise => { + const user = { email: 'test@example.com' } as User; + return app.services.segmentService.create(postData, user); +}; + +const createFeatureToggle = ( + postData: object, + expectStatusCode = 201, +): Promise => { + return app.request + .post(FEATURES_ADMIN_BASE_PATH) + .send(postData) + .expect(expectStatusCode); +}; + +const addSegmentToStrategy = ( + segmentId: number, + strategyId: string, +): Promise => { + return app.services.segmentService.addToStrategy(segmentId, strategyId); +}; + +const mockFeatureToggle = (): object => { + return { + name: randomId(), + strategies: [{ name: randomId(), constraints: [], parameters: {} }], + }; +}; + +const mockConstraints = (): IConstraint[] => { + return Array.from({ length: 5 }).map(() => ({ + values: ['x', 'y', 'z'], + operator: 'IN', + contextName: 'a', + })); +}; + +const createTestSegments = async (): Promise => { + 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); +}; + +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, + }); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +afterEach(async () => { + await db.stores.segmentStore.deleteAll(); + await db.stores.featureToggleStore.deleteAll(); +}); + +test('should return segments in base of toggle response if inline is disabled', async () => { + await createTestSegments(); + + const clientFeatures = await fetchClientResponse(); + expect(clientFeatures.segments.length).toBeDefined(); +}); + +test('should only send segments that are in use', async () => { + await createTestSegments(); + + const clientFeatures = await fetchClientResponse(); + //3 segments were created in createTestSegments, only 2 are in use + expect(clientFeatures.segments.length).toEqual(2); +}); + +test('should send all segments that are in use by feature', async () => { + await createTestSegments(); + + const clientFeatures = await fetchClientResponse(); + const globalSegments = clientFeatures.segments; + const globalSegmentIds = globalSegments.map((segment) => segment.id); + const allSegmentIds = clientFeatures.features + .map((feat) => feat.strategies.map((strategy) => strategy.segments)) + .flat() + .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 3aa6786e62..921fe7298f 100644 --- a/src/test/e2e/api/client/segment.e2e.test.ts +++ b/src/test/e2e/api/client/segment.e2e.test.ts @@ -38,6 +38,13 @@ const fetchClientFeatures = (): Promise => { .then((res) => res.body.features); }; +const fetchGlobalSegments = (): Promise => { + return app.request + .get(FEATURES_CLIENT_BASE_PATH) + .expect(200) + .then((res) => res.body.segments); +}; + const fetchClientSegmentsActive = (): Promise => { return app.request .get('/api/client/segments/active') @@ -216,3 +223,22 @@ test('should validate feature strategy segment limit', async () => { addSegmentToStrategy(segments[5].id, feature1.strategies[0].id), ).rejects.toThrow(`Strategies may not have more than ${limit} segments`); }); + +test('should not return segments in base of toggle response if inline is enabled', 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 globalSegments = await fetchGlobalSegments(); + expect(globalSegments).toBe(undefined); +}); diff --git a/src/test/e2e/helpers/database-init.ts b/src/test/e2e/helpers/database-init.ts index 9f76c31919..4122c2ba5b 100644 --- a/src/test/e2e/helpers/database-init.ts +++ b/src/test/e2e/helpers/database-init.ts @@ -10,6 +10,7 @@ import EnvironmentStore from '../../../lib/db/environment-store'; import { IUnleashStores } from '../../../lib/types'; import { IFeatureEnvironmentStore } from '../../../lib/types/stores/feature-environment-store'; import { DEFAULT_ENV } from '../../../lib/util/constants'; +import { IUnleashOptions } from 'lib/server-impl'; // require('db-migrate-shared').log.silence(false); @@ -79,6 +80,7 @@ export interface ITestDb { export default async function init( databaseSchema: string = 'test', getLogger: LogProvider = noLoggerProvider, + configOverride: Partial = {}, ): Promise { const config = createTestConfig({ db: { @@ -87,6 +89,7 @@ export default async function init( schema: databaseSchema, ssl: false, }, + ...configOverride, getLogger, });