Raise error when specifying compatibility inside forbidden Android range in the API (#21198)

* Raise error when specifying compatibility inside forbidden Android range in the API

Only do it when a compatibility object is specified with a min and/or max.

src/olympia/addons/fields.py

@@ -31,6 +31,7 @@ from olympia.versions.models import (
     VALID_SOURCE_EXTENSIONS,
     ApplicationsVersions,
     License,
+    Version,
 )
 
 
@@ -251,6 +252,7 @@ class VersionCompatibilityField(serializers.Field):
             raise exceptions.ValidationError(gettext('Invalid value'))
 
         version = self.parent.instance
+        addon = self.parent.addon
         existing = version.compatible_apps if version else {}
         internal = {}
         for app_name, min_max in data.items():
@@ -262,17 +264,15 @@ class VersionCompatibilityField(serializers.Field):
             existing_app = existing.get(app)
             # we need to copy() to avoid changing the instance before save
             apps_versions = copy.copy(existing_app) or ApplicationsVersions(
-                application=app.id
+                application=app.id, version=version or Version(addon=addon)
             )
-
             app_version_qs = AppVersion.objects.filter(application=app.id)
             try:
                 if 'max' in min_max:
                     apps_versions.max = app_version_qs.get(version=min_max['max'])
                 elif version:
-                    apps_versions.max = app_version_qs.get(
-                        version=amo.DEFAULT_WEBEXT_MAX_VERSION
-                    )
+                    apps_versions.max = apps_versions.get_default_maximum_appversion()
+
             except AppVersion.DoesNotExist:
                 raise exceptions.ValidationError(
                     gettext('Unknown max app version specified')
@@ -283,14 +283,33 @@ class VersionCompatibilityField(serializers.Field):
                 if 'min' in min_max:
                     apps_versions.min = app_version_qs.get(version=min_max['min'])
                 elif version:
-                    apps_versions.min = app_version_qs.get(
-                        version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[app]
-                    )
+                    apps_versions.min = apps_versions.get_default_minimum_appversion()
             except AppVersion.DoesNotExist:
                 raise exceptions.ValidationError(
                     gettext('Unknown min app version specified')
                 )
 
+            # We want to validate whether or not the android range contains
+            # forbidden versions. At this point the ApplicationsVersions
+            # instance might be partial and completed by Version.from_upload()
+            # which looks at the manifest. We can't do that here, so if either
+            # the min or max provided would lead to a problematic range on its
+            # own, we force the developer to explicitly set both, even though
+            # what's in the manifest could resulted in a valid range.
+            if (
+                ('min' in min_max or 'max' in min_max)
+                and apps_versions.application == amo.ANDROID.id
+                and apps_versions.version_range_contains_forbidden_compatibility()
+            ):
+                valid_range = ApplicationsVersions.ANDROID_LIMITED_COMPATIBILITY_RANGE
+                raise exceptions.ValidationError(
+                    gettext(
+                        'Invalid version range. For Firefox for Android, you may only '
+                        'pick a range that starts with version %(max)s or higher, '
+                        'or ends with lower than version %(min)s.'
+                    )
+                    % {'min': valid_range[0], 'max': valid_range[1]}
+                )
             if existing_app and existing_app.locked_from_manifest:
                 if (
                     existing_app.min != apps_versions.min

src/olympia/addons/tests/test_views.py

@@ -3146,6 +3146,38 @@ class VersionViewSetCreateUpdateMixin(RequestMixin):
         assert response.status_code == 400, response.content
         assert response.data == {'compatibility': ['Unknown min app version specified']}
 
+    def test_compatibility_forbidden_range_android(self):
+        response = self.request(compatibility={'android': {'min': '48.0', 'max': '*'}})
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'compatibility': [
+                'Invalid version range. For Firefox for Android, you may only pick a '
+                'range that starts with version 119.0a1 or higher, or ends with lower '
+                'than version 79.0a1.'
+            ]
+        }
+
+        # Recommended add-ons for Android don't have that restriction.
+        self.make_addon_promoted(self.addon, RECOMMENDED, approve_version=True)
+        response = self.request(compatibility={'android': {'min': '48.0', 'max': '*'}})
+        assert response.status_code == self.SUCCESS_STATUS_CODE, response.content
+
+    def test_compatibility_forbidden_range_android_only_min_specified(self):
+        response = self.request(compatibility={'android': {'min': '48.0'}})
+        assert response.status_code == 400, response.content
+        assert response.data == {
+            'compatibility': [
+                'Invalid version range. For Firefox for Android, you may only pick a '
+                'range that starts with version 119.0a1 or higher, or ends with lower '
+                'than version 79.0a1.'
+            ]
+        }
+
+        # Recommended add-ons for Android don't have that restriction.
+        self.make_addon_promoted(self.addon, RECOMMENDED, approve_version=True)
+        response = self.request(compatibility={'android': {'min': '48.0'}})
+        assert response.status_code == self.SUCCESS_STATUS_CODE, response.content
+
     @staticmethod
     def _parse_xpi_mock(pkg, addon, minimal, user):
         return {**parse_xpi(pkg, addon, minimal, user), 'type': addon.type}
@@ -4266,7 +4298,7 @@ class TestVersionViewSetUpdate(UploadMixin, VersionViewSetCreateUpdateMixin, Tes
         response = self.request(
             compatibility={
                 'firefox': {'min': amo.DEFAULT_WEBEXT_MIN_VERSION},
-                'android': {'min': amo.DEFAULT_WEBEXT_MIN_VERSION_GECKO_ANDROID},
+                'android': {'min': amo.MIN_VERSION_FENIX_GENERAL_AVAILABILITY},
             }
         )
         assert response.status_code == 400, response.content

src/olympia/versions/models.py

@@ -1416,6 +1416,16 @@ class ApplicationsVersions(models.Model):
             .first()
         )
 
+    def get_default_minimum_appversion(self):
+        return AppVersion.objects.filter(application=self.application).get(
+            version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[amo.APPS_ALL[self.application]]
+        )
+
+    def get_default_maximum_appversion(self):
+        return AppVersion.objects.filter(application=self.application).get(
+            version=amo.DEFAULT_WEBEXT_MAX_VERSION
+        )
+
     @property
     def locked_from_manifest(self):
         """Whether the manifest is the source of truth for this ApplicationsVersions or
@@ -1468,9 +1478,16 @@ class ApplicationsVersions(models.Model):
         limited_range_start = version_int(self.ANDROID_LIMITED_COMPATIBILITY_RANGE[0])
         limited_range_end = version_int(self.ANDROID_LIMITED_COMPATIBILITY_RANGE[1])
 
+        # Min or max could be absent if it's a partial instance we'll be
+        # completing later. Use default values in that case (increases the
+        # chance that the range would be considered forbbiden, forcing caller
+        # to provide a complete instance).
+        min_ = getattr(self, 'min', None) or self.get_default_minimum_appversion()
+        max_ = getattr(self, 'max', None) or self.get_default_maximum_appversion()
+
         return (
-            self.min.version_int < limited_range_end
-            and self.max.version_int >= limited_range_start
+            min_.version_int < limited_range_end
+            and max_.version_int >= limited_range_start
         )
 
     def save(self, *args, **kwargs):

src/olympia/versions/tests/test_models.py

@@ -3052,6 +3052,57 @@ class TestApplicationsVersionsVersionRangeContainsForbiddenCompatibility(TestCas
         self.assert_min_and_max_are_set_to_fenix_ga_on_save(avs)
         assert not avs.version.file.reload().strict_compatibility
 
+    def test_get_default_minimum_appversion(self):
+        assert ApplicationsVersions(
+            application=amo.FIREFOX.id
+        ).get_default_minimum_appversion() == AppVersion.objects.get(
+            application=amo.FIREFOX.id,
+            version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[amo.FIREFOX],
+        )
+
+        assert ApplicationsVersions(
+            application=amo.ANDROID.id
+        ).get_default_minimum_appversion() == AppVersion.objects.get(
+            application=amo.ANDROID.id,
+            version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[amo.ANDROID],
+        )
+
+    def get_default_maximum_appversion(self):
+        self.star_firefox_appversion = AppVersion.objects.filter(
+            application=amo.FIREFOX.id, version=amo.DEFAULT_WEBEXT_MAX_VERSION
+        )
+        assert (
+            ApplicationsVersions(
+                application=amo.FIREFOX.id
+            ).get_default_maximum_appversion()
+            == self.star_firefox_appversion
+        )
+
+        assert (
+            ApplicationsVersions(
+                application=amo.ANDROID.id
+            ).get_default_maximum_appversion()
+            == self.star_android_appversion
+        )
+
+    def test_min_not_set_fallback_to_default(self):
+        addon = addon_factory()
+        avs = ApplicationsVersions(
+            application=amo.ANDROID.id,
+            version=addon.current_version,
+            max=self.fenix_appversion_star,
+        )
+        assert avs.version_range_contains_forbidden_compatibility()
+
+    def test_max_not_set_fallback_to_default(self):
+        addon = addon_factory()
+        avs = ApplicationsVersions(
+            application=amo.ANDROID.id,
+            version=addon.current_version,
+            min=self.fennec_appversion,
+        )
+        assert avs.version_range_contains_forbidden_compatibility()
+
 
 class TestVersionPreview(BasePreviewMixin, TestCase):
     def get_object(self):