Skip to content

Commit 0879bee

Browse files
danoswaltCLdanoswaltCL
authored
Merge pull request #3192 from CarnegieLearningWeb/feature/optimized-feature-flag-from-context
parrallel get flag from context
2 parents 504b1ce + 411505f commit 0879bee

2 files changed

Lines changed: 127 additions & 20 deletions

File tree

‎packages/backend/src/api/repositories/FeatureFlagRepository.ts‎

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Repository, EntityManager } from 'typeorm';
1+
import { Repository, EntityManager, SelectQueryBuilder } from 'typeorm';
22
import { EntityRepository } from '../../typeorm-typedi-extensions';
33
import { FeatureFlag } from '../models/FeatureFlag';
44
import repositoryError from './utils/repositoryError';
@@ -91,26 +91,48 @@ export class FeatureFlagRepository extends Repository<FeatureFlag> {
9191
}
9292

9393
public async getFlagsFromContext(context: string): Promise<FeatureFlag[]> {
94-
const result = await this.createQueryBuilder('feature_flag')
95-
.leftJoinAndSelect('feature_flag.featureFlagSegmentInclusion', 'featureFlagSegmentInclusion')
96-
.leftJoinAndSelect('featureFlagSegmentInclusion.segment', 'segmentInclusion')
97-
.leftJoinAndSelect('segmentInclusion.individualForSegment', 'individualForSegment')
98-
.leftJoinAndSelect('segmentInclusion.groupForSegment', 'groupForSegment')
99-
.leftJoinAndSelect('segmentInclusion.subSegments', 'subSegment')
100-
.leftJoinAndSelect('feature_flag.featureFlagSegmentExclusion', 'featureFlagSegmentExclusion')
101-
.leftJoinAndSelect('featureFlagSegmentExclusion.segment', 'segmentExclusion')
102-
.leftJoinAndSelect('segmentExclusion.individualForSegment', 'individualForSegmentExclusion')
103-
.leftJoinAndSelect('segmentExclusion.groupForSegment', 'groupForSegmentExclusion')
104-
.leftJoinAndSelect('segmentExclusion.subSegments', 'subSegmentExclusion')
105-
.where('feature_flag.context @> :searchContext', { searchContext: [context] })
106-
.andWhere('feature_flag.status = :status', { status: FEATURE_FLAG_STATUS.ENABLED })
107-
.getMany()
108-
.catch((errorMsg: any) => {
109-
const errorMsgString = repositoryError('FeatureFlagRepository', 'getFlagsFromContext', { context }, errorMsg);
110-
throw errorMsgString;
111-
});
94+
const addExclusionJoins = (qb: SelectQueryBuilder<FeatureFlag>) =>
95+
qb
96+
.leftJoinAndSelect('feature_flag.featureFlagSegmentExclusion', 'featureFlagSegmentExclusion')
97+
.leftJoinAndSelect('featureFlagSegmentExclusion.segment', 'segmentExclusion')
98+
.leftJoinAndSelect('segmentExclusion.individualForSegment', 'individualForSegmentExclusion')
99+
.leftJoinAndSelect('segmentExclusion.groupForSegment', 'groupForSegmentExclusion')
100+
.leftJoinAndSelect('segmentExclusion.subSegments', 'subSegmentExclusion');
112101

113-
return result;
102+
const addInclusionJoins = (qb: SelectQueryBuilder<FeatureFlag>) =>
103+
qb
104+
.leftJoinAndSelect('feature_flag.featureFlagSegmentInclusion', 'featureFlagSegmentInclusion')
105+
.leftJoinAndSelect('featureFlagSegmentInclusion.segment', 'segmentInclusion')
106+
.leftJoinAndSelect('segmentInclusion.individualForSegment', 'individualForSegment')
107+
.leftJoinAndSelect('segmentInclusion.groupForSegment', 'groupForSegment')
108+
.leftJoinAndSelect('segmentInclusion.subSegments', 'subSegment');
109+
110+
const addBaseConditions = (qb: SelectQueryBuilder<FeatureFlag>) =>
111+
qb
112+
.where('feature_flag.context @> :searchContext', { searchContext: [context] })
113+
.andWhere('feature_flag.status = :status', { status: FEATURE_FLAG_STATUS.ENABLED });
114+
115+
// INCLUDE_ALL flags: inclusion segments are irrelevant — skip all inclusion joins
116+
const includeAllQuery = addBaseConditions(addExclusionJoins(this.createQueryBuilder('feature_flag'))).andWhere(
117+
'feature_flag.filterMode = :includeAll',
118+
{ includeAll: FILTER_MODE.INCLUDE_ALL }
119+
);
120+
121+
// EXCLUDE_ALL flags: both inclusion and exclusion segments are needed
122+
const excludeAllQuery = addBaseConditions(
123+
addInclusionJoins(addExclusionJoins(this.createQueryBuilder('feature_flag')))
124+
).andWhere('feature_flag.filterMode = :excludeAll', { excludeAll: FILTER_MODE.EXCLUDE_ALL });
125+
126+
const [includeAllFlags, excludeAllFlags] = await Promise.all([
127+
includeAllQuery.getMany().catch((errorMsg: any) => {
128+
throw repositoryError('FeatureFlagRepository', 'getFlagsFromContext', { context }, errorMsg);
129+
}),
130+
excludeAllQuery.getMany().catch((errorMsg: any) => {
131+
throw repositoryError('FeatureFlagRepository', 'getFlagsFromContext', { context }, errorMsg);
132+
}),
133+
]);
134+
135+
return [...includeAllFlags, ...excludeAllFlags];
114136
}
115137

116138
public async validateUniqueKey(flagDTO: FeatureFlagValidation) {

‎packages/backend/test/unit/repositories/FeatureFlagRepository.test.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,89 @@ describe('FeatureFlagRepository Testing', () => {
176176
expect(mock.returning).toHaveBeenCalledWith('*');
177177
expect(mock.execute).toHaveBeenCalledTimes(1);
178178
});
179+
180+
describe('getFlagsFromContext', () => {
181+
const context = 'test-context';
182+
183+
beforeEach(() => {
184+
mock.getMany.mockResolvedValue([]);
185+
});
186+
187+
it('should run two separate queries in parallel, one per filterMode', async () => {
188+
await repo.getFlagsFromContext(context);
189+
190+
expect(repo.createQueryBuilder).toHaveBeenCalledTimes(2);
191+
expect(mock.getMany).toHaveBeenCalledTimes(2);
192+
expect(mock.andWhere).toHaveBeenCalledWith('feature_flag.filterMode = :includeAll', {
193+
includeAll: FILTER_MODE.INCLUDE_ALL,
194+
});
195+
expect(mock.andWhere).toHaveBeenCalledWith('feature_flag.filterMode = :excludeAll', {
196+
excludeAll: FILTER_MODE.EXCLUDE_ALL,
197+
});
198+
});
199+
200+
it('should apply context and status conditions to both queries', async () => {
201+
await repo.getFlagsFromContext(context);
202+
203+
expect(mock.where).toHaveBeenCalledTimes(2);
204+
expect(mock.where).toHaveBeenCalledWith('feature_flag.context @> :searchContext', {
205+
searchContext: [context],
206+
});
207+
expect(mock.andWhere).toHaveBeenCalledWith('feature_flag.status = :status', {
208+
status: FEATURE_FLAG_STATUS.ENABLED,
209+
});
210+
});
211+
212+
it('should not join inclusion segment member data for INCLUDE_ALL flags', async () => {
213+
await repo.getFlagsFromContext(context);
214+
215+
// Collect all first-args passed to leftJoinAndSelect across both queries
216+
const joinedRelations = mock.leftJoinAndSelect.mock.calls.map(([relation]) => relation);
217+
218+
// Inclusion-side member joins should appear exactly once (only from the EXCLUDE_ALL query)
219+
expect(joinedRelations.filter((r) => r === 'segmentInclusion.individualForSegment')).toHaveLength(1);
220+
expect(joinedRelations.filter((r) => r === 'segmentInclusion.groupForSegment')).toHaveLength(1);
221+
expect(joinedRelations.filter((r) => r === 'segmentInclusion.subSegments')).toHaveLength(1);
222+
223+
// Exclusion-side member joins should appear twice (once per query)
224+
expect(joinedRelations.filter((r) => r === 'segmentExclusion.individualForSegment')).toHaveLength(2);
225+
expect(joinedRelations.filter((r) => r === 'segmentExclusion.groupForSegment')).toHaveLength(2);
226+
expect(joinedRelations.filter((r) => r === 'segmentExclusion.subSegments')).toHaveLength(2);
227+
});
228+
229+
it('should join inclusion segment member data for EXCLUDE_ALL flags', async () => {
230+
await repo.getFlagsFromContext(context);
231+
232+
expect(mock.leftJoinAndSelect).toHaveBeenCalledWith(
233+
'feature_flag.featureFlagSegmentInclusion',
234+
'featureFlagSegmentInclusion'
235+
);
236+
expect(mock.leftJoinAndSelect).toHaveBeenCalledWith('featureFlagSegmentInclusion.segment', 'segmentInclusion');
237+
expect(mock.leftJoinAndSelect).toHaveBeenCalledWith(
238+
'segmentInclusion.individualForSegment',
239+
'individualForSegment'
240+
);
241+
expect(mock.leftJoinAndSelect).toHaveBeenCalledWith('segmentInclusion.groupForSegment', 'groupForSegment');
242+
expect(mock.leftJoinAndSelect).toHaveBeenCalledWith('segmentInclusion.subSegments', 'subSegment');
243+
});
244+
245+
it('should combine results from both queries', async () => {
246+
const includeAllFlag = new FeatureFlag();
247+
includeAllFlag.id = 'include-all-flag';
248+
const excludeAllFlag = new FeatureFlag();
249+
excludeAllFlag.id = 'exclude-all-flag';
250+
251+
mock.getMany.mockResolvedValueOnce([includeAllFlag]).mockResolvedValueOnce([excludeAllFlag]);
252+
253+
const results = await repo.getFlagsFromContext(context);
254+
255+
expect(results).toEqual([includeAllFlag, excludeAllFlag]);
256+
});
257+
258+
it('should throw when either query fails', async () => {
259+
mock.getMany.mockRejectedValueOnce(err);
260+
261+
await expect(repo.getFlagsFromContext(context)).rejects.toThrow();
262+
});
263+
});
179264
});

0 commit comments

Comments
 (0)