Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 = [email protected]:Flagsmith/engine-test-data.git
branch = v3.0.0
branch = v3.1.0
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite enjoying the Custom prefix here

Suggested change
export interface CustomFeatureMetadata extends FeatureMetadata {
export interface SDKFeatureMetadata extends FeatureMetadata {

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we use named arguments here?

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.'
);
});