Bug 654355 Multiple Client Match Rules

apps/homesnippets/models.py

@@ -1,19 +1,15 @@
 """
 homesnippets models
 """
-import logging
 import hashlib
 from datetime import datetime
 from time import mktime, gmtime
+
 from django.conf import settings
-from django.db import models
-from django import forms
 from django.core import urlresolvers
-from django.contrib.contenttypes.models import ContentType
-from django.db.models.signals import pre_save, post_save, pre_delete, post_delete
 from django.core.cache import cache
-from django.contrib.sites.models import Site
-from django.contrib.auth.models import User
+from django.db import models
+from django.db.models.signals import post_save, post_delete
 from django.utils.translation import ugettext_lazy as _
 
 
@@ -36,7 +32,13 @@ def _key_from_client(args):
 class ClientMatchRuleManager(models.Manager):
     """Manager for client match rules, allows filtering against match logic"""
 
-    def find_ids_for_matches(self, args):
+    def find_match_ids_for_request(self, args):
+        """
+        Finds all match rules that affect the given request. Returns two lists
+        containing the rules that will exclude or include snippets in the
+        response.
+        """
+
         cache_key = '%s%s' % (CACHE_RULE_MATCH_PREFIX, _key_from_client(args))
         cache_hit = cache.get(cache_key)
 
@@ -55,20 +57,30 @@ def find_ids_for_matches(self, args):
 
         if not cache_hit:
             # Cache miss, so recalculate the results and cache them.
-            matches = [ rule for rule in self._cached_all() 
-                    if rule.is_match(args) ]
-            (include_ids, exclude_ids) = (
-                [str(rule.id) for rule in matches if not rule.exclude],
-                [str(rule.id) for rule in matches if rule.exclude],
-            )
+            rules = self._cached_all()
+            include_ids, exclude_ids = [], []
+
+            # Check every rule
+            for rule in rules:
+                if rule.is_match(args):
+                    if rule.exclude:
+                        exclude_ids.append(str(rule.id))
+                    else:
+                        include_ids.append(str(rule.id))
+                elif not rule.exclude:
+                    # Include rule that doesn't match? Add as an exclude rule
+                    # so that snippets can split required matches into multiple
+                    # rules and combine them together.
+                    exclude_ids.append(str(rule.id))
+
             cache_hit = (mktime(gmtime()), (include_ids, exclude_ids))
             cache.set(cache_key, cache_hit, CACHE_TIMEOUT)
 
         return cache_hit[1]
 
     def _cached_all(self):
         """Cached version of self.all(), invalidated by change to any rule."""
-        c_data = cache.get_many([CACHE_RULE_ALL_PREFIX, 
+        c_data = cache.get_many([CACHE_RULE_ALL_PREFIX,
             CACHE_RULE_ALL_LASTMOD_PREFIX])
 
         lastmod   = c_data.get(CACHE_RULE_ALL_LASTMOD_PREFIX, None)
@@ -97,34 +109,34 @@ class Meta():
     exclude = models.BooleanField( _('exclusion rule?'),
             default=False)
 
-    # browser/components/nsBrowserContentHandler.js:911:    
-    # const SNIPPETS_URL = "http://snippets.mozilla.com/" + STARTPAGE_VERSION + 
+    # browser/components/nsBrowserContentHandler.js:911:
+    # const SNIPPETS_URL = "http://snippets.mozilla.com/" + STARTPAGE_VERSION +
     # "/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%
     # /%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/";
 
-    startpage_version = models.CharField( _('start page version'), 
+    startpage_version = models.CharField( _('start page version'),
             null=True, blank=True, max_length=64)
-    name = models.CharField( _('product name'), 
+    name = models.CharField( _('product name'),
             null=True, blank=True, max_length=64)
-    version = models.CharField( _('product version'), 
+    version = models.CharField( _('product version'),
             null=True, blank=True, max_length=64)
-    appbuildid = models.CharField( _('app build id'), 
+    appbuildid = models.CharField( _('app build id'),
             null=True, blank=True, max_length=64)
-    build_target = models.CharField( _('build target'), 
+    build_target = models.CharField( _('build target'),
             null=True, blank=True, max_length=64)
-    locale = models.CharField( _('locale'), 
+    locale = models.CharField( _('locale'),
             null=True, blank=True, max_length=64)
-    channel = models.CharField( _('channel'), 
+    channel = models.CharField( _('channel'),
             null=True, blank=True, max_length=64)
-    os_version = models.CharField( _('os version'), 
+    os_version = models.CharField( _('os version'),
             null=True, blank=True, max_length=64)
-    distribution = models.CharField( _('distribution'), 
+    distribution = models.CharField( _('distribution'),
             null=True, blank=True, max_length=64)
-    distribution_version = models.CharField( _('distribution version'), 
+    distribution_version = models.CharField( _('distribution version'),
             null=True, blank=True, max_length=64)
-    created = models.DateTimeField( _('date created'), 
+    created = models.DateTimeField( _('date created'),
             auto_now_add=True, blank=False)
-    modified = models.DateTimeField( _('date last modified'), 
+    modified = models.DateTimeField( _('date last modified'),
             auto_now=True, blank=False)
 
     def __unicode__(self):
@@ -136,17 +148,17 @@ def __unicode__(self):
             return self.description
         else:
             return '%s /%s' % (
-                self.exclude and 'EXCLUDE' or 'INCLUDE', 
-                '/'.join(vals) 
+                self.exclude and 'EXCLUDE' or 'INCLUDE',
+                '/'.join(vals)
             )
 
     def is_match(self, args):
         is_match = True
         for ak,av in args.items():
             mv = getattr(self, ak, None)
-            if not mv: 
+            if not mv:
                 continue
-            
+
             if mv.startswith('/'):
                 # Regex match
                 import re
@@ -173,15 +185,15 @@ def related_snippets(self):
         count = self.snippet_set.count()
 
         # TODO: Needs l10n? Maybe not a priority for an admin page.
-        return ( 
-            ( (count != 1) and 
+        return (
+            ( (count != 1) and
                 '<a href="%(link)s" title="Click to list snippets matched by this rule"><strong>%(count)d snippets</strong></a>' or
                 '<a href="%(link)s" title="Click to list snippets matched by this rule"><strong>%(count)d snippet</strong></a>' )
-            % dict(count=count, link=link) 
+            % dict(count=count, link=link)
         )
 
     related_snippets.allow_tags = True
-        
+
 
 def rule_update_lastmods(sender, instance, created=False, **kwargs):
     """On a change to a rule, bump lastmod timestamps for that rule and the set
@@ -205,28 +217,28 @@ def rule_update_lastmods(sender, instance, created=False, **kwargs):
 class SnippetManager(models.Manager):
 
     def find_snippets_with_match_rules(self, args, time_now=None):
-        """Find snippets data using match rules. 
+        """Find snippets data using match rules.
 
         Returned is a list of dicts with id and body of snippets found, rather
-        than full Snippet model objects. This makes things easier to cache - 
+        than full Snippet model objects. This makes things easier to cache -
         if full snippets are required, try using the id's to look them up.
         """
         if time_now is None:
             time_now = datetime.now()
 
         preview = ( 'preview' in args ) and args['preview']
         include_ids, exclude_ids = \
-            ClientMatchRule.objects.find_ids_for_matches(args)
+            ClientMatchRule.objects.find_match_ids_for_request(args)
         snippets = self.find_snippets_for_rule_ids(preview, include_ids, exclude_ids)
 
-        # Filter for date ranges here, rather than in SQL. 
+        # Filter for date ranges here, rather than in SQL.
         #
         # This is a compromise to make snippet match results more cacheable -
         # ie. cached data should only be recalculated in response to content
         # changes, not the passage of time.
-        snippets_data = [ s for s in snippets if ( 
+        snippets_data = [ s for s in snippets if (
             ( not s['pub_start'] or time_now >= s['pub_start'] ) and
-            ( not s['pub_end']   or time_now <  s['pub_end'] ) 
+            ( not s['pub_end']   or time_now <  s['pub_end'] )
         ) ]
 
         return snippets_data
@@ -235,7 +247,7 @@ def find_snippets_for_rule_ids(self, preview, include_ids, exclude_ids):
         """Given a set of matching inclusion & exclusion rule IDs, look up the
         corresponding snippets."""
 
-        if not include_ids and not exclude_ids: 
+        if not include_ids and not exclude_ids:
             return []
 
         # Could base the cache key on the entire text of the SQL query
@@ -263,7 +275,7 @@ def find_snippets_for_rule_ids(self, preview, include_ids, exclude_ids):
         if not cache_hit:
             # No cache hit, look up the snippets associated with rules.
             sql_base = """
-                SELECT homesnippets_snippet.* 
+                SELECT homesnippets_snippet.*
                 FROM homesnippets_snippet
                 WHERE ( %s )
                 ORDER BY priority, pub_start, modified
@@ -274,28 +286,28 @@ def find_snippets_for_rule_ids(self, preview, include_ids, exclude_ids):
             if not preview:
                 where.append('( homesnippets_snippet.preview <> 1 )')
             if include_ids:
-                where.append(""" 
+                where.append("""
                     homesnippets_snippet.id IN (
                         SELECT snippet_id
                         FROM homesnippets_snippet_client_match_rules
                         WHERE clientmatchrule_id IN (%s)
-                    ) 
+                    )
                 """ % ",".join(include_ids))
             if exclude_ids:
-                where.append(""" 
+                where.append("""
                     homesnippets_snippet.id NOT IN (
                         SELECT snippet_id
                         FROM homesnippets_snippet_client_match_rules
                         WHERE clientmatchrule_id IN (%s)
-                    ) 
+                    )
                 """ % ",".join(exclude_ids))
             sql = sql_base % (' AND '.join(where))
 
             # Reduce snippet model objects to more cacheable dicts
             snippet_objs = self.raw(sql)
-            snippets = [ 
+            snippets = [
                 dict(
-                    id=snippet.id, 
+                    id=snippet.id,
                     name=snippet.name,
                     body=snippet.body,
                     pub_start=snippet.pub_start,
@@ -318,10 +330,10 @@ class Meta():
 
     client_match_rules = models.ManyToManyField(
             ClientMatchRule, blank=False)
-    
-    name = models.CharField( _("short name (only shown to admins)"), 
+
+    name = models.CharField( _("short name (only shown to admins)"),
             blank=False, max_length=255)
-    body = models.TextField( _("content body"), 
+    body = models.TextField( _("content body"),
             blank=False)
 
     priority = models.IntegerField( _('sort order priority'),
@@ -331,16 +343,16 @@ class Meta():
     preview = models.BooleanField( _('preview only?'),
             default=False)
     pub_start = models.DateTimeField( _('start time'),
-            blank=True, null=True) 
+            blank=True, null=True)
     pub_end = models.DateTimeField( _('end time'),
-            blank=True, null=True) 
+            blank=True, null=True)
 
-    created = models.DateTimeField( _('date created'), 
+    created = models.DateTimeField( _('date created'),
             auto_now_add=True, blank=False)
-    modified = models.DateTimeField( _('date last modified'), 
+    modified = models.DateTimeField( _('date last modified'),
             auto_now=True, blank=False)
 
-    
+
 def snippet_update_lastmod(sender, instance, **kwargs):
     """On a change to a snippet, bump its cached lastmod timestamp"""
     now = mktime(gmtime())
@@ -353,4 +365,3 @@ def snippet_update_lastmod(sender, instance, **kwargs):
 
 post_save.connect(snippet_update_lastmod, sender=Snippet)
 post_delete.connect(snippet_update_lastmod, sender=Snippet)
-