From 6eb1f6aee4142e1047b665134e72a6898f3dfab6 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 08:42:22 -0700 Subject: [PATCH 1/6] [FSSDK-12813] Normalize decision event campaign_id, variation_id, and entity_id --- .../ab/event/internal/EventFactory.java | 18 +- .../ab/event/internal/EventIdNormalizer.java | 93 +++++ .../com/optimizely/ab/OptimizelyTest.java | 100 +++-- .../ab/OptimizelyUserContextTest.java | 14 +- .../EventFactoryNormalizationTest.java | 352 ++++++++++++++++++ .../event/internal/EventIdNormalizerTest.java | 211 +++++++++++ 6 files changed, 746 insertions(+), 42 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java create mode 100644 core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java create mode 100644 core-api/src/test/java/com/optimizely/ab/event/internal/EventIdNormalizerTest.java diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java index f200f963d..090739da3 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2020, 2022, Optimizely and contributors + * Copyright 2016-2020, 2022, 2025, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -97,10 +97,20 @@ private static Visitor createVisitor(ImpressionEvent impressionEvent) { UserContext userContext = impressionEvent.getUserContext(); + // FSSDK-12813: Normalize identifier fields uniformly across all decision types + // (experiment, feature test, rollout, holdout). Empty / null / non-numeric + // campaign_id falls back to experiment_id. Empty / null / non-numeric + // variation_id becomes null. entity_id mirrors the normalized campaign_id + // byte-for-byte to guarantee wire equivalence. + String normalizedCampaignId = EventIdNormalizer.normalizeCampaignId( + impressionEvent.getLayerId(), impressionEvent.getExperimentId()); + String normalizedVariationId = EventIdNormalizer.normalizeVariationId( + impressionEvent.getVariationId()); + Decision decision = new Decision.Builder() - .setCampaignId(impressionEvent.getLayerId()) + .setCampaignId(normalizedCampaignId) .setExperimentId(impressionEvent.getExperimentId()) - .setVariationId(impressionEvent.getVariationId()) + .setVariationId(normalizedVariationId) .setMetadata(impressionEvent.getMetadata()) .setIsCampaignHoldback(false) .build(); @@ -108,7 +118,7 @@ private static Visitor createVisitor(ImpressionEvent impressionEvent) { Event event = new Event.Builder() .setTimestamp(impressionEvent.getTimestamp()) .setUuid(impressionEvent.getUUID()) - .setEntityId(impressionEvent.getLayerId()) + .setEntityId(normalizedCampaignId) .setKey(ACTIVATE_EVENT_KEY) .setType(ACTIVATE_EVENT_KEY) .build(); diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java new file mode 100644 index 000000000..7f34ec4ef --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java @@ -0,0 +1,93 @@ +/** + * + * Copyright 2025, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.event.internal; + +/** + * EventIdNormalizer normalizes decision-event identifier fields prior to wire serialization. + * + *

Implements FSSDK-12813: + *

+ * + *

A "numeric string" is a non-empty string consisting entirely of decimal digits + * {@code [0-9]}. Leading zeros are allowed. Whitespace, negatives, decimals, and + * exponents are INVALID. + * + *

Normalization applies uniformly to all decision types (experiment, feature test, + * rollout, holdout). It must not drop, defer, or fail event dispatch, and it must not + * emit any log or warning on the normalization path. + */ +final class EventIdNormalizer { + + private EventIdNormalizer() { + // Utility class — not instantiable. + } + + /** + * @return {@code true} iff {@code value} is non-null and consists entirely of decimal digits. + * Empty strings, whitespace, negatives, decimals, and exponents are all invalid. + */ + static boolean isNumericString(String value) { + if (value == null) { + return false; + } + int length = value.length(); + if (length == 0) { + return false; + } + for (int i = 0; i < length; i++) { + char c = value.charAt(i); + if (c < '0' || c > '9') { + return false; + } + } + return true; + } + + /** + * Normalize a {@code campaign_id} or impression {@code entity_id}. + * + * @param campaignId the candidate campaign_id (may be null, empty, or non-numeric) + * @param experimentId fallback experiment_id (returned as-is; not re-validated) + * @return {@code campaignId} when it is a non-empty numeric string, + * otherwise {@code experimentId} (which may itself be {@code null}). + */ + static String normalizeCampaignId(String campaignId, String experimentId) { + if (isNumericString(campaignId)) { + return campaignId; + } + return experimentId; + } + + /** + * Normalize a {@code variation_id}. + * + * @param variationId the candidate variation_id (may be null, empty, or non-numeric) + * @return {@code variationId} when it is a non-empty numeric string, otherwise {@code null}. + */ + static String normalizeVariationId(String variationId) { + if (isNumericString(variationId)) { + return variationId; + } + return null; + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 15c3eea5f..dd5341b69 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -618,7 +618,8 @@ public void isFeatureEnabledWithExperimentKeyForced() throws Exception { assertTrue(optimizely.setForcedVariation(activatedExperiment.getKey(), testUserId, null)); assertNull(optimizely.getForcedVariation(activatedExperiment.getKey(), testUserId)); assertFalse(optimizely.isFeatureEnabled(FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), testUserId)); - eventHandler.expectImpression(null, "", testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); } /** @@ -1810,13 +1811,19 @@ public void getEnabledFeaturesWithListenerMultipleFeatureEnabled() throws Except List featureFlags = optimizely.getEnabledFeatures(testUserId, Collections.emptyMap()); assertEquals(2, featureFlags.size()); - eventHandler.expectImpression(null, "", testUserId); - eventHandler.expectImpression(null, "", testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); eventHandler.expectImpression("3794675122", "589640735", testUserId); - eventHandler.expectImpression(null, "", testUserId); - eventHandler.expectImpression(null, "", testUserId); - eventHandler.expectImpression(null, "", testUserId); - eventHandler.expectImpression(null, "", testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, testUserId); eventHandler.expectImpression("1786133852", "1619235542", testUserId); // Verify that listener being called @@ -1853,14 +1860,22 @@ public void getEnabledFeaturesWithNoFeatureEnabled() throws Exception { // Verify that listener not being called assertFalse(isListenerCalled); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); } @@ -2013,7 +2028,8 @@ public void isFeatureEnabledWithListenerUserNotInExperimentAndNotInRollOut() thr "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); // Verify that listener being called assertTrue(isListenerCalled); @@ -3339,7 +3355,8 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), @@ -3384,7 +3401,8 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? true" ); - eventHandler.expectImpression("3421010877", "variationId", genericUserId); + // FSSDK-12813: non-numeric variation_id "variationId" is normalized to null on the wire. + eventHandler.expectImpression("3421010877", null, genericUserId); verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), @@ -3456,7 +3474,8 @@ public void isFeatureEnabledTrueWhenFeatureEnabledOfVariationIsTrue() throws Exc ); assertTrue(optimizely.isFeatureEnabled(validFeatureKey, genericUserId)); - eventHandler.expectImpression("3421010877", "variationId", genericUserId); + // FSSDK-12813: non-numeric variation_id "variationId" is normalized to null on the wire. + eventHandler.expectImpression("3421010877", null, genericUserId); } @@ -3485,7 +3504,8 @@ public void isFeatureEnabledFalseWhenFeatureEnabledOfVariationIsFalse() throws E ); assertFalse(spyOptimizely.isFeatureEnabled(FEATURE_MULTI_VARIATE_FEATURE_KEY, genericUserId)); - eventHandler.expectImpression("3421010877", "variationId", genericUserId); + // FSSDK-12813: non-numeric variation_id "variationId" is normalized to null on the wire. + eventHandler.expectImpression("3421010877", null, genericUserId); } @@ -3582,10 +3602,13 @@ public void getEnabledFeatureWithValidUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); eventHandler.expectImpression("3794675122", "589640735", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); eventHandler.expectImpression("1785077004", "1566407342", genericUserId); eventHandler.expectImpression("828245624", "3137445031", genericUserId); eventHandler.expectImpression("828245624", "3137445031", genericUserId); @@ -3606,10 +3629,11 @@ public void getEnabledFeatureWithEmptyUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures("", Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); - eventHandler.expectImpression(null, "", ""); - eventHandler.expectImpression(null, "", ""); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression(null, null, ""); eventHandler.expectImpression("3794675122", "589640735", ""); - eventHandler.expectImpression(null, "", ""); + eventHandler.expectImpression(null, null, ""); eventHandler.expectImpression("1785077004", "1566407342", ""); eventHandler.expectImpression("828245624", "3137445031", ""); eventHandler.expectImpression("828245624", "3137445031", ""); @@ -3660,14 +3684,22 @@ public void getEnabledFeatureWithMockDecisionService() throws Exception { List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertTrue(featureFlags.isEmpty()); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); - eventHandler.expectImpression(null, "", genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, genericUserId); } /** diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 2e479d2ea..19b65c194 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -267,7 +267,8 @@ public void decide_nullVariation() { .setVariationKey("") .setEnabled(false) .build(); - eventHandler.expectImpression(null, "", userId, Collections.emptyMap(), metadata); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, userId, Collections.emptyMap(), metadata); } // decideAll @@ -639,7 +640,8 @@ public void decide_sendEvent_rollout_withSendFlagDecisionsOn() { user.decide(flagKey); assertTrue(isListenerCalled); - eventHandler.expectImpression(null, "", userId, attributes); + // FSSDK-12813: empty-string variation_id is normalized to null on the wire. + eventHandler.expectImpression(null, null, userId, attributes); } @Test @@ -2102,6 +2104,8 @@ public void decisionNotification_with_holdout() throws Exception { String variationKey = "ho_off_key"; // holdout (off) variation key String experimentId = "10075323428"; // holdout experiment id in holdouts-project-config.json String variationId = "$opt_dummy_variation_id";// dummy variation id used for holdout impressions + // FSSDK-12813: dummy variation_id is non-numeric and normalized to null on the wire. + String expectedDispatchedVariationId = null; String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (" + ruleKey + ")."; Map attrs = new HashMap<>(); @@ -2153,7 +2157,7 @@ public void decisionNotification_with_holdout() throws Exception { .setVariationKey(variationKey) .setEnabled(false) .build(); - eventHandler.expectImpression(experimentId, variationId, userId, Collections.singletonMap("nationality", "English"), metadata); + eventHandler.expectImpression(experimentId, expectedDispatchedVariationId, userId, Collections.singletonMap("nationality", "English"), metadata); // Log expectation (reuse existing pattern) logbackVerifier.expectMessage(Level.INFO, expectedReason); @@ -2177,6 +2181,8 @@ public void decide_for_keys_with_holdout() throws Exception { String holdoutExperimentId = "10075323428"; // basic_holdout id String variationId = "$opt_dummy_variation_id"; + // FSSDK-12813: dummy variation_id is non-numeric and normalized to null on the wire. + String expectedDispatchedVariationId = null; String variationKey = "ho_off_key"; String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (basic_holdout)."; @@ -2195,7 +2201,7 @@ public void decide_for_keys_with_holdout() throws Exception { .setEnabled(false) .build(); // attributes map expected empty (reserved $opt_ attribute filtered out) - eventHandler.expectImpression(holdoutExperimentId, variationId, userId, Collections.emptyMap(), metadata); + eventHandler.expectImpression(holdoutExperimentId, expectedDispatchedVariationId, userId, Collections.emptyMap(), metadata); } // At least one log message confirming holdout membership diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java new file mode 100644 index 000000000..62164aa79 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java @@ -0,0 +1,352 @@ +/** + * + * Copyright 2025, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.event.internal; + +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.event.LogEvent; +import com.optimizely.ab.event.internal.payload.Decision; +import com.optimizely.ab.event.internal.payload.DecisionMetadata; +import com.optimizely.ab.event.internal.payload.Event; +import com.optimizely.ab.event.internal.payload.EventBatch; +import com.optimizely.ab.event.internal.payload.Snapshot; +import com.optimizely.ab.event.internal.payload.Visitor; +import org.junit.Before; +import org.junit.Test; + +import java.util.Collections; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Integration tests for FSSDK-12813 decision-event identifier normalization, + * exercising the full path from {@link ImpressionEvent} through + * {@link EventFactory#createLogEvent} to the wire payload. + * + *

Verifies: + *

+ */ +public class EventFactoryNormalizationTest { + + private static final String USER_ID = "test-user"; + + private ProjectConfig projectConfig; + + @Before + public void setUp() { + projectConfig = mock(ProjectConfig.class); + when(projectConfig.getAccountId()).thenReturn("1"); + when(projectConfig.getProjectId()).thenReturn("100"); + when(projectConfig.getRevision()).thenReturn("3"); + when(projectConfig.getAnonymizeIP()).thenReturn(true); + when(projectConfig.getBotFiltering()).thenReturn(null); + when(projectConfig.getRegion()).thenReturn("US"); + } + + private UserContext userContext() { + return new UserContext.Builder() + .withUserId(USER_ID) + .withAttributes(Collections.emptyMap()) + .withProjectConfig(projectConfig) + .build(); + } + + private ImpressionEvent buildImpression(String ruleType, + String layerId, + String experimentId, + String variationId) { + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey("test_flag") + .setRuleKey("test_rule") + .setRuleType(ruleType) + .setVariationKey("variationKey") + .setEnabled(true) + .build(); + + return new ImpressionEvent.Builder() + .withUserContext(userContext()) + .withLayerId(layerId) + .withExperimentId(experimentId) + .withExperimentKey("experimentKey") + .withVariationId(variationId) + .withVariationKey("variationKey") + .withMetadata(metadata) + .build(); + } + + private static Decision firstDecision(LogEvent logEvent) { + EventBatch batch = logEvent.getEventBatch(); + Visitor visitor = batch.getVisitors().get(0); + Snapshot snapshot = visitor.getSnapshots().get(0); + return snapshot.getDecisions().get(0); + } + + private static Event firstEvent(LogEvent logEvent) { + EventBatch batch = logEvent.getEventBatch(); + Visitor visitor = batch.getVisitors().get(0); + Snapshot snapshot = visitor.getSnapshots().get(0); + return snapshot.getEvents().get(0); + } + + // --------------------------------------------------------------------- + // Happy path + // --------------------------------------------------------------------- + + /** + * FR-001/FR-009 happy path: a valid numeric campaign_id is passed through, + * and entity_id matches it byte-for-byte. + */ + @Test + public void happyPath_numericIds_passThroughUnchanged() { + ImpressionEvent imp = buildImpression( + "experiment", "1111", "2222", "3333"); + + LogEvent log = EventFactory.createLogEvent(imp); + + assertNotNull(log); + Decision d = firstDecision(log); + Event e = firstEvent(log); + + assertEquals("1111", d.getCampaignId()); + assertEquals("2222", d.getExperimentId()); + assertEquals("3333", d.getVariationId()); + assertEquals("1111", e.getEntityId()); + // FR-009 byte-for-byte: same reference content + assertEquals(d.getCampaignId(), e.getEntityId()); + } + + // --------------------------------------------------------------------- + // FR-001/FR-002: campaign_id fallback + // --------------------------------------------------------------------- + + @Test + public void campaignId_null_fallsBackToExperimentId() { + ImpressionEvent imp = buildImpression( + "experiment", null, "2222", "3333"); + LogEvent log = EventFactory.createLogEvent(imp); + assertEquals("2222", firstDecision(log).getCampaignId()); + assertEquals("2222", firstEvent(log).getEntityId()); + } + + @Test + public void campaignId_empty_fallsBackToExperimentId() { + ImpressionEvent imp = buildImpression( + "experiment", "", "2222", "3333"); + LogEvent log = EventFactory.createLogEvent(imp); + assertEquals("2222", firstDecision(log).getCampaignId()); + assertEquals("2222", firstEvent(log).getEntityId()); + } + + @Test + public void campaignId_whitespace_fallsBackToExperimentId() { + ImpressionEvent imp = buildImpression( + "experiment", " ", "2222", "3333"); + LogEvent log = EventFactory.createLogEvent(imp); + assertEquals("2222", firstDecision(log).getCampaignId()); + assertEquals("2222", firstEvent(log).getEntityId()); + } + + @Test + public void campaignId_nonNumericString_fallsBackToExperimentId() { + ImpressionEvent imp = buildImpression( + "experiment", "layerKey", "2222", "3333"); + LogEvent log = EventFactory.createLogEvent(imp); + assertEquals("2222", firstDecision(log).getCampaignId()); + assertEquals("2222", firstEvent(log).getEntityId()); + } + + // --------------------------------------------------------------------- + // FR-003/FR-004: variation_id null replacement + // --------------------------------------------------------------------- + + @Test + public void variationId_null_remainsNull() { + ImpressionEvent imp = buildImpression( + "experiment", "1111", "2222", null); + LogEvent log = EventFactory.createLogEvent(imp); + assertNull(firstDecision(log).getVariationId()); + } + + @Test + public void variationId_empty_becomesNull() { + ImpressionEvent imp = buildImpression( + "experiment", "1111", "2222", ""); + LogEvent log = EventFactory.createLogEvent(imp); + assertNull(firstDecision(log).getVariationId()); + } + + @Test + public void variationId_whitespace_becomesNull() { + ImpressionEvent imp = buildImpression( + "experiment", "1111", "2222", " "); + LogEvent log = EventFactory.createLogEvent(imp); + assertNull(firstDecision(log).getVariationId()); + } + + @Test + public void variationId_nonNumericString_becomesNull() { + ImpressionEvent imp = buildImpression( + "experiment", "1111", "2222", "variationKey"); + LogEvent log = EventFactory.createLogEvent(imp); + assertNull(firstDecision(log).getVariationId()); + } + + // --------------------------------------------------------------------- + // FR-005: Uniform across decision types + // --------------------------------------------------------------------- + + @Test + public void normalization_uniformAcrossAllRuleTypes() { + String[] ruleTypes = {"experiment", "feature-test", "rollout", "holdout"}; + for (String ruleType : ruleTypes) { + ImpressionEvent imp = buildImpression( + ruleType, "bad-layer", "2222", "bad-variation"); + LogEvent log = EventFactory.createLogEvent(imp); + + Decision d = firstDecision(log); + Event e = firstEvent(log); + + // For every rule type, the same normalization rules apply. + assertEquals( + "campaign_id should fall back to experiment_id for ruleType=" + ruleType, + "2222", d.getCampaignId()); + assertEquals( + "entity_id must mirror campaign_id for ruleType=" + ruleType, + "2222", e.getEntityId()); + assertNull( + "variation_id should become null for ruleType=" + ruleType, + d.getVariationId()); + } + } + + // --------------------------------------------------------------------- + // FR-009: byte-for-byte equality of entity_id and campaign_id + // --------------------------------------------------------------------- + + @Test + public void entityId_alwaysMirrorsNormalizedCampaignId() { + // 1) Both valid → equal. + ImpressionEvent imp1 = buildImpression("experiment", "1111", "2222", "3333"); + LogEvent log1 = EventFactory.createLogEvent(imp1); + assertEquals(firstDecision(log1).getCampaignId(), firstEvent(log1).getEntityId()); + + // 2) campaign_id invalid → both equal to experiment_id. + ImpressionEvent imp2 = buildImpression("experiment", "bad", "2222", "3333"); + LogEvent log2 = EventFactory.createLogEvent(imp2); + assertEquals(firstDecision(log2).getCampaignId(), firstEvent(log2).getEntityId()); + assertEquals("2222", firstEvent(log2).getEntityId()); + + // 3) campaign_id null → both equal to experiment_id. + ImpressionEvent imp3 = buildImpression("experiment", null, "2222", "3333"); + LogEvent log3 = EventFactory.createLogEvent(imp3); + assertEquals(firstDecision(log3).getCampaignId(), firstEvent(log3).getEntityId()); + } + + // --------------------------------------------------------------------- + // FR-006/FR-007: Event must never be dropped, no exceptions + // --------------------------------------------------------------------- + + @Test + public void event_isNeverDropped_evenWhenAllIdsAreInvalid() { + ImpressionEvent imp = buildImpression( + "experiment", "bad", "2222", "bad"); + LogEvent log = EventFactory.createLogEvent(imp); + assertNotNull("Event must not be dropped due to id normalization", log); + assertNotNull(firstDecision(log)); + assertNotNull(firstEvent(log)); + } + + // --------------------------------------------------------------------- + // FR-010: Conversion entity_id is unchanged + // --------------------------------------------------------------------- + + @Test + public void conversionEvent_entityId_isUnchanged() { + // Conversion events derive entity_id from a different source (event_id) and + // must NOT be normalized. We use a non-numeric event_id ("event_abc") and + // verify it passes through verbatim. + ConversionEvent conversion = new ConversionEvent.Builder() + .withUserContext(userContext()) + .withEventId("event_abc") + .withEventKey("checkout") + .withRevenue(null) + .withValue(null) + .withTags(Collections.emptyMap()) + .build(); + + LogEvent log = EventFactory.createLogEvent(conversion); + assertNotNull(log); + + Event e = firstEvent(log); + assertEquals("event_abc", e.getEntityId()); + assertEquals("checkout", e.getKey()); + } + + // --------------------------------------------------------------------- + // FR-008: Wire output is identical for identical inputs (determinism). + // --------------------------------------------------------------------- + + @Test + public void wireOutput_isDeterministic_forSameInput() { + ImpressionEvent imp1 = buildImpression("experiment", "1111", "2222", "3333"); + ImpressionEvent imp2 = buildImpression("experiment", "1111", "2222", "3333"); + + LogEvent log1 = EventFactory.createLogEvent(imp1); + LogEvent log2 = EventFactory.createLogEvent(imp2); + + Decision d1 = firstDecision(log1); + Decision d2 = firstDecision(log2); + assertEquals(d1.getCampaignId(), d2.getCampaignId()); + assertEquals(d1.getExperimentId(), d2.getExperimentId()); + assertEquals(d1.getVariationId(), d2.getVariationId()); + assertEquals(firstEvent(log1).getEntityId(), firstEvent(log2).getEntityId()); + } + + // --------------------------------------------------------------------- + // Reference safety: when campaign_id is valid, the same string is used + // for both decision.campaign_id and event.entity_id (no defensive copy). + // --------------------------------------------------------------------- + + @Test + public void validCampaignId_usedForBothDecisionAndEntity() { + String layerId = "9876543210"; + ImpressionEvent imp = buildImpression("experiment", layerId, "2222", "3333"); + LogEvent log = EventFactory.createLogEvent(imp); + + assertSame( + "Same numeric campaign_id must be reused for both fields", + layerId, firstDecision(log).getCampaignId()); + assertSame( + "entity_id must reuse the same normalized campaign_id reference", + layerId, firstEvent(log).getEntityId()); + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventIdNormalizerTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventIdNormalizerTest.java new file mode 100644 index 000000000..9565cc3c4 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventIdNormalizerTest.java @@ -0,0 +1,211 @@ +/** + * + * Copyright 2025, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.event.internal; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * Unit tests for {@link EventIdNormalizer}. + * + *

Covers FSSDK-12813 normalization rules: + *

+ */ +public class EventIdNormalizerTest { + + // --------------------------------------------------------------------- + // isNumericString + // --------------------------------------------------------------------- + + @Test + public void isNumericString_validDecimalDigits_returnsTrue() { + assertTrue(EventIdNormalizer.isNumericString("0")); + assertTrue(EventIdNormalizer.isNumericString("1")); + assertTrue(EventIdNormalizer.isNumericString("12345")); + assertTrue(EventIdNormalizer.isNumericString("9999999999999")); + } + + @Test + public void isNumericString_leadingZerosAllowed_returnsTrue() { + assertTrue(EventIdNormalizer.isNumericString("0123")); + assertTrue(EventIdNormalizer.isNumericString("00")); + assertTrue(EventIdNormalizer.isNumericString("000000001")); + } + + @Test + public void isNumericString_null_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString(null)); + } + + @Test + public void isNumericString_empty_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString("")); + } + + @Test + public void isNumericString_whitespace_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString(" ")); + assertFalse(EventIdNormalizer.isNumericString(" ")); + assertFalse(EventIdNormalizer.isNumericString("\t")); + assertFalse(EventIdNormalizer.isNumericString("\n")); + // surrounding whitespace is also invalid + assertFalse(EventIdNormalizer.isNumericString(" 123")); + assertFalse(EventIdNormalizer.isNumericString("123 ")); + assertFalse(EventIdNormalizer.isNumericString(" 123 ")); + } + + @Test + public void isNumericString_nonDigits_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString("abc")); + assertFalse(EventIdNormalizer.isNumericString("variation_a")); + assertFalse(EventIdNormalizer.isNumericString("exp_42")); + assertFalse(EventIdNormalizer.isNumericString("layerId")); + assertFalse(EventIdNormalizer.isNumericString("12a")); + assertFalse(EventIdNormalizer.isNumericString("a12")); + } + + @Test + public void isNumericString_negativesAreInvalid_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString("-1")); + assertFalse(EventIdNormalizer.isNumericString("-123")); + } + + @Test + public void isNumericString_decimalsAreInvalid_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString("1.0")); + assertFalse(EventIdNormalizer.isNumericString("123.456")); + assertFalse(EventIdNormalizer.isNumericString(".")); + } + + @Test + public void isNumericString_exponentsAreInvalid_returnsFalse() { + assertFalse(EventIdNormalizer.isNumericString("1e5")); + assertFalse(EventIdNormalizer.isNumericString("1E5")); + assertFalse(EventIdNormalizer.isNumericString("1.0e3")); + } + + @Test + public void isNumericString_unicodeDigitsAreInvalid_returnsFalse() { + // Unicode digit U+0660 (Arabic-Indic 0) — not ASCII [0-9], must be rejected. + assertFalse(EventIdNormalizer.isNumericString("٠١٢")); + // Fullwidth digits U+FF10..U+FF19 — must be rejected. + assertFalse(EventIdNormalizer.isNumericString("１２３")); + } + + // --------------------------------------------------------------------- + // normalizeCampaignId + // --------------------------------------------------------------------- + + @Test + public void normalizeCampaignId_validNumeric_returnsCampaignId() { + assertEquals("12345", EventIdNormalizer.normalizeCampaignId("12345", "67890")); + assertEquals("0", EventIdNormalizer.normalizeCampaignId("0", "67890")); + assertEquals("0123", EventIdNormalizer.normalizeCampaignId("0123", "67890")); + } + + @Test + public void normalizeCampaignId_null_fallsBackToExperimentId() { + assertEquals("67890", EventIdNormalizer.normalizeCampaignId(null, "67890")); + } + + @Test + public void normalizeCampaignId_empty_fallsBackToExperimentId() { + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("", "67890")); + } + + @Test + public void normalizeCampaignId_whitespace_fallsBackToExperimentId() { + assertEquals("67890", EventIdNormalizer.normalizeCampaignId(" ", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId(" ", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("\t", "67890")); + } + + @Test + public void normalizeCampaignId_nonNumeric_fallsBackToExperimentId() { + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("abc", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("layerId", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("exp_42", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("12a", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("-1", "67890")); + assertEquals("67890", EventIdNormalizer.normalizeCampaignId("1.0", "67890")); + } + + @Test + public void normalizeCampaignId_invalidCampaignAndNullExperiment_returnsNull() { + // Spec is silent on this combination; library returns experiment_id as-is, which may be null. + // This is intentional: no logging, no failure, just pass-through. + assertNull(EventIdNormalizer.normalizeCampaignId(null, null)); + assertNull(EventIdNormalizer.normalizeCampaignId("", null)); + assertNull(EventIdNormalizer.normalizeCampaignId("abc", null)); + } + + @Test + public void normalizeCampaignId_invalidCampaignAndNonNumericExperiment_returnsExperimentAsIs() { + // Fallback is verbatim — not re-validated. This matches the spec. + assertEquals("expKey", EventIdNormalizer.normalizeCampaignId(null, "expKey")); + assertEquals("", EventIdNormalizer.normalizeCampaignId("abc", "")); + } + + // --------------------------------------------------------------------- + // normalizeVariationId + // --------------------------------------------------------------------- + + @Test + public void normalizeVariationId_validNumeric_returnsVariationId() { + assertEquals("12345", EventIdNormalizer.normalizeVariationId("12345")); + assertEquals("0", EventIdNormalizer.normalizeVariationId("0")); + assertEquals("0123", EventIdNormalizer.normalizeVariationId("0123")); + } + + @Test + public void normalizeVariationId_null_returnsNull() { + assertNull(EventIdNormalizer.normalizeVariationId(null)); + } + + @Test + public void normalizeVariationId_empty_returnsNull() { + assertNull(EventIdNormalizer.normalizeVariationId("")); + } + + @Test + public void normalizeVariationId_whitespace_returnsNull() { + assertNull(EventIdNormalizer.normalizeVariationId(" ")); + assertNull(EventIdNormalizer.normalizeVariationId(" ")); + assertNull(EventIdNormalizer.normalizeVariationId("\t")); + } + + @Test + public void normalizeVariationId_nonNumeric_returnsNull() { + assertNull(EventIdNormalizer.normalizeVariationId("abc")); + assertNull(EventIdNormalizer.normalizeVariationId("variation_a")); + assertNull(EventIdNormalizer.normalizeVariationId("variationId")); + assertNull(EventIdNormalizer.normalizeVariationId("12a")); + assertNull(EventIdNormalizer.normalizeVariationId("-1")); + assertNull(EventIdNormalizer.normalizeVariationId("1.0")); + } +} From 77f3c224b3ec1cd0af10d3d048d379fdeac77a6b Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 11:12:17 -0700 Subject: [PATCH 2/6] [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. --- .../ab/event/internal/EventIdNormalizer.java | 40 ++++-- .../EventFactoryNormalizationTest.java | 117 +++++++++++++++--- .../event/internal/EventIdNormalizerTest.java | 85 +++++++++---- 3 files changed, 188 insertions(+), 54 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java index 7f34ec4ef..b2c4b1fe9 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventIdNormalizer.java @@ -22,15 +22,17 @@ *

Implements FSSDK-12813: *

* - *

A "numeric string" is a non-empty string consisting entirely of decimal digits - * {@code [0-9]}. Leading zeros are allowed. Whitespace, negatives, decimals, and - * exponents are INVALID. + *

For {@code variation_id}, a "numeric string" is a non-empty string consisting + * entirely of decimal digits {@code [0-9]}. Leading zeros are allowed. Whitespace, + * negatives, decimals, and exponents are INVALID. * *

Normalization applies uniformly to all decision types (experiment, feature test, * rollout, holdout). It must not drop, defer, or fail event dispatch, and it must not @@ -42,9 +44,20 @@ private EventIdNormalizer() { // Utility class — not instantiable. } + /** + * @return {@code true} iff {@code value} is non-null and has length ≥ 1. + * Character content is not validated — any non-empty string is accepted. + * Used to validate {@code campaign_id} and impression {@code entity_id} + * per the FSSDK-12813 relaxed contract. + */ + static boolean isNonEmptyString(String value) { + return value != null && !value.isEmpty(); + } + /** * @return {@code true} iff {@code value} is non-null and consists entirely of decimal digits. * Empty strings, whitespace, negatives, decimals, and exponents are all invalid. + * Used to validate {@code variation_id} per the FSSDK-12813 strict numeric-string contract. */ static boolean isNumericString(String value) { if (value == null) { @@ -66,13 +79,18 @@ static boolean isNumericString(String value) { /** * Normalize a {@code campaign_id} or impression {@code entity_id}. * - * @param campaignId the candidate campaign_id (may be null, empty, or non-numeric) + *

Per FSSDK-12813, any non-empty string is accepted as-is (IDs may be opaque, + * e.g. {@code "default-12345"}, {@code "layer_abc"}). The fallback to + * {@code experiment_id} fires ONLY when {@code campaignId} is {@code null} or + * the empty string {@code ""}. + * + * @param campaignId the candidate campaign_id (may be null or empty) * @param experimentId fallback experiment_id (returned as-is; not re-validated) - * @return {@code campaignId} when it is a non-empty numeric string, + * @return {@code campaignId} when it is a non-empty string of any content, * otherwise {@code experimentId} (which may itself be {@code null}). */ static String normalizeCampaignId(String campaignId, String experimentId) { - if (isNumericString(campaignId)) { + if (isNonEmptyString(campaignId)) { return campaignId; } return experimentId; @@ -81,6 +99,10 @@ static String normalizeCampaignId(String campaignId, String experimentId) { /** * Normalize a {@code variation_id}. * + *

Per FSSDK-12813, {@code variation_id} retains the stricter contract: must be + * a non-empty decimal-digit string. Anything else (null, empty, whitespace, or + * non-numeric) is replaced with {@code null}. + * * @param variationId the candidate variation_id (may be null, empty, or non-numeric) * @return {@code variationId} when it is a non-empty numeric string, otherwise {@code null}. */ diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java index 62164aa79..ebcde9032 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryNormalizationTest.java @@ -44,7 +44,8 @@ *

Verifies: *