feat(nimbus): add validation to nimbus review serializer for targeting context fields within version range

experimenter/experimenter/experiments/api/v5/serializers.py

@@ -22,7 +22,7 @@
     NimbusConstants,
     TargetingMultipleKintoCollectionsError,
 )
-from experimenter.experiments.jexl_utils import JEXLParser
+from experimenter.experiments.jexl_utils import JEXLParser, collect_exprs
 from experimenter.experiments.models import (
     NimbusBranch,
     NimbusBranchFeatureValue,
@@ -34,6 +34,7 @@
     NimbusVersionedSchema,
 )
 from experimenter.features.manifests.nimbus_fml_loader import NimbusFmlLoader
+from experimenter.targeting.targeting_context_parser import TargetingContextFields
 
 logger = logging.getLogger()
 
@@ -1811,6 +1812,85 @@ def _validate_firefox_labs(self, data):
 
         return data
 
+    def _validate_targeting_configs(self, data):
+        targeting_config_slug = data.get("targeting_config_slug")
+        targeting_config = NimbusExperiment.TARGETING_CONFIGS[targeting_config_slug]
+        result = self.ValidateFeatureResult()
+
+        if not targeting_config.targeting:
+            return data
+
+        min_version = None
+        max_version = None
+        assume_unversioned = False
+        if not NimbusExperiment.Application.is_web(self.instance.application):
+            raw_min_version = data.get("firefox_min_version", "")
+            raw_max_version = data.get("firefox_max_version", "")
+
+            min_version = NimbusExperiment.Version.parse(raw_min_version)
+            if raw_max_version:
+                max_version = NimbusExperiment.Version.parse(raw_max_version)
+
+        if min_supported_version := NimbusConstants.MIN_VERSIONED_FEATURE_VERSION.get(
+            data.get("application")
+        ):
+            min_supported_version = NimbusExperiment.Version.parse(min_supported_version)
+
+            if min_supported_version > min_version:
+                if max_version is not None and min_supported_version > max_version:
+                    assume_unversioned = True
+                elif max_version is None or min_supported_version < max_version:
+                    min_version = min_supported_version
+
+        extracted_root_fields = set()
+        for subexpr in collect_exprs(targeting_config.targeting):
+            try:
+                json.loads(subexpr)
+                continue
+            except json.JSONDecodeError:
+                if subexpr.startswith("."):
+                    continue
+                extracted_root_fields.add(subexpr.partition(".")[0])
+
+        versions = (
+            NimbusFeatureVersion.objects.filter(
+                NimbusFeatureVersion.objects.between_versions_q(min_version, max_version)
+            )
+            .order_by("-major", "-minor", "-patch")
+            .distinct()
+        )
+
+        unsupported_versions = defaultdict(list)
+        for version in versions:
+            targeting_context_version = None if assume_unversioned else f"v{version}"
+            fields = set(
+                TargetingContextFields.for_application(
+                    data.get("application"), targeting_context_version
+                )
+            )
+
+            unknown_fields = extracted_root_fields - fields
+
+            for field in unknown_fields:
+                unsupported_versions[field].append(str(version))
+
+        for field, versions in unsupported_versions.items():
+            result.append(
+                NimbusConstants.ERROR_TARGETING_FIELD_UNSUPPORTED_IN_VERSIONS.format(
+                    field=field,
+                    versions=versions,
+                ),
+                data.get("warn_feature_schema", False),
+            )
+
+        if result.errors:
+            raise serializers.ValidationError({"targeting_config_slug": result.errors})
+
+        if result.warnings:
+            self.warnings["targeting_config_slug"] = result.warnings
+
+        return data
+
     def validate(self, data):
         application = data.get("application")
         channel = data.get("channel")
@@ -1836,6 +1916,7 @@ def validate(self, data):
         data = self._validate_feature_value_variables(data)
         data = self._validate_primary_secondary_outcomes(data)
         data = self._validate_firefox_labs(data)
+        data = self._validate_targeting_configs(data)
         if application == NimbusExperiment.Application.DESKTOP:
             data = self._validate_desktop_pref_rollouts(data)
             data = self._validate_desktop_pref_flips(data)

experimenter/experimenter/experiments/constants.py

@@ -1067,6 +1067,14 @@ class FirefoxLabsGroups(models.TextChoices):
         "In versions {versions}: Feature {feature_config} is not supported"
     )
 
+    ERROR_TARGETING_FIELD_UNSUPPORTED_IN_RANGE = (
+        "Targeting field {field} is not supported by any version in this range."
+    )
+
+    ERROR_TARGETING_FIELD_UNSUPPORTED_IN_VERSIONS = (
+        "In versions {versions}: Targeting field {field} is not supported"
+    )
+
     ERROR_FEATURE_VALUE_IN_VERSIONS = "In versions {versions}: {error}"
     WARNING_FEATURE_VALUE_IN_VERSIONS = "Warning: In versions {versions}: {warning}"
 

experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py

@@ -4404,6 +4404,139 @@ def test_fml_validate_feature_versioned_unbounded_range_valid(self):
         self.assertTrue(serializer.is_valid())
         self.assertEqual(serializer.errors, {})
 
+    @parameterized.expand(
+        [
+            (False, False),
+            (True, True),
+        ]
+    )
+    def test_validate_targeting_configs_unknown_field_is_error_or_warning(
+        self,
+        warn_feature_schema,
+        expected_valid,
+    ):
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            firefox_min_version=NimbusExperiment.Version.FIREFOX_120,
+            firefox_max_version=NimbusExperiment.Version.FIREFOX_121,
+            targeting_config_slug=NimbusExperiment.TargetingConfig.ATTRIBUTION_MEDIUM_EMAIL,
+            warn_feature_schema=warn_feature_schema,
+        )
+        serializer = NimbusReviewSerializer(
+            experiment,
+            data=NimbusReviewSerializer(experiment, context={"user": self.user}).data,
+            context={"user": self.user},
+        )
+
+        with (
+            patch(
+                "experimenter.experiments.api.v5.serializers.collect_exprs",
+                return_value=["knownField", "unknownField", ".id"],
+            ),
+            patch(
+                "experimenter.experiments.api.v5.serializers.TargetingContextFields.for_application",
+                return_value={"knownField"},
+            ),
+        ):
+            self.assertEqual(serializer.is_valid(), expected_valid, serializer.errors)
+
+        if expected_valid:
+            self.assertEqual(len(serializer.warnings["targeting_config_slug"]), 1)
+            self.assertIn("unknownField", serializer.warnings["targeting_config_slug"][0])
+        else:
+            self.assertEqual(len(serializer.errors["targeting_config_slug"]), 1)
+            self.assertIn("unknownField", serializer.errors["targeting_config_slug"][0])
+
+    @parameterized.expand(
+        [
+            # min == max
+            (NimbusExperiment.Version.FIREFOX_120, NimbusExperiment.Version.FIREFOX_120),
+            (NimbusExperiment.Version.FIREFOX_121, NimbusExperiment.Version.FIREFOX_121),
+            (NimbusExperiment.Version.FIREFOX_122, NimbusExperiment.Version.FIREFOX_122),
+            # min <= max (bounded)
+            (NimbusExperiment.Version.FIREFOX_120, NimbusExperiment.Version.FIREFOX_121),
+            (NimbusExperiment.Version.FIREFOX_120, NimbusExperiment.Version.FIREFOX_122),
+            (NimbusExperiment.Version.FIREFOX_121, NimbusExperiment.Version.FIREFOX_122),
+            # min <= max (unbounded)
+            (NimbusExperiment.Version.FIREFOX_120, NimbusExperiment.Version.NO_VERSION),
+            (NimbusExperiment.Version.FIREFOX_121, NimbusExperiment.Version.NO_VERSION),
+            (NimbusExperiment.Version.FIREFOX_122, NimbusExperiment.Version.NO_VERSION),
+        ]
+    )
+    def test_validate_targeting_configs_valid_version_ranges(
+        self,
+        min_version,
+        max_version,
+    ):
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            firefox_min_version=min_version,
+            firefox_max_version=max_version,
+            targeting_config_slug=NimbusExperiment.TargetingConfig.ATTRIBUTION_MEDIUM_EMAIL,
+        )
+        serializer = NimbusReviewSerializer(
+            experiment,
+            data=NimbusReviewSerializer(experiment, context={"user": self.user}).data,
+            context={"user": self.user},
+        )
+
+        def fields_for_version(_app, version=None):
+            fields_by_version = {
+                "v120.0.0": {"knownField"},
+                "v121.0.0": {"knownField", "anotherKnownField"},
+                "v122.0.0": {"knownField", "anotherKnownField", "thirdKnownField"},
+                "v123.0.0": {"knownField"},
+            }
+            return fields_by_version.get(version, {"knownField"})
+
+        with (
+            patch(
+                "experimenter.experiments.api.v5.serializers.collect_exprs",
+                return_value=["knownField"],
+            ),
+            patch(
+                "experimenter.experiments.api.v5.serializers.TargetingContextFields.for_application",
+                side_effect=fields_for_version,
+            ),
+        ):
+            self.assertTrue(serializer.is_valid(), serializer.errors)
+
+        self.assertEqual(serializer.warnings, {})
+
+    def test_validate_targeting_configs_assume_unversioned_uses_unversioned_context(self):
+        experiment = NimbusExperimentFactory.create_with_lifecycle(
+            NimbusExperimentFactory.Lifecycles.CREATED,
+            application=NimbusExperiment.Application.DESKTOP,
+            firefox_min_version=NimbusExperiment.Version.FIREFOX_119,
+            firefox_max_version=NimbusExperiment.Version.FIREFOX_119,
+            targeting_config_slug=NimbusExperiment.TargetingConfig.ATTRIBUTION_MEDIUM_EMAIL,
+        )
+        serializer = NimbusReviewSerializer(
+            experiment,
+            data=NimbusReviewSerializer(experiment, context={"user": self.user}).data,
+            context={"user": self.user},
+        )
+
+        def fields_for_version(_app, version=None):
+            self.assertIsNone(version)
+            return {"knownField"}
+
+        with (
+            patch(
+                "experimenter.experiments.api.v5.serializers.collect_exprs",
+                return_value=["knownField"],
+            ),
+            patch(
+                "experimenter.experiments.api.v5.serializers.TargetingContextFields.for_application",
+                side_effect=fields_for_version,
+            ),
+        ):
+            self.assertTrue(serializer.is_valid(), serializer.errors)
+
+        self.assertEqual(serializer.warnings, {})
+
 
 class TestNimbusReviewSerializerMultiFeature(MockFmlErrorMixin, TestCase):
     def setUp(self):