From 1b7df08106a63018ec5896dc1f4a5df1c3e72730 Mon Sep 17 00:00:00 2001 From: "linear-code[bot]" <222613912+linear-code[bot]@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:16:57 +0000 Subject: [PATCH 1/2] feat(Engine): Evaluate flags that depend on other flags Port Flag Dependency from flagsmith-engine 11.1.1. Segment conditions can read other flags' evaluated results through `$.flags.` JSONPath properties, resolved lazily and transitively. Flags caught in a dependency cycle report `ERROR; code=CIRCULAR_DEPENDENCY`. Align the rest of the engine with flagsmith-engine 11.1.1, as required by engine-test-data v3.12.1: - Flag results carry the selected multivariate `variant`. - Flags without segment overrides report the `STATIC` reason. - Segments are evaluated for contexts without an identity, so environment flags are evaluated without segments. Fix engine type generation now that the evaluation context schema references the evaluation result schema. Generated with [Linear](https://linear.app/flagsmith/agent-session/389c0494-a028-440c-93fd-4dfab403905d) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com> --- .gitmodules | 2 +- .../evaluationContext.types.ts | 42 ++ .../evaluation/evaluationContext/mappers.ts | 3 +- flagsmith-engine/evaluation/models.ts | 3 +- flagsmith-engine/features/types.ts | 3 + flagsmith-engine/index.ts | 368 ++++++++++++------ flagsmith-engine/segments/evaluators.ts | 30 +- package.json | 2 +- tests/engine/engine-tests/engine-test-data | 2 +- tests/engine/unit/engine.test.ts | 6 +- tests/engine/unit/mappers.test.ts | 4 +- tests/sdk/flagsmith-environment-flags.test.ts | 21 + tests/sdk/flagsmith-identity-flags.test.ts | 47 +++ 13 files changed, 394 insertions(+), 139 deletions(-) diff --git a/.gitmodules b/.gitmodules index e48e5296..1b07fdc9 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.7.0 + branch = v3.12.1 diff --git a/flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts b/flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts index 87cc8576..9cebefb8 100644 --- a/flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts +++ b/flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts @@ -128,6 +128,26 @@ export type FeaturePriority = number; * Feature overrides for the segment. */ export type Overrides = FeatureContext[]; +/** + * Feature name. + */ +export type Name3 = string; +/** + * Indicates if the feature flag is enabled. + */ +export type Enabled1 = boolean; +/** + * Feature flag value. + */ +export type Value4 = string | number | boolean | null; +/** + * Reason for the feature flag evaluation. + */ +export type Reason = string; +/** + * A stable identifier of the multivariate variant the identity was bucketed into: the variant's key, "control" for the control bucket, or null when no multivariate split applied (a standard feature, an unkeyed variant, or evaluation without an identity). + */ +export type Variant = string | null; /** * A context object containing the necessary information to evaluate Flagsmith feature flags. @@ -140,6 +160,7 @@ export interface EvaluationContext { identity?: IdentityContext | null; segments?: Segments; features?: Features; + flags?: FlagResults; [k: string]: unknown; } /** @@ -250,3 +271,24 @@ export interface SegmentMetadata { export interface Features { [k: string]: FeatureContext; } +/** + * Results of features already evaluated for this context, mapped by feature name. Populated by the engine as evaluation progresses, so that segment conditions may reference `$.flags..enabled`, `.value` and `.variant` to express feature dependencies. + */ +export interface FlagResults { + [k: string]: FlagResult; +} +export interface FlagResult { + name: Name3; + enabled: Enabled1; + value: Value4; + reason: Reason; + variant: Variant; + metadata?: FeatureMetadata1; + [k: string]: unknown; +} +/** + * Additional metadata associated with the feature. + */ +export interface FeatureMetadata1 { + [k: string]: unknown; +} diff --git a/flagsmith-engine/evaluation/evaluationContext/mappers.ts b/flagsmith-engine/evaluation/evaluationContext/mappers.ts index e84849c6..c925af47 100644 --- a/flagsmith-engine/evaluation/evaluationContext/mappers.ts +++ b/flagsmith-engine/evaluation/evaluationContext/mappers.ts @@ -25,7 +25,8 @@ export function getEvaluationContext( ): GenericEvaluationContext { const environmentContext = mapEnvironmentModelToEvaluationContext(environment); if (isEnvironmentEvaluation) { - return environmentContext; + const { segments, ...environmentContextWithoutSegments } = environmentContext; + return environmentContextWithoutSegments; } const identityContext = identity ? mapIdentityModelToIdentityContext(identity, overrideTraits) diff --git a/flagsmith-engine/evaluation/models.ts b/flagsmith-engine/evaluation/models.ts index 72f98ddd..6908354f 100644 --- a/flagsmith-engine/evaluation/models.ts +++ b/flagsmith-engine/evaluation/models.ts @@ -65,7 +65,7 @@ export type SegmentsWithMetadata = export interface SegmentResultWithMetadata { name: string; - metadata: SDKSegmentMetadata; + metadata?: SDKSegmentMetadata; } export type EvaluationResultSegments = SegmentResultWithMetadata[]; @@ -79,6 +79,7 @@ export interface GenericEvaluationContext< identity?: IdentityContext | null; segments?: SegmentsWithMetadata; features?: FeaturesWithMetadata; + flags?: EvaluationResultFlags; [k: string]: unknown; } diff --git a/flagsmith-engine/features/types.ts b/flagsmith-engine/features/types.ts index f792e2d4..867ff12f 100644 --- a/flagsmith-engine/features/types.ts +++ b/flagsmith-engine/features/types.ts @@ -1,5 +1,8 @@ export enum TARGETING_REASONS { DEFAULT = 'DEFAULT', + STATIC = 'STATIC', TARGETING_MATCH = 'TARGETING_MATCH', SPLIT = 'SPLIT' } + +export const CIRCULAR_DEPENDENCY_REASON = 'ERROR; code=CIRCULAR_DEPENDENCY'; diff --git a/flagsmith-engine/index.ts b/flagsmith-engine/index.ts index 2064bb7b..426387fa 100644 --- a/flagsmith-engine/index.ts +++ b/flagsmith-engine/index.ts @@ -7,9 +7,9 @@ import { FlagResultWithMetadata, GenericEvaluationContext } from './evaluation/models.js'; -import { getIdentitySegments } from './segments/evaluators.js'; +import { isContextInSegment } from './segments/evaluators.js'; import { EvaluationResultFlags } from './evaluation/models.js'; -import { TARGETING_REASONS } from './features/types.js'; +import { CIRCULAR_DEPENDENCY_REASON, TARGETING_REASONS } from './features/types.js'; import { getHashedPercentageForObjIds } from './utils/hashing/index.js'; export { EnvironmentModel } from './environments/models.js'; export { IdentityModel } from './identities/models.js'; @@ -32,15 +32,30 @@ export type SegmentOverrides = Record; * applies feature overrides based on segment priority, and returns the final flag states with * evaluation reasons. * + * Segment conditions may read other flags' evaluated results through `$.flags..enabled`, + * `$.flags..value` or `$.flags..variant`. Flags caught in a dependency cycle + * serve their environment default with the reason {@link CIRCULAR_DEPENDENCY_REASON}. + * * @param context - EvaluationContext containing environment, identity, and segment data * @returns EvaluationResult with flags, segments, and original context */ export function getEvaluationResult( context: EvaluationContextWithMetadata ): EvaluationResultWithMetadata { - const enrichedContext = getEnrichedContext(context); - const { segments, segmentOverrides } = evaluateSegments(enrichedContext); - const flags = evaluateFeatures(enrichedContext, segmentOverrides); + const flagDependencies = new FlagDependencies(getEnrichedContext(context)); + const { segments, segmentOverrides } = evaluateSegments( + flagDependencies.context, + flagDependencies + ); + const flags = evaluateFeatures(flagDependencies.context, segmentOverrides); + + if (flagDependencies.used) { + Object.assign(flags, flagDependencies.resolved); + for (const featureName of flagDependencies.cyclic) { + const flag = flags[featureName]; + if (flag) flag.reason = CIRCULAR_DEPENDENCY_REASON; + } + } return { flags, segments }; } @@ -62,66 +77,51 @@ function getEnrichedContext(context: EvaluationContextWithMetadata): EvaluationC } /** - * Evaluates which segments the identity belongs to and collects feature overrides. + * Evaluates which segments the context belongs to and collects feature overrides. + * + * When multiple segments override the same feature, the segment with + * higher priority (lower numeric value) takes precedence. * * @param context - EvaluationContext containing identity and segment definitions - * @returns Object containing segments the identity belongs to and any feature overrides + * @param flagDependencies - Flags read by segment conditions, if the context exposes them + * @returns Object containing segments the context belongs to and any feature overrides */ -export function evaluateSegments(context: EvaluationContextWithMetadata): { +export function evaluateSegments( + context: EvaluationContextWithMetadata, + flagDependencies?: FlagDependencies +): { segments: EvaluationResultSegments; segmentOverrides: Record; } { - if (!context.identity || !context.segments) { - return { - segments: [], - segmentOverrides: {} as Record - }; - } - const identitySegments = getIdentitySegments(context); - - const segments = identitySegments.map(segment => ({ - name: segment.name, - ...(segment.metadata - ? { - metadata: { - ...segment.metadata - } - } - : {}) - })) as EvaluationResultSegments; - const segmentOverrides = processSegmentOverrides(identitySegments); - - return { segments, segmentOverrides }; -} - -/** - * Processes feature overrides from segments, applying priority rules. - * - * When multiple segments override the same feature, the segment with - * higher priority (lower numeric value) takes precedence. - * - * @param identitySegments - Segments that the identity belongs to - * @returns Map of feature keys to their highest-priority segment overrides - */ -export function processSegmentOverrides(identitySegments: any[]): Record { + const segments: EvaluationResultSegments = []; const segmentOverrides: Record = {}; + let cycleHits = flagDependencies?.cycleHits; + + for (const segment of Object.values(context.segments || {})) { + const matches = isContextInSegment(context, segment); - for (const segment of identitySegments) { - if (!segment.overrides) continue; + if (flagDependencies && flagDependencies.cycleHits !== cycleHits) { + cycleHits = flagDependencies.cycleHits; // Matched or not on an unresolvable flag + continue; + } + if (!matches) continue; - const overridesList = Array.isArray(segment.overrides) ? segment.overrides : []; + segments.push({ + name: segment.name, + ...(segment.metadata ? { metadata: { ...segment.metadata } } : {}) + }); - for (const override of overridesList) { + for (const override of segment.overrides || []) { if (shouldApplyOverride(override, segmentOverrides)) { segmentOverrides[override.name] = { - feature: override, + feature: override as FeatureContextWithMetadata, segmentName: segment.name }; } } } - return segmentOverrides; + return { segments, segmentOverrides }; } /** @@ -130,7 +130,7 @@ export function processSegmentOverrides(identitySegments: any[]): Record ): EvaluationResultFlags { const flags: EvaluationResultFlags = {}; + const targetedFeatureNames = getTargetedFeatureNames(context); for (const feature of Object.values(context.features || {})) { const segmentOverride = segmentOverrides[feature.name]; - const finalFeature = segmentOverride ? segmentOverride.feature : feature; - - const { value: evaluatedValue, reason: evaluatedReason } = evaluateFeatureValue( - finalFeature, - getIdentityKey(context) - ); - - flags[finalFeature.name] = { - name: finalFeature.name, - enabled: finalFeature.enabled, - value: evaluatedValue, - ...(finalFeature.metadata ? { metadata: finalFeature.metadata } : {}), - reason: - evaluatedReason ?? - getTargetingMatchReason({ type: 'SEGMENT', override: segmentOverride }) - } as FlagResultWithMetadata; + flags[feature.name] = segmentOverride + ? getFlagResult( + context, + segmentOverride.feature, + getTargetingMatchReason(segmentOverride.segmentName) + ) + : getFlagResult( + context, + feature, + getFallbackReason(feature.name, targetedFeatureNames) + ); } return flags; } -function evaluateFeatureValue( - feature: FeatureContextWithMetadata, - identityKey?: string -): { value: any; reason?: string } { - if (!!feature.variants && feature.variants.length > 0 && !!identityKey) { - return getMultivariateFeatureValue(feature, identityKey); +/** + * Names of the features overridden by at least one segment. + */ +function getTargetedFeatureNames(context: EvaluationContextWithMetadata): Set { + const targetedFeatureNames = new Set(); + for (const segment of Object.values(context.segments || {})) { + for (const override of segment.overrides || []) { + targetedFeatureNames.add(override.name); + } } + return targetedFeatureNames; +} + +/** + * Reason for a flag evaluated to its environment default: `DEFAULT` if the feature has + * targeting rules that did not match, `STATIC` if it has none. + */ +function getFallbackReason( + featureName: string, + targetedFeatureNames: { has(featureName: string): boolean } +): string { + return targetedFeatureNames.has(featureName) + ? TARGETING_REASONS.DEFAULT + : TARGETING_REASONS.STATIC; +} - return { value: feature.value, reason: undefined }; +function getTargetingMatchReason(segmentName: string): string { + return `${TARGETING_REASONS.TARGETING_MATCH}; segment=${segmentName}`; } /** - * Evaluates a multivariate feature flag to determine which variant value to return for a given identity. + * Evaluates a feature to a flag result, selecting a multivariate variant for identities. * * Uses deterministic hashing to ensure the same identity always receives the same variant, * while distributing variants according to their configured weight percentages. * - * @param feature - The feature context containing variants and their weights - * @param identityKey - The identity key used for deterministic variant selection - * @returns The variant value if the identity falls within a variant's range, otherwise the default feature value + * @param context - The evaluation context, providing the identity key + * @param feature - The feature context, or the segment override, to evaluate + * @param reason - The reason to report when no variant is selected + * @returns The flag result */ -function getMultivariateFeatureValue( - feature: FeatureContextWithMetadata, - identityKey?: string -): { value: any; reason?: string } { - const percentageValue = getHashedPercentageForObjIds([feature.key, identityKey]); - const sortedVariants = [...(feature?.variants || [])].sort((a, b) => { - const aPriority = a.priority ?? Infinity; - const bPriority = b.priority ?? Infinity; - if (aPriority < bPriority) return -1; - if (aPriority > bPriority) return 1; - return 0; - }); - - let startPercentage = 0; - for (const variant of sortedVariants) { - const limit = startPercentage + variant.weight; - if (startPercentage <= percentageValue && percentageValue < limit) { - return { - value: variant.value, - reason: getTargetingMatchReason({ type: 'SPLIT', weight: variant.weight }) - }; +function getFlagResult( + context: EvaluationContextWithMetadata, + feature: FeatureContextWithMetadata, + reason: string +): FlagResultWithMetadata { + const identityKey = getIdentityKey(context); + let value = feature.value; + let variant: string | null = null; + + if (identityKey && feature.variants && feature.variants.length > 0) { + variant = 'control'; + const percentageValue = getHashedPercentageForObjIds([feature.key, identityKey]); + const sortedVariants = [...feature.variants].sort((a, b) => { + const aPriority = a.priority ?? Infinity; + const bPriority = b.priority ?? Infinity; + if (aPriority < bPriority) return -1; + if (aPriority > bPriority) return 1; + return 0; + }); + + let startPercentage = 0; + for (const featureVariant of sortedVariants) { + const limit = startPercentage + featureVariant.weight; + if (startPercentage <= percentageValue && percentageValue < limit) { + value = featureVariant.value; + variant = featureVariant.key ?? null; + reason = `${TARGETING_REASONS.SPLIT}; weight=${featureVariant.weight}`; + break; + } + startPercentage = limit; } - startPercentage = limit; } - return { value: feature.value, reason: undefined }; + return { + name: feature.name, + enabled: feature.enabled, + value, + reason, + variant, + ...(feature.metadata ? { metadata: feature.metadata } : {}) + } as FlagResultWithMetadata; } export function shouldApplyOverride( - override: any, + override: { name: string; priority?: number }, existingOverrides: Record ): boolean { const currentOverride = existingOverrides[override.name]; @@ -231,31 +262,128 @@ export function isHigherPriority( return (priorityA ?? Infinity) < (priorityB ?? Infinity); } -export type TargetingMatchReason = - | { - type: 'SEGMENT'; - override: SegmentOverride; - } - | { - type: 'SPLIT'; - weight: number; - }; - -const getTargetingMatchReason = (matchObject: TargetingMatchReason) => { - const { type } = matchObject; - - if (type === 'SEGMENT') { - return matchObject.override - ? `${TARGETING_REASONS.TARGETING_MATCH}; segment=${matchObject.override.segmentName}` - : TARGETING_REASONS.DEFAULT; +/** + * The `$.flags` mapping of an evaluation context, resolving a flag when a segment condition + * first reads it, transitively and with a guard against dependency cycles. + */ +export class FlagDependencies { + /** Flags resolved so far. */ + readonly resolved: EvaluationResultFlags = {}; + /** Names of the flags found to be in a dependency cycle. */ + readonly cyclic = new Set(); + /** Whether a segment condition has read a flag. */ + used = false; + /** Number of cycles cut. A segment whose evaluation changes it must not match. */ + cycleHits = 0; + /** The evaluation context, exposing the `$.flags` mapping to segment conditions. */ + readonly context: EvaluationContextWithMetadata; + + private readonly resolving: string[] = []; + private readonly segmentMatches = new Map(); + private segmentKeysByFeatureName?: Map; + + constructor(context: EvaluationContextWithMetadata) { + const flags = new Proxy(this.resolved, { + get: (target, property) => + this.read(property) ? Reflect.get(target, property) : undefined, + getOwnPropertyDescriptor: (target, property) => + this.read(property) ? Reflect.getOwnPropertyDescriptor(target, property) : undefined + }); + this.context = { ...context, flags }; } - if (type === 'SPLIT') { - return `${TARGETING_REASONS.SPLIT}; weight=${matchObject.weight}`; + private read(property: string | symbol): boolean { + if (typeof property !== 'string') return false; + if (!hasOwn(this.resolved, property)) { + this.used = true; + this.resolveFeature(property); + } + return hasOwn(this.resolved, property); } - return TARGETING_REASONS.DEFAULT; -}; + private resolveFeature(featureName: string): void { + const cycleStart = this.resolving.indexOf(featureName); + if (cycleStart !== -1) { + for (const name of this.resolving.slice(cycleStart)) this.cyclic.add(name); + this.cycleHits++; + return; + } + const features = this.context.features || {}; + if (!hasOwn(features, featureName)) return; + const feature = features[featureName]; + + this.resolving.push(featureName); + let segmentOverride: SegmentOverride | undefined; + try { + segmentOverride = this.getSegmentOverride(featureName); + } finally { + this.resolving.pop(); // Errors are swallowed by JSONPath evaluation + } + + if (this.cyclic.has(featureName)) return; + + this.resolved[featureName] = segmentOverride + ? getFlagResult( + this.context, + segmentOverride.feature, + getTargetingMatchReason(segmentOverride.segmentName) + ) + : getFlagResult( + this.context, + feature, + getFallbackReason(featureName, this.getSegmentKeysByFeatureName()) + ); + } + + private matchesSegment(segmentKey: string): boolean { + const memoised = this.segmentMatches.get(segmentKey); + if (memoised !== undefined) return memoised; + + const cycleHits = this.cycleHits; + const matches = isContextInSegment(this.context, this.context.segments![segmentKey]); + if (this.cycleHits !== cycleHits) return false; // Matched or not on an unresolvable flag + + this.segmentMatches.set(segmentKey, matches); + return matches; + } + + private getSegmentOverride(featureName: string): SegmentOverride | undefined { + const segmentOverrides: Record = {}; + + for (const segmentKey of this.getSegmentKeysByFeatureName().get(featureName) || []) { + if (!this.matchesSegment(segmentKey)) continue; + const segment = this.context.segments![segmentKey]; + for (const override of segment.overrides || []) { + if (override.name !== featureName) continue; + if (shouldApplyOverride(override, segmentOverrides)) { + segmentOverrides[featureName] = { + feature: override as FeatureContextWithMetadata, + segmentName: segment.name + }; + } + } + } + + return segmentOverrides[featureName]; + } + + private getSegmentKeysByFeatureName(): Map { + if (!this.segmentKeysByFeatureName) { + this.segmentKeysByFeatureName = new Map(); + for (const [segmentKey, segment] of Object.entries(this.context.segments || {})) { + for (const override of segment.overrides || []) { + const segmentKeys = this.segmentKeysByFeatureName.get(override.name) || []; + segmentKeys.push(segmentKey); + this.segmentKeysByFeatureName.set(override.name, segmentKeys); + } + } + } + return this.segmentKeysByFeatureName; + } +} + +const hasOwn = (object: object, property: string): boolean => + Object.prototype.hasOwnProperty.call(object, property); const getIdentityKey = (context: GenericEvaluationContext): string | undefined => { if (!context.identity) return undefined; diff --git a/flagsmith-engine/segments/evaluators.ts b/flagsmith-engine/segments/evaluators.ts index 82551e8b..fb59ce94 100644 --- a/flagsmith-engine/segments/evaluators.ts +++ b/flagsmith-engine/segments/evaluators.ts @@ -11,21 +11,33 @@ import { SegmentConditionModel } from './models.js'; import { IS_NOT_SET, IS_SET, PERCENTAGE_SPLIT } from './constants.js'; /** - * Returns all segments that the identity belongs to based on segment rules evaluation. + * Returns all segments that the context belongs to based on segment rules evaluation. * - * An identity belongs to a segment if it matches ALL of the segment's rules. - * If the context has no identity or segments, returns an empty array. + * If the context has no segments, returns an empty array. * * @param context - Evaluation context containing identity and segment definitions - * @returns Array of segments that the identity matches + * @returns Array of segments that the context matches */ export function getIdentitySegments(context: GenericEvaluationContext): SegmentContext[] { - if (!context.identity || !context.segments) return []; + if (!context.segments) return []; - return Object.values(context.segments).filter(segment => { - if (segment.rules.length === 0) return false; - return segment.rules.every(rule => traitsMatchSegmentRule(rule, segment.key, context)); - }); + return Object.values(context.segments).filter(segment => isContextInSegment(context, segment)); +} + +/** + * Whether the context belongs to a segment, i.e. it matches ALL of the segment's rules. + * A segment without rules matches no context. + * + * @param context - Evaluation context containing identity, traits, and environment + * @param segment - The segment to evaluate + * @returns true if the context matches every rule of the segment + */ +export function isContextInSegment( + context: GenericEvaluationContext, + segment: SegmentContext +): boolean { + if (segment.rules.length === 0) return false; + return segment.rules.every(rule => traitsMatchSegmentRule(rule, segment.key, context)); } /** diff --git a/package.json b/package.json index e774f572..473ae90e 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "deploy:beta": "npm i && npm run build && npm publish --tag beta", "prepare": "husky install", "generate-evaluation-result-types": "curl -o evaluation-result.json https://raw.githubusercontent.com/Flagsmith/flagsmith/main/sdk/evaluation-result.json && npx json2ts -i evaluation-result.json -o flagsmith-engine/evaluation/evaluationResult/evaluationResult.types.ts && rm evaluation-result.json", - "generate-evaluation-context-types": "curl -o evaluation-context.json https://raw.githubusercontent.com/Flagsmith/flagsmith/main/sdk/evaluation-context.json && npx json2ts -i evaluation-context.json -o flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts && rm evaluation-context.json", + "generate-evaluation-context-types": "curl -o evaluation-context.json https://raw.githubusercontent.com/Flagsmith/flagsmith/main/sdk/evaluation-context.json && curl -o evaluation-result.json https://raw.githubusercontent.com/Flagsmith/flagsmith/main/sdk/evaluation-result.json && npx json2ts -i evaluation-context.json -o flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts && rm evaluation-context.json evaluation-result.json", "generate-engine-types": "npm run generate-evaluation-result-types && npm run generate-evaluation-context-types" }, "dependencies": { diff --git a/tests/engine/engine-tests/engine-test-data b/tests/engine/engine-tests/engine-test-data index 4b29dc77..cb067401 160000 --- a/tests/engine/engine-tests/engine-test-data +++ b/tests/engine/engine-tests/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit cb067401093766e55c3354c1de7c73d9331334df diff --git a/tests/engine/unit/engine.test.ts b/tests/engine/unit/engine.test.ts index 89b50784..ad202fb4 100644 --- a/tests/engine/unit/engine.test.ts +++ b/tests/engine/unit/engine.test.ts @@ -39,7 +39,7 @@ test('test_get_evaluation_result_without_any_override', () => { const flag = Object.values(result.flags).find(f => f.name === feature1().name); expect(flag).toBeDefined(); expect(flag?.name).toBe(feature1().name); - expect(flag?.reason).toBe(TARGETING_REASONS.DEFAULT); + expect(flag?.reason).toBe(TARGETING_REASONS.STATIC); }); test('test_get_evaluation_result_with_identity_override_and_no_segment_override', () => { @@ -67,7 +67,7 @@ test('test_get_evaluation_result_with_identity_override_and_no_segment_override' expect(flag.reason).toBe( flag.name === 'overridden_feature' ? `${TARGETING_REASONS.TARGETING_MATCH}; segment=${IDENTITY_OVERRIDE_SEGMENT_NAME}` - : TARGETING_REASONS.DEFAULT + : TARGETING_REASONS.STATIC ); } }); @@ -97,7 +97,7 @@ test('test_environment_get_all_feature_states', () => { expect(Object.keys(result.flags).length).toBe(Object.keys(context.features || {}).length); Object.values(result.flags).forEach(flag => { - expect(flag.reason).toBe(TARGETING_REASONS.DEFAULT); + expect(flag.reason).toBe(TARGETING_REASONS.STATIC); }); for (const flag of Object.values(result.flags)) { diff --git a/tests/engine/unit/mappers.test.ts b/tests/engine/unit/mappers.test.ts index 7b294734..94f0aa48 100644 --- a/tests/engine/unit/mappers.test.ts +++ b/tests/engine/unit/mappers.test.ts @@ -278,7 +278,7 @@ describe('getEvaluationContext', () => { }); }); - test('produces evaluation context without identity when isEnvironmentEvaluation is true', () => { + test('produces evaluation context without identity or segments when isEnvironmentEvaluation is true', () => { // Given const identity = new IdentityModel( '2024-01-01T00:00:00Z', @@ -297,7 +297,7 @@ describe('getEvaluationContext', () => { expect(context.identity).toBeUndefined(); expect(context.environment).toBeDefined(); expect(context.features).toBeDefined(); - expect(context.segments).toBeDefined(); + expect(context.segments).toBeUndefined(); }); test('handles identity without django_id', () => { diff --git a/tests/sdk/flagsmith-environment-flags.test.ts b/tests/sdk/flagsmith-environment-flags.test.ts index e4244247..2df4d707 100644 --- a/tests/sdk/flagsmith-environment-flags.test.ts +++ b/tests/sdk/flagsmith-environment-flags.test.ts @@ -181,3 +181,24 @@ test('test_local_evaluation', async () => { expect(flag.value).not.toBe(defaultFlag.value); expect(flag.value).toBe('some-value'); }); + +test('local evaluation of environment flags ignores segment overrides', async () => { + // Given + const environmentWithSegmentWithoutTraits = JSON.parse(environmentJSON); + environmentWithSegmentWithoutTraits.project.segments[0].rules = [ + { + type: 'ALL', + conditions: [{ property_: 'age', operator: 'IS_NOT_SET', value: null }], + rules: [] + } + ]; + fetch.mockResolvedValue(new Response(JSON.stringify(environmentWithSegmentWithoutTraits))); + const flg = flagsmith({ environmentKey: 'ser.key', enableLocalEvaluation: true }); + + // When + const flag = (await flg.getEnvironmentFlags()).getFlag('some_feature'); + + // Then + expect(flag.value).toBe('some-value'); + expect(flag.enabled).toBe(true); +}); diff --git a/tests/sdk/flagsmith-identity-flags.test.ts b/tests/sdk/flagsmith-identity-flags.test.ts index 9bccd6a2..1a3c003c 100644 --- a/tests/sdk/flagsmith-identity-flags.test.ts +++ b/tests/sdk/flagsmith-identity-flags.test.ts @@ -335,3 +335,50 @@ test('getIdentityFlags fails if API call failed and no default flag handler was 'getIdentityFlags failed and no default flag handler was provided' ); }); + +test.each([ + [41, true], + [21, false] +])( + 'local evaluation enables a flag only for identities who get its prerequisite flag enabled (age %s)', + async (age, expectedEnabled) => { + // Given + const environmentWithFlagDependency = JSON.parse(environmentJSON); + environmentWithFlagDependency.project.segments.push({ + id: 2, + name: 'follows_some_feature', + rules: [ + { + type: 'ALL', + conditions: [ + { + property_: '$.flags.some_feature.enabled', + operator: 'EQUAL', + value: 'true' + } + ], + rules: [] + } + ], + feature_states: [ + { + feature: { id: 2, name: 'mv_feature', type: 'STANDARD' }, + featurestate_uuid: '33333333-3333-3333-3333-333333333333', + django_id: 81028, + feature_state_value: 'foo', + enabled: true, + multivariate_feature_state_values: [] + } + ] + }); + fetch.mockResolvedValue(new Response(JSON.stringify(environmentWithFlagDependency))); + const flg = flagsmith({ environmentKey: 'ser.key', enableLocalEvaluation: true }); + + // When + const identityFlags = await flg.getIdentityFlags('identifier', { age }); + + // Then + expect(identityFlags.isFeatureEnabled('some_feature')).toBe(expectedEnabled); + expect(identityFlags.isFeatureEnabled('mv_feature')).toBe(expectedEnabled); + } +); From e9f4444bd3b520f7ff00f397ad0d4b5a68681be2 Mon Sep 17 00:00:00 2001 From: Evandro Myller <22429+emyller@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:22:54 +0000 Subject: [PATCH 2/2] perf(Engine): Avoid repeated work when resolving flag dependencies Resolve JSONPaths made of member names only, such as `$.flags..enabled` or `$.identity.identifier`, without a JSONPath evaluation. Other paths are still evaluated by jsonpath-plus. Reuse the segment matches memoised while resolving flag dependencies when evaluating segments, so segments read through a dependency are not evaluated twice. Memoise only once a segment condition has read a flag, so environments without flag dependencies do not pay for it. Generated with [Linear](https://linear.app/flagsmith/agent-session/389c0494-a028-440c-93fd-4dfab403905d) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com> --- flagsmith-engine/index.ts | 22 +++++++++---------- flagsmith-engine/segments/evaluators.ts | 22 +++++++++++++++++++ .../unit/segments/segment_evaluators.test.ts | 5 ++++- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/flagsmith-engine/index.ts b/flagsmith-engine/index.ts index 426387fa..37783622 100644 --- a/flagsmith-engine/index.ts +++ b/flagsmith-engine/index.ts @@ -95,15 +95,11 @@ export function evaluateSegments( } { const segments: EvaluationResultSegments = []; const segmentOverrides: Record = {}; - let cycleHits = flagDependencies?.cycleHits; - for (const segment of Object.values(context.segments || {})) { - const matches = isContextInSegment(context, segment); - - if (flagDependencies && flagDependencies.cycleHits !== cycleHits) { - cycleHits = flagDependencies.cycleHits; // Matched or not on an unresolvable flag - continue; - } + for (const [segmentKey, segment] of Object.entries(context.segments || {})) { + const matches = flagDependencies + ? flagDependencies.matchesSegment(segmentKey) + : isContextInSegment(context, segment); if (!matches) continue; segments.push({ @@ -335,15 +331,19 @@ export class FlagDependencies { ); } - private matchesSegment(segmentKey: string): boolean { - const memoised = this.segmentMatches.get(segmentKey); + /** + * Whether the context belongs to a segment. A segment matching, or not, on a flag in a + * dependency cycle does not match. + */ + matchesSegment(segmentKey: string): boolean { + const memoised = this.used ? this.segmentMatches.get(segmentKey) : undefined; if (memoised !== undefined) return memoised; const cycleHits = this.cycleHits; const matches = isContextInSegment(this.context, this.context.segments![segmentKey]); if (this.cycleHits !== cycleHits) return false; // Matched or not on an unresolvable flag - this.segmentMatches.set(segmentKey, matches); + if (this.used) this.segmentMatches.set(segmentKey, matches); return matches; } diff --git a/flagsmith-engine/segments/evaluators.ts b/flagsmith-engine/segments/evaluators.ts index fb59ce94..868972aa 100644 --- a/flagsmith-engine/segments/evaluators.ts +++ b/flagsmith-engine/segments/evaluators.ts @@ -186,6 +186,25 @@ function isPrimitive(value: any): boolean { return typeof value !== 'object'; } +const MEMBER_NAMES_PATH_PATTERN = /^\$(?:\.[A-Za-z_][A-Za-z0-9_]*)+$/; +const UNSUPPORTED_PATH = Symbol('UNSUPPORTED_PATH'); + +/** + * Resolves a JSONPath made of object member names only, e.g. `$.flags.my_feature.enabled`, + * without the cost of a JSONPath evaluation. Other paths are left to JSONPath. + */ +function getMemberValue(jsonPath: string, json: object): unknown { + if (!MEMBER_NAMES_PATH_PATTERN.test(jsonPath)) return UNSUPPORTED_PATH; + + let value: unknown = json; + for (const memberName of jsonPath.slice(2).split('.')) { + if (typeof value !== 'object' || value === null) return UNSUPPORTED_PATH; + if (!Object.prototype.hasOwnProperty.call(value, memberName)) return undefined; + value = (value as Record)[memberName]; + } + return value; +} + /** * Evaluates JSONPath expressions against the evaluation context. * @@ -203,6 +222,9 @@ function isPrimitive(value: any): boolean { export function getContextValue(jsonPath: string, context?: GenericEvaluationContext): any { if (!context || !jsonPath?.startsWith('$.')) return undefined; + const memberValue = getMemberValue(jsonPath, context); + if (memberValue !== UNSUPPORTED_PATH) return memberValue; + try { const results = JSONPath({ path: jsonPath, json: context }); return results.length > 0 ? results[0] : undefined; diff --git a/tests/engine/unit/segments/segment_evaluators.test.ts b/tests/engine/unit/segments/segment_evaluators.test.ts index 08fc0f5e..49fdfdf2 100644 --- a/tests/engine/unit/segments/segment_evaluators.test.ts +++ b/tests/engine/unit/segments/segment_evaluators.test.ts @@ -433,7 +433,9 @@ describe('getContextValue', () => { test.each([ ['$.identity.identifier', 'user@example.com'], ['$.environment.name', 'Test Environment'], - ['$.environment.key', 'test-env-key'] + ['$.environment.key', 'test-env-key'], + ["$.environment['name']", 'Test Environment'], + ['$.environment.name.length', 16] ])('returns correct value for path %s', (jsonPath, expected) => { const result = getContextValue(jsonPath, mockContext); expect(result).toBe(expected); @@ -445,6 +447,7 @@ describe('getContextValue', () => { ['identity.identifier', 'missing $ prefix'], ['$.invalid.path', 'completely invalid path'], ['$.identity.nonexistent', 'valid structure but missing property'], + ['$.identity.identifier.nonexistent', 'missing property of a text value'], ['', 'empty string'], ['$', 'just $ symbol'] ])('returns undefined for %s (%s)', jsonPath => {