[FSSDK-12813] Normalize decision event campaign_id, variation_id, and entity_id

jaeopt · jaeopt · commit b11dd6fe54f5 · 2026-06-25T08:56:03.000-07:00
diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py
@@ -18,6 +18,7 @@
 from optimizely.helpers import enums
 from optimizely.helpers import event_tag_utils
 from optimizely.helpers import validator
+from . import event_id_normalizer
 from . import log_event
 from . import payload
 from . import user_event
@@ -134,12 +135,25 @@ def _create_visitor(cls, event: Optional[user_event.UserEvent], logger: Logger)
                 if isinstance(event.experiment, entities.Experiment):
                     experiment_layerId = event.experiment.layerId
 
+            # FSSDK-12813: Normalize decision-event IDs uniformly across all
+            # decision types (experiment, feature test, rollout, holdout).
+            # campaign_id falls back to experiment_id when invalid.
+            # variation_id becomes None when invalid.
+            # entity_id on the impression event mirrors campaign_id (FR-009)
+            # so the two fields are byte-equivalent for the same event.
+            normalized_campaign_id = event_id_normalizer.normalize_campaign_id(
+                experiment_layerId, experiment_id
+            )
+            normalized_variation_id = event_id_normalizer.normalize_variation_id(variation_id)
+
             metadata = payload.Metadata(event.flag_key, event.rule_key,
                                         event.rule_type, variation_key,
                                         event.enabled, event.cmab_uuid)
-            decision = payload.Decision(experiment_layerId, experiment_id, variation_id, metadata)
+            decision = payload.Decision(
+                normalized_campaign_id, experiment_id, normalized_variation_id, metadata
+            )
             snapshot_event = payload.SnapshotEvent(
-                experiment_layerId, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp,
+                normalized_campaign_id, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp,
             )
 
             snapshot = payload.Snapshot([snapshot_event], [decision])
diff --git a/optimizely/event/event_id_normalizer.py b/optimizely/event/event_id_normalizer.py
@@ -0,0 +1,84 @@
+# Copyright 2026, Optimizely
+# 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.
+
+"""Normalization helpers for decision-event ID fields.
+
+This module provides byte-equivalent, cross-SDK normalization for the
+``campaign_id``, ``variation_id``, and impression ``entity_id`` fields that
+appear in dispatched decision events. See FSSDK-12813.
+
+Rules:
+  * A "numeric ID string" is a non-empty :class:`str` consisting entirely of
+    decimal digits ``0-9``. Leading zeros are allowed. Whitespace, negatives,
+    decimals, and exponents are INVALID.
+  * ``campaign_id`` -> when invalid, falls back to ``experiment_id`` (which is
+    itself passed through :func:`normalize_string_id`).
+  * ``variation_id`` -> when invalid, becomes ``None``.
+  * ``entity_id`` on impression events shares the campaign_id normalization
+    and is therefore byte-equivalent to the normalized campaign_id for the
+    same impression (FR-009).
+
+The normalization path MUST NOT log, warn, or raise. It must never drop or
+defer event dispatch.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Optional
+
+
+def is_numeric_id_string(value: Any) -> bool:
+    """Return ``True`` if ``value`` is a non-empty decimal-digit string.
+
+    Whitespace, signs, decimal points, exponents, and non-string types all
+    return ``False``. Leading zeros are accepted.
+    """
+    if not isinstance(value, str):
+        return False
+    if value == '':
+        return False
+    # ``str.isdigit`` rejects everything except [0-9] characters and the
+    # empty string. We've already excluded the empty case above. Note that
+    # ``isdigit`` also accepts some non-ASCII digit code points; ``isascii``
+    # combined with ``isdigit`` restricts us to plain decimal digits.
+    return value.isascii() and value.isdigit()
+
+
+def normalize_string_id(value: Any) -> Optional[str]:
+    """Return ``value`` if it's a numeric ID string, otherwise ``None``."""
+    return value if is_numeric_id_string(value) else None
+
+
+def normalize_campaign_id(campaign_id: Any, experiment_id: Any) -> str:
+    """Normalize a decision-event ``campaign_id`` (FR-001/FR-002, FR-009).
+
+    If ``campaign_id`` is a valid numeric ID string it is returned unchanged.
+    Otherwise the function falls back to ``experiment_id`` (after applying
+    the same validation). If neither is a numeric ID string, an empty string
+    is returned so the event still dispatches (FR-006).
+    """
+    if is_numeric_id_string(campaign_id):
+        return campaign_id  # type: ignore[no-any-return]
+    if is_numeric_id_string(experiment_id):
+        return experiment_id  # type: ignore[no-any-return]
+    return ''
+
+
+def normalize_variation_id(variation_id: Any) -> Optional[str]:
+    """Normalize a decision-event ``variation_id`` (FR-003/FR-004).
+
+    Returns the original value if it is a valid numeric ID string. Otherwise
+    returns ``None`` so the event payload omits/clears the field for the
+    downstream consumer.
+    """
+    return variation_id if is_numeric_id_string(variation_id) else None
diff --git a/optimizely/event/payload.py b/optimizely/event/payload.py
@@ -71,9 +71,18 @@ def get_event_params(self) -> dict[str, Any]:
 class Decision:
     """ Class respresenting Decision. """
 
-    def __init__(self, campaign_id: str, experiment_id: str, variation_id: str, metadata: Metadata):
+    def __init__(
+        self,
+        campaign_id: str,
+        experiment_id: str,
+        variation_id: Optional[str],
+        metadata: Metadata,
+    ):
         self.campaign_id = campaign_id
         self.experiment_id = experiment_id
+        # FSSDK-12813: variation_id may be None when input is invalid /
+        # non-numeric (FR-003/FR-004). All other decision fields remain
+        # strings.
         self.variation_id = variation_id
         self.metadata = metadata
 
diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py
@@ -18,6 +18,7 @@
 from sys import version_info
 
 from . import version
+from .event import event_id_normalizer
 from .helpers import enums
 from .helpers import event_tag_utils
 from .helpers import validator
@@ -178,7 +179,7 @@ def _get_common_params(
 
     def _get_required_params_for_impression(
         self, experiment: Experiment, variation_id: str
-    ) -> dict[str, list[dict[str, str | int]]]:
+    ) -> dict[str, list[dict[str, Any]]]:
         """ Get parameters that are required for the impression event to register.
 
     Args:
@@ -188,19 +189,28 @@ def _get_required_params_for_impression(
     Returns:
       Dict consisting of decisions and events info for impression event.
     """
-        snapshot: dict[str, list[dict[str, str | int]]] = {}
+        snapshot: dict[str, list[dict[str, Any]]] = {}
+
+        # FSSDK-12813: Normalize decision-event IDs uniformly across all
+        # decision types. campaign_id falls back to experiment_id when
+        # invalid; variation_id becomes None when invalid; entity_id mirrors
+        # the normalized campaign_id (FR-009).
+        normalized_campaign_id = event_id_normalizer.normalize_campaign_id(
+            experiment.layerId, experiment.id
+        )
+        normalized_variation_id = event_id_normalizer.normalize_variation_id(variation_id)
 
         snapshot[self.EventParams.DECISIONS] = [
             {
                 self.EventParams.EXPERIMENT_ID: experiment.id,
-                self.EventParams.VARIATION_ID: variation_id,
-                self.EventParams.CAMPAIGN_ID: experiment.layerId,
+                self.EventParams.VARIATION_ID: normalized_variation_id,
+                self.EventParams.CAMPAIGN_ID: normalized_campaign_id,
             }
         ]
 
         snapshot[self.EventParams.EVENTS] = [
             {
-                self.EventParams.EVENT_ID: experiment.layerId,
+                self.EventParams.EVENT_ID: normalized_campaign_id,
                 self.EventParams.TIME: self._get_time(),
                 self.EventParams.KEY: 'campaign_activated',
                 self.EventParams.UUID: str(uuid.uuid4()),
diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py
@@ -1237,3 +1237,203 @@ def test_create_impression_event_without_cmab_uuid(self):
             EventFactory.HTTP_VERB,
             EventFactory.HTTP_HEADERS,
         )
+
+
+class EventFactoryIdNormalizationIntegrationTest(base.BaseTest):
+    """FSSDK-12813: end-to-end decision-event ID normalization.
+
+    These tests build real ``ImpressionEvent`` instances using crafted
+    Experiment/Variation objects, then call ``EventFactory.create_log_event``
+    and inspect the dispatched payload. They exercise FR-001..FR-009.
+    """
+
+    def setUp(self, *args, **kwargs):
+        base.BaseTest.setUp(self, 'config_dict_with_multiple_experiments')
+        self.logger = logger.NoOpLogger()
+
+    def _build_impression(
+        self,
+        experiment_id,
+        layer_id,
+        variation_id,
+        rule_type='experiment',
+    ):
+        """Build an ImpressionEvent with the provided raw ID values.
+
+        ``experiment_id``/``layer_id``/``variation_id`` are inserted verbatim
+        so tests can exercise empty/non-string/non-numeric inputs.
+        """
+        from optimizely.entities import Experiment, Variation
+        from optimizely.event.user_event import EventContext, ImpressionEvent
+
+        experiment = Experiment(
+            id=experiment_id,
+            key='exp_key',
+            status='Running',
+            audienceIds=[],
+            variations=[],
+            forcedVariations={},
+            trafficAllocation=[],
+            layerId=layer_id,
+        )
+        variation = Variation(
+            id=variation_id,
+            key='variation_key',
+            featureEnabled=True,
+        ) if isinstance(variation_id, str) else None
+
+        event_context = EventContext(
+            account_id='12001',
+            project_id='111001',
+            revision='42',
+            anonymize_ip=False,
+            region='US',
+        )
+        return ImpressionEvent(
+            event_context=event_context,
+            user_id='test_user',
+            experiment=experiment,
+            visitor_attributes=[],
+            variation=variation,
+            flag_key='flag_key',
+            rule_key='rule_key',
+            rule_type=rule_type,
+            enabled=True,
+        )
+
+    def _dispatched_decision(self, impression_event):
+        """Return (decision_dict, event_dict) for an impression event."""
+        log_event = EventFactory.create_log_event(impression_event, self.logger)
+        snapshot = log_event.params['visitors'][0]['snapshots'][0]
+        return snapshot['decisions'][0], snapshot['events'][0]
+
+    # ------------------------------------------------------------------ FR-001
+    def test_valid_campaign_id_is_passed_through(self):
+        impression = self._build_impression('111127', '111182', '111129')
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111182', decision['campaign_id'])
+        # FR-009: entity_id mirrors campaign_id byte-for-byte.
+        self.assertEqual(decision['campaign_id'], event['entity_id'])
+
+    # ------------------------------------------------------------------ FR-002
+    def test_empty_campaign_id_falls_back_to_experiment_id(self):
+        impression = self._build_impression('111127', '', '111129')
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    def test_non_numeric_campaign_id_falls_back_to_experiment_id(self):
+        impression = self._build_impression('111127', 'campaign_a', '111129')
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    def test_whitespace_campaign_id_falls_back_to_experiment_id(self):
+        impression = self._build_impression('111127', ' ', '111129')
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    # ------------------------------------------------------------------ FR-003
+    def test_valid_variation_id_is_passed_through(self):
+        impression = self._build_impression('111127', '111182', '111129')
+        decision, _ = self._dispatched_decision(impression)
+        self.assertEqual('111129', decision['variation_id'])
+
+    # ------------------------------------------------------------------ FR-004
+    def test_empty_variation_id_becomes_none(self):
+        impression = self._build_impression('111127', '111182', '')
+        decision, _ = self._dispatched_decision(impression)
+        self.assertIsNone(decision['variation_id'])
+
+    def test_non_numeric_variation_id_becomes_none(self):
+        impression = self._build_impression('111127', '111182', 'variation_a')
+        decision, _ = self._dispatched_decision(impression)
+        self.assertIsNone(decision['variation_id'])
+
+    def test_whitespace_variation_id_becomes_none(self):
+        impression = self._build_impression('111127', '111182', ' ')
+        decision, _ = self._dispatched_decision(impression)
+        self.assertIsNone(decision['variation_id'])
+
+    # ------------------------------------------------------------------ FR-005
+    def test_normalization_applies_to_rollout_decisions(self):
+        impression = self._build_impression(
+            '111127', 'bad_layer', 'bad_var', rule_type='rollout'
+        )
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertIsNone(decision['variation_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    def test_normalization_applies_to_feature_test_decisions(self):
+        impression = self._build_impression(
+            '111127', '', '', rule_type='feature-test'
+        )
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertIsNone(decision['variation_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    def test_normalization_applies_to_holdout_decisions(self):
+        impression = self._build_impression(
+            '111127', '', '', rule_type='holdout'
+        )
+        decision, event = self._dispatched_decision(impression)
+        self.assertEqual('111127', decision['campaign_id'])
+        self.assertIsNone(decision['variation_id'])
+        self.assertEqual('111127', event['entity_id'])
+
+    # ------------------------------------------------------------------ FR-006
+    def test_event_still_dispatches_when_all_ids_invalid(self):
+        """FR-006: never drop / fail dispatch."""
+        impression = self._build_impression('', '', '')
+        log_event = EventFactory.create_log_event(impression, self.logger)
+        self.assertIsNotNone(log_event)
+        decision, event = self._dispatched_decision(impression)
+        # campaign_id and entity_id end up as '' but the event still
+        # dispatches and the two fields remain byte-equivalent.
+        self.assertEqual('', decision['campaign_id'])
+        self.assertEqual('', event['entity_id'])
+        self.assertIsNone(decision['variation_id'])
+
+    # ------------------------------------------------------------------ FR-009
+    def test_entity_id_equals_campaign_id_byte_for_byte(self):
+        """FR-009: ``events[].entity_id`` must equal ``decisions[].campaign_id``."""
+        for layer_id, exp_id, expected in [
+            ('111182', '111127', '111182'),  # campaign_id wins
+            ('', '111127', '111127'),         # falls back to experiment_id
+            ('bad', '111127', '111127'),      # fallback on invalid
+            ('007', '111127', '007'),         # leading zeros preserved
+        ]:
+            with self.subTest(layer_id=layer_id, exp_id=exp_id):
+                impression = self._build_impression(exp_id, layer_id, '111129')
+                decision, event = self._dispatched_decision(impression)
+                self.assertEqual(expected, decision['campaign_id'])
+                self.assertEqual(decision['campaign_id'], event['entity_id'])
+
+    # ----------------------------------------------------------------- FR-010
+    def test_conversion_event_entity_id_unchanged(self):
+        """FR-010: conversion events derive entity_id from event.id, not the
+        normalizer.
+        """
+        from optimizely.event.user_event_factory import UserEventFactory
+
+        with mock.patch('time.time', return_value=42.123), mock.patch(
+            'uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'
+        ):
+            conversion_event = UserEventFactory.create_conversion_event(
+                self.project_config,
+                'test_event',
+                'test_user',
+                None,
+                None,
+            )
+        log_event = EventFactory.create_log_event(conversion_event, self.logger)
+        snapshot = log_event.params['visitors'][0]['snapshots'][0]
+        # Conversion entity_id comes from the event.id of the conversion event
+        # and must NOT pass through the campaign_id normalizer.
+        self.assertEqual(
+            self.project_config.get_event('test_event').id,
+            snapshot['events'][0]['entity_id'],
+        )
diff --git a/tests/test_event_id_normalizer.py b/tests/test_event_id_normalizer.py
