Skip to content

Commit 28ec399

Browse files
committed
[FSSDK-12813] Relax campaign_id/entity_id validation to non-empty string per updated spec
1 parent 717048f commit 28ec399

2 files changed

Lines changed: 75 additions & 44 deletions

File tree

lib/event_processor/event_builder/log_event.spec.ts

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -887,10 +887,12 @@ describe('buildLogEvent', () => {
887887
* These tests pin the normalization contract for decisions[].campaign_id,
888888
* decisions[].variation_id, and events[].entity_id on impression events:
889889
*
890-
* - campaign_id MUST be a non-empty decimal-digit string; if not, fall back
891-
* to experiment_id; if experiment_id is also invalid, emit null.
890+
* - campaign_id MUST be a non-empty string (any character content; IDs
891+
* may be opaque, e.g. "default-12345", "layer_abc"); if empty/null/
892+
* missing, fall back to experiment_id; if experiment_id is also empty/
893+
* null, emit null.
892894
* - variation_id MUST be a non-empty decimal-digit string OR null; any
893-
* invalid input normalizes to null.
895+
* invalid input (empty, whitespace, non-numeric) normalizes to null.
894896
* - events[].entity_id (impression only) follows the same rule as
895897
* campaign_id and MUST equal decisions[].campaign_id byte-for-byte.
896898
* - The rule applies uniformly across all decision types (experiment,
@@ -960,65 +962,73 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
960962
expect(decision.campaign_id).toBe('00042');
961963
});
962964

963-
it('substitutes experiment_id when layerId is null', () => {
965+
it('preserves an opaque non-numeric layerId unchanged', () => {
966+
// FSSDK-12813: campaign_id contract is "any non-empty string". Opaque
967+
// IDs like "layer_abc" or "default-12345" pass through unchanged.
964968
const decision = getDecision(
965-
makeImpression({ layerId: null, experimentId: '67890', variationId: '111', ruleType: 'experiment' })
969+
makeImpression({ layerId: 'layer_abc', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
966970
);
967-
expect(decision.campaign_id).toBe('67890');
971+
expect(decision.campaign_id).toBe('layer_abc');
968972
});
969973

970-
it('substitutes experiment_id when layerId is an empty string', () => {
974+
it('preserves a layerId with a negative sign unchanged', () => {
975+
// FSSDK-12813: Any non-empty string is valid for campaign_id.
971976
const decision = getDecision(
972-
makeImpression({ layerId: '', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
977+
makeImpression({ layerId: '-123', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
973978
);
974-
expect(decision.campaign_id).toBe('67890');
979+
expect(decision.campaign_id).toBe('-123');
975980
});
976981

977-
it('substitutes experiment_id when layerId is whitespace', () => {
982+
it('preserves a decimal-formatted layerId unchanged', () => {
983+
// FSSDK-12813: Any non-empty string is valid for campaign_id.
978984
const decision = getDecision(
979-
makeImpression({ layerId: ' ', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
985+
makeImpression({ layerId: '123.45', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
980986
);
981-
expect(decision.campaign_id).toBe('67890');
987+
expect(decision.campaign_id).toBe('123.45');
982988
});
983989

984-
it('substitutes experiment_id when layerId is a non-numeric string', () => {
990+
it('preserves an exponential-notation layerId unchanged', () => {
991+
// FSSDK-12813: Any non-empty string is valid for campaign_id.
985992
const decision = getDecision(
986-
makeImpression({ layerId: 'layer_abc', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
993+
makeImpression({ layerId: '1e5', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
987994
);
988-
expect(decision.campaign_id).toBe('67890');
995+
expect(decision.campaign_id).toBe('1e5');
989996
});
990997

991-
it('substitutes experiment_id when layerId contains negative sign', () => {
998+
it('preserves a whitespace-only layerId unchanged', () => {
999+
// FSSDK-12813: Whitespace is a non-empty string; only empty string,
1000+
// null, and undefined trigger the experiment_id fallback.
9921001
const decision = getDecision(
993-
makeImpression({ layerId: '-123', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
1002+
makeImpression({ layerId: ' ', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
9941003
);
995-
expect(decision.campaign_id).toBe('67890');
1004+
expect(decision.campaign_id).toBe(' ');
9961005
});
9971006

998-
it('substitutes experiment_id when layerId is a decimal', () => {
1007+
it('substitutes experiment_id when layerId is null', () => {
9991008
const decision = getDecision(
1000-
makeImpression({ layerId: '123.45', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
1009+
makeImpression({ layerId: null, experimentId: '67890', variationId: '111', ruleType: 'experiment' })
10011010
);
10021011
expect(decision.campaign_id).toBe('67890');
10031012
});
10041013

1005-
it('substitutes experiment_id when layerId is in exponential notation', () => {
1014+
it('substitutes experiment_id when layerId is an empty string', () => {
10061015
const decision = getDecision(
1007-
makeImpression({ layerId: '1e5', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
1016+
makeImpression({ layerId: '', experimentId: '67890', variationId: '111', ruleType: 'experiment' })
10081017
);
10091018
expect(decision.campaign_id).toBe('67890');
10101019
});
10111020

1012-
it('emits null when both layerId and experiment_id are invalid', () => {
1021+
it('substitutes an opaque experiment_id when layerId is null', () => {
1022+
// FSSDK-12813: experiment_id fallback also accepts any non-empty string.
10131023
const decision = getDecision(
1014-
makeImpression({ layerId: null, experimentId: null, variationId: '111', ruleType: 'experiment' })
1024+
makeImpression({ layerId: null, experimentId: 'exp_42', variationId: '111', ruleType: 'experiment' })
10151025
);
1016-
expect(decision.campaign_id).toBeNull();
1026+
expect(decision.campaign_id).toBe('exp_42');
10171027
});
10181028

1019-
it('emits null when both layerId and experiment_id are non-numeric strings', () => {
1029+
it('emits null when both layerId and experiment_id are null', () => {
10201030
const decision = getDecision(
1021-
makeImpression({ layerId: 'abc', experimentId: 'exp_42', variationId: '111', ruleType: 'experiment' })
1031+
makeImpression({ layerId: null, experimentId: null, variationId: '111', ruleType: 'experiment' })
10221032
);
10231033
expect(decision.campaign_id).toBeNull();
10241034
});
@@ -1100,8 +1110,10 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
11001110

11011111
ruleTypes.forEach((ruleType) => {
11021112
it(`normalizes campaign_id identically for ruleType=${ruleType}`, () => {
1113+
// FSSDK-12813: null layerId triggers fallback to experiment_id; the
1114+
// fallback fires identically regardless of rule type.
11031115
const decision = getDecision(
1104-
makeImpression({ layerId: 'bad', experimentId: '67890', variationId: '111', ruleType })
1116+
makeImpression({ layerId: null, experimentId: '67890', variationId: '111', ruleType })
11051117
);
11061118
expect(decision.campaign_id).toBe('67890');
11071119
});
@@ -1173,14 +1185,17 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
11731185
// ---------------------------------------------------------------------------
11741186
describe('byte-equivalent output (FR-008)', () => {
11751187
it('produces identical JSON for two identical event inputs', () => {
1188+
// FSSDK-12813: identical inputs must produce identical wire output.
1189+
// layerId here is a valid non-empty string (passes through unchanged);
1190+
// variationId is non-numeric (normalizes to null).
11761191
const e1 = makeImpression({
1177-
layerId: 'bad',
1192+
layerId: 'layer_abc',
11781193
experimentId: '67890',
11791194
variationId: 'bad',
11801195
ruleType: 'experiment',
11811196
});
11821197
const e2 = makeImpression({
1183-
layerId: 'bad',
1198+
layerId: 'layer_abc',
11841199
experimentId: '67890',
11851200
variationId: 'bad',
11861201
ruleType: 'experiment',
@@ -1200,10 +1215,11 @@ describe('decision event ID normalization (FSSDK-12813)', () => {
12001215
layerId: string | null;
12011216
experimentId: string | null;
12021217
}> = [
1203-
{ name: 'valid layerId', layerId: '12345', experimentId: '67890' },
1204-
{ name: 'invalid layerId falls back to experiment_id', layerId: 'bad', experimentId: '67890' },
1218+
{ name: 'valid numeric layerId', layerId: '12345', experimentId: '67890' },
1219+
{ name: 'opaque non-numeric layerId preserved', layerId: 'layer_abc', experimentId: '67890' },
1220+
{ name: 'empty-string layerId falls back to experiment_id', layerId: '', experimentId: '67890' },
12051221
{ name: 'null layerId falls back to experiment_id', layerId: null, experimentId: '67890' },
1206-
{ name: 'both invalid -> null', layerId: 'bad', experimentId: 'also_bad' },
1222+
{ name: 'both empty/null -> null', layerId: '', experimentId: '' },
12071223
{ name: 'both null -> null', layerId: null, experimentId: null },
12081224
{ name: 'layerId leading zeros preserved', layerId: '00099', experimentId: '67890' },
12091225
];

lib/event_processor/event_builder/log_event.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,19 @@ function makeConversionSnapshot(conversion: ConversionEvent): Snapshot {
161161
}
162162

163163
/**
164-
* FSSDK-12813: Numeric-string validator used to normalize decision-event IDs.
164+
* FSSDK-12813: Non-empty string validator used to normalize campaign_id and
165+
* entity_id (decision-event ID fields whose contract is "any non-empty
166+
* string"). IDs may be opaque, e.g. "default-12345" or "layer_abc".
167+
*
168+
* Returns true if value is a string of length >= 1. Any character content is
169+
* accepted. Empty string, non-string types, null, and undefined are invalid.
170+
*/
171+
function isNonEmptyString(value: unknown): value is string {
172+
return typeof value === 'string' && value.length > 0;
173+
}
174+
175+
/**
176+
* FSSDK-12813: Numeric-string validator used to normalize variation_id.
165177
*
166178
* Returns true if value is a non-empty string consisting entirely of decimal
167179
* digits [0-9]. Leading zeros are allowed. Whitespace, negatives, decimals,
@@ -174,20 +186,22 @@ function isNumericString(value: unknown): value is string {
174186
/**
175187
* FSSDK-12813: Normalize a campaign_id / entity_id field.
176188
*
177-
* Rule (FR-001/FR-002, FR-009): if the provided id is a non-empty
178-
* numeric-string return it unchanged; otherwise substitute experimentId
179-
* when experimentId itself is a non-empty numeric-string. If neither is
180-
* valid, return null so the wire payload is byte-equivalent across SDKs.
189+
* Rule (FR-001/FR-002, FR-009): if the provided id is a non-empty string
190+
* return it unchanged (any character content is accepted — IDs may be
191+
* opaque, e.g. "default-12345", "layer_abc"); otherwise substitute
192+
* experimentId when experimentId itself is a non-empty string. If neither
193+
* is valid (empty string, null, or undefined), return null so the wire
194+
* payload is byte-equivalent across SDKs.
181195
*
182196
* Applies uniformly to ALL decision types (experiment, feature test,
183197
* rollout, holdout) — there is no per-type branching here (FR-005).
184198
* Does not drop or fail event dispatch (FR-006) and does not log (FR-007).
185199
*/
186200
function normalizeCampaignId(id: unknown, experimentId: unknown): string | null {
187-
if (isNumericString(id)) {
201+
if (isNonEmptyString(id)) {
188202
return id;
189203
}
190-
if (isNumericString(experimentId)) {
204+
if (isNonEmptyString(experimentId)) {
191205
return experimentId;
192206
}
193207
return null;
@@ -196,10 +210,11 @@ function normalizeCampaignId(id: unknown, experimentId: unknown): string | null
196210
/**
197211
* FSSDK-12813: Normalize a variation_id field.
198212
*
199-
* Rule (FR-003/FR-004): if the provided id is a non-empty numeric-string
200-
* return it unchanged; otherwise substitute null. Applies uniformly to ALL
201-
* decision types (FR-005). Does not drop or fail event dispatch (FR-006)
202-
* and does not log (FR-007).
213+
* Rule (FR-003/FR-004): variation_id retains the stricter numeric-string
214+
* contract. If the provided id is a non-empty numeric-string return it
215+
* unchanged; otherwise substitute null. Applies uniformly to ALL decision
216+
* types (FR-005). Does not drop or fail event dispatch (FR-006) and does
217+
* not log (FR-007).
203218
*/
204219
function normalizeVariationId(id: unknown): string | null {
205220
return isNumericString(id) ? id : null;

0 commit comments

Comments
 (0)