Skip to content

Commit 2e60f40

Browse files
Zaimwa9Zaimwa9
authored
fix: environment evaluation skip segments (#213)
* feat: implemented-implicit-identity-key * fix: skip-segment-if-environment-flag-and-moved-flagsmith-id-to-id * fix: rename-custom-to-sdk-metadata
1 parent 93c261e commit 2e60f40

File tree

9 files changed

+61
-52
lines changed

9 files changed

+61
-52
lines changed

‎flagsmith-engine/evaluation/evaluationContext/mappers.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import {
22
FeaturesWithMetadata,
3-
Segments,
43
Traits,
54
GenericEvaluationContext,
65
EnvironmentContext,
76
IdentityContext,
8-
SegmentSource
7+
SegmentSource,
8+
SDKFeatureMetadata,
9+
SegmentsWithMetadata,
10+
SDKSegmentMetadata
911
} from '../models.js';
1012
import { EnvironmentModel } from '../../environments/models.js';
1113
import { IdentityModel } from '../../identities/models.js';
@@ -17,9 +19,13 @@ import { uuidToBigInt } from '../../features/util.js';
1719
export function getEvaluationContext(
1820
environment: EnvironmentModel,
1921
identity?: IdentityModel,
20-
overrideTraits?: TraitModel[]
22+
overrideTraits?: TraitModel[],
23+
isEnvironmentEvaluation: boolean = false
2124
): GenericEvaluationContext {
2225
const environmentContext = mapEnvironmentModelToEvaluationContext(environment);
26+
if (isEnvironmentEvaluation) {
27+
return environmentContext;
28+
}
2329
const identityContext = identity
2430
? mapIdentityModelToIdentityContext(identity, overrideTraits)
2531
: undefined;
@@ -40,7 +46,7 @@ function mapEnvironmentModelToEvaluationContext(
4046
name: environment.project.name
4147
};
4248

43-
const features: FeaturesWithMetadata = {};
49+
const features: FeaturesWithMetadata<SDKFeatureMetadata> = {};
4450
for (const fs of environment.featureStates) {
4551
const variants =
4652
fs.multivariateFeatureStateValues?.length > 0
@@ -59,12 +65,12 @@ function mapEnvironmentModelToEvaluationContext(
5965
variants,
6066
priority: fs.featureSegment?.priority,
6167
metadata: {
62-
flagsmithId: fs.feature.id
68+
id: fs.feature.id
6369
}
6470
};
6571
}
6672

67-
const segmentOverrides: Segments = {};
73+
const segmentOverrides: SegmentsWithMetadata<SDKSegmentMetadata> = {};
6874
for (const segment of environment.project.segments) {
6975
segmentOverrides[segment.id.toString()] = {
7076
key: segment.id.toString(),
@@ -79,18 +85,18 @@ function mapEnvironmentModelToEvaluationContext(
7985
value: fs.getValue(),
8086
priority: fs.featureSegment?.priority,
8187
metadata: {
82-
flagsmithId: fs.feature.id
88+
id: fs.feature.id
8389
}
8490
}))
8591
: [],
8692
metadata: {
8793
source: SegmentSource.API,
88-
flagsmithId: segment.id
94+
id: segment.id
8995
}
9096
};
9197
}
9298

93-
let identityOverrideSegments: Segments = {};
99+
let identityOverrideSegments: SegmentsWithMetadata<SDKSegmentMetadata> = {};
94100
if (environment.identityOverrides && environment.identityOverrides.length > 0) {
95101
identityOverrideSegments = mapIdentityOverridesToSegments(environment.identityOverrides);
96102
}
@@ -140,8 +146,10 @@ function mapSegmentRuleModelToRule(rule: any): any {
140146
};
141147
}
142148

143-
function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Segments {
144-
const segments: Segments = {};
149+
function mapIdentityOverridesToSegments(
150+
identityOverrides: IdentityModel[]
151+
): SegmentsWithMetadata<SDKSegmentMetadata> {
152+
const segments: SegmentsWithMetadata<SDKSegmentMetadata> = {};
145153
const featuresToIdentifiers = new Map<string, { identifiers: string[]; overrides: any[] }>();
146154

147155
for (const identity of identityOverrides) {
@@ -158,7 +166,7 @@ function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Seg
158166
value: fs.getValue(),
159167
priority: -Infinity,
160168
metadata: {
161-
flagsmithId: fs.feature.id
169+
id: fs.feature.id
162170
}
163171
}));
164172

‎flagsmith-engine/evaluation/models.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ export enum SegmentSource {
2424
}
2525

2626
// Feature types
27-
export interface CustomFeatureMetadata extends FeatureMetadata {
28-
flagsmithId: number;
27+
export interface SDKFeatureMetadata extends FeatureMetadata {
28+
id: number;
2929
}
3030

3131
export interface FeatureContextWithMetadata<T extends FeatureMetadata = FeatureMetadata>
@@ -48,8 +48,8 @@ export type EvaluationResultFlags<T extends FeatureMetadata = FeatureMetadata> =
4848
>;
4949

5050
// Segment types
51-
export interface CustomSegmentMetadata extends SegmentMetadata {
52-
flagsmithId: number;
51+
export interface SDKSegmentMetadata extends SegmentMetadata {
52+
id?: number;
5353
source?: SegmentSource;
5454
}
5555

@@ -65,10 +65,10 @@ export type SegmentsWithMetadata<T extends SegmentMetadata = SegmentMetadata> =
6565

6666
export interface SegmentResultWithMetadata {
6767
name: string;
68-
metadata: CustomSegmentMetadata;
68+
metadata: SDKSegmentMetadata;
6969
}
7070

71-
export type EvaluationResultSegments = EvaluationContextResult['segments'];
71+
export type EvaluationResultSegments = SegmentResultWithMetadata[];
7272

7373
// Evaluation context types
7474
export interface GenericEvaluationContext<
@@ -83,8 +83,8 @@ export interface GenericEvaluationContext<
8383
}
8484

8585
export type EvaluationContextWithMetadata = GenericEvaluationContext<
86-
CustomFeatureMetadata,
87-
CustomSegmentMetadata
86+
SDKFeatureMetadata,
87+
SDKSegmentMetadata
8888
>;
8989

9090
// Evaluation result types
@@ -93,4 +93,4 @@ export type EvaluationResult<T extends FeatureMetadata = FeatureMetadata> = {
9393
segments: EvaluationResultSegments;
9494
};
9595

96-
export type EvaluationResultWithMetadata = EvaluationResult<CustomFeatureMetadata>;
96+
export type EvaluationResultWithMetadata = EvaluationResult<SDKFeatureMetadata>;

‎flagsmith-engine/index.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
EvaluationResultSegments,
44
EvaluationResultWithMetadata,
55
FeatureContextWithMetadata,
6-
CustomFeatureMetadata,
6+
SDKFeatureMetadata,
77
FlagResultWithMetadata
88
} from './evaluation/models.js';
99
import { getIdentitySegments, getIdentityKey } from './segments/evaluators.js';
@@ -18,7 +18,7 @@ export { FeatureModel, FeatureStateModel } from './features/models.js';
1818
export { OrganisationModel } from './organisations/models.js';
1919

2020
type SegmentOverride = {
21-
feature: FeatureContextWithMetadata<CustomFeatureMetadata>;
21+
feature: FeatureContextWithMetadata<SDKFeatureMetadata>;
2222
segmentName: string;
2323
};
2424

@@ -121,8 +121,8 @@ export function processSegmentOverrides(identitySegments: any[]): Record<string,
121121
export function evaluateFeatures(
122122
context: EvaluationContextWithMetadata,
123123
segmentOverrides: Record<string, SegmentOverride>
124-
): EvaluationResultFlags<CustomFeatureMetadata> {
125-
const flags: EvaluationResultFlags<CustomFeatureMetadata> = {};
124+
): EvaluationResultFlags<SDKFeatureMetadata> {
125+
const flags: EvaluationResultFlags<SDKFeatureMetadata> = {};
126126

127127
for (const feature of Object.values(context.features || {})) {
128128
const segmentOverride = segmentOverrides[feature.name];
@@ -141,7 +141,7 @@ export function evaluateFeatures(
141141
reason:
142142
evaluatedReason ??
143143
getTargetingMatchReason({ type: 'SEGMENT', override: segmentOverride })
144-
} as FlagResultWithMetadata<CustomFeatureMetadata>;
144+
} as FlagResultWithMetadata<SDKFeatureMetadata>;
145145
}
146146

147147
return flags;

‎flagsmith-engine/segments/models.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,13 @@ export class SegmentModel {
227227
if (segmentResult.metadata?.source === SegmentSource.IDENTITY_OVERRIDE) {
228228
continue;
229229
}
230-
const flagsmithId = segmentResult.metadata?.flagsmithId;
231-
if (!flagsmithId) {
230+
const segmentMetadataId = segmentResult.metadata?.id;
231+
if (!segmentMetadataId) {
232232
continue;
233233
}
234-
const segmentContext = evaluationContext.segments[flagsmithId.toString()];
234+
const segmentContext = evaluationContext.segments[segmentMetadataId.toString()];
235235
if (segmentContext) {
236-
const segment = new SegmentModel(flagsmithId, segmentContext.name);
236+
const segment = new SegmentModel(segmentMetadataId, segmentContext.name);
237237
segment.rules = segmentContext.rules.map(rule => new SegmentRuleModel(rule.type));
238238
segment.featureStates = SegmentModel.createFeatureStatesFromOverrides(
239239
segmentContext.overrides || []
@@ -249,13 +249,13 @@ export class SegmentModel {
249249
if (!overrides) return [];
250250
return overrides
251251
.filter(override => {
252-
const flagsmithId = override?.metadata?.flagsmithId;
253-
return typeof flagsmithId === 'number';
252+
const overrideMetadataId = override?.metadata?.id;
253+
return typeof overrideMetadataId === 'number';
254254
})
255255
.map(override => {
256-
const flagsmithId = override.metadata!.flagsmithId as number;
256+
const overrideMetadataId = override.metadata!.id as number;
257257
const feature = new FeatureModel(
258-
flagsmithId,
258+
overrideMetadataId,
259259
override.name,
260260
override.variants?.length && override.variants.length > 0
261261
? CONSTANTS.MULTIVARIATE

‎sdk/index.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
} from './types.js';
2626
import { pino, Logger } from 'pino';
2727
import { getEvaluationContext } from '../flagsmith-engine/evaluation/evaluationContext/mappers.js';
28+
import { EvaluationContextWithMetadata } from '../flagsmith-engine/evaluation/models.js';
2829

2930
export { AnalyticsProcessor, AnalyticsProcessorOptions } from './analytics.js';
3031
export { FlagsmithAPIError, FlagsmithClientError } from './errors.js';
@@ -282,7 +283,7 @@ export class Flagsmith {
282283
if (!context) {
283284
throw new FlagsmithClientError('Local evaluation required to obtain identity segments');
284285
}
285-
const evaluationResult = getEvaluationResult(context);
286+
const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
286287

287288
return SegmentModel.fromSegmentResult(evaluationResult.segments, context);
288289
}
@@ -449,11 +450,11 @@ export class Flagsmith {
449450

450451
private async getEnvironmentFlagsFromDocument(): Promise<Flags> {
451452
const environment = await this.getEnvironment();
452-
const context = getEvaluationContext(environment);
453+
const context = getEvaluationContext(environment, undefined, undefined, true);
453454
if (!context) {
454455
throw new FlagsmithClientError('Unable to get flags. No environment present.');
455456
}
456-
const evaluationResult = getEvaluationResult(context);
457+
const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
457458
const flags = Flags.fromEvaluationResult(evaluationResult);
458459

459460
if (!!this.cache) {
@@ -481,7 +482,7 @@ export class Flagsmith {
481482
if (!context) {
482483
throw new FlagsmithClientError('Unable to get flags. No environment present.');
483484
}
484-
const evaluationResult = getEvaluationResult(context);
485+
const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata);
485486

486487
const flags = Flags.fromEvaluationResult(
487488
evaluationResult,

‎sdk/models.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {
2-
CustomFeatureMetadata,
2+
SDKFeatureMetadata,
33
FlagResultWithMetadata,
44
EvaluationResultWithMetadata
55
} from '../flagsmith-engine/evaluation/models.js';
@@ -118,18 +118,18 @@ export class Flags {
118118
): Flags {
119119
const flags: { [key: string]: Flag } = {};
120120
for (const flag of Object.values(evaluationResult.flags)) {
121-
const flagsmithId = flag.metadata?.flagsmithId;
122-
if (!flagsmithId) {
121+
const flagMetadataId = flag.metadata?.id;
122+
if (!flagMetadataId) {
123123
throw new Error(
124-
`FlagResult metadata.flagsmithId is missing for feature "${flag.name}". ` +
124+
`FlagResult metadata.id is missing for feature "${flag.name}". ` +
125125
`This indicates a bug in the SDK, please report it.`
126126
);
127127
}
128128

129129
flags[flag.name] = new Flag({
130130
enabled: flag.enabled,
131131
value: flag.value ?? null,
132-
featureId: flagsmithId,
132+
featureId: flagMetadataId,
133133
featureName: flag.name,
134134
reason: flag.reason
135135
});

‎tests/engine/unit/engine.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ test('shouldApplyOverride with priority conflicts', () => {
115115
enabled: true,
116116
value: 'value',
117117
priority: 5,
118-
metadata: { flagsmithId: 1 }
118+
metadata: { id: 1 }
119119
},
120120
segmentName: 'segment1'
121121
}
@@ -185,7 +185,7 @@ test('evaluateSegments handles segments with identity identifier matching', () =
185185
name: 'feature1',
186186
enabled: false,
187187
value: 'default_value',
188-
metadata: { flagsmithId: 1 }
188+
metadata: { id: 1 }
189189
}
190190
}
191191
};
@@ -272,7 +272,7 @@ test('evaluateSegments handles priority conflicts correctly', () => {
272272
name: 'feature1',
273273
enabled: false,
274274
value: 'default_value',
275-
metadata: { flagsmithId: 1 }
275+
metadata: { id: 1 }
276276
}
277277
}
278278
};
@@ -343,7 +343,7 @@ test('evaluateFeatures with multivariate evaluation', () => {
343343
{ value: 'variant_a', weight: 0, priority: 1 },
344344
{ value: 'variant_b', weight: 100, priority: 2 }
345345
],
346-
metadata: { flagsmithId: 1 }
346+
metadata: { id: 1 }
347347
}
348348
},
349349
identity: { key: 'test_user', identifier: 'test_user' },

‎tests/engine/unit/segments/segments_model.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ test('test_segment_rule_matching_function', () => {
141141
});
142142

143143
test('test_fromSegmentResult_with_multiple_variants', () => {
144-
const segmentResults = [{ name: 'test_segment', metadata: { flagsmithId: '1' } }];
144+
const segmentResults = [{ name: 'test_segment', metadata: { id: '1' } }];
145145

146146
const evaluationContext: EvaluationContext = {
147147
identity: {
@@ -159,7 +159,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => {
159159
name: 'test_segment',
160160
metadata: {
161161
source: SegmentSource.API,
162-
flagsmithId: '1'
162+
id: '1'
163163
},
164164
rules: [
165165
{
@@ -181,7 +181,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => {
181181
value: 'default_value',
182182
priority: 1,
183183
metadata: {
184-
flagsmithId: 1
184+
id: 1
185185
},
186186
variants: [
187187
{ id: 1, value: 'variant_a', weight: 30 },

‎tests/sdk/flagsmith.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ test('get_user_agent_extracts_version_from_package_json', async () => {
520520
expect(userAgent).toBe(`flagsmith-nodejs-sdk/${packageJson.version}`);
521521
});
522522

523-
test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missing', () => {
523+
test('Flags.fromEvaluationResult throws error when metadata.id is missing', () => {
524524
const evaluationResult = {
525525
flags: {
526526
test_feature: {
@@ -535,7 +535,7 @@ test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missi
535535
};
536536

537537
expect(() => Flags.fromEvaluationResult(evaluationResult as any)).toThrow(
538-
'FlagResult metadata.flagsmithId is missing for feature "test_feature". ' +
538+
'FlagResult metadata.id is missing for feature "test_feature". ' +
539539
'This indicates a bug in the SDK, please report it.'
540540
);
541541
});

0 commit comments

Comments
 (0)