From d9731f7a1e63dd710210f6cb898dc1337aa1b81b Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 23 Jul 2026 20:44:03 -0400 Subject: [PATCH 1/3] feat(flags): add rules engine boundary --- .../__tests__/__utils__/rulesTestUtils.ts | 82 +++ .../configuration/__tests__/rules.test.ts | 175 +++++ .../configuration/__tests__/wire.test.ts | 59 ++ .../core/src/flags/configuration/rules.ts | 614 ++++++++++++++++++ packages/core/src/flags/configuration/wire.ts | 94 ++- 5 files changed, 1021 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts create mode 100644 packages/core/src/flags/configuration/__tests__/rules.test.ts create mode 100644 packages/core/src/flags/configuration/rules.ts diff --git a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts new file mode 100644 index 000000000..4eb2aef52 --- /dev/null +++ b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts @@ -0,0 +1,82 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://github.com/DataDog). + * Copyright 2016-Present Datadog, Inc. + */ + +import { OperatorType } from '@datadog/flagging-core'; +import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; + +import type { + RulesEngine, + RulesEvaluationDetails, + RulesEvaluationRequest, + RulesValueType +} from '../../rules'; + +export const buildRulesConfiguration = (): UniversalFlagConfigurationV1 => ({ + createdAt: '2026-07-23T12:00:00.000Z', + format: 'SERVER', + environment: { name: 'test' }, + flags: { + 'dynamic-flag': { + key: 'dynamic-flag', + enabled: true, + variationType: 'BOOLEAN', + variations: { + enabled: { key: 'enabled', value: true }, + disabled: { key: 'disabled', value: false } + }, + allocations: [ + { + key: 'allocation-1', + rules: [ + { + conditions: [ + { + operator: OperatorType.ONE_OF, + attribute: 'country', + value: ['US'] + } + ] + } + ], + splits: [ + { + variationKey: 'enabled', + serialId: 7, + extraLogging: { experiment: 'checkout' }, + shards: [ + { + salt: 'test-salt', + ranges: [{ start: 0, end: 100 }], + totalShards: 100 + } + ] + } + ], + doLog: false + } + ] + } + } +}); + +type FakeRulesEvaluation = RulesEvaluationDetails; + +export interface FakeRulesEngine extends RulesEngine { + evaluate: jest.Mock< + FakeRulesEvaluation, + [RulesEvaluationRequest] + >; +} + +// TODO(FFL-2837): Remove this fake after the upstream rules wire and engine +// contract are published and the state-matrix tests can use canonical vectors. +export const createFakeRulesEngine = ( + result: FakeRulesEvaluation +): FakeRulesEngine => { + return { + evaluate: jest.fn(() => result) + } as FakeRulesEngine; +}; diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts new file mode 100644 index 000000000..932851cdd --- /dev/null +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -0,0 +1,175 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://github.com/DataDog). + * Copyright 2016-Present Datadog, Inc. + */ + +import { + flaggingCoreRulesEngine, + getNoopRulesLogger, + prepareRulesConfiguration, + toRulesEvaluationContext +} from '../rules'; + +import { + buildRulesConfiguration, + createFakeRulesEngine +} from './__utils__/rulesTestUtils'; + +describe('rules configuration', () => { + it('converts an SDK context to a flat rules context and reserves identifiers', () => { + expect( + toRulesEvaluationContext({ + targetingKey: 'user-1', + attributes: { + country: 'US', + id: 'customer-id', + targetingKey: 'attribute-key', + enabled: true + } + }) + ).toEqual({ + targetingKey: 'user-1', + country: 'US', + enabled: true + }); + }); + + it('clones and freezes a valid rules configuration', () => { + const source = buildRulesConfiguration(); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + + source.flags['dynamic-flag'].enabled = false; + + expect(prepared.configuration.flags['dynamic-flag'].enabled).toBe(true); + expect(Object.isFrozen(prepared.configuration)).toBe(true); + expect( + Object.isFrozen( + prepared.configuration.flags['dynamic-flag'].allocations[0] + ) + ).toBe(true); + }); + + it('rejects an unsupported operator', () => { + const source = buildRulesConfiguration(); + const condition = + source.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + + if (!condition) { + throw new Error('The fixture has no condition.'); + } + (condition as { operator: string }).operator = 'ONE_OF_SHA256'; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: + 'The rules configuration uses the unsupported operator "ONE_OF_SHA256".' + }); + }); + + it('rejects an invalid regular expression', () => { + const source = buildRulesConfiguration(); + const conditions = + source.flags['dynamic-flag'].allocations[0].rules?.[0].conditions; + if (!conditions) { + throw new Error('The fixture has no conditions.'); + } + conditions[0] = { + operator: 'MATCHES', + attribute: 'country', + value: '[' + } as typeof conditions[number]; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: 'A regular expression condition is not valid.' + }); + }); + + it('rejects a split that points to an absent variation', () => { + const source = buildRulesConfiguration(); + source.flags['dynamic-flag'].allocations[0].splits[0].variationKey = + 'absent'; + + expect(prepareRulesConfiguration(source)).toEqual({ + status: 'error', + errorMessage: 'A split has an invalid variation key.' + }); + }); + + it('normalizes a real flagging-core evaluation', () => { + const configuration = buildRulesConfiguration(); + + const result = flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { + targetingKey: 'user-1', + country: 'US' + }, + logger: getNoopRulesLogger() + }); + + expect(result).toMatchObject({ + value: true, + variant: 'enabled', + reason: 'TARGETING_MATCH', + metadata: { + allocationKey: 'allocation-1', + variationType: 'boolean', + doLog: false, + extraLogging: { experiment: 'checkout' }, + splitSerialId: 7 + } + }); + expect(result.metadata.evaluationTimestampMs).toEqual( + expect.any(Number) + ); + }); + + it('checks own properties before it calls flagging-core', () => { + const result = flaggingCoreRulesEngine.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey: 'toString', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }); + + expect(result).toEqual({ + value: false, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }); + }); + + it('provides a deterministic fake engine for client tests', () => { + const fake = createFakeRulesEngine({ + value: true, + variant: 'fake', + reason: 'TARGETING_MATCH', + metadata: {} + }); + + expect( + fake.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey: 'dynamic-flag', + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }) + ).toMatchObject({ value: true, variant: 'fake' }); + }); +}); diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index 23f1d359b..e464a701b 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -7,6 +7,8 @@ import type { ParsedFlagsConfiguration } from '../types'; import { configurationFromString, configurationToString } from '../wire'; +import { buildRulesConfiguration } from './__utils__/rulesTestUtils'; + const buildResponse = () => ({ data: { id: '2', @@ -125,3 +127,60 @@ describe('configurationToString round-trip', () => { ); }); }); + +describe('rules configuration wire compatibility', () => { + it('parses and serializes a rules configuration', () => { + const rulesBased = { + response: buildRulesConfiguration(), + fetchedAt: 123, + etag: 'rules-etag' + }; + const wire = JSON.stringify({ + version: 1, + rulesBased: { + ...rulesBased, + response: JSON.stringify(rulesBased.response) + } + }); + + const parsed = configurationFromString(wire) as { + rulesBased?: typeof rulesBased; + }; + + expect(parsed.rulesBased).toEqual(rulesBased); + expect( + configurationFromString( + configurationToString( + (parsed as unknown) as ParsedFlagsConfiguration + ) + ) + ).toEqual(parsed); + }); + + it('keeps both branches in a mixed configuration', () => { + const mixedWire = buildWire({ + rulesBased: { + response: JSON.stringify(buildRulesConfiguration()) + } + }); + + const parsed = configurationFromString(mixedWire) as { + precomputed?: unknown; + rulesBased?: unknown; + }; + + expect(parsed.precomputed).toBeDefined(); + expect(parsed.rulesBased).toBeDefined(); + }); + + it('returns an empty configuration for malformed rules JSON', () => { + expect( + configurationFromString( + JSON.stringify({ + version: 1, + rulesBased: { response: '{' } + }) + ) + ).toEqual({}); + }); +}); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts new file mode 100644 index 000000000..592f4216d --- /dev/null +++ b/packages/core/src/flags/configuration/rules.ts @@ -0,0 +1,614 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { + evaluateRulesBasedConfiguration, + OperatorType +} from '@datadog/flagging-core'; +import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; + +import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; + +export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; + +type RulesValueByType = { + boolean: boolean; + string: string; + number: number; + object: JsonValue; +}; + +export interface RulesLogger { + debug: (message: string, ...args: unknown[]) => void; + info: (message: string, ...args: unknown[]) => void; + warn: (message: string, ...args: unknown[]) => void; + error: (message: string, ...args: unknown[]) => void; +} + +export interface RulesEvaluationContext { + targetingKey?: string; + [key: string]: PrimitiveValue | undefined; +} + +export interface RulesEvaluationMetadata { + allocationKey?: string; + variationType?: RulesValueType; + doLog?: boolean; + extraLogging?: Record; + splitSerialId?: number; + evaluationTimestampMs?: number; +} + +export interface RulesEvaluationDetails { + value: T; + reason?: string; + variant?: string; + errorCode?: string; + errorMessage?: string; + metadata: RulesEvaluationMetadata; +} + +export interface RulesEvaluationRequest { + configuration: UniversalFlagConfigurationV1; + type: T; + flagKey: string; + defaultValue: RulesValueByType[T]; + context: RulesEvaluationContext; + logger: RulesLogger; +} + +export interface RulesEngine { + evaluate( + request: RulesEvaluationRequest + ): RulesEvaluationDetails; +} + +type RawEvaluationDetails = { + value: T; + reason?: string; + variant?: string; + errorCode?: string; + errorMessage?: string; + flagMetadata?: Record; +}; + +type EvaluateRules = ( + configuration: UniversalFlagConfigurationV1, + type: T, + flagKey: string, + defaultValue: RulesValueByType[T], + context: RulesEvaluationContext, + logger: RulesLogger +) => RawEvaluationDetails; + +const evaluateRules = evaluateRulesBasedConfiguration as EvaluateRules; + +const NOOP_LOGGER: RulesLogger = { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {} +}; + +export const getNoopRulesLogger = (): RulesLogger => NOOP_LOGGER; + +/** + * Convert the SDK context to the flat context that flagging-core uses. + * + * `id` and `targetingKey` are reserved. The adapter always derives them from + * `EvaluationContext.targetingKey`. + */ +export const toRulesEvaluationContext = ( + context: EvaluationContext +): RulesEvaluationContext => { + const attributes = new Map(); + + for (const [key, value] of Object.entries(context.attributes ?? {})) { + if (key === 'id' || key === 'targetingKey' || value === undefined) { + continue; + } + attributes.set(key, value); + } + + return { + ...Object.fromEntries(attributes), + targetingKey: context.targetingKey + }; +}; + +const hasOwn = (value: object, key: PropertyKey): boolean => + Object.prototype.hasOwnProperty.call(value, key); + +const isRecord = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + +const isStringRecord = (value: unknown): value is Record => { + if (!isRecord(value)) { + return false; + } + + return Object.values(value).every(item => typeof item === 'string'); +}; + +const isJsonValue = (value: unknown): value is JsonValue => { + if ( + value === null || + typeof value === 'string' || + typeof value === 'boolean' + ) { + return true; + } + if (typeof value === 'number') { + return Number.isFinite(value); + } + if (Array.isArray(value)) { + return value.every(isJsonValue); + } + if (isRecord(value)) { + return Object.values(value).every(isJsonValue); + } + return false; +}; + +const variationValueIsValid = ( + variationType: string, + value: unknown +): boolean => { + switch (variationType) { + case 'BOOLEAN': + return typeof value === 'boolean'; + case 'STRING': + return typeof value === 'string'; + case 'INTEGER': + case 'NUMERIC': + return typeof value === 'number' && Number.isFinite(value); + case 'JSON': + return isJsonValue(value); + default: + return false; + } +}; + +const SUPPORTED_OPERATORS: ReadonlySet = new Set( + Object.values(OperatorType) +); + +const validateCondition = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.attribute !== 'string' || + typeof value.operator !== 'string' + ) { + return 'A rule condition has an invalid shape.'; + } + + if (!SUPPORTED_OPERATORS.has(value.operator)) { + return `The rules configuration uses the unsupported operator "${value.operator}".`; + } + + switch (value.operator) { + case OperatorType.MATCHES: + case OperatorType.NOT_MATCHES: + if (typeof value.value !== 'string') { + return 'A regular expression condition must contain a string.'; + } + try { + // TODO(FFL-2837): Replace this compile-only check with the upstream + // safe-regex policy before dynamic offline rules leave draft state. + RegExp(value.value); // dd-iac-scan ignore-line + } catch { + return 'A regular expression condition is not valid.'; + } + return undefined; + case OperatorType.ONE_OF: + case OperatorType.NOT_ONE_OF: + return Array.isArray(value.value) && + value.value.every(item => typeof item === 'string') + ? undefined + : 'A membership condition must contain a string array.'; + case OperatorType.GTE: + case OperatorType.GT: + case OperatorType.LTE: + case OperatorType.LT: + return typeof value.value === 'number' && + Number.isFinite(value.value) + ? undefined + : 'A numeric condition must contain a finite number.'; + case OperatorType.IS_NULL: + return typeof value.value === 'boolean' + ? undefined + : 'A null condition must contain a boolean.'; + default: + return 'The rules configuration uses an unsupported operator.'; + } +}; + +const validateRules = (value: unknown): string | undefined => { + if (value === undefined) { + return undefined; + } + if (!Array.isArray(value)) { + return 'An allocation rules field must be an array.'; + } + + for (const rule of value) { + if (!isRecord(rule) || !Array.isArray(rule.conditions)) { + return 'A rule has an invalid shape.'; + } + for (const condition of rule.conditions) { + const error = validateCondition(condition); + if (error) { + return error; + } + } + } + + return undefined; +}; + +const validateShards = (value: unknown): string | undefined => { + if (!Array.isArray(value)) { + return 'A split shards field must be an array.'; + } + + for (const shard of value) { + if ( + !isRecord(shard) || + typeof shard.salt !== 'string' || + !Number.isInteger(shard.totalShards) || + (shard.totalShards as number) <= 0 || + !Array.isArray(shard.ranges) + ) { + return 'A shard has an invalid shape.'; + } + + for (const range of shard.ranges) { + if ( + !isRecord(range) || + !Number.isInteger(range.start) || + !Number.isInteger(range.end) || + (range.start as number) < 0 || + (range.end as number) <= (range.start as number) || + (range.end as number) > (shard.totalShards as number) + ) { + return 'A shard range is not valid.'; + } + } + } + + return undefined; +}; + +const isValidDate = (value: unknown): boolean => + value instanceof Date + ? !Number.isNaN(value.getTime()) + : typeof value === 'string' && !Number.isNaN(Date.parse(value)); + +const validateAllocation = ( + value: unknown, + variations: Record +): string | undefined => { + if ( + !isRecord(value) || + typeof value.key !== 'string' || + !Array.isArray(value.splits) + ) { + return 'An allocation has an invalid shape.'; + } + + if (value.startAt !== undefined && !isValidDate(value.startAt)) { + return 'An allocation start time is not valid.'; + } + if (value.endAt !== undefined && !isValidDate(value.endAt)) { + return 'An allocation end time is not valid.'; + } + + const rulesError = validateRules(value.rules); + if (rulesError) { + return rulesError; + } + + for (const split of value.splits) { + if ( + !isRecord(split) || + typeof split.variationKey !== 'string' || + !hasOwn(variations, split.variationKey) + ) { + return 'A split has an invalid variation key.'; + } + if (split.serialId !== undefined && !Number.isInteger(split.serialId)) { + return 'A split serial ID is not valid.'; + } + if ( + split.extraLogging !== undefined && + !isStringRecord(split.extraLogging) + ) { + return 'A split extraLogging field is not valid.'; + } + + const shardsError = validateShards(split.shards); + if (shardsError) { + return shardsError; + } + } + + return undefined; +}; + +const validateFlag = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.key !== 'string' || + typeof value.enabled !== 'boolean' || + typeof value.variationType !== 'string' || + !isRecord(value.variations) || + !Array.isArray(value.allocations) + ) { + return 'A flag has an invalid shape.'; + } + + if ( + !['BOOLEAN', 'INTEGER', 'NUMERIC', 'STRING', 'JSON'].includes( + value.variationType + ) + ) { + return `A flag uses the unsupported variation type "${value.variationType}".`; + } + + for (const variation of Object.values(value.variations)) { + if ( + !isRecord(variation) || + typeof variation.key !== 'string' || + !variationValueIsValid(value.variationType, variation.value) + ) { + return 'A variation has an invalid shape or value.'; + } + } + + for (const allocation of value.allocations) { + const error = validateAllocation(allocation, value.variations); + if (error) { + return error; + } + } + + return undefined; +}; + +const validateRulesConfiguration = (value: unknown): string | undefined => { + if ( + !isRecord(value) || + typeof value.createdAt !== 'string' || + typeof value.format !== 'string' || + !isRecord(value.environment) || + typeof value.environment.name !== 'string' || + !isRecord(value.flags) + ) { + return 'The rules configuration has an invalid envelope.'; + } + + for (const flag of Object.values(value.flags)) { + const error = validateFlag(flag); + if (error) { + return error; + } + } + + return undefined; +}; + +const cloneValue = (value: unknown): unknown => { + if (value instanceof Date) { + return new Date(value.getTime()); + } + if (Array.isArray(value)) { + return value.map(cloneValue); + } + if (isRecord(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, item]) => [key, cloneValue(item)]) + ); + } + return value; +}; + +const freezeValue = (value: unknown): void => { + if (typeof value !== 'object' || value === null || Object.isFrozen(value)) { + return; + } + + Object.freeze(value); + for (const item of Object.values(value)) { + freezeValue(item); + } +}; + +export type PreparedRulesConfiguration = + | { + status: 'ready'; + configuration: UniversalFlagConfigurationV1; + } + | { + status: 'error'; + errorMessage: string; + }; + +/** + * Clone and validate untrusted rules before `FlagsClient` stores them. + */ +export const prepareRulesConfiguration = ( + value: unknown +): PreparedRulesConfiguration => { + const clone = cloneValue(value); + + // TODO(FFL-2837): Replace the temporary SDK validator with the published + // flagging-core validation API. Keep the branch-level result contract. + const errorMessage = validateRulesConfiguration(clone); + if (errorMessage) { + return { status: 'error', errorMessage }; + } + + freezeValue(clone); + return { + status: 'ready', + configuration: clone as UniversalFlagConfigurationV1 + }; +}; + +const normalizeVariationType = ( + variationType: unknown +): RulesValueType | undefined => { + switch (variationType) { + case 'boolean': + case 'string': + case 'number': + case 'object': + return variationType; + case 'BOOLEAN': + return 'boolean'; + case 'STRING': + return 'string'; + case 'INTEGER': + case 'NUMERIC': + return 'number'; + case 'JSON': + return 'object'; + default: + return undefined; + } +}; + +const recoverSplitMetadata = ( + configuration: UniversalFlagConfigurationV1, + flagKey: string, + variant: string | undefined, + allocationKey: string | undefined, + splitSerialId: number | undefined +): Pick => { + const flags = configuration.flags as Record; + if (!hasOwn(flags, flagKey)) { + return {}; + } + + const flag = flags[flagKey]; + if (!isRecord(flag)) { + return {}; + } + + const variationType = normalizeVariationType(flag.variationType); + if (!Array.isArray(flag.allocations)) { + return { variationType }; + } + + const variations = isRecord(flag.variations) ? flag.variations : {}; + const variationEntry = Object.entries(variations).find(([, value]) => { + return isRecord(value) && value.key === variant; + }); + const variationKey = variationEntry?.[0]; + + for (const allocation of flag.allocations) { + if ( + !isRecord(allocation) || + allocation.key !== allocationKey || + !Array.isArray(allocation.splits) + ) { + continue; + } + const split = allocation.splits.find(candidate => { + if (!isRecord(candidate)) { + return false; + } + if ( + splitSerialId !== undefined && + candidate.serialId === splitSerialId + ) { + return true; + } + return ( + splitSerialId === undefined && + variationKey !== undefined && + candidate.variationKey === variationKey + ); + }); + if (isRecord(split) && isStringRecord(split.extraLogging)) { + return { extraLogging: split.extraLogging, variationType }; + } + } + + return { variationType }; +}; + +export const flaggingCoreRulesEngine: RulesEngine = { + evaluate( + request: RulesEvaluationRequest + ): RulesEvaluationDetails { + const flags = request.configuration.flags as Record; + if (!hasOwn(flags, request.flagKey)) { + return { + value: request.defaultValue, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }; + } + + const result = evaluateRules( + request.configuration, + request.type, + request.flagKey, + request.defaultValue, + request.context, + request.logger + ); + const rawMetadata = result.flagMetadata ?? {}; + const allocationKey = + typeof rawMetadata.allocationKey === 'string' + ? rawMetadata.allocationKey + : typeof rawMetadata.__dd_allocation_key === 'string' + ? rawMetadata.__dd_allocation_key + : undefined; + const splitSerialId = + typeof rawMetadata.__dd_split_serial_id === 'number' + ? rawMetadata.__dd_split_serial_id + : undefined; + + // TODO(FFL-2837): Remove this metadata lookup when flagging-core + // returns extraLogging and the original UFC variation type. + const recoveredMetadata = recoverSplitMetadata( + request.configuration, + request.flagKey, + result.variant, + allocationKey, + splitSerialId + ); + + return { + value: result.value, + reason: result.reason, + variant: result.variant, + errorCode: result.errorCode, + errorMessage: result.errorMessage, + metadata: { + allocationKey, + variationType: + normalizeVariationType(rawMetadata.variationType) ?? + recoveredMetadata.variationType, + doLog: + typeof rawMetadata.doLog === 'boolean' + ? rawMetadata.doLog + : typeof rawMetadata.__dd_do_log === 'boolean' + ? rawMetadata.__dd_do_log + : undefined, + extraLogging: recoveredMetadata.extraLogging, + splitSerialId, + evaluationTimestampMs: + typeof rawMetadata.__dd_eval_timestamp_ms === 'number' + ? rawMetadata.__dd_eval_timestamp_ms + : undefined + } + }; + } +}; diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 952636fd0..f47a6fb84 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -9,7 +9,95 @@ // it returns an empty configuration (`{}`) for malformed input or an unsupported wire // version rather than throwing. `configurationToString` is the inverse (its fix from // https://github.com/DataDog/openfeature-js-client/pull/331 shipped in flagging-core 2.0.0). -export { - configurationFromString, - configurationToString +import { + configurationFromString as coreConfigurationFromString, + configurationToString as coreConfigurationToString } from '@datadog/flagging-core'; +import type { + FlagsConfiguration, + UniversalFlagConfigurationV1 +} from '@datadog/flagging-core'; + +type PendingRulesConfiguration = FlagsConfiguration & { + rulesBased?: { + response: UniversalFlagConfigurationV1; + fetchedAt?: number; + etag?: string; + }; +}; + +type PendingRulesWire = { + version: 1; + rulesBased?: { + response: string; + fetchedAt?: number; + etag?: string; + }; +}; + +const readPendingRulesWire = ( + source: string +): PendingRulesWire['rulesBased'] | undefined => { + try { + const wire = JSON.parse(source) as PendingRulesWire; + if ( + wire.version !== 1 || + !wire.rulesBased || + typeof wire.rulesBased.response !== 'string' + ) { + return undefined; + } + + return wire.rulesBased; + } catch { + return undefined; + } +}; + +/** + * Use flagging-core to parse a configuration wire. + */ +export const configurationFromString = (source: string): FlagsConfiguration => { + const configuration = coreConfigurationFromString( + source + ) as PendingRulesConfiguration; + + // TODO(FFL-2837): Delete this JSON compatibility shim after + // DataDog/openfeature-js-client#336 is published by flagging-core. + const pendingRules = readPendingRulesWire(source); + if (pendingRules) { + try { + configuration.rulesBased = { + ...pendingRules, + response: JSON.parse(pendingRules.response) + }; + } catch { + return {}; + } + } + + return configuration; +}; + +/** + * Use flagging-core to serialize a parsed configuration. + */ +export const configurationToString = ( + configuration: FlagsConfiguration +): string => { + const serialized = coreConfigurationToString(configuration); + const pendingConfiguration = configuration as PendingRulesConfiguration; + if (!pendingConfiguration.rulesBased) { + return serialized; + } + + // TODO(FFL-2837): Delete this JSON compatibility shim after + // DataDog/openfeature-js-client#336 is published by flagging-core. + const wire = JSON.parse(serialized) as PendingRulesWire; + wire.rulesBased = { + fetchedAt: pendingConfiguration.rulesBased.fetchedAt, + etag: pendingConfiguration.rulesBased.etag, + response: JSON.stringify(pendingConfiguration.rulesBased.response) + }; + return JSON.stringify(wire); +}; From c6c3ea9fd820e6d1bc810fad1e6d4d857f86f750 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 27 Jul 2026 16:10:42 -0400 Subject: [PATCH 2/3] fix(flags): align rules boundary with upstream --- .../__tests__/__utils__/rulesTestUtils.ts | 5 +- .../configuration/__tests__/rules.test.ts | 122 +++++++++++------ .../configuration/__tests__/wire.test.ts | 46 ++++--- .../core/src/flags/configuration/rules.ts | 128 ++++-------------- packages/core/src/flags/configuration/wire.ts | 23 ++-- 5 files changed, 148 insertions(+), 176 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts index 4eb2aef52..81190c964 100644 --- a/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts +++ b/packages/core/src/flags/configuration/__tests__/__utils__/rulesTestUtils.ts @@ -45,7 +45,6 @@ export const buildRulesConfiguration = (): UniversalFlagConfigurationV1 => ({ { variationKey: 'enabled', serialId: 7, - extraLogging: { experiment: 'checkout' }, shards: [ { salt: 'test-salt', @@ -71,8 +70,8 @@ export interface FakeRulesEngine extends RulesEngine { >; } -// TODO(FFL-2837): Remove this fake after the upstream rules wire and engine -// contract are published and the state-matrix tests can use canonical vectors. +// Client tests use this fake to control evaluation independently of the +// flagging-core implementation and its canonical integration vectors. export const createFakeRulesEngine = ( result: FakeRulesEvaluation ): FakeRulesEngine => { diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index 932851cdd..aeb3abdbf 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -35,6 +35,17 @@ describe('rules configuration', () => { }); }); + it('preserves the difference between a missing and empty targeting key', () => { + expect(toRulesEvaluationContext({})).toHaveProperty( + 'targetingKey', + undefined + ); + expect(toRulesEvaluationContext({ targetingKey: '' })).toHaveProperty( + 'targetingKey', + '' + ); + }); + it('clones and freezes a valid rules configuration', () => { const source = buildRulesConfiguration(); const prepared = prepareRulesConfiguration(source); @@ -55,7 +66,7 @@ describe('rules configuration', () => { ).toBe(true); }); - it('rejects an unsupported operator', () => { + it('omits a flag that uses an unsupported operator', () => { const source = buildRulesConfiguration(); const condition = source.flags['dynamic-flag'].allocations[0].rules?.[0] @@ -64,16 +75,18 @@ describe('rules configuration', () => { if (!condition) { throw new Error('The fixture has no condition.'); } - (condition as { operator: string }).operator = 'ONE_OF_SHA256'; + (condition as { operator: string }).operator = 'FUTURE_OPERATOR'; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: - 'The rules configuration uses the unsupported operator "ONE_OF_SHA256".' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); }); - it('rejects an invalid regular expression', () => { + it('omits a flag that contains an invalid regular expression', () => { const source = buildRulesConfiguration(); const conditions = source.flags['dynamic-flag'].allocations[0].rules?.[0].conditions; @@ -86,21 +99,52 @@ describe('rules configuration', () => { value: '[' } as typeof conditions[number]; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: 'A regular expression condition is not valid.' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); }); - it('rejects a split that points to an absent variation', () => { + it('omits a flag whose split points to an absent variation', () => { const source = buildRulesConfiguration(); source.flags['dynamic-flag'].allocations[0].splits[0].variationKey = 'absent'; - expect(prepareRulesConfiguration(source)).toEqual({ - status: 'error', - errorMessage: 'A split has an invalid variation key.' - }); + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(prepared.configuration.flags).toEqual({}); + }); + + it('keeps valid flags when it omits an invalid flag', () => { + const source = buildRulesConfiguration(); + const validFlag = buildRulesConfiguration().flags['dynamic-flag']; + validFlag.key = 'valid-flag'; + source.flags['valid-flag'] = validFlag; + + const condition = + source.flags['dynamic-flag'].allocations[0].rules?.[0] + .conditions[0]; + if (!condition) { + throw new Error('The fixture has no condition.'); + } + (condition as { operator: string }).operator = 'FUTURE_OPERATOR'; + + const prepared = prepareRulesConfiguration(source); + + expect(prepared.status).toBe('ready'); + if (prepared.status !== 'ready') { + throw new Error(prepared.errorMessage); + } + expect(Object.keys(prepared.configuration.flags)).toEqual([ + 'valid-flag' + ]); }); it('normalizes a real flagging-core evaluation', () => { @@ -125,33 +169,31 @@ describe('rules configuration', () => { metadata: { allocationKey: 'allocation-1', variationType: 'boolean', - doLog: false, - extraLogging: { experiment: 'checkout' }, - splitSerialId: 7 + doLog: false } }); - expect(result.metadata.evaluationTimestampMs).toEqual( - expect.any(Number) - ); }); - it('checks own properties before it calls flagging-core', () => { - const result = flaggingCoreRulesEngine.evaluate({ - configuration: buildRulesConfiguration(), - type: 'boolean', - flagKey: 'toString', - defaultValue: false, - context: { targetingKey: 'user-1' }, - logger: getNoopRulesLogger() - }); - - expect(result).toEqual({ - value: false, - reason: 'ERROR', - errorCode: 'FLAG_NOT_FOUND', - metadata: {} - }); - }); + it.each(['toString', 'constructor', '__proto__'])( + 'checks own properties before it evaluates %s', + flagKey => { + const result = flaggingCoreRulesEngine.evaluate({ + configuration: buildRulesConfiguration(), + type: 'boolean', + flagKey, + defaultValue: false, + context: { targetingKey: 'user-1' }, + logger: getNoopRulesLogger() + }); + + expect(result).toEqual({ + value: false, + reason: 'ERROR', + errorCode: 'FLAG_NOT_FOUND', + metadata: {} + }); + } + ); it('provides a deterministic fake engine for client tests', () => { const fake = createFakeRulesEngine({ diff --git a/packages/core/src/flags/configuration/__tests__/wire.test.ts b/packages/core/src/flags/configuration/__tests__/wire.test.ts index e464a701b..9be51f2b6 100644 --- a/packages/core/src/flags/configuration/__tests__/wire.test.ts +++ b/packages/core/src/flags/configuration/__tests__/wire.test.ts @@ -128,8 +128,8 @@ describe('configurationToString round-trip', () => { }); }); -describe('rules configuration wire compatibility', () => { - it('parses and serializes a rules configuration', () => { +describe('temporary rules configuration wire compatibility', () => { + it('parses a legacy rules configuration', () => { const rulesBased = { response: buildRulesConfiguration(), fetchedAt: 123, @@ -148,13 +148,22 @@ describe('rules configuration wire compatibility', () => { }; expect(parsed.rulesBased).toEqual(rulesBased); - expect( - configurationFromString( - configurationToString( - (parsed as unknown) as ParsedFlagsConfiguration - ) + }); + + it('does not serialize a rules configuration', () => { + const configuration = { + rulesBased: { + response: buildRulesConfiguration() + } + }; + + expect(() => + configurationToString( + (configuration as unknown) as ParsedFlagsConfiguration ) - ).toEqual(parsed); + ).toThrow( + 'Rules configurations cannot be serialized to the wire format' + ); }); it('keeps both branches in a mixed configuration', () => { @@ -173,14 +182,17 @@ describe('rules configuration wire compatibility', () => { expect(parsed.rulesBased).toBeDefined(); }); - it('returns an empty configuration for malformed rules JSON', () => { - expect( - configurationFromString( - JSON.stringify({ - version: 1, - rulesBased: { response: '{' } - }) - ) - ).toEqual({}); + it('keeps a valid precomputed branch when rules JSON is malformed', () => { + const parsed = configurationFromString( + buildWire({ + rulesBased: { response: '{' } + }) + ) as { + precomputed?: unknown; + rulesBased?: unknown; + }; + + expect(parsed.precomputed).toBeDefined(); + expect(parsed.rulesBased).toBeUndefined(); }); }); diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 592f4216d..0ee103dd5 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -37,9 +37,6 @@ export interface RulesEvaluationMetadata { allocationKey?: string; variationType?: RulesValueType; doLog?: boolean; - extraLogging?: Record; - splitSerialId?: number; - evaluationTimestampMs?: number; } export interface RulesEvaluationDetails { @@ -125,14 +122,6 @@ const hasOwn = (value: object, key: PropertyKey): boolean => const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); -const isStringRecord = (value: unknown): value is Record => { - if (!isRecord(value)) { - return false; - } - - return Object.values(value).every(item => typeof item === 'string'); -}; - const isJsonValue = (value: unknown): value is JsonValue => { if ( value === null || @@ -196,8 +185,9 @@ const validateCondition = (value: unknown): string | undefined => { return 'A regular expression condition must contain a string.'; } try { - // TODO(FFL-2837): Replace this compile-only check with the upstream - // safe-regex policy before dynamic offline rules leave draft state. + // TODO(FFL-2837): Define a bounded regular expression policy before + // dynamic offline rules leave draft state. Upstream PR #344 validates + // regular expression syntax, but it does not limit expensive patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -322,12 +312,6 @@ const validateAllocation = ( if (split.serialId !== undefined && !Number.isInteger(split.serialId)) { return 'A split serial ID is not valid.'; } - if ( - split.extraLogging !== undefined && - !isStringRecord(split.extraLogging) - ) { - return 'A split extraLogging field is not valid.'; - } const shardsError = validateShards(split.shards); if (shardsError) { @@ -378,7 +362,9 @@ const validateFlag = (value: unknown): string | undefined => { return undefined; }; -const validateRulesConfiguration = (value: unknown): string | undefined => { +const validateRulesConfigurationEnvelope = ( + value: unknown +): string | undefined => { if ( !isRecord(value) || typeof value.createdAt !== 'string' || @@ -390,13 +376,6 @@ const validateRulesConfiguration = (value: unknown): string | undefined => { return 'The rules configuration has an invalid envelope.'; } - for (const flag of Object.values(value.flags)) { - const error = validateFlag(flag); - if (error) { - return error; - } - } - return undefined; }; @@ -444,13 +423,21 @@ export const prepareRulesConfiguration = ( ): PreparedRulesConfiguration => { const clone = cloneValue(value); - // TODO(FFL-2837): Replace the temporary SDK validator with the published - // flagging-core validation API. Keep the branch-level result contract. - const errorMessage = validateRulesConfiguration(clone); + // TODO(FFL-2837): Delete this legacy JSON clone and validator after a + // flagging-core release contains upstream PR #344. That implementation + // decodes the protobuf response and omits unsupported or invalid flags. + const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; } + const flags = (clone as UniversalFlagConfigurationV1).flags; + for (const [flagKey, flag] of Object.entries(flags)) { + if (validateFlag(flag)) { + delete flags[flagKey]; + } + } + freezeValue(clone); return { status: 'ready', @@ -481,64 +468,21 @@ const normalizeVariationType = ( } }; -const recoverSplitMetadata = ( +const recoverVariationType = ( configuration: UniversalFlagConfigurationV1, - flagKey: string, - variant: string | undefined, - allocationKey: string | undefined, - splitSerialId: number | undefined -): Pick => { + flagKey: string +): RulesValueType | undefined => { const flags = configuration.flags as Record; if (!hasOwn(flags, flagKey)) { - return {}; + return undefined; } const flag = flags[flagKey]; if (!isRecord(flag)) { - return {}; - } - - const variationType = normalizeVariationType(flag.variationType); - if (!Array.isArray(flag.allocations)) { - return { variationType }; - } - - const variations = isRecord(flag.variations) ? flag.variations : {}; - const variationEntry = Object.entries(variations).find(([, value]) => { - return isRecord(value) && value.key === variant; - }); - const variationKey = variationEntry?.[0]; - - for (const allocation of flag.allocations) { - if ( - !isRecord(allocation) || - allocation.key !== allocationKey || - !Array.isArray(allocation.splits) - ) { - continue; - } - const split = allocation.splits.find(candidate => { - if (!isRecord(candidate)) { - return false; - } - if ( - splitSerialId !== undefined && - candidate.serialId === splitSerialId - ) { - return true; - } - return ( - splitSerialId === undefined && - variationKey !== undefined && - candidate.variationKey === variationKey - ); - }); - if (isRecord(split) && isStringRecord(split.extraLogging)) { - return { extraLogging: split.extraLogging, variationType }; - } + return undefined; } - return { variationType }; + return normalizeVariationType(flag.variationType); }; export const flaggingCoreRulesEngine: RulesEngine = { @@ -570,21 +514,6 @@ export const flaggingCoreRulesEngine: RulesEngine = { : typeof rawMetadata.__dd_allocation_key === 'string' ? rawMetadata.__dd_allocation_key : undefined; - const splitSerialId = - typeof rawMetadata.__dd_split_serial_id === 'number' - ? rawMetadata.__dd_split_serial_id - : undefined; - - // TODO(FFL-2837): Remove this metadata lookup when flagging-core - // returns extraLogging and the original UFC variation type. - const recoveredMetadata = recoverSplitMetadata( - request.configuration, - request.flagKey, - result.variant, - allocationKey, - splitSerialId - ); - return { value: result.value, reason: result.reason, @@ -595,18 +524,15 @@ export const flaggingCoreRulesEngine: RulesEngine = { allocationKey, variationType: normalizeVariationType(rawMetadata.variationType) ?? - recoveredMetadata.variationType, + recoverVariationType( + request.configuration, + request.flagKey + ), doLog: typeof rawMetadata.doLog === 'boolean' ? rawMetadata.doLog : typeof rawMetadata.__dd_do_log === 'boolean' ? rawMetadata.__dd_do_log - : undefined, - extraLogging: recoveredMetadata.extraLogging, - splitSerialId, - evaluationTimestampMs: - typeof rawMetadata.__dd_eval_timestamp_ms === 'number' - ? rawMetadata.__dd_eval_timestamp_ms : undefined } }; diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index f47a6fb84..681f68b94 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -62,8 +62,8 @@ export const configurationFromString = (source: string): FlagsConfiguration => { source ) as PendingRulesConfiguration; - // TODO(FFL-2837): Delete this JSON compatibility shim after - // DataDog/openfeature-js-client#336 is published by flagging-core. + // TODO(FFL-2837): Delete this legacy JSON compatibility shim after a + // flagging-core release contains DataDog/openfeature-js-client#344. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -72,7 +72,7 @@ export const configurationFromString = (source: string): FlagsConfiguration => { response: JSON.parse(pendingRules.response) }; } catch { - return {}; + return configuration; } } @@ -85,19 +85,12 @@ export const configurationFromString = (source: string): FlagsConfiguration => { export const configurationToString = ( configuration: FlagsConfiguration ): string => { - const serialized = coreConfigurationToString(configuration); const pendingConfiguration = configuration as PendingRulesConfiguration; - if (!pendingConfiguration.rulesBased) { - return serialized; + if (pendingConfiguration.rulesBased) { + throw new Error( + 'Rules configurations cannot be serialized to the wire format' + ); } - // TODO(FFL-2837): Delete this JSON compatibility shim after - // DataDog/openfeature-js-client#336 is published by flagging-core. - const wire = JSON.parse(serialized) as PendingRulesWire; - wire.rulesBased = { - fetchedAt: pendingConfiguration.rulesBased.fetchedAt, - etag: pendingConfiguration.rulesBased.etag, - response: JSON.stringify(pendingConfiguration.rulesBased.response) - }; - return JSON.stringify(wire); + return coreConfigurationToString(configuration); }; From a50f71d4fb0a12c2ea8303b7473930a3c1be0ba4 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 28 Jul 2026 14:55:17 -0400 Subject: [PATCH 3/3] fix(flags): refresh upstream compatibility TODOs --- .../configuration/__tests__/rules.test.ts | 33 +++++++++++++++++++ .../core/src/flags/configuration/rules.ts | 30 ++++++++++++----- packages/core/src/flags/configuration/wire.ts | 11 +++++-- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/rules.test.ts b/packages/core/src/flags/configuration/__tests__/rules.test.ts index aeb3abdbf..58488478f 100644 --- a/packages/core/src/flags/configuration/__tests__/rules.test.ts +++ b/packages/core/src/flags/configuration/__tests__/rules.test.ts @@ -174,6 +174,39 @@ describe('rules configuration', () => { }); }); + it.each([ + ['INTEGER', 42], + ['NUMERIC', 1.5] + ] as const)( + 'normalizes %s variation metadata to number', + (variationType, variationValue) => { + const configuration = buildRulesConfiguration(); + const flag = configuration.flags['dynamic-flag']; + flag.variationType = variationType; + flag.variations.enabled.value = variationValue; + flag.variations.disabled.value = 0; + + const result = flaggingCoreRulesEngine.evaluate({ + configuration, + type: 'number', + flagKey: 'dynamic-flag', + defaultValue: 0, + context: { + targetingKey: 'user-1', + country: 'US' + }, + logger: getNoopRulesLogger() + }); + + expect(result).toMatchObject({ + value: variationValue, + metadata: { + variationType: 'number' + } + }); + } + ); + it.each(['toString', 'constructor', '__proto__'])( 'checks own properties before it evaluates %s', flagKey => { diff --git a/packages/core/src/flags/configuration/rules.ts b/packages/core/src/flags/configuration/rules.ts index 0ee103dd5..024f2060b 100644 --- a/packages/core/src/flags/configuration/rules.ts +++ b/packages/core/src/flags/configuration/rules.ts @@ -12,6 +12,11 @@ import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types'; +// TODO(FFL-2837): Replace this legacy UFC v1 alias with +// `NonNullable['response']` after a flagging-core +// release contains DataDog/openfeature-js-client#344. +type RulesConfigurationResponse = UniversalFlagConfigurationV1; + export type RulesValueType = 'boolean' | 'string' | 'number' | 'object'; type RulesValueByType = { @@ -49,7 +54,7 @@ export interface RulesEvaluationDetails { } export interface RulesEvaluationRequest { - configuration: UniversalFlagConfigurationV1; + configuration: RulesConfigurationResponse; type: T; flagKey: string; defaultValue: RulesValueByType[T]; @@ -73,7 +78,7 @@ type RawEvaluationDetails = { }; type EvaluateRules = ( - configuration: UniversalFlagConfigurationV1, + configuration: RulesConfigurationResponse, type: T, flagKey: string, defaultValue: RulesValueByType[T], @@ -187,7 +192,7 @@ const validateCondition = (value: unknown): string | undefined => { try { // TODO(FFL-2837): Define a bounded regular expression policy before // dynamic offline rules leave draft state. Upstream PR #344 validates - // regular expression syntax, but it does not limit expensive patterns. + // the protobuf indexes, but it does not limit expensive patterns. RegExp(value.value); // dd-iac-scan ignore-line } catch { return 'A regular expression condition is not valid.'; @@ -408,7 +413,7 @@ const freezeValue = (value: unknown): void => { export type PreparedRulesConfiguration = | { status: 'ready'; - configuration: UniversalFlagConfigurationV1; + configuration: RulesConfigurationResponse; } | { status: 'error'; @@ -425,13 +430,14 @@ export const prepareRulesConfiguration = ( // TODO(FFL-2837): Delete this legacy JSON clone and validator after a // flagging-core release contains upstream PR #344. That implementation - // decodes the protobuf response and omits unsupported or invalid flags. + // decodes a generated Protobuf-ES response and omits unsupported or invalid + // flags. Do not adapt this validator to the generated response type. const errorMessage = validateRulesConfigurationEnvelope(clone); if (errorMessage) { return { status: 'error', errorMessage }; } - const flags = (clone as UniversalFlagConfigurationV1).flags; + const flags = (clone as RulesConfigurationResponse).flags; for (const [flagKey, flag] of Object.entries(flags)) { if (validateFlag(flag)) { delete flags[flagKey]; @@ -441,7 +447,7 @@ export const prepareRulesConfiguration = ( freezeValue(clone); return { status: 'ready', - configuration: clone as UniversalFlagConfigurationV1 + configuration: clone as RulesConfigurationResponse }; }; @@ -468,8 +474,12 @@ const normalizeVariationType = ( } }; +// TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the +// flagging-core dependency contains DataDog/openfeature-js-client#344. +// The protobuf evaluator supplies `variationType` and maps integer and numeric +// variations to the OpenFeature type `number`. const recoverVariationType = ( - configuration: UniversalFlagConfigurationV1, + configuration: RulesConfigurationResponse, flagKey: string ): RulesValueType | undefined => { const flags = configuration.flags as Record; @@ -490,6 +500,10 @@ export const flaggingCoreRulesEngine: RulesEngine = { request: RulesEvaluationRequest ): RulesEvaluationDetails { const flags = request.configuration.flags as Record; + + // TODO(FFL-2837): Delete this local compatibility guard after the + // flagging-core dependency contains DataDog/openfeature-js-client#344. + // Keep the reserved-name contract tests for the upstream implementation. if (!hasOwn(flags, request.flagKey)) { return { value: request.defaultValue, diff --git a/packages/core/src/flags/configuration/wire.ts b/packages/core/src/flags/configuration/wire.ts index 681f68b94..7a3fe7cd1 100644 --- a/packages/core/src/flags/configuration/wire.ts +++ b/packages/core/src/flags/configuration/wire.ts @@ -18,6 +18,9 @@ import type { UniversalFlagConfigurationV1 } from '@datadog/flagging-core'; +// TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers +// after a flagging-core release contains DataDog/openfeature-js-client#344. +// Re-export the upstream functions and use `FlagsConfiguration.rules`. type PendingRulesConfiguration = FlagsConfiguration & { rulesBased?: { response: UniversalFlagConfigurationV1; @@ -62,8 +65,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => { source ) as PendingRulesConfiguration; - // TODO(FFL-2837): Delete this legacy JSON compatibility shim after a - // flagging-core release contains DataDog/openfeature-js-client#344. + // TODO(FFL-2837): Delete this legacy JSON compatibility shim with the + // pending types above. The upstream parser decodes `rules.response` as a + // generated Protobuf-ES message. const pendingRules = readPendingRulesWire(source); if (pendingRules) { try { @@ -86,6 +90,9 @@ export const configurationToString = ( configuration: FlagsConfiguration ): string => { const pendingConfiguration = configuration as PendingRulesConfiguration; + + // TODO(FFL-2837): Delete this local serialization guard with the pending + // types above. PR #344 makes the upstream serializer reject `rules`. if (pendingConfiguration.rulesBased) { throw new Error( 'Rules configurations cannot be serialized to the wire format'