[FSSDK-12839] Normalize impression event campaign_id, variation_id, entity_id

Author: jaeopt

optimizely/event/event_factory.py

@@ -1,4 +1,4 @@
-# Copyright 2019, 2022, Optimizely
+# Copyright 2019, 2022, 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
@@ -134,12 +134,21 @@ def _create_visitor(cls, event: Optional[user_event.UserEvent], logger: Logger)
                 if isinstance(event.experiment, entities.Experiment):
                     experiment_layerId = event.experiment.layerId
 
+            campaign_id: str = (
+                experiment_layerId
+                if validator.is_numeric_string_id(experiment_layerId)
+                else experiment_id
+            )
+            normalized_variation_id: Optional[str] = (
+                variation_id if validator.is_numeric_string_id(variation_id) else None
+            )
+
             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(campaign_id, experiment_id, normalized_variation_id, metadata)
             snapshot_event = payload.SnapshotEvent(
-                experiment_layerId, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp,
+                campaign_id, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp,
             )
 
             snapshot = payload.Snapshot([snapshot_event], [decision])

optimizely/event/payload.py

@@ -1,4 +1,4 @@
-# Copyright 2019, 2022, Optimizely
+# Copyright 2019, 2022, 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
@@ -71,7 +71,13 @@ 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
         self.variation_id = variation_id

optimizely/event_builder.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2019, 2022, Optimizely
+# Copyright 2016-2019, 2022, 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
@@ -178,7 +178,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 +188,26 @@ 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]]] = {}
+        campaign_id: str = (
+            experiment.layerId if validator.is_numeric_string_id(experiment.layerId) else experiment.id
+        )
+        normalized_variation_id: Optional[str] = (
+            variation_id if validator.is_numeric_string_id(variation_id) else None
+        )
+
+        snapshot: dict[str, list[dict[str, Any]]] = {}
 
         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: campaign_id,
             }
         ]
 
         snapshot[self.EventParams.EVENTS] = [
             {
-                self.EventParams.EVENT_ID: experiment.layerId,
+                self.EventParams.EVENT_ID: campaign_id,
                 self.EventParams.TIME: self._get_time(),
                 self.EventParams.KEY: 'campaign_activated',
                 self.EventParams.UUID: str(uuid.uuid4()),

optimizely/helpers/validator.py

@@ -228,6 +228,27 @@ def is_non_empty_string(input_id_key: str) -> bool:
     return False
 
 
+def is_numeric_string_id(value: Any) -> bool:
+    """ Determine if value is a non-empty string consisting entirely of decimal digits.
+
+  Booleans are rejected even though they are technically instances of int in Python; the
+  field contract is "string of decimal digits", and a bool is neither a string nor a digit.
+
+  Args:
+    value: Variable which needs to be validated.
+
+  Returns:
+    True if value is a non-empty string of decimal digits [0-9]. False otherwise
+    (including for None, empty string, whitespace, non-string types, or strings
+    containing any non-digit character such as a sign, decimal point, or letter).
+  """
+    if not isinstance(value, str) or isinstance(value, bool):
+        return False
+    if not value:
+        return False
+    return value.isdigit() and value.isascii()
+
+
 def is_attribute_valid(attribute_key: str, attribute_value: Any) -> bool:
     """ Determine if given attribute is valid.
 

tests/test_event_builder.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2019, Optimizely
+# Copyright 2016-2019, 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
@@ -11,6 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import copy
 from unittest import mock
 import unittest
 from operator import itemgetter
@@ -1015,3 +1016,123 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel
             event_builder.EventBuilder.HTTP_VERB,
             event_builder.EventBuilder.HTTP_HEADERS,
         )
+
+
+class ImpressionEventIdNormalizationTest(base.BaseTest):
+    """Impression-event normalization rules for campaign_id, variation_id, and entity_id."""
+
+    def setUp(self, *args, **kwargs):
+        base.BaseTest.setUp(self, 'config_dict_with_multiple_experiments')
+        self.event_builder = self.optimizely.event_builder
+        self.experiment = self.project_config.get_experiment_from_key('test_experiment')
+
+    def _build_impression(self, experiment, variation_id):
+        return self.event_builder._get_required_params_for_impression(experiment, variation_id)
+
+    def _with_layer_id(self, layer_id):
+        experiment = copy.deepcopy(self.experiment)
+        experiment.layerId = layer_id
+        return experiment
+
+    def _decision(self, snapshot):
+        return snapshot[event_builder.EventBuilder.EventParams.DECISIONS][0]
+
+    def _event(self, snapshot):
+        return snapshot[event_builder.EventBuilder.EventParams.EVENTS][0]
+
+    # campaign_id normalization (US1) ----------------------------------------------
+
+    def test_campaign_id_valid_numeric_layer_id_passes_through(self):
+        experiment = self._with_layer_id('111182')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], '111182')
+
+    def test_campaign_id_empty_string_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id('')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+
+    def test_campaign_id_none_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id(None)
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+
+    def test_campaign_id_non_numeric_string_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id('abc')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+
+    def test_campaign_id_whitespace_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id('   ')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+
+    def test_campaign_id_integer_value_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id(111182)
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+
+    # variation_id normalization (US2) ---------------------------------------------
+
+    def test_variation_id_valid_numeric_passes_through(self):
+        snapshot = self._build_impression(self.experiment, '111129')
+        self.assertEqual(self._decision(snapshot)['variation_id'], '111129')
+
+    def test_variation_id_empty_string_becomes_none(self):
+        snapshot = self._build_impression(self.experiment, '')
+        self.assertIsNone(self._decision(snapshot)['variation_id'])
+
+    def test_variation_id_non_numeric_string_becomes_none(self):
+        snapshot = self._build_impression(self.experiment, 'variation_a')
+        self.assertIsNone(self._decision(snapshot)['variation_id'])
+
+    def test_variation_id_none_stays_none(self):
+        snapshot = self._build_impression(self.experiment, None)
+        self.assertIsNone(self._decision(snapshot)['variation_id'])
+
+    # entity_id normalization (US3) and US3 acceptance #5: byte-equality with campaign_id
+
+    def test_entity_id_valid_layer_id_passes_through(self):
+        experiment = self._with_layer_id('111182')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._event(snapshot)['entity_id'], '111182')
+
+    def test_entity_id_empty_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id('')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._event(snapshot)['entity_id'], experiment.id)
+
+    def test_entity_id_non_numeric_falls_back_to_experiment_id(self):
+        experiment = self._with_layer_id('abc')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(self._event(snapshot)['entity_id'], experiment.id)
+
+    def test_entity_id_equals_campaign_id_when_layer_invalid(self):
+        experiment = self._with_layer_id('')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(
+            self._event(snapshot)['entity_id'],
+            self._decision(snapshot)['campaign_id'],
+        )
+
+    def test_entity_id_equals_campaign_id_when_layer_valid(self):
+        experiment = self._with_layer_id('111182')
+        snapshot = self._build_impression(experiment, '111129')
+        self.assertEqual(
+            self._event(snapshot)['entity_id'],
+            self._decision(snapshot)['campaign_id'],
+        )
+
+    # Negative regression: conversion events are out of scope (FR-010).
+
+    def test_conversion_event_entity_id_uses_event_id_unchanged(self):
+        with mock.patch('time.time', return_value=42.123), mock.patch(
+            'uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'
+        ):
+            event_obj = self.event_builder.create_conversion_event(
+                self.project_config, 'test_event', 'test_user', None, None,
+            )
+
+        snapshot = event_obj.params['visitors'][0]['snapshots'][0]
+        event = snapshot['events'][0]
+        self.assertEqual(event['entity_id'], self.project_config.get_event('test_event').id)

tests/test_event_factory.py

@@ -1,4 +1,4 @@
-# Copyright 2019, Optimizely
+# Copyright 2019, 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
@@ -11,6 +11,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import copy
 from unittest import mock
 import time
 import unittest
@@ -1237,3 +1238,91 @@ def test_create_impression_event_without_cmab_uuid(self):
             EventFactory.HTTP_VERB,
             EventFactory.HTTP_HEADERS,
         )
+
+
+class ImpressionEventIdNormalizationFactoryTest(base.BaseTest):
+    """Impression-event normalization rules in EventFactory._create_visitor."""
+
+    def setUp(self, *args, **kwargs):
+        base.BaseTest.setUp(self, 'config_dict_with_multiple_experiments')
+        self.logger = logger.NoOpLogger()
+        self.experiment = self.project_config.get_experiment_from_key('test_experiment')
+
+    def _build_visitor(self, layer_id, variation_id):
+        from optimizely.event import user_event
+
+        experiment = copy.deepcopy(self.experiment)
+        experiment.layerId = layer_id
+
+        variation_payload = {'id': variation_id, 'key': 'whatever'} if variation_id is not None else None
+
+        ctx = user_event.EventContext(
+            self.project_config.get_account_id(),
+            self.project_config.get_project_id(),
+            self.project_config.get_revision(),
+            self.project_config.get_anonymize_ip_value(),
+            'US',
+        )
+        impression = user_event.ImpressionEvent(
+            event_context=ctx,
+            user_id='test_user',
+            experiment=experiment,
+            visitor_attributes=[],
+            variation=variation_payload,
+            flag_key='flag_a',
+            rule_key=experiment.key,
+            rule_type='experiment',
+            enabled=True,
+        )
+        visitor = EventFactory._create_visitor(impression, self.logger)
+        self.assertIsNotNone(visitor)
+        snapshot = visitor.snapshots[0]
+        decision = snapshot.decisions[0]
+        snapshot_event = snapshot.events[0]
+        return decision, snapshot_event, experiment
+
+    def test_campaign_id_valid_layer_id_passes_through(self):
+        decision, _event, _exp = self._build_visitor('111182', '111129')
+        self.assertEqual(decision.campaign_id, '111182')
+
+    def test_campaign_id_empty_layer_id_falls_back_to_experiment_id(self):
+        decision, _event, experiment = self._build_visitor('', '111129')
+        self.assertEqual(decision.campaign_id, experiment.id)
+
+    def test_campaign_id_non_numeric_layer_id_falls_back_to_experiment_id(self):
+        decision, _event, experiment = self._build_visitor('abc', '111129')
+        self.assertEqual(decision.campaign_id, experiment.id)
+
+    def test_variation_id_empty_becomes_none(self):
+        decision, _event, _exp = self._build_visitor('111182', '')
+        self.assertIsNone(decision.variation_id)
+
+    def test_variation_id_non_numeric_becomes_none(self):
+        decision, _event, _exp = self._build_visitor('111182', 'variation_a')
+        self.assertIsNone(decision.variation_id)
+
+    def test_variation_id_valid_passes_through(self):
+        decision, _event, _exp = self._build_visitor('111182', '111129')
+        self.assertEqual(decision.variation_id, '111129')
+
+    def test_entity_id_matches_campaign_id_when_layer_invalid(self):
+        decision, snapshot_event, _exp = self._build_visitor('', '111129')
+        self.assertEqual(snapshot_event.entity_id, decision.campaign_id)
+
+    def test_entity_id_matches_campaign_id_when_layer_valid(self):
+        decision, snapshot_event, _exp = self._build_visitor('111182', '111129')
+        self.assertEqual(snapshot_event.entity_id, decision.campaign_id)
+
+    def test_variation_id_serializes_to_json_null(self):
+        decision, _event, _exp = self._build_visitor('111182', 'not_numeric')
+        # The EventBatch serializer renders the variation_id field as JSON null,
+        # which is the wire contract this fix exists to honor.
+        from optimizely.event.payload import EventBatch, Snapshot, Visitor
+        batch = EventBatch('acc', 'proj', 'rev', 'python-sdk', '1.0', False, True)
+        snapshot = Snapshot([], [decision])
+        visitor = Visitor([snapshot], [], 'u')
+        batch.visitors = [visitor]
+        params = batch.get_event_params()
+        rendered = params['visitors'][0]['snapshots'][0]['decisions'][0]
+        self.assertIn('variation_id', rendered)
+        self.assertIsNone(rendered['variation_id'])