diff --git a/.gitmodules b/.gitmodules index 06c9e18..93de146 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine/engine-tests/engine-test-data"] path = tests/engine/engine-tests/engine-test-data url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.0.0 + branch = v3.1.0 diff --git a/flagsmith-engine/evaluation/evaluationContext/mappers.ts b/flagsmith-engine/evaluation/evaluationContext/mappers.ts index 1fa898f..b0bfde0 100644 --- a/flagsmith-engine/evaluation/evaluationContext/mappers.ts +++ b/flagsmith-engine/evaluation/evaluationContext/mappers.ts @@ -5,7 +5,10 @@ import { GenericEvaluationContext, EnvironmentContext, IdentityContext, - SegmentSource + SegmentSource, + CustomFeatureMetadata, + SegmentsWithMetadata, + CustomSegmentMetadata } from '../models.js'; import { EnvironmentModel } from '../../environments/models.js'; import { IdentityModel } from '../../identities/models.js'; @@ -17,9 +20,13 @@ import { uuidToBigInt } from '../../features/util.js'; export function getEvaluationContext( environment: EnvironmentModel, identity?: IdentityModel, - overrideTraits?: TraitModel[] + overrideTraits?: TraitModel[], + isEnvironmentEvaluation: boolean = false ): GenericEvaluationContext { const environmentContext = mapEnvironmentModelToEvaluationContext(environment); + if (isEnvironmentEvaluation) { + return environmentContext; + } const identityContext = identity ? mapIdentityModelToIdentityContext(identity, overrideTraits) : undefined; @@ -40,7 +47,7 @@ function mapEnvironmentModelToEvaluationContext( name: environment.project.name }; - const features: FeaturesWithMetadata = {}; + const features: FeaturesWithMetadata = {}; for (const fs of environment.featureStates) { const variants = fs.multivariateFeatureStateValues?.length > 0 @@ -59,12 +66,12 @@ function mapEnvironmentModelToEvaluationContext( variants, priority: fs.featureSegment?.priority, metadata: { - flagsmithId: fs.feature.id + id: fs.feature.id } }; } - const segmentOverrides: Segments = {}; + const segmentOverrides: SegmentsWithMetadata = {}; for (const segment of environment.project.segments) { segmentOverrides[segment.id.toString()] = { key: segment.id.toString(), @@ -79,18 +86,18 @@ function mapEnvironmentModelToEvaluationContext( value: fs.getValue(), priority: fs.featureSegment?.priority, metadata: { - flagsmithId: fs.feature.id + id: fs.feature.id } })) : [], metadata: { source: SegmentSource.API, - flagsmithId: segment.id + id: segment.id } }; } - let identityOverrideSegments: Segments = {}; + let identityOverrideSegments: SegmentsWithMetadata = {}; if (environment.identityOverrides && environment.identityOverrides.length > 0) { identityOverrideSegments = mapIdentityOverridesToSegments(environment.identityOverrides); } @@ -116,11 +123,16 @@ function mapIdentityModelToIdentityContext( traitsContext[trait.traitKey] = trait.traitValue; } - return { + const identityContext: IdentityContext = { identifier: identity.identifier, - key: identity.djangoID?.toString() || identity.compositeKey, traits: traitsContext }; + + if (identity.djangoID !== undefined) { + identityContext.key = identity.djangoID.toString(); + } + + return identityContext; } function mapSegmentRuleModelToRule(rule: any): any { @@ -135,8 +147,10 @@ function mapSegmentRuleModelToRule(rule: any): any { }; } -function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Segments { - const segments: Segments = {}; +function mapIdentityOverridesToSegments( + identityOverrides: IdentityModel[] +): SegmentsWithMetadata { + const segments: SegmentsWithMetadata = {}; const featuresToIdentifiers = new Map(); for (const identity of identityOverrides) { @@ -153,7 +167,7 @@ function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Seg value: fs.getValue(), priority: -Infinity, metadata: { - flagsmithId: fs.feature.id + id: fs.feature.id } })); diff --git a/flagsmith-engine/evaluation/models.ts b/flagsmith-engine/evaluation/models.ts index 9b8cc06..c6d0c17 100644 --- a/flagsmith-engine/evaluation/models.ts +++ b/flagsmith-engine/evaluation/models.ts @@ -25,7 +25,7 @@ export enum SegmentSource { // Feature types export interface CustomFeatureMetadata extends FeatureMetadata { - flagsmithId: number; + id: number; } export interface FeatureContextWithMetadata @@ -49,7 +49,7 @@ export type EvaluationResultFlags = // Segment types export interface CustomSegmentMetadata extends SegmentMetadata { - flagsmithId: number; + id?: number; source?: SegmentSource; } @@ -68,7 +68,7 @@ export interface SegmentResultWithMetadata { metadata: CustomSegmentMetadata; } -export type EvaluationResultSegments = EvaluationContextResult['segments']; +export type EvaluationResultSegments = SegmentResultWithMetadata[]; // Evaluation context types export interface GenericEvaluationContext< diff --git a/flagsmith-engine/index.ts b/flagsmith-engine/index.ts index 58f9adc..c57abd0 100644 --- a/flagsmith-engine/index.ts +++ b/flagsmith-engine/index.ts @@ -6,7 +6,7 @@ import { CustomFeatureMetadata, FlagResultWithMetadata } from './evaluation/models.js'; -import { getIdentitySegments } from './segments/evaluators.js'; +import { getIdentitySegments, getIdentityKey } from './segments/evaluators.js'; import { EvaluationResultFlags } from './evaluation/models.js'; import { TARGETING_REASONS } from './features/types.js'; import { getHashedPercentageForObjIds } from './utils/hashing/index.js'; @@ -131,7 +131,7 @@ export function evaluateFeatures( const { value: evaluatedValue, reason: evaluatedReason } = hasOverride ? { value: finalFeature.value, reason: undefined } - : evaluateFeatureValue(finalFeature, context.identity?.key); + : evaluateFeatureValue(finalFeature, getIdentityKey(context)); flags[finalFeature.name] = { name: finalFeature.name, diff --git a/flagsmith-engine/segments/evaluators.ts b/flagsmith-engine/segments/evaluators.ts index 0b08526..aecae7f 100644 --- a/flagsmith-engine/segments/evaluators.ts +++ b/flagsmith-engine/segments/evaluators.ts @@ -49,7 +49,7 @@ export function traitsMatchSegmentCondition( ): boolean { if (condition.operator === PERCENTAGE_SPLIT) { const contextValueKey = - getContextValue(condition.property, context) || context?.identity?.key; + getContextValue(condition.property, context) || getIdentityKey(context); const hashedPercentage = getHashedPercentageForObjIds([segmentKey, contextValueKey]); return hashedPercentage <= parseFloat(String(condition.value)); } @@ -173,3 +173,9 @@ export function getContextValue(jsonPath: string, context?: GenericEvaluationCon return undefined; } } + +export function getIdentityKey(context?: GenericEvaluationContext): string | undefined { + if (!context?.identity) return undefined; + + return context.identity.key || `${context.environment.key}_${context.identity.identifier}`; +} diff --git a/flagsmith-engine/segments/models.ts b/flagsmith-engine/segments/models.ts index 8f3fb4b..a61c0c1 100644 --- a/flagsmith-engine/segments/models.ts +++ b/flagsmith-engine/segments/models.ts @@ -227,13 +227,13 @@ export class SegmentModel { if (segmentResult.metadata?.source === SegmentSource.IDENTITY_OVERRIDE) { continue; } - const flagsmithId = segmentResult.metadata?.flagsmithId; - if (!flagsmithId) { + const segmentMetadataId = segmentResult.metadata?.id; + if (!segmentMetadataId) { continue; } - const segmentContext = evaluationContext.segments[flagsmithId.toString()]; + const segmentContext = evaluationContext.segments[segmentMetadataId.toString()]; if (segmentContext) { - const segment = new SegmentModel(flagsmithId, segmentContext.name); + const segment = new SegmentModel(segmentMetadataId, segmentContext.name); segment.rules = segmentContext.rules.map(rule => new SegmentRuleModel(rule.type)); segment.featureStates = SegmentModel.createFeatureStatesFromOverrides( segmentContext.overrides || [] @@ -249,13 +249,13 @@ export class SegmentModel { if (!overrides) return []; return overrides .filter(override => { - const flagsmithId = override?.metadata?.flagsmithId; - return typeof flagsmithId === 'number'; + const overrideMetadataId = override?.metadata?.id; + return typeof overrideMetadataId === 'number'; }) .map(override => { - const flagsmithId = override.metadata!.flagsmithId as number; + const overrideMetadataId = override.metadata!.id as number; const feature = new FeatureModel( - flagsmithId, + overrideMetadataId, override.name, override.variants?.length && override.variants.length > 0 ? CONSTANTS.MULTIVARIATE diff --git a/sdk/index.ts b/sdk/index.ts index 81cbde7..6618ed8 100644 --- a/sdk/index.ts +++ b/sdk/index.ts @@ -25,6 +25,7 @@ import { } from './types.js'; import { pino, Logger } from 'pino'; import { getEvaluationContext } from '../flagsmith-engine/evaluation/evaluationContext/mappers.js'; +import { EvaluationContextWithMetadata } from '../flagsmith-engine/evaluation/models.js'; export { AnalyticsProcessor, AnalyticsProcessorOptions } from './analytics.js'; export { FlagsmithAPIError, FlagsmithClientError } from './errors.js'; @@ -282,7 +283,7 @@ export class Flagsmith { if (!context) { throw new FlagsmithClientError('Local evaluation required to obtain identity segments'); } - const evaluationResult = getEvaluationResult(context); + const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata); return SegmentModel.fromSegmentResult(evaluationResult.segments, context); } @@ -449,11 +450,11 @@ export class Flagsmith { private async getEnvironmentFlagsFromDocument(): Promise { const environment = await this.getEnvironment(); - const context = getEvaluationContext(environment); + const context = getEvaluationContext(environment, undefined, undefined, true); if (!context) { throw new FlagsmithClientError('Unable to get flags. No environment present.'); } - const evaluationResult = getEvaluationResult(context); + const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata); const flags = Flags.fromEvaluationResult(evaluationResult); if (!!this.cache) { @@ -481,7 +482,7 @@ export class Flagsmith { if (!context) { throw new FlagsmithClientError('Unable to get flags. No environment present.'); } - const evaluationResult = getEvaluationResult(context); + const evaluationResult = getEvaluationResult(context as EvaluationContextWithMetadata); const flags = Flags.fromEvaluationResult( evaluationResult, diff --git a/sdk/models.ts b/sdk/models.ts index 17b0b22..ebea0d2 100644 --- a/sdk/models.ts +++ b/sdk/models.ts @@ -118,10 +118,10 @@ export class Flags { ): Flags { const flags: { [key: string]: Flag } = {}; for (const flag of Object.values(evaluationResult.flags)) { - const flagsmithId = flag.metadata?.flagsmithId; - if (!flagsmithId) { + const flagMetadataId = flag.metadata?.id; + if (!flagMetadataId) { throw new Error( - `FlagResult metadata.flagsmithId is missing for feature "${flag.name}". ` + + `FlagResult metadata.id is missing for feature "${flag.name}". ` + `This indicates a bug in the SDK, please report it.` ); } @@ -129,7 +129,7 @@ export class Flags { flags[flag.name] = new Flag({ enabled: flag.enabled, value: flag.value ?? null, - featureId: flagsmithId, + featureId: flagMetadataId, featureName: flag.name, reason: flag.reason }); diff --git a/tests/engine/engine-tests/engine-test-data b/tests/engine/engine-tests/engine-test-data index 8d19e96..6ab57ec 160000 --- a/tests/engine/engine-tests/engine-test-data +++ b/tests/engine/engine-tests/engine-test-data @@ -1 +1 @@ -Subproject commit 8d19e9627013c0e0213c29f3318fd45a179868fa +Subproject commit 6ab57ec67bc84659e8b5aa41534b04fe45cc4cbe diff --git a/tests/engine/unit/engine.test.ts b/tests/engine/unit/engine.test.ts index c1a60a3..1c43f8e 100644 --- a/tests/engine/unit/engine.test.ts +++ b/tests/engine/unit/engine.test.ts @@ -115,7 +115,7 @@ test('shouldApplyOverride with priority conflicts', () => { enabled: true, value: 'value', priority: 5, - metadata: { flagsmithId: 1 } + metadata: { id: 1 } }, segmentName: 'segment1' } @@ -185,7 +185,7 @@ test('evaluateSegments handles segments with identity identifier matching', () = name: 'feature1', enabled: false, value: 'default_value', - metadata: { flagsmithId: 1 } + metadata: { id: 1 } } } }; @@ -272,7 +272,7 @@ test('evaluateSegments handles priority conflicts correctly', () => { name: 'feature1', enabled: false, value: 'default_value', - metadata: { flagsmithId: 1 } + metadata: { id: 1 } } } }; @@ -343,7 +343,7 @@ test('evaluateFeatures with multivariate evaluation', () => { { value: 'variant_a', weight: 0, priority: 1 }, { value: 'variant_b', weight: 100, priority: 2 } ], - metadata: { flagsmithId: 1 } + metadata: { id: 1 } } }, identity: { key: 'test_user', identifier: 'test_user' }, diff --git a/tests/engine/unit/segments/segments_model.test.ts b/tests/engine/unit/segments/segments_model.test.ts index 0c84448..1210998 100644 --- a/tests/engine/unit/segments/segments_model.test.ts +++ b/tests/engine/unit/segments/segments_model.test.ts @@ -141,7 +141,7 @@ test('test_segment_rule_matching_function', () => { }); test('test_fromSegmentResult_with_multiple_variants', () => { - const segmentResults = [{ name: 'test_segment', metadata: { flagsmithId: '1' } }]; + const segmentResults = [{ name: 'test_segment', metadata: { id: '1' } }]; const evaluationContext: EvaluationContext = { identity: { @@ -159,7 +159,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => { name: 'test_segment', metadata: { source: SegmentSource.API, - flagsmithId: '1' + id: '1' }, rules: [ { @@ -181,7 +181,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => { value: 'default_value', priority: 1, metadata: { - flagsmithId: 1 + id: 1 }, variants: [ { id: 1, value: 'variant_a', weight: 30 }, diff --git a/tests/sdk/flagsmith.test.ts b/tests/sdk/flagsmith.test.ts index caa80f6..b0b66f3 100644 --- a/tests/sdk/flagsmith.test.ts +++ b/tests/sdk/flagsmith.test.ts @@ -520,7 +520,7 @@ test('get_user_agent_extracts_version_from_package_json', async () => { expect(userAgent).toBe(`flagsmith-nodejs-sdk/${packageJson.version}`); }); -test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missing', () => { +test('Flags.fromEvaluationResult throws error when metadata.id is missing', () => { const evaluationResult = { flags: { test_feature: { @@ -535,7 +535,7 @@ test('Flags.fromEvaluationResult throws error when metadata.flagsmithId is missi }; expect(() => Flags.fromEvaluationResult(evaluationResult as any)).toThrow( - 'FlagResult metadata.flagsmithId is missing for feature "test_feature". ' + + 'FlagResult metadata.id is missing for feature "test_feature". ' + 'This indicates a bug in the SDK, please report it.' ); });