Merge pull request #74 from gijsk/version-check-addons-and-plugins

Apply version checks to add-ons and plugins in the blocklist

amo2kinto/exporter.py

@@ -43,23 +43,42 @@ def build_version_range(root, item, app_id, app_ver=None):
                             maxVersion=tA['maxVersion'])
 
 
-def is_related_to(item, app_id):
-    """Return True if the item relates to the given app_id."""
-    if not item.get('versionRange'):
+def is_related_to(item, app_id, app_ver=None):
+    """Return True if the item relates to the given app_id (and app_ver, if passed)."""
+    versionRange = item.get('versionRange')
+    if not versionRange:
         return True
 
-    for vR in item['versionRange']:
+    for vR in versionRange:
         if not vR.get('targetApplication'):
             return True
+        if get_related_targetApplication(vR, app_id, app_ver) is not None:
+            return True
+    return False
 
-        for tA in vR['targetApplication']:
-            if tA['guid'] == app_id:
-                return True
 
-    return False
+def get_related_targetApplication(vR, app_id, app_ver):
+    """Return the first matching target application in this version range.
+    Returns None if there are no target applications or no matching ones."""
+    targetApplication = vR.get('targetApplication')
+    if not targetApplication:
+        return None
+
+    for tA in targetApplication:
+        guid = tA.get('guid')
+        if not guid or guid == app_id:
+            if not app_ver:
+                return tA
+            # We purposefully use maxVersion only, so that the blocklist contains items
+            # whose minimum version is ahead of the version we get passed. This means
+            # the blocklist we serve is "future-proof" for app upgrades.
+            if between(version_int(app_ver), '0', tA.get('maxVersion', '*')):
+                return tA
 
+    return None
 
-def write_addons_items(xml_tree, records, app_id, api_ver=3):
+
+def write_addons_items(xml_tree, records, app_id, api_ver=3, app_ver=None):
     """Generate the addons blocklists.
 
     <emItem blockID="i372" id="[email protected]">
@@ -80,7 +99,7 @@ def write_addons_items(xml_tree, records, app_id, api_ver=3):
     emItems = etree.SubElement(xml_tree, 'emItems')
     groupby = {}
     for item in records:
-        if is_related_to(item, app_id):
+        if is_related_to(item, app_id, app_ver):
             if item['guid'] in groupby:
                 emItem = groupby[item['guid']]
                 # When creating new records from the Kinto Admin we don't have proper blockID.
@@ -137,35 +156,17 @@ def write_plugin_items(xml_tree, records, app_id, api_ver=3, app_ver=None):
 
     pluginItems = etree.SubElement(xml_tree, 'pluginItems')
     for item in records:
-        if is_related_to(item, app_id):
-            for versionRange in item.get('versionRange', []):
-                if not versionRange.get('targetApplication'):
-                    add_plugin_item(pluginItems, item, versionRange,
+        for versionRange in item.get('versionRange', []):
+            if not versionRange.get('targetApplication'):
+                add_plugin_item(pluginItems, item, versionRange,
+                                app_id=app_id, api_ver=api_ver,
+                                app_ver=app_ver)
+            else:
+                targetApplication = get_related_targetApplication(versionRange, app_id, app_ver)
+                if targetApplication is not None:
+                    add_plugin_item(pluginItems, item, versionRange, targetApplication,
                                     app_id=app_id, api_ver=api_ver,
                                     app_ver=app_ver)
-                else:
-                    for targetApplication in versionRange['targetApplication']:
-                        is_targetApplication_related = (
-                            'guid' not in targetApplication or
-                            targetApplication['guid'] == app_id
-                        )
-                        if is_targetApplication_related:
-                            if api_ver < 3 and app_ver is not None:
-                                app_version = version_int(app_ver)
-                                is_version_related = between(
-                                    app_version,
-                                    targetApplication.get('minVersion', 0),
-                                    targetApplication.get('maxVersion', '*'))
-                                if is_version_related:
-                                    add_plugin_item(
-                                        pluginItems, item, versionRange,
-                                        targetApplication, app_id=app_id,
-                                        api_ver=api_ver, app_ver=app_ver)
-                            else:
-                                add_plugin_item(
-                                    pluginItems, item, versionRange,
-                                    targetApplication, app_id=app_id,
-                                    api_ver=api_ver, app_ver=app_ver)
 
 
 def add_plugin_item(pluginItems, item, version, tA=None, app_id=None,
@@ -485,7 +486,8 @@ def main(args=None):
 
     write_addons_items(xml_tree, addons_records,
                        api_ver=args.api_version,
-                       app_id=args.app)
+                       app_id=args.app,
+                       app_ver=args.app_version)
     write_plugin_items(xml_tree, plugin_records,
                        api_ver=args.api_version,
                        app_id=args.app,

amo2kinto/tests/test_exporter.py

@@ -234,6 +234,78 @@ def test_addon_record_with_no_targetApplication_matching():
 """.decode('utf-8')
 
 
+def test_addon_record_with_no_targetApplication_version_matching():
+    xml_tree = etree.Element(
+        'blocklist',
+        xmlns="http://www.mozilla.org/2006/addons-blocklist",
+        lastupdate='1459262434336'
+    )
+
+    data = ADDONS_DATA.copy()
+    data['versionRange'] = [{
+        "targetApplication": [
+            {"guid": constants.FIREFOX_APPID,
+             "minVersion": "3.6",
+             "maxVersion": "3.6.*"}
+        ],
+        "minVersion": "0",
+        "maxVersion": "*",
+        "severity": 3
+    }]
+
+    exporter.write_addons_items(xml_tree, [data],
+                                constants.FIREFOX_APPID, app_ver="3.7")
+
+    result = etree.tostring(
+        etree.ElementTree(xml_tree),
+        pretty_print=True,
+        xml_declaration=True,
+        encoding='UTF-8').decode('utf-8')
+
+    assert result == b"""<?xml version='1.0' encoding='UTF-8'?>
+<blocklist lastupdate="1459262434336" \
+xmlns="http://www.mozilla.org/2006/addons-blocklist">
+  <emItems/>
+</blocklist>
+""".decode('utf-8')
+
+
+def test_addon_record_with_future_targetApplication_version_matching():
+    xml_tree = etree.Element(
+        'blocklist',
+        xmlns="http://www.mozilla.org/2006/addons-blocklist",
+        lastupdate='1459262434336'
+    )
+
+    exporter.write_addons_items(xml_tree, [ADDONS_DATA],
+                                constants.FIREFOX_APPID, app_ver="3.1")
+
+    result = etree.tostring(
+        etree.ElementTree(xml_tree),
+        pretty_print=True,
+        xml_declaration=True,
+        encoding='UTF-8').decode('utf-8')
+
+    assert result == b"""<?xml version='1.0' encoding='UTF-8'?>
+<blocklist lastupdate="1459262434336" \
+xmlns="http://www.mozilla.org/2006/addons-blocklist">
+  <emItems>
+    <emItem blockID="i454" id="[email protected]">
+      <prefs>
+        <pref>test.blocklist</pref>
+      </prefs>
+      <versionRange minVersion="0" maxVersion="*" severity="3">
+        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
+          <versionRange maxVersion="3.6.*" minVersion="3.6"/>
+        </targetApplication>
+      </versionRange>
+      <versionRange minVersion="0" maxVersion="*"/>
+    </emItem>
+  </emItems>
+</blocklist>
+""".decode('utf-8')
+
+
 def test_addon_record_group_by_blockID():
     xml_tree = etree.Element(
         'blocklist',
@@ -335,6 +407,10 @@ def test_between_does_not_fails_with_none():
     assert exporter.between(compare.version_int("46.2"), None, None)
 
 
+def test_get_related_targetApplication_accepts_versionRange_without_targetApplication():
+    assert exporter.get_related_targetApplication({}, None, None) is None
+
+
 def test_plugin_record():
     xml_tree = etree.Element(
         'blocklist',