[FSSDK-12839] Relax campaign_id/entity_id to non-empty string per spec

Author: jaeopt

optimizely/event/event_factory.py

@@ -136,7 +136,7 @@ def _create_visitor(cls, event: Optional[user_event.UserEvent], logger: Logger)
 
             campaign_id: str = (
                 experiment_layerId
-                if validator.is_numeric_string_id(experiment_layerId)
+                if validator.is_non_empty_string(experiment_layerId)
                 else experiment_id
             )
             normalized_variation_id: Optional[str] = (

optimizely/event_builder.py

@@ -189,7 +189,7 @@ def _get_required_params_for_impression(
       Dict consisting of decisions and events info for impression event.
     """
         campaign_id: str = (
-            experiment.layerId if validator.is_numeric_string_id(experiment.layerId) else experiment.id
+            experiment.layerId if validator.is_non_empty_string(experiment.layerId) else experiment.id
         )
         normalized_variation_id: Optional[str] = (
             variation_id if validator.is_numeric_string_id(variation_id) else None

tests/test_event_builder.py

@@ -1057,17 +1057,24 @@ def test_campaign_id_none_falls_back_to_experiment_id(self):
         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')
+    def test_campaign_id_opaque_string_passes_through(self):
+        # Per FR-001: any non-empty string (numeric or opaque like "default-12345"
+        # or "layer_abc") is valid and passes through unchanged.
+        experiment = self._with_layer_id('default-12345')
         snapshot = self._build_impression(experiment, '111129')
-        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+        self.assertEqual(self._decision(snapshot)['campaign_id'], 'default-12345')
 
-    def test_campaign_id_whitespace_falls_back_to_experiment_id(self):
+    def test_campaign_id_whitespace_string_passes_through(self):
+        # Per FR-001 / spec Assumptions: a non-empty string of any character
+        # content is valid; whitespace-only strings are non-empty and accepted.
         experiment = self._with_layer_id('   ')
         snapshot = self._build_impression(experiment, '111129')
-        self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
+        self.assertEqual(self._decision(snapshot)['campaign_id'], '   ')
 
     def test_campaign_id_integer_value_falls_back_to_experiment_id(self):
+        # Non-string types are out of scope per spec Assumptions; the
+        # is_non_empty_string predicate returns False for them, so the
+        # fallback to experiment_id fires.
         experiment = self._with_layer_id(111182)
         snapshot = self._build_impression(experiment, '111129')
         self.assertEqual(self._decision(snapshot)['campaign_id'], experiment.id)
@@ -1102,10 +1109,13 @@ def test_entity_id_empty_falls_back_to_experiment_id(self):
         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')
+    def test_entity_id_opaque_layer_id_passes_through(self):
+        # Per FR-009: entity_id accepts any non-empty string (numeric or opaque
+        # like "layer_abc") and passes through unchanged. The fallback to
+        # experiment_id fires only when the value is empty, null, or missing.
+        experiment = self._with_layer_id('layer_abc')
         snapshot = self._build_impression(experiment, '111129')
-        self.assertEqual(self._event(snapshot)['entity_id'], experiment.id)
+        self.assertEqual(self._event(snapshot)['entity_id'], 'layer_abc')
 
     def test_entity_id_equals_campaign_id_when_layer_invalid(self):
         experiment = self._with_layer_id('')

tests/test_event_factory.py

@@ -1289,9 +1289,13 @@ 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_campaign_id_opaque_layer_id_passes_through(self):
+        # Per FR-001: any non-empty string (numeric or opaque like
+        # "default-12345" / "layer_abc") is valid and passes through unchanged.
+        decision, snapshot_event, _exp = self._build_visitor('layer_abc', '111129')
+        self.assertEqual(decision.campaign_id, 'layer_abc')
+        # FR-009: entity_id MUST stay byte-equal to campaign_id even for opaque IDs.
+        self.assertEqual(snapshot_event.entity_id, 'layer_abc')
 
     def test_variation_id_empty_becomes_none(self):
         decision, _event, _exp = self._build_visitor('111182', '')