From 81963572279ee30e2230591f174577110f17c430 Mon Sep 17 00:00:00 2001 From: Alissa Sobo Date: Tue, 10 Mar 2020 13:32:33 -0700 Subject: [PATCH 1/4] Add addon filter objects --- docs/user/filters.rst | 2 + normandy/recipes/filters.py | 92 ++++++++++++++++++++++++++ normandy/recipes/tests/test_filters.py | 50 ++++++++++++++ 3 files changed, 144 insertions(+) diff --git a/docs/user/filters.rst b/docs/user/filters.rst index ab4438070..1b86fed08 100644 --- a/docs/user/filters.rst +++ b/docs/user/filters.rst @@ -32,6 +32,8 @@ Filter Objects .. autoclass:: WindowsBuildNumberFilter() .. autoclass:: WindowsVersionFilter() .. autoclass:: NegateFilter() +.. autoclass:: AddonActiveFilter() +.. autoclass:: AddonInstalledFilter() Filter Expressions diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py index d223eb333..ee04cbe09 100644 --- a/normandy/recipes/filters.py +++ b/normandy/recipes/filters.py @@ -224,6 +224,98 @@ def capabilities(self): return set() +class AddonActiveFilter(BaseFilter): + """Match a user based on if a particular addon is active. + + .. attribute:: type + + ``addon_active`` + + .. attribute:: addons + List of addon ids to filter against. + + :example: ``["uBlock0@raymondhill.net", "pioneer-opt-in@mozilla.org"]`` + + .. attribute:: any_or_all + This will determine whether the addons are connected with an "&&" operator, + meaning all the addons must be active for the filter to evaluate to true, + or an "||" operator, meaning any of the addons can be active to evaluate to + true. + + :example: ``any`` or ``all`` + """ + + type = "addon_active" + addons = serializers.ListField(child=serializers.CharField(), min_length=1) + any_or_all = serializers.CharField() + + def to_jexl(self): + any_or_all = self.initial_data["any_or_all"] + + if any_or_all == "all": + return "&&".join( + (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) + ) + elif any_or_all == "any": + return "||".join( + (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) + ) + else: + raise serializers.ValidationError( + f"Unrecognized string for any_or_all: {any_or_all!r}" + ) + + @property + def capabilities(self): + return set() + + +class AddonInstalledFilter(BaseFilter): + """Match a user based on if a particular addon is installed. + + .. attribute:: type + + ``addon_installed`` + + .. attribute:: addons + List of addon ids to filter against. + + :example: ``["uBlock0@raymondhill.net", "pioneer-opt-in@mozilla.org"]`` + + .. attribute:: any_or_all + This will determine whether the addons are connected with an "&&" operator, + meaning all the addons must be installed for the filter to evaluate to true, + or an "||" operator, meaning any of the addons can be installed to + evaluate to true. + + :example: ``any`` or ``all`` + """ + + type = "addon_active" + addons = serializers.ListField(child=serializers.CharField(), min_length=1) + any_or_all = serializers.CharField() + + def to_jexl(self): + any_or_all = self.initial_data["any_or_all"] + + if any_or_all == "all": + return "&&".join( + (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) + ) + elif any_or_all == "any": + return "||".join( + (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) + ) + else: + raise serializers.ValidationError( + f"Unrecognized string for any_or_all: {any_or_all!r}" + ) + + @property + def capabilities(self): + return set() + + class PrefCompareFilter(BaseFilter): """Match based on a user's pref having a particular value. diff --git a/normandy/recipes/tests/test_filters.py b/normandy/recipes/tests/test_filters.py index 196d6cebd..f62f51883 100644 --- a/normandy/recipes/tests/test_filters.py +++ b/normandy/recipes/tests/test_filters.py @@ -18,6 +18,8 @@ WindowsBuildNumberFilter, WindowsVersionFilter, NegateFilter, + AddonActiveFilter, + AddonInstalledFilter, ) from normandy.recipes.tests import ( ChannelFactory, @@ -256,6 +258,54 @@ def test_generates_jexl(self): assert negate_filter.to_jexl() == '!(normandy.channel in ["release","beta"])' +class TestAddonInstalledFilter(FilterTestsBase): + def create_basic_filter(self, addons=["@abcdef", "ghijk@lmnop"], any_or_all="any"): + return AddonInstalledFilter.create(addons=addons, any_or_all=any_or_all) + + def test_generates_jexl_installed_not_active(self): + filter = self.create_basic_filter() + assert set(filter.to_jexl().split("||")) == { + 'normandy.addons["@abcdef"]', + 'normandy.addons["ghijk@lmnop"]', + } + + def test_generates_jexl_installed_active(self): + filter = self.create_basic_filter(any_or_all="all") + assert set(filter.to_jexl().split("&&")) == { + 'normandy.addons["@abcdef"]', + 'normandy.addons["ghijk@lmnop"]', + } + + def test_throws_error_on_bad_any_or_all(self): + filter = self.create_basic_filter(any_or_all="error") + with pytest.raises(serializers.ValidationError): + filter.to_jexl() + + +class TestAddonActiveFilter(FilterTestsBase): + def create_basic_filter(self, addons=["@abcdef", "ghijk@lmnop"], any_or_all="any"): + return AddonActiveFilter.create(addons=addons, any_or_all=any_or_all) + + def test_generates_jexl_installed_not_active(self): + filter = self.create_basic_filter() + assert set(filter.to_jexl().split("||")) == { + 'normandy.addons["@abcdef"].isActive', + 'normandy.addons["ghijk@lmnop"].isActive', + } + + def test_generates_jexl_installed_active(self): + filter = self.create_basic_filter(any_or_all="all") + assert set(filter.to_jexl().split("&&")) == { + 'normandy.addons["@abcdef"].isActive', + 'normandy.addons["ghijk@lmnop"].isActive', + } + + def test_throws_error_on_bad_any_or_all(self): + filter = self.create_basic_filter(any_or_all="error") + with pytest.raises(serializers.ValidationError): + filter.to_jexl() + + class TestPrefCompareFilter(FilterTestsBase): def create_basic_filter( self, pref="browser.urlbar.maxRichResults", value=10, comparison="equal" From 3f3cf68717e23ec6c1f5ef59e82f36f80f3e9ff1 Mon Sep 17 00:00:00 2001 From: Alissa Sobo Date: Tue, 10 Mar 2020 15:54:55 -0700 Subject: [PATCH 2/4] refactor with 'symbol' --- normandy/recipes/filters.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py index ee04cbe09..759dd7fc1 100644 --- a/normandy/recipes/filters.py +++ b/normandy/recipes/filters.py @@ -253,18 +253,18 @@ def to_jexl(self): any_or_all = self.initial_data["any_or_all"] if any_or_all == "all": - return "&&".join( - (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) - ) + symbol = "&&" elif any_or_all == "any": - return "||".join( - (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) - ) + symbol = "||" else: raise serializers.ValidationError( f"Unrecognized string for any_or_all: {any_or_all!r}" ) + return symbol.join( + (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) + ) + @property def capabilities(self): return set() @@ -299,18 +299,18 @@ def to_jexl(self): any_or_all = self.initial_data["any_or_all"] if any_or_all == "all": - return "&&".join( - (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) - ) + symbol = "&&" elif any_or_all == "any": - return "||".join( - (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) - ) + symbol = "||" else: raise serializers.ValidationError( f"Unrecognized string for any_or_all: {any_or_all!r}" ) + return symbol.join( + (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) + ) + @property def capabilities(self): return set() From 685a179fef87236bf3e943129d956d3368ae395e Mon Sep 17 00:00:00 2001 From: Alissa Sobo Date: Tue, 10 Mar 2020 21:56:34 -0700 Subject: [PATCH 3/4] Add BaseAddonFilter --- normandy/recipes/filters.py | 66 ++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py index 759dd7fc1..81d57c4de 100644 --- a/normandy/recipes/filters.py +++ b/normandy/recipes/filters.py @@ -49,6 +49,28 @@ def to_jexl(self): raise NotImplementedError +class BaseAddonFilter(BaseFilter): + addons = serializers.ListField(child=serializers.CharField(), min_length=1) + any_or_all = serializers.CharField() + + def get_formatted_string(self, addon): + raise NotImplementedError("Not correctly implemented.") + + def to_jexl(self): + any_or_all = self.initial_data["any_or_all"] + + symbol = {"all": "&&", "any": "||"}.get(any_or_all) + + if not symbol: + raise serializers.ValidationError( + f"Unrecognized string for any_or_all: {any_or_all!r}" + ) + + return symbol.join( + self.get_formatted_string(addon) for addon in self.initial_data["addons"] + ) + + class BaseComparisonFilter(BaseFilter): value = serializers.IntegerField() comparison = serializers.CharField() @@ -224,7 +246,7 @@ def capabilities(self): return set() -class AddonActiveFilter(BaseFilter): +class AddonActiveFilter(BaseAddonFilter): """Match a user based on if a particular addon is active. .. attribute:: type @@ -246,31 +268,16 @@ class AddonActiveFilter(BaseFilter): """ type = "addon_active" - addons = serializers.ListField(child=serializers.CharField(), min_length=1) - any_or_all = serializers.CharField() - - def to_jexl(self): - any_or_all = self.initial_data["any_or_all"] - if any_or_all == "all": - symbol = "&&" - elif any_or_all == "any": - symbol = "||" - else: - raise serializers.ValidationError( - f"Unrecognized string for any_or_all: {any_or_all!r}" - ) - - return symbol.join( - (f'normandy.addons["{addon}"].isActive' for addon in self.initial_data["addons"]) - ) + def get_formatted_string(self, addon): + return f'normandy.addons["{addon}"].isActive' @property def capabilities(self): return set() -class AddonInstalledFilter(BaseFilter): +class AddonInstalledFilter(BaseAddonFilter): """Match a user based on if a particular addon is installed. .. attribute:: type @@ -291,25 +298,10 @@ class AddonInstalledFilter(BaseFilter): :example: ``any`` or ``all`` """ - type = "addon_active" - addons = serializers.ListField(child=serializers.CharField(), min_length=1) - any_or_all = serializers.CharField() + type = "addon_installed" - def to_jexl(self): - any_or_all = self.initial_data["any_or_all"] - - if any_or_all == "all": - symbol = "&&" - elif any_or_all == "any": - symbol = "||" - else: - raise serializers.ValidationError( - f"Unrecognized string for any_or_all: {any_or_all!r}" - ) - - return symbol.join( - (f'normandy.addons["{addon}"]' for addon in self.initial_data["addons"]) - ) + def get_formatted_string(self, addon): + return f'normandy.addons["{addon}"]' @property def capabilities(self): From 40db750f067b67b63dbaf4d2f1ec3350e1f14ea7 Mon Sep 17 00:00:00 2001 From: Alissa Sobo Date: Mon, 30 Mar 2020 12:12:28 -0700 Subject: [PATCH 4/4] Update test names and add to by_type --- normandy/recipes/filters.py | 2 ++ normandy/recipes/tests/test_filters.py | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/normandy/recipes/filters.py b/normandy/recipes/filters.py index 81d57c4de..170b201b7 100644 --- a/normandy/recipes/filters.py +++ b/normandy/recipes/filters.py @@ -835,6 +835,8 @@ def capabilities(self): WindowsVersionFilter, WindowsBuildNumberFilter, NegateFilter, + AddonActiveFilter, + AddonInstalledFilter, ] } diff --git a/normandy/recipes/tests/test_filters.py b/normandy/recipes/tests/test_filters.py index f62f51883..defe9b3e8 100644 --- a/normandy/recipes/tests/test_filters.py +++ b/normandy/recipes/tests/test_filters.py @@ -262,14 +262,14 @@ class TestAddonInstalledFilter(FilterTestsBase): def create_basic_filter(self, addons=["@abcdef", "ghijk@lmnop"], any_or_all="any"): return AddonInstalledFilter.create(addons=addons, any_or_all=any_or_all) - def test_generates_jexl_installed_not_active(self): + def test_generates_jexl_installed_any(self): filter = self.create_basic_filter() assert set(filter.to_jexl().split("||")) == { 'normandy.addons["@abcdef"]', 'normandy.addons["ghijk@lmnop"]', } - def test_generates_jexl_installed_active(self): + def test_generates_jexl_installed_all(self): filter = self.create_basic_filter(any_or_all="all") assert set(filter.to_jexl().split("&&")) == { 'normandy.addons["@abcdef"]', @@ -286,14 +286,14 @@ class TestAddonActiveFilter(FilterTestsBase): def create_basic_filter(self, addons=["@abcdef", "ghijk@lmnop"], any_or_all="any"): return AddonActiveFilter.create(addons=addons, any_or_all=any_or_all) - def test_generates_jexl_installed_not_active(self): + def test_generates_jexl_active_any(self): filter = self.create_basic_filter() assert set(filter.to_jexl().split("||")) == { 'normandy.addons["@abcdef"].isActive', 'normandy.addons["ghijk@lmnop"].isActive', } - def test_generates_jexl_installed_active(self): + def test_generates_jexl_active_all(self): filter = self.create_basic_filter(any_or_all="all") assert set(filter.to_jexl().split("&&")) == { 'normandy.addons["@abcdef"].isActive',