Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -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
Comment thread
khvn26 marked this conversation as resolved.
40 changes: 27 additions & 13 deletions flagsmith-engine/evaluation/evaluationContext/mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -40,7 +47,7 @@ function mapEnvironmentModelToEvaluationContext(
name: environment.project.name
};

const features: FeaturesWithMetadata = {};
const features: FeaturesWithMetadata<CustomFeatureMetadata> = {};
for (const fs of environment.featureStates) {
const variants =
fs.multivariateFeatureStateValues?.length > 0
Expand All @@ -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<CustomSegmentMetadata> = {};
for (const segment of environment.project.segments) {
segmentOverrides[segment.id.toString()] = {
key: segment.id.toString(),
Expand All @@ -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<CustomSegmentMetadata> = {};
if (environment.identityOverrides && environment.identityOverrides.length > 0) {
identityOverrideSegments = mapIdentityOverridesToSegments(environment.identityOverrides);
}
Expand All @@ -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 {
Expand All @@ -135,8 +147,10 @@ function mapSegmentRuleModelToRule(rule: any): any {
};
}

function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Segments {
const segments: Segments = {};
function mapIdentityOverridesToSegments(
identityOverrides: IdentityModel[]
): SegmentsWithMetadata<CustomSegmentMetadata> {
const segments: SegmentsWithMetadata<CustomSegmentMetadata> = {};
const featuresToIdentifiers = new Map<string, { identifiers: string[]; overrides: any[] }>();

for (const identity of identityOverrides) {
Expand All @@ -153,7 +167,7 @@ function mapIdentityOverridesToSegments(identityOverrides: IdentityModel[]): Seg
value: fs.getValue(),
priority: -Infinity,
metadata: {
flagsmithId: fs.feature.id
id: fs.feature.id
}
}));

Expand Down
6 changes: 3 additions & 3 deletions flagsmith-engine/evaluation/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export enum SegmentSource {

// Feature types
export interface CustomFeatureMetadata extends FeatureMetadata {
Comment thread
Zaimwa9 marked this conversation as resolved.
Outdated
flagsmithId: number;
id: number;
}

export interface FeatureContextWithMetadata<T extends FeatureMetadata = FeatureMetadata>
Expand All @@ -49,7 +49,7 @@ export type EvaluationResultFlags<T extends FeatureMetadata = FeatureMetadata> =

// Segment types
export interface CustomSegmentMetadata extends SegmentMetadata {
flagsmithId: number;
id?: number;
source?: SegmentSource;
}

Expand All @@ -68,7 +68,7 @@ export interface SegmentResultWithMetadata {
metadata: CustomSegmentMetadata;
}

export type EvaluationResultSegments = EvaluationContextResult['segments'];
export type EvaluationResultSegments = SegmentResultWithMetadata[];

// Evaluation context types
export interface GenericEvaluationContext<
Expand Down
4 changes: 2 additions & 2 deletions flagsmith-engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion flagsmith-engine/segments/evaluators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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}`;
}
16 changes: 8 additions & 8 deletions flagsmith-engine/segments/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || []
Expand All @@ -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
Expand Down
9 changes: 5 additions & 4 deletions sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -449,11 +450,11 @@ export class Flagsmith {

private async getEnvironmentFlagsFromDocument(): Promise<Flags> {
const environment = await this.getEnvironment();
const context = getEvaluationContext(environment);
const context = getEvaluationContext(environment, undefined, undefined, true);
Comment thread
khvn26 marked this conversation as resolved.
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) {
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions sdk/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,18 @@ 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.`
);
}

flags[flag.name] = new Flag({
enabled: flag.enabled,
value: flag.value ?? null,
featureId: flagsmithId,
featureId: flagMetadataId,
featureName: flag.name,
reason: flag.reason
});
Expand Down
8 changes: 4 additions & 4 deletions tests/engine/unit/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ test('shouldApplyOverride with priority conflicts', () => {
enabled: true,
value: 'value',
priority: 5,
metadata: { flagsmithId: 1 }
metadata: { id: 1 }
},
segmentName: 'segment1'
}
Expand Down Expand Up @@ -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 }
}
}
};
Expand Down Expand Up @@ -272,7 +272,7 @@ test('evaluateSegments handles priority conflicts correctly', () => {
name: 'feature1',
enabled: false,
value: 'default_value',
metadata: { flagsmithId: 1 }
metadata: { id: 1 }
}
}
};
Expand Down Expand Up @@ -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' },
Expand Down
6 changes: 3 additions & 3 deletions tests/engine/unit/segments/segments_model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -159,7 +159,7 @@ test('test_fromSegmentResult_with_multiple_variants', () => {
name: 'test_segment',
metadata: {
source: SegmentSource.API,
flagsmithId: '1'
id: '1'
},
rules: [
{
Expand All @@ -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 },
Expand Down
4 changes: 2 additions & 2 deletions tests/sdk/flagsmith.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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.'
);
});