bug 748689: Support OS deprecation in Balrog. r=nthomas

auslib/admin/views/rules.py

@@ -68,16 +68,16 @@ def _post(self, transaction, changed_by):
                 priority=form.priority.data,
                 product = form.product.data,
                 version = form.version.data,
-                build_id = form.build_id.data,
+                buildID = form.build_id.data,
                 channel = form.channel.data,
                 locale = form.locale.data,
                 distribution = form.distribution.data,
-                build_target = form.build_target.data,
-                os_version = form.os_version.data,
-                dist_version = form.dist_version.data,
+                buildTarget = form.build_target.data,
+                osVersion = form.os_version.data,
+                distVersion = form.dist_version.data,
                 comment = form.comment.data,
                 update_type = form.update_type.data,
-                header_arch = form.header_arch.data)
+                headerArchitecturerch = form.header_arch.data)
         rule_id = db.rules.addRule(changed_by=changed_by, what=what,
             transaction=transaction)
         return Response(status=200, response=rule_id)

auslib/db.py

@@ -9,6 +9,7 @@
 from sqlalchemy import Table, Column, Integer, Text, String, MetaData, \
   create_engine, select, BigInteger
 from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.sql.expression import literal
 
 import migrate.versioning.schema
 import migrate.versioning.api
@@ -634,7 +635,7 @@ def _versionMatchesRule(self, ruleVersion, queryVersion):
             return True
 
     def _channelMatchesRule(self, ruleChannel, queryChannel, fallbackChannel):
-        """Decides whether a channel from the rules matchs an incoming one.
+        """Decides whether a channel from the rules matches an incoming one.
            If the ruleChannel is null, we match any queryChannel. We also match
            if the channels match exactly, or match after wildcards in ruleChannel
            are resolved. Channels may have a fallback specified, too, so we must
@@ -646,6 +647,19 @@ def _channelMatchesRule(self, ruleChannel, queryChannel, fallbackChannel):
         if self._matchesRegex(ruleChannel, fallbackChannel):
             return True
 
+    def _osVersionMatchesRule(self, ruleOsVersion, queryOsVersion):
+        """Decides whether an osVersion from a rule matches an incoming one.
+           osVersion columns in a rule may specify multiple OS versions,
+           delimited by a comma. Once split we do simple substring matching
+           against the query's OS version. Unlike versions and channels, we
+           assume each OS version is a substring of what will be in the query,
+           thus we don't need to support globbing."""
+        if ruleOsVersion is None:
+            return True
+        for os in ruleOsVersion.split(','):
+            if os in queryOsVersion:
+                return True
+
     def addRule(self, changed_by, what, transaction=None):
         ret = self.insert(changed_by=changed_by, transaction=transaction, **what)
         return ret.inserted_primary_key
@@ -669,7 +683,6 @@ def getRulesMatchingQuery(self, updateQuery, fallbackChannel, transaction=None):
             ((self.buildTarget==updateQuery['buildTarget']) | (self.buildTarget==None)) &
             ((self.buildID==updateQuery['buildID']) | (self.buildID==None)) &
             ((self.locale==updateQuery['locale']) | (self.locale==None)) &
-            ((self.osVersion==updateQuery['osVersion']) | (self.osVersion==None)) &
             ((self.headerArchitecture==updateQuery['headerArchitecture']) | (self.headerArchitecture==None))
         ]
         # Query version 2 doesn't have distribution information, and to keep
@@ -699,6 +712,12 @@ def getRulesMatchingQuery(self, updateQuery, fallbackChannel, transaction=None):
             if not self._channelMatchesRule(rule['channel'], updateQuery['channel'], fallbackChannel):
                 self.log.debug("%s doesn't match %s", rule['channel'], updateQuery['channel'])
                 continue
+            # To help keep the rules table compact, multiple OS versions may be
+            # specified in a single rule. They are comma delimited, so we need to
+            # break them out and create clauses for each one.
+            if not self._osVersionMatchesRule(rule['osVersion'], updateQuery['osVersion']):
+                self.log.debug("%s doesn't match %s", rule['osVersion'], updateQuery['osVersion'])
+                continue
             matchingRules.append(rule)
         self.log.debug("Reduced matches:")
         if self.log.isEnabledFor(logging.DEBUG):

auslib/test/test_db.py

@@ -523,12 +523,16 @@ def setUp(self):
         self.paths.t.insert().execute(id=3, priority=100, version='3.5', buildTarget='a', backgroundRate=100, mapping='a', update_type='z', data_version=1)
         self.paths.t.insert().execute(id=4, priority=80, buildTarget='d', backgroundRate=100, mapping='a', update_type='z', data_version=1)
         self.paths.t.insert().execute(id=5, priority=80, buildTarget='d', version='3.3', backgroundRate=0, mapping='c', update_type='z', data_version=1)
+        self.paths.t.insert().execute(id=6, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 1', update_type='z', data_version=1)
+        self.paths.t.insert().execute(id=7, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 2,blah 6', update_type='z', data_version=1)
 
     def testGetOrderedRules(self):
         rules = self._stripNullColumns(self.paths.getOrderedRules())
         expected = [
             dict(rule_id=4, priority=80, backgroundRate=100, buildTarget='d', mapping='a', update_type='z', data_version=1),
             dict(rule_id=5, priority=80, backgroundRate=0, version='3.3', buildTarget='d', mapping='c', update_type='z', data_version=1),
+            dict(rule_id=6, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 1', update_type='z', data_version=1),
+            dict(rule_id=7, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 2,blah 6', update_type='z', data_version=1),
             dict(rule_id=2, priority=100, backgroundRate=100, version='3.3', buildTarget='d', mapping='b', update_type='z', data_version=1),
             dict(rule_id=3, priority=100, backgroundRate=100, version='3.5', buildTarget='a', mapping='a', update_type='z', data_version=1),
             dict(rule_id=1, priority=100, backgroundRate=100, version='3.5', buildTarget='d', mapping='c', update_type='z', data_version=1),
@@ -537,7 +541,7 @@ def testGetOrderedRules(self):
 
     def testGetRulesMatchingQuery(self):
         rules = self.paths.getRulesMatchingQuery(
-            dict(name='', product='', version='3.5', channel='',
+            dict(product='', version='3.5', channel='',
                  buildTarget='a', buildID='', locale='', osVersion='',
                  distribution='', distVersion='', headerArchitecture='',
                  force=False, queryVersion=3,
@@ -550,7 +554,7 @@ def testGetRulesMatchingQuery(self):
 
     def testGetRulesMatchingQueryWithNullColumn(self):
         rules = self.paths.getRulesMatchingQuery(
-            dict(name='', product='', version='3.5', channel='',
+            dict(product='', version='3.5', channel='',
                  buildTarget='d', buildID='', locale='', osVersion='',
                  distribution='', distVersion='', headerArchitecture='',
                  force=False, queryVersion=3,
@@ -566,7 +570,7 @@ def testGetRulesMatchingQueryWithNullColumn(self):
 
     def testGetRulesMatchingQueryDontReturnBackgroundThrottled(self):
         rules = self.paths.getRulesMatchingQuery(
-            dict(name='', product='', version='3.3', channel='',
+            dict(product='', version='3.3', channel='',
                  buildTarget='d', buildID='', locale='', osVersion='',
                  distribution='', distVersion='', headerArchitecture='',
                  force=False, queryVersion=3,
@@ -582,7 +586,7 @@ def testGetRulesMatchingQueryDontReturnBackgroundThrottled(self):
 
     def testGetRulesMatchingQueryReturnBackgroundThrottled(self):
         rules = self.paths.getRulesMatchingQuery(
-            dict(name='', product='', version='3.3', channel='',
+            dict(product='', version='3.3', channel='',
                  buildTarget='d', buildID='', locale='', osVersion='',
                  distribution='', distVersion='', headerArchitecture='',
                  force=True, queryVersion=3,
@@ -593,7 +597,55 @@ def testGetRulesMatchingQueryReturnBackgroundThrottled(self):
         expected = [
             dict(rule_id=2, priority=100, backgroundRate=100, version='3.3', buildTarget='d', mapping='b', update_type='z', data_version=1),
             dict(rule_id=4, priority=80, backgroundRate=100, buildTarget='d', mapping='a', update_type='z', data_version=1),
-            dict(rule_id=5, priority=80, backgroundRate=0, version='3.3', buildTarget='d', mapping='c', update_type='z', data_version=1)
+            dict(rule_id=5, priority=80, backgroundRate=0, version='3.3', buildTarget='d', mapping='c', update_type='z', data_version=1),
+        ]
+        self.assertEquals(rules, expected)
+
+    def testGetRulesMatchingQueryOsVersionSubstring(self):
+        rules = self.paths.getRulesMatchingQuery(
+            dict(product='', version='5', channel='', buildTarget='d',
+                 buildID='', locale='', osVersion='foo 1.2.3', distribution='',
+                 distVersion='', headerArchitecture='', force=False,
+                 queryVersion=3,
+            ),
+            fallbackChannel='',
+        )
+        rules = self._stripNullColumns(rules)
+        expected = [
+            dict(rule_id=4, priority=80, backgroundRate=100, buildTarget='d', mapping='a', update_type='z', data_version=1),
+            dict(rule_id=6, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 1', update_type='z', data_version=1)
+        ]
+        self.assertEquals(rules, expected)
+
+    def testGetRulesMatchingQueryOsVersionSubstringNotAtStart(self):
+        rules = self.paths.getRulesMatchingQuery(
+            dict(product='', version='5', channel='', buildTarget='d',
+                 buildID='', locale='', osVersion='bbb foo 1.2.3', distribution='',
+                 distVersion='', headerArchitecture='', force=False,
+                 queryVersion=3,
+            ),
+            fallbackChannel='',
+        )
+        rules = self._stripNullColumns(rules)
+        expected = [
+            dict(rule_id=4, priority=80, backgroundRate=100, buildTarget='d', mapping='a', update_type='z', data_version=1),
+            dict(rule_id=6, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 1', update_type='z', data_version=1)
+        ]
+        self.assertEquals(rules, expected)
+
+    def testGetRulesMatchingQueryOsVersionMultipleSubstring(self):
+        rules = self.paths.getRulesMatchingQuery(
+            dict(product='', version='5', channel='', buildTarget='d',
+                 buildID='', locale='', osVersion='blah 6.3.2', distribution='',
+                 distVersion='', headerArchitecture='', force=False,
+                 queryVersion=3,
+            ),
+            fallbackChannel='',
+        )
+        rules = self._stripNullColumns(rules)
+        expected = [
+            dict(rule_id=4, priority=80, backgroundRate=100, buildTarget='d', mapping='a', update_type='z', data_version=1),
+            dict(rule_id=7, priority=100, buildTarget='d', mapping='a', backgroundRate=100, osVersion='foo 2,blah 6', update_type='z', data_version=1)
         ]
         self.assertEquals(rules, expected)
 
@@ -632,8 +684,7 @@ def testUpdateRule(self):
         self.assertEquals(rule, expected)
 
     def testGetNumberOfRules(self):
-        # because 5 rules were set up in the setUp()
-        self.assertEquals(self.paths.countRules(), 5)
+        self.assertEquals(self.paths.countRules(), 7)
 
 
 class TestRulesSpecial(unittest.TestCase, RulesTestMixin, MemoryDatabaseMixin):

auslib/web/views/client.py

@@ -1,3 +1,5 @@
+import urllib
+
 from flask import make_response, request
 from flask.views import MethodView
 
@@ -21,6 +23,7 @@ def getHeaderArchitecture(self, buildTarget, ua):
 
     def getQueryFromURL(self, url):
         query = url.copy()
+        query['osVersion'] = urllib.unquote(query['osVersion'])
         ua = request.headers.get('User-Agent')
         query['headerArchitecture'] = self.getHeaderArchitecture(query['buildTarget'], ua)
         query['force'] = (int(request.args.get('force', 0)) == 1)