Skip to content

Commit 77f3c22

Browse files
committed
[FSSDK-12813] Relax campaign_id/entity_id validation to non-empty string per updated spec
Per the updated FSSDK-12813 spec, decision-event campaign_id and impression entity_id only require a non-empty string (IDs may be opaque, e.g. "default-12345" or "layer_abc"). Fallback to experiment_id now fires only when the value is null or "". variation_id retains the stricter numeric-string-only contract. - EventIdNormalizer: add isNonEmptyString predicate and route normalizeCampaignId through it; isNumericString and normalizeVariationId unchanged. - EventIdNormalizerTest: add isNonEmptyString coverage; flip normalizeCampaignId tests so non-numeric/whitespace inputs assert passthrough; variation_id assertions unchanged. - EventFactoryNormalizationTest: flip whitespace and non-numeric campaign_id tests to passthrough; split the uniform-across-rule-types test into opaque-passthrough and null-fallback variants; update entity_id-mirrors-campaign_id and event-never-dropped tests to use null/empty for the campaign_id fallback path and a non-numeric value only for the variation_id null path.
1 parent 6eb1f6a commit 77f3c22

3 files changed

Lines changed: 188 additions & 54 deletions

File tree

‎core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,17 @@
2222
* <p>Implements FSSDK-12813:
2323
* <ul>
2424
* <li>{@code campaign_id} and impression {@code entity_id} must be a non-empty
25-
* decimal-digit string. If empty / null / non-numeric / whitespace, substitute
26-
* {@code experiment_id}.</li>
25+
* string of any character content (IDs may be opaque, e.g. {@code "default-12345"},
26+
* {@code "layer_abc"}). If null or empty string, substitute {@code experiment_id}.
27+
* Non-numeric strings pass through unchanged.</li>
2728
* <li>{@code variation_id} must be a non-empty decimal-digit string OR {@code null}.
28-
* If empty / non-numeric / non-string / whitespace, substitute {@code null}.</li>
29+
* If empty / non-numeric / whitespace, substitute {@code null}. This field retains
30+
* the stricter numeric-string-only contract.</li>
2931
* </ul>
3032
*
31-
* <p>A "numeric string" is a non-empty string consisting entirely of decimal digits
32-
* {@code [0-9]}. Leading zeros are allowed. Whitespace, negatives, decimals, and
33-
* exponents are INVALID.
33+
* <p>For {@code variation_id}, a "numeric string" is a non-empty string consisting
34+
* entirely of decimal digits {@code [0-9]}. Leading zeros are allowed. Whitespace,
35+
* negatives, decimals, and exponents are INVALID.
3436
*
3537
* <p>Normalization applies uniformly to all decision types (experiment, feature test,
3638
* rollout, holdout). It must not drop, defer, or fail event dispatch, and it must not
@@ -42,9 +44,20 @@ private EventIdNormalizer() {
4244
// Utility class — not instantiable.
4345
}
4446

47+
/**
48+
* @return {@code true} iff {@code value} is non-null and has length &ge; 1.
49+
* Character content is not validated — any non-empty string is accepted.
50+
* Used to validate {@code campaign_id} and impression {@code entity_id}
51+
* per the FSSDK-12813 relaxed contract.
52+
*/
53+
static boolean isNonEmptyString(String value) {
54+
return value != null && !value.isEmpty();
55+
}
56+
4557
/**
4658
* @return {@code true} iff {@code value} is non-null and consists entirely of decimal digits.
4759
* Empty strings, whitespace, negatives, decimals, and exponents are all invalid.
60+
* Used to validate {@code variation_id} per the FSSDK-12813 strict numeric-string contract.
4861
*/
4962
static boolean isNumericString(String value) {
5063
if (value == null) {
@@ -66,13 +79,18 @@ static boolean isNumericString(String value) {
6679
/**
6780
* Normalize a {@code campaign_id} or impression {@code entity_id}.
6881
*
69-
* @param campaignId the candidate campaign_id (may be null, empty, or non-numeric)
82+
* <p>Per FSSDK-12813, any non-empty string is accepted as-is (IDs may be opaque,
83+
* e.g. {@code "default-12345"}, {@code "layer_abc"}). The fallback to
84+
* {@code experiment_id} fires ONLY when {@code campaignId} is {@code null} or
85+
* the empty string {@code ""}.
86+
*
87+
* @param campaignId the candidate campaign_id (may be null or empty)
7088
* @param experimentId fallback experiment_id (returned as-is; not re-validated)
71-
* @return {@code campaignId} when it is a non-empty numeric string,
89+
* @return {@code campaignId} when it is a non-empty string of any content,
7290
* otherwise {@code experimentId} (which may itself be {@code null}).
7391
*/
7492
static String normalizeCampaignId(String campaignId, String experimentId) {
75-
if (isNumericString(campaignId)) {
93+
if (isNonEmptyString(campaignId)) {
7694
return campaignId;
7795
}
7896
return experimentId;
@@ -81,6 +99,10 @@ static String normalizeCampaignId(String campaignId, String experimentId) {
8199
/**
82100
* Normalize a {@code variation_id}.
83101
*
102+
* <p>Per FSSDK-12813, {@code variation_id} retains the stricter contract: must be
103+
* a non-empty decimal-digit string. Anything else (null, empty, whitespace, or
104+
* non-numeric) is replaced with {@code null}.
105+
*
84106
* @param variationId the candidate variation_id (may be null, empty, or non-numeric)
85107
* @return {@code variationId} when it is a non-empty numeric string, otherwise {@code null}.
86108
*/

‎core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java‎

Lines changed: 99 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
* <p>Verifies:
4545
* <ul>
4646
* <li>FR-001/FR-002: {@code campaign_id} substitution from {@code experiment_id}
47-
* when campaign_id is empty / null / non-numeric / whitespace.</li>
47+
* when campaign_id is null or empty string. Non-numeric / opaque strings
48+
* (e.g. {@code "layer_abc"}) are valid and pass through unchanged.</li>
4849
* <li>FR-003/FR-004: {@code variation_id} replacement with {@code null} when
4950
* empty / non-numeric / whitespace.</li>
5051
* <li>FR-005: Uniform behavior across all rule types (experiment / feature-test /
@@ -167,21 +168,37 @@ public void campaignId_empty_fallsBackToExperimentId() {
167168
}
168169

169170
@Test
170-
public void campaignId_whitespace_fallsBackToExperimentId() {
171+
public void campaignId_whitespace_passesThroughUnchanged() {
172+
// FSSDK-12813 (relaxed): any non-empty string is valid for campaign_id.
173+
// Whitespace-only strings have length >= 1, so they pass through as-is.
171174
ImpressionEvent imp = buildImpression(
172175
"experiment", " ", "2222", "3333");
173176
LogEvent log = EventFactory.createLogEvent(imp);
174-
assertEquals("2222", firstDecision(log).getCampaignId());
175-
assertEquals("2222", firstEvent(log).getEntityId());
177+
assertEquals(" ", firstDecision(log).getCampaignId());
178+
assertEquals(" ", firstEvent(log).getEntityId());
176179
}
177180

178181
@Test
179-
public void campaignId_nonNumericString_fallsBackToExperimentId() {
182+
public void campaignId_nonNumericString_passesThroughUnchanged() {
183+
// FSSDK-12813 (relaxed): opaque/non-numeric IDs (e.g. "layerKey",
184+
// "default-12345", "layer_abc") are valid campaign_id values and must
185+
// pass through to both decisions[].campaign_id and events[].entity_id.
180186
ImpressionEvent imp = buildImpression(
181187
"experiment", "layerKey", "2222", "3333");
182188
LogEvent log = EventFactory.createLogEvent(imp);
183-
assertEquals("2222", firstDecision(log).getCampaignId());
184-
assertEquals("2222", firstEvent(log).getEntityId());
189+
assertEquals("layerKey", firstDecision(log).getCampaignId());
190+
assertEquals("layerKey", firstEvent(log).getEntityId());
191+
}
192+
193+
@Test
194+
public void campaignId_opaqueDashSeparatedString_passesThroughUnchanged() {
195+
// FSSDK-12813 (relaxed): explicit coverage for the canonical opaque-ID
196+
// example from the spec ("default-12345").
197+
ImpressionEvent imp = buildImpression(
198+
"experiment", "default-12345", "2222", "3333");
199+
LogEvent log = EventFactory.createLogEvent(imp);
200+
assertEquals("default-12345", firstDecision(log).getCampaignId());
201+
assertEquals("default-12345", firstEvent(log).getEntityId());
185202
}
186203

187204
// ---------------------------------------------------------------------
@@ -226,6 +243,14 @@ public void variationId_nonNumericString_becomesNull() {
226243

227244
@Test
228245
public void normalization_uniformAcrossAllRuleTypes() {
246+
// FSSDK-12813 (relaxed):
247+
// - campaign_id: any non-empty string passes through unchanged
248+
// (including opaque IDs like "bad-layer"). Fallback to experiment_id
249+
// only fires for null / empty string.
250+
// - variation_id: still strict numeric-string-only; non-numeric
251+
// inputs like "bad-variation" become null.
252+
// This test verifies the SAME relaxed-campaign / strict-variation
253+
// behavior applies uniformly across every rule type.
229254
String[] ruleTypes = {"experiment", "feature-test", "rollout", "holdout"};
230255
for (String ruleType : ruleTypes) {
231256
ImpressionEvent imp = buildImpression(
@@ -235,9 +260,36 @@ public void normalization_uniformAcrossAllRuleTypes() {
235260
Decision d = firstDecision(log);
236261
Event e = firstEvent(log);
237262

238-
// For every rule type, the same normalization rules apply.
263+
// campaign_id: opaque string is valid under the relaxed contract,
264+
// so it passes through unchanged for every rule type.
265+
assertEquals(
266+
"campaign_id should pass through unchanged for ruleType=" + ruleType,
267+
"bad-layer", d.getCampaignId());
268+
assertEquals(
269+
"entity_id must mirror campaign_id for ruleType=" + ruleType,
270+
"bad-layer", e.getEntityId());
271+
// variation_id: still strict numeric-only — non-numeric becomes null.
272+
assertNull(
273+
"variation_id should become null for ruleType=" + ruleType,
274+
d.getVariationId());
275+
}
276+
}
277+
278+
@Test
279+
public void normalization_uniformAcrossAllRuleTypes_nullCampaignFallsBack() {
280+
// FSSDK-12813 (relaxed): when campaign_id is null, fallback to
281+
// experiment_id still fires uniformly across every rule type.
282+
String[] ruleTypes = {"experiment", "feature-test", "rollout", "holdout"};
283+
for (String ruleType : ruleTypes) {
284+
ImpressionEvent imp = buildImpression(
285+
ruleType, null, "2222", "bad-variation");
286+
LogEvent log = EventFactory.createLogEvent(imp);
287+
288+
Decision d = firstDecision(log);
289+
Event e = firstEvent(log);
290+
239291
assertEquals(
240-
"campaign_id should fall back to experiment_id for ruleType=" + ruleType,
292+
"campaign_id should fall back to experiment_id when null for ruleType=" + ruleType,
241293
"2222", d.getCampaignId());
242294
assertEquals(
243295
"entity_id must mirror campaign_id for ruleType=" + ruleType,
@@ -254,21 +306,30 @@ public void normalization_uniformAcrossAllRuleTypes() {
254306

255307
@Test
256308
public void entityId_alwaysMirrorsNormalizedCampaignId() {
257-
// 1) Both valid → equal.
309+
// 1) Both valid numeric → equal, passed through.
258310
ImpressionEvent imp1 = buildImpression("experiment", "1111", "2222", "3333");
259311
LogEvent log1 = EventFactory.createLogEvent(imp1);
260312
assertEquals(firstDecision(log1).getCampaignId(), firstEvent(log1).getEntityId());
313+
assertEquals("1111", firstEvent(log1).getEntityId());
261314

262-
// 2) campaign_id invalid → both equal to experiment_id.
315+
// 2) campaign_id is opaque non-numeric string → passes through unchanged
316+
// under the relaxed FSSDK-12813 contract; entity_id still mirrors it.
263317
ImpressionEvent imp2 = buildImpression("experiment", "bad", "2222", "3333");
264318
LogEvent log2 = EventFactory.createLogEvent(imp2);
265319
assertEquals(firstDecision(log2).getCampaignId(), firstEvent(log2).getEntityId());
266-
assertEquals("2222", firstEvent(log2).getEntityId());
320+
assertEquals("bad", firstEvent(log2).getEntityId());
267321

268-
// 3) campaign_id null → both equal to experiment_id.
322+
// 3) campaign_id null → fallback to experiment_id, entity_id mirrors it.
269323
ImpressionEvent imp3 = buildImpression("experiment", null, "2222", "3333");
270324
LogEvent log3 = EventFactory.createLogEvent(imp3);
271325
assertEquals(firstDecision(log3).getCampaignId(), firstEvent(log3).getEntityId());
326+
assertEquals("2222", firstEvent(log3).getEntityId());
327+
328+
// 4) campaign_id empty string → fallback to experiment_id, entity_id mirrors it.
329+
ImpressionEvent imp4 = buildImpression("experiment", "", "2222", "3333");
330+
LogEvent log4 = EventFactory.createLogEvent(imp4);
331+
assertEquals(firstDecision(log4).getCampaignId(), firstEvent(log4).getEntityId());
332+
assertEquals("2222", firstEvent(log4).getEntityId());
272333
}
273334

274335
// ---------------------------------------------------------------------
@@ -277,12 +338,32 @@ public void entityId_alwaysMirrorsNormalizedCampaignId() {
277338

278339
@Test
279340
public void event_isNeverDropped_evenWhenAllIdsAreInvalid() {
280-
ImpressionEvent imp = buildImpression(
341+
// FSSDK-12813 (FR-006/FR-007): Even when EVERY id is null or non-numeric,
342+
// the event must still be created — normalization never drops events and
343+
// never throws. We use null/empty campaign_id (triggers fallback) and a
344+
// non-numeric variation_id (becomes null) to exercise both fallback paths.
345+
ImpressionEvent impNullCampaign = buildImpression(
346+
"experiment", null, "2222", "bad");
347+
LogEvent logNull = EventFactory.createLogEvent(impNullCampaign);
348+
assertNotNull("Event must not be dropped due to id normalization", logNull);
349+
assertNotNull(firstDecision(logNull));
350+
assertNotNull(firstEvent(logNull));
351+
352+
ImpressionEvent impEmptyCampaign = buildImpression(
353+
"experiment", "", "2222", "bad");
354+
LogEvent logEmpty = EventFactory.createLogEvent(impEmptyCampaign);
355+
assertNotNull("Event must not be dropped due to id normalization", logEmpty);
356+
assertNotNull(firstDecision(logEmpty));
357+
assertNotNull(firstEvent(logEmpty));
358+
359+
// Also verify an opaque-but-non-empty campaign_id (now valid under the
360+
// relaxed contract) still produces a non-null event.
361+
ImpressionEvent impOpaqueCampaign = buildImpression(
281362
"experiment", "bad", "2222", "bad");
282-
LogEvent log = EventFactory.createLogEvent(imp);
283-
assertNotNull("Event must not be dropped due to id normalization", log);
284-
assertNotNull(firstDecision(log));
285-
assertNotNull(firstEvent(log));
363+
LogEvent logOpaque = EventFactory.createLogEvent(impOpaqueCampaign);
364+
assertNotNull("Event must not be dropped due to id normalization", logOpaque);
365+
assertNotNull(firstDecision(logOpaque));
366+
assertNotNull(firstEvent(logOpaque));
286367
}
287368

288369
// ---------------------------------------------------------------------

0 commit comments

Comments
 (0)