diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py index 5b76038ff..c9d52d234 100644 --- 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 index 14dd8b039..dba432d7e 100644 --- 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 index c02c419b9..e1ef357d5 100644 --- 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)', }