wip

normandy/classifier/models.py

@@ -2,23 +2,24 @@
 
 import uuid
 
-from django.utils.functional import cached_property
-
 from normandy.classifier.geolocation import get_country_code
 from normandy.recipes.models import Recipe
 
 
 class Client(object):
     """A client attempting to fetch a set of recipes."""
-    def __init__(self, request, locale=None, release_channel=None):
+    def __init__(self, request, locale=None, release_channel=None, country=None):
         self.request = request
         self.locale = locale
         self.release_channel = release_channel
+        self._country = country
 
-    @cached_property
+    @property
     def country(self):
-        ip_address = self.request.META.get('REMOTE_ADDR')
-        return get_country_code(ip_address)
+        if not self._country:
+            ip_address = self.request.META.get('REMOTE_ADDR')
+            self._country = get_country_code(ip_address)
+        return self._country
 
     @property
     def request_time(self):

normandy/classifier/tests/test_api.py

@@ -2,7 +2,7 @@
 from rest_framework.reverse import reverse
 
 from normandy.base.tests import Whatever
-from normandy.recipes.tests import RecipeActionFactory, RecipeFactory
+from normandy.recipes.tests import RecipeActionFactory, RecipeFactory, LocaleFactory
 
 
 @pytest.mark.django_db
@@ -38,13 +38,17 @@ def test_it_serves_recipes(self, client):
         }]
 
     def test_it_filters_by_locale(self, client):
-        RecipeFactory(name='english', enabled=True, locale='en-US')
-        RecipeFactory(name='german', enabled=True, locale='de')
-        RecipeFactory(name='any', enabled=True, locale='')
-        RecipeFactory(name='disabled', enabled=False, locale='de')
+        english = LocaleFactory(code='en-US')
+        german = LocaleFactory(code='de')
+
+        RecipeFactory(name='english', enabled=True, locales=[english])
+        RecipeFactory(name='german', enabled=True, locales=[german])
+        RecipeFactory(name='any', enabled=True, locales=[])
+        RecipeFactory(name='both', enabled=True, locales=[english, german])
+        RecipeFactory(name='disabled', enabled=False, locales=[german])
 
         res = client.post('/api/v1/fetch_bundle/', {'locale': 'de'})
         assert res.status_code == 200
 
         recipe_names = set(r['name'] for r in res.data['recipes'])
-        assert recipe_names == {'german', 'any'}
+        assert recipe_names == {'german', 'any', 'both'}

normandy/recipes/models.py

@@ -20,14 +20,17 @@ class Locale(models.Model):
     code = models.CharField(max_length=255, unique=True)
     english_name = models.CharField(max_length=255, blank=True)
     native_name = models.CharField(max_length=255, blank=True)
-    order = models.IntegerField()
+    order = models.IntegerField(default=100)
 
     class Meta:
         ordering = ['order', 'code']
 
     def __str__(self):
         return '{self.code} ({self.english_name})'.format(self=self)
 
+    def matches(self, other):
+        return self.code.lower() == other.lower()
+
 
 class ReleaseChannel(models.Model):
     """Release channel of Firefox"""
@@ -40,19 +43,25 @@ class Meta:
     def __str__(self):
         return self.name
 
+    def matches(self, other):
+        return self.slug == other or self.name == other
+
 
 class Country(models.Model):
     """Database table for countries from django_countries."""
     code = models.CharField(max_length=255, unique=True)
     name = models.CharField(max_length=255)
-    order = models.IntegerField()
+    order = models.IntegerField(default=100)
 
     class Meta:
         ordering = ['order', 'name']
 
     def __str__(self):
         return '{self.name} ({self.code})'.format(self=self)
 
+    def matches(self, other):
+        return self.code == other or self.name == other
+
 
 class Recipe(models.Model):
     """A set of actions to be fetched and executed by users."""
@@ -71,26 +80,31 @@ class Recipe(models.Model):
     def log_rejection(self, msg):
         logger.debug('{} rejected: {}'.format(self, msg))
 
+    def check_many(self, name, field_qs, client_val):
+        field_vals = list(field_qs.all())
+
+        if field_vals == []:
+            return True
+
+        for val in field_vals:
+            if val.matches(client_val):
+                return True
+
+        field_vals_str = ', '.join(str(o) for o in field_vals)
+        self.log_rejection('client {name} ({client_val}) does not match recipe '
+                           '(choices are {field_vals})'
+                           .format(name=name, client_val=client_val, field_vals=field_vals_str))
+        return False
+
     def matches(self, client):
         """
         Return whether this Recipe should be sent to the given client.
         """
+        # This should be ordered roughly by performance cost
         if not self.enabled:
             self.log_rejection('not enabled')
             return False
 
-        if self.locale and client.locale and self.locale.code.lower() != client.locale.lower():
-            self.log_rejection('recipe locale ({self.locale!r}) != '
-                               'client locale ({client.locale!r})'
-                               .format(self=self, client=client))
-            return False
-
-        if self.country and self.country != client.country:
-            self.log_rejection('recipe country ({self.country!r}) != '
-                               'client country ({client.country!r})'
-                               .format(self=self, client=client))
-            return False
-
         if self.start_time and self.start_time > client.request_time:
             self.log_rejection('start time not met ({})'.format(self.start_time))
             return False
@@ -105,20 +119,14 @@ def matches(self, client):
                 self.log_rejection('did not match sample')
                 return False
 
-        recipe_channels = list(self.release_channels.all())
-        if recipe_channels:
-            match = False
-            for channel in recipe_channels:
-                if channel.slug == client.release_channel:
-                    match = True
-                    break
-            if not match:
-                channels = ', '.join(c.slug for c in self.release_channels.all())
-                self.log_rejection(
-                    'client channel ({client.release_channel}) not in '
-                    'recipe channels ({channels})'
-                    .format(channels=channels, client=client))
-                return False
+        if not self.check_many('locale', self.locales, client.locale):
+            return False
+
+        if not self.check_many('country', self.countries, client.country):
+            return False
+
+        if not self.check_many('channel', self.release_channels, client.release_channel):
+            return False
 
         return True
 

normandy/recipes/tests/__init__.py

@@ -3,7 +3,7 @@
 from django.template.defaultfilters import slugify
 
 from normandy.base.tests import FuzzyUnicode
-from normandy.recipes.models import Action, Locale, Recipe, RecipeAction, ReleaseChannel
+from normandy.recipes.models import Action, Country, Locale, Recipe, RecipeAction, ReleaseChannel
 
 
 class RecipeFactory(factory.DjangoModelFactory):
@@ -14,12 +14,22 @@ class Meta:
     enabled = True
 
     @factory.post_generation
-    def locale(self, create, extracted, **kwargs):
+    def countries(self, create, extracted, **kwargs):
         if not create:
             return
 
-        if extracted and isinstance(extracted, str):
-            self.locale, _ = Locale.objects.get_or_create(code=extracted)
+        if extracted:
+            for country in extracted:
+                self.countries.add(country)
+
+    @factory.post_generation
+    def locales(self, create, extracted, **kwargs):
+        if not create:
+            return
+
+        if extracted:
+            for locale in extracted:
+                self.locales.add(locale)
 
     @factory.post_generation
     def release_channels(self, create, extracted, **kwargs):
@@ -47,10 +57,19 @@ class Meta:
     recipe = factory.SubFactory(RecipeFactory)
 
 
+class CountryFactory(factory.DjangoModelFactory):
+    class Meta:
+        model = Country
+
+    code = factory.fuzzy.FuzzyText(length=3)
+
+
 class LocaleFactory(factory.DjangoModelFactory):
     class Meta:
         model = Locale
 
+    code = factory.fuzzy.FuzzyText(length=2)
+
 
 class ReleaseChannelFactory(factory.DjangoModelFactory):
     class Meta: