Added strict compatibility checking (bug 698358, 698355)

apps/addons/fixtures/addons/update-strict-compat-version.json

@@ -0,0 +1,42 @@
+[
+  {
+    "pk": 123456,
+    "model": "versions.version",
+    "fields": {
+      "license": 5,
+      "created": "2011-08-17 13:32:40",
+      "releasenotes": null,
+      "approvalnotes": "",
+      "modified": "2011-09-14 12:50:58",
+      "version": "1.3.0",
+      "addon": 1865
+    }
+  },
+  {
+    "pk": 96878,
+    "model": "files.file",
+    "fields": {
+      "status": 4,
+      "codereview": false,
+      "hash": "sha256:2bb766a6b096e9724df3a584bbe4f2e0ca687f1cd199e1a2c0ce58d4d8487284",
+      "created": "2010-08-17 13:32:40",
+      "modified": "2010-09-14 12:50:57",
+      "filename": "adblock_plus-1.3.0-fx+sm+tb+fn.xpi",
+      "platform": 1,
+      "version": 123456,
+      "datestatuschanged": "2011-08-17 13:31:08",
+      "size": 301,
+      "strict_compatibility": 1
+    }
+  },
+  {
+    "pk": 88590,
+    "model": "versions.applicationsversions",
+    "fields": {
+      "application": 1,
+      "max": 364,
+      "version": 123456,
+      "min": 296
+    }
+  }
+]

apps/addons/fixtures/addons/update.json

@@ -633,7 +633,29 @@
       "created": "2008-10-28 18:25:09"
     }
   },
-{
+  {
+    "pk": 411,
+    "model": "applications.appversion",
+    "fields": {
+      "version_int": 5000000200100,
+      "application": 1,
+      "version": "5.0",
+      "modified": null,
+      "created": null
+    }
+  },
+  {
+    "pk": 364,
+    "model": "applications.appversion",
+    "fields": {
+      "version_int": 4000000200100,
+      "application": 1,
+      "version": "4.0",
+      "modified": null,
+      "created": null
+    }
+  },
+  {
     "pk": 350,
     "model": "applications.appversion",
     "fields": {

apps/addons/tests/test_update.py

@@ -357,6 +357,85 @@ def test_file_preliminary_addon(self):
             eq_(version, self.version_1_2_1)
 
 
+class TestDefaultToCompat(amo.tests.TestCase):
+    """
+    This test adds a fixture with a version/file with strict compatibility
+    enabled.
+    """
+    fixtures = ['addons/update', 'base/platforms',
+                'addons/update-strict-compat-version']
+
+    def setUp(self):
+        self.addon = Addon.objects.get(id=1865)
+        self.platform = None
+        self.version_int = 5000000200100
+
+        self.app = Application.objects.get(id=1)
+        self.version_1_0_2 = 66463
+        self.version_1_1_3 = 90149
+        self.version_1_2_0 = 105387
+        self.version_1_2_1 = 112396
+        self.version_1_2_2 = 115509
+        self.version_1_3_0 = 123456
+
+    def get(self, *args):
+        up = update.Update({
+            'id': self.addon.guid,
+            'version': args[0],
+            'appID': args[2].guid,
+            'appVersion': 1,  # this is going to be overridden
+            'appOS': args[3].api_name if args[3] else '',
+            'reqVersion': '',
+            })
+        up.cursor = connection.cursor()
+        assert up.is_valid()
+        up.data['version_int'] = args[1]
+        if len(args) == 5:
+            up.compat_mode = args[4]
+        up.get_update()
+        return (up.data['row'].get('version_id'),
+                up.data['row'].get('file_id'))
+
+    def test_strict_and_strict(self):
+        # File has strict_compatibility and compatMode is strict.
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'strict')
+        eq_(version, None)
+
+    def test_strict_and_normal(self):
+        # File has strict_compatibility and compatMode is normal.
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'normal')
+        eq_(version, self.version_1_2_2)
+
+    def test_strict_and_ignore(self):
+        # File has strict_compatibility and compatMode is ignore.
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'ignore')
+        eq_(version, self.version_1_3_0)
+
+    def test_no_strict_and_strict(self):
+        # File has strict_compatibility off and compatMode is strict.
+        File.objects.get(pk=96878).update(strict_compatibility=False)
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'strict')
+        eq_(version, None)
+
+    def test_no_strict_and_normal(self):
+        # File has strict_compatibility off and compatMode is normal.
+        File.objects.get(pk=96878).update(strict_compatibility=False)
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'normal')
+        eq_(version, self.version_1_3_0)
+
+    def test_no_strict_and_ignore(self):
+        # File has strict_compatibility off and compatMode is ignore.
+        File.objects.get(pk=96878).update(strict_compatibility=False)
+        version, file = self.get('', self.version_int,
+                                 self.app, self.platform, 'ignore')
+        eq_(version, self.version_1_3_0)
+
+
 class TestResponse(amo.tests.TestCase):
     fixtures = ['base/addon_3615',
                 'base/platforms',

apps/amo/fixtures/base/appversion.json

@@ -436,5 +436,16 @@
             "modified": null,
             "created": null
         }
+    },
+    {
+        "pk": 364,
+        "model": "applications.appversion",
+        "fields": {
+            "version_int": 4000000200100,
+            "application": 1,
+            "version": "4.0",
+            "modified": null,
+            "created": "2007-03-05 13:09:25"
+        }
     }
 ]

apps/files/models.py

@@ -56,6 +56,7 @@ class File(amo.models.OnChangeMixin, amo.models.ModelBase):
                                               default=amo.STATUS_UNREVIEWED)
     datestatuschanged = models.DateTimeField(null=True, auto_now_add=True)
     no_restart = models.BooleanField(default=False)
+    strict_compatibility = models.BooleanField(default=False)
     # The XPI contains JS that calls require("chrome").
     requires_chrome = models.BooleanField(default=False)
     reviewed = models.DateTimeField(null=True)
@@ -141,6 +142,7 @@ def from_upload(cls, upload, version, platform, parse_data={}):
         f.size = int(max(1, round(upload.path.size / 1024, 0)))  # Kilobytes.
         f.jetpack_version = cls.get_jetpack_version(upload.path)
         f.no_restart = parse_data.get('no_restart', False)
+        f.strict_compatibility = parse_data.get('strict_compatibility', False)
         if version.addon.status == amo.STATUS_PUBLIC:
             if amo.VERSION_BETA.search(parse_data.get('version', '')):
                 f.status = amo.STATUS_BETA

apps/files/tests/test_models.py

@@ -408,6 +408,14 @@ def test_long_version_number(self):
         msg = e.exception.messages[0]
         eq_(msg, 'Version numbers should have fewer than 32 characters.')
 
+    def test_strict_compat_undefined(self):
+        result = self.parse()
+        eq_(result['strict_compatibility'], False)
+
+    def test_strict_compat_enabled(self):
+        result = self.parse(filename='strict-compat.xpi')
+        eq_(result['strict_compatibility'], True)
+
 
 class TestParseAlternateXpi(amo.tests.TestCase, amo.tests.AMOPaths):
     # This install.rdf is completely different from our other xpis.
@@ -694,6 +702,12 @@ def test_file_hash_no_paranoia(self):
         f = File.from_upload(upload, self.version, self.platform)
         assert f.hash.startswith('oops')
 
+    def test_strict_compat(self):
+        upload = self.upload('strict-compat')
+        data = parse_addon(upload.path)
+        f = File.from_upload(upload, self.version, self.platform, data)
+        eq_(f.strict_compatibility, True)
+
 
 class TestZip(amo.tests.TestCase, amo.tests.AMOPaths):
 

apps/files/utils.py

@@ -76,6 +76,7 @@ def __init__(self, path):
             'homepage': self.find('homepageURL'),
             'summary': self.find('description'),
             'no_restart': self.find('bootstrap') == 'true',
+            'strict_compatibility': self.find('strictCompatibility') == 'true',
             'apps': self.apps(),
         }
 

migrations/276-file-strict-compat.sql

@@ -0,0 +1 @@
+ALTER TABLE `files` ADD COLUMN `strict_compatibility` bool NOT NULL DEFAULT '0';

services/update.py

@@ -88,13 +88,14 @@ def getconn():
 
 class Update(object):
 
-    def __init__(self, data):
+    def __init__(self, data, compat_mode='strict'):
         self.conn, self.cursor = None, None
         self.data = data.copy()
         self.data['row'] = {}
         self.flags = {'use_version': False, 'multiple_status': False}
         self.is_beta_version = False
         self.version_int = 0
+        self.compat_mode = compat_mode
 
     def is_valid(self):
         # If you accessing this from unit tests, then before calling
@@ -174,7 +175,7 @@ def get_update(self):
         self.get_beta()
         data = self.data
 
-        sql = """
+        sql = ["""
             SELECT
                 addons.guid as guid, addons.addontype_id as type,
                 addons.inactive as disabled_by_user,
@@ -183,6 +184,7 @@ def get_update(self):
                 files.status as file_status, files.hash,
                 files.filename, versions.id as version_id,
                 files.datestatuschanged as datestatuschanged,
+                files.strict_compatibility as strict_compat,
                 versions.releasenotes, versions.version as version,
                 addons.premium_type
             FROM versions
@@ -199,36 +201,48 @@ def get_update(self):
                 ON appmax.id = applications_versions.max
             INNER JOIN files
                 ON files.version_id = versions.id AND (files.platform_id = 1
-            """
+            """]
         if data.get('appOS'):
-            sql += ' OR files.platform_id = %(appOS)s'
+            sql.append(' OR files.platform_id = %(appOS)s')
 
         if self.flags['use_version']:
-            sql += (') WHERE files.status > %(status)s AND '
+            sql.append(') WHERE files.status > %(status)s AND '
                     'versions.version = %(version)s ')
         else:
             if self.flags['multiple_status']:
                 # Note that getting this properly escaped is a pain.
                 # Suggestions for improvement welcome.
-                sql += (') WHERE files.status in (%(STATUS_PUBLIC)s,'
-                        '%(STATUS_LITE)s,%(STATUS_LITE_AND_NOMINATED)s)')
+                sql.append(') WHERE files.status in (%(STATUS_PUBLIC)s,'
+                        '%(STATUS_LITE)s,%(STATUS_LITE_AND_NOMINATED)s) ')
             else:
-                sql += ') WHERE files.status = %(status)s '
+                sql.append(') WHERE files.status = %(status)s ')
 
-        sql += """
-            AND (appmin.version_int <= %(version_int)s
-            AND appmax.version_int >= %(version_int)s)
-            ORDER BY versions.id DESC LIMIT 1;
-            """
+        sql.append('AND appmin.version_int <= %(version_int)s ')
 
-        self.cursor.execute(sql, data)
+        if self.compat_mode == 'ignore':
+            pass  # no further SQL modification required.
+
+        elif self.compat_mode == 'normal':
+            # normal checks the file's strict_compatibility flag
+            sql.append("""
+                AND (CASE WHEN files.strict_compatibility = 1 THEN
+                        appmax.version_int >= %(version_int)s ELSE 1 END)
+            """)
+
+        else:  # Not defined or 'strict'.
+            sql.append('AND appmax.version_int >= %(version_int)s ')
+
+        sql.append('ORDER BY versions.id DESC LIMIT 1;')
+
+        self.cursor.execute(''.join(sql), data)
         result = self.cursor.fetchone()
+
         if result:
             row = dict(zip([
                 'guid', 'type', 'disabled_by_user', 'appguid', 'min', 'max',
                 'file_id', 'file_status', 'hash', 'filename', 'version_id',
-                'datestatuschanged', 'releasenotes', 'version',
-                'premium_type'],
+                'datestatuschanged', 'strict_compat', 'releasenotes',
+                'version', 'premium_type'],
                 list(result)))
             row['type'] = base.ADDON_SLUGS_UPDATE[row['type']]
             if row['premium_type'] == base.ADDON_PREMIUM:
@@ -319,8 +333,9 @@ def application(environ, start_response):
               (environ['SCRIPT_NAME'], environ['QUERY_STRING']))
     with statsd.timer('services.update'):
         data = dict(parse_qsl(environ['QUERY_STRING']))
+        compat_mode = data.pop('compatMode', 'strict')
         try:
-            update = Update(data)
+            update = Update(data, compat_mode)
             output = update.get_rdf()
             start_response(status, update.get_headers(len(output)))
         except: