Skip to content

Commit

Permalink
Merge #2136
Browse files Browse the repository at this point in the history
2136: Add addon filter objects r=uhlissuh a=uhlissuh

fixes #2104 

So, splitting these into two filter objects came from Rehan, which I thought was a good idea. He mentioned seeing about having them share some code, because there is duplication here, and I kinda looked at that for a while and arrived at wondering if we will really gain much here trying to have these two share code? There's only two that are shaped like this right now. There isn't a clear thing I could pull out, maybe someone sees something. Perhaps that's an argument for making them one filter? 

Co-authored-by: Alissa Sobo <alissasobo@gmail.com>
  • Loading branch information
bors[bot] and uhlissuh committed Mar 30, 2020
2 parents fc6d717 + 40db750 commit 7b912b6
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 0 deletions.
2 changes: 2 additions & 0 deletions docs/user/filters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Filter Objects
.. autoclass:: WindowsBuildNumberFilter()
.. autoclass:: WindowsVersionFilter()
.. autoclass:: NegateFilter()
.. autoclass:: AddonActiveFilter()
.. autoclass:: AddonInstalledFilter()


Filter Expressions
Expand Down
86 changes: 86 additions & 0 deletions normandy/recipes/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -224,6 +246,68 @@ def capabilities(self):
return set()


class AddonActiveFilter(BaseAddonFilter):
"""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"

def get_formatted_string(self, addon):
return f'normandy.addons["{addon}"].isActive'

@property
def capabilities(self):
return set()


class AddonInstalledFilter(BaseAddonFilter):
"""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_installed"

def get_formatted_string(self, addon):
return f'normandy.addons["{addon}"]'

@property
def capabilities(self):
return set()


class PrefCompareFilter(BaseFilter):
"""Match based on a user's pref having a particular value.
Expand Down Expand Up @@ -751,6 +835,8 @@ def capabilities(self):
WindowsVersionFilter,
WindowsBuildNumberFilter,
NegateFilter,
AddonActiveFilter,
AddonInstalledFilter,
]
}

Expand Down
50 changes: 50 additions & 0 deletions normandy/recipes/tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
WindowsBuildNumberFilter,
WindowsVersionFilter,
NegateFilter,
AddonActiveFilter,
AddonInstalledFilter,
)
from normandy.recipes.tests import (
ChannelFactory,
Expand Down Expand Up @@ -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_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_all(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_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_active_all(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"
Expand Down

0 comments on commit 7b912b6

Please sign in to comment.