Merge #2332

2332: version 100: first pass r=mythmon a=tiftran Co-authored-by: tiftran <ttran@mozilla.com>
mozilla · Oct 25, 2021 · b173d6e · b173d6e
2 parents 858b62e + d15396b
commit b173d6e
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 25 deletions.
diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py
@@ -621,26 +621,34 @@ class VersionFilter(BaseFilter):
     """Version's doc string"""
 
     def to_jexl(self, revision):
-        # This could be improved to generate more compact JEXL by noticing
-        # adjacent versions, and combining them into a single range. i.e. if
-        # `versions` is [55, 56, 57], this could generate
-        #
-        #   (normandy.version >= 55 && normandy.version < 58)
-        #
-        # instead of the current, more verbose
-        #
-        #   (normandy.version >= 55 && normandy.version < 56) ||
-        #   (normandy.version >= 56 && normandy.version < 57) ||
-        #   (normandy.version >= 57 && normandy.version < 58)
-
-        return "||".join(
-            f'(normandy.version>="{v}"&&normandy.version<"{v + 1}")'
-            for v in self.initial_data["versions"]
+        versions = self.initial_data["versions"]
+        versions.sort()
+        smallest_version = versions[0]
+        largest_version = versions[-1]
+
+        # check whether versions are non-continuous
+        if largest_version - smallest_version + 1 != len(versions):
+            result = [
+                "&&".join(
+                    [
+                        f'(env.version|versionCompare("{v}.!")>=0)',  # lowest v version
+                        f'(env.version|versionCompare("{v}.*")<0)',  # highest v version
+                    ]
+                )
+                for v in versions
+            ]
+
+            return "||".join(result)
+
+        return "&&".join(
+            [
+                f'(env.version|versionCompare("{smallest_version}.!")>=0)',
+                f'(env.version|versionCompare("{largest_version}.*")<0)',
+            ]
         )
 
     def get_capabilities(self):
-        # no special capabilities needed
-        return set()
+        return {"jexl.context.env.version", "jexl.transform.versionCompare"}
 
 
 class VersionRangeFilter(BaseFilter):

diff --git a/normandy/recipes/tests/api/v3/test_api.py b/normandy/recipes/tests/api/v3/test_api.py
@@ -1030,15 +1030,19 @@ def test_stable_sample_correct_fields(self, api_client):
 
         def test_version_works(self, api_client):
             res = self.make_recipe(
-                api_client, filter_object=[{"type": "version", "versions": [57, 58]}]
+                api_client, filter_object=[{"type": "version", "versions": [57, 58, 62]}]
             )
             assert res.status_code == 201, res.json()
             recipe_data = res.json()
 
             Recipe.objects.get(id=recipe_data["id"])
             assert recipe_data["latest_revision"]["filter_expression"] == (
-                '((normandy.version>="57"&&normandy.version<"58")||'
-                '(normandy.version>="58"&&normandy.version<"59")) && (true)'
+                '((env.version|versionCompare("57.!")>=0)&&'
+                '(env.version|versionCompare("57.*")<0)||'
+                '(env.version|versionCompare("58.!")>=0)&&'
+                '(env.version|versionCompare("58.*")<0)||'
+                '(env.version|versionCompare("62.!")>=0)&&'
+                '(env.version|versionCompare("62.*")<0)) && (true)'
             )
 
         def test_version_correct_fields(self, api_client):
@@ -2033,12 +2037,11 @@ def test_stable_sample_correct_fields(self, api_client):
 
     def test_version_works(self, api_client):
         res = self.make_recipe(
-            api_client, filter_object=[{"type": "version", "versions": [57, 58]}]
+            api_client, filter_object=[{"type": "version", "versions": [57, 58, 59, 60]}]
         )
         assert res.status_code == 201, res.json()
         assert res.json()["latest_revision"]["filter_expression"] == (
-            '(normandy.version>="57"&&normandy.version<"58")||'
-            '(normandy.version>="58"&&normandy.version<"59")'
+            '(env.version|versionCompare("57.!")>=0)&&' '(env.version|versionCompare("60.*")<0)'
         )
 
     def test_version_correct_fields(self, api_client):

diff --git a/normandy/recipes/tests/test_filters.py b/normandy/recipes/tests/test_filters.py
@@ -109,6 +109,8 @@ def test_throws_error_on_bad_date(self):
 
 
 class TestVersionFilter(FilterTestsBase):
+    should_be_baseline = False
+
     def create_basic_filter(self, versions=None):
         if versions is None:
             versions = [72, 74]
@@ -117,8 +119,8 @@ def create_basic_filter(self, versions=None):
     def test_generates_jexl(self):
         filter = self.create_basic_filter(versions=[72, 74])
         assert set(filter.to_jexl(self.create_revision()).split("||")) == {
-            '(normandy.version>="72"&&normandy.version<"73")',
-            '(normandy.version>="74"&&normandy.version<"75")',
+            '(env.version|versionCompare("72.!")>=0)&&(env.version|versionCompare("72.*")<0)',
+            '(env.version|versionCompare("74.!")>=0)&&(env.version|versionCompare("74.*")<0)',
         }