Use jsonschema for add-on manifests

.travis.sh

@@ -2,11 +2,11 @@
 
 set -e
 
-echo "running unit tests..."
-nosetests ./tests
-
 echo "building ui..."
 ./tools/build_ui.sh
 
+echo "running unit tests..."
+nosetests ./tests
+
 echo "linting..."
 ./tools/lint.sh

aqt/addons.py

@@ -8,6 +8,8 @@ import zipfile
 from collections import defaultdict
 import markdown
 from send2trash import send2trash
+import jsonschema
+from jsonschema.exceptions import ValidationError
 
 from aqt.qt import *
 from aqt.utils import showInfo, openFolder, isWin, openLink, \
@@ -24,12 +26,19 @@ from anki.sync import AnkiRequestsClient
 class AddonManager:
 
     ext = ".ankiaddon"
-    # todo?: use jsonschema package
     _manifest_schema = {
-        "package": {"type": str, "req": True, "meta": False},
-        "name": {"type": str, "req": True, "meta": True},
-        "mod": {"type": int, "req": False, "meta": True},
-        "conflicts": {"type": list, "req": False, "meta": True}
+        "type": "object",
+        "properties": {
+            "package": {"type": "string", "meta": False},
+            "name": {"type": "string", "meta": True},
+            "mod": {"type": "number", "meta": True},
+            "conflicts": {
+                "type": "array",
+                "items": {"type": "string"},
+                "meta": True
+            }
+        },
+        "required": ["package", "name"]
     }
 
     def __init__(self, mw):
@@ -161,18 +170,15 @@ and have been disabled: %(found)s") % dict(name=self.addonName(dir), found=addon
     # Installing and deleting add-ons
     ######################################################################
 
-    def _readManifestFile(self, zfile):
+    def readManifestFile(self, zfile):
         try:
             with zfile.open("manifest.json") as f:
                 data = json.loads(f.read())
-            manifest = {}  # build new manifest from recognized keys
-            for key, attrs in self._manifest_schema.items():
-                if not attrs["req"] and key not in data:
-                    continue
-                val = data[key]
-                assert isinstance(val, attrs["type"])
-                manifest[key] = val
-        except (KeyError, json.decoder.JSONDecodeError, AssertionError):
+            jsonschema.validate(data, self._manifest_schema)
+            # build new manifest from recognized keys
+            schema = self._manifest_schema["properties"]
+            manifest = {key: data[key] for key in data.keys() & schema.keys()}
+        except (KeyError, json.decoder.JSONDecodeError, ValidationError):
             # raised for missing manifest, invalid json, missing/invalid keys
             return {}
         return manifest
@@ -187,7 +193,7 @@ and have been disabled: %(found)s") % dict(name=self.addonName(dir), found=addon
             return False, "zip"
         
         with zfile:
-            file_manifest = self._readManifestFile(zfile)
+            file_manifest = self.readManifestFile(zfile)
             if manifest:
                 file_manifest.update(manifest)
             manifest = file_manifest
@@ -200,7 +206,7 @@ and have been disabled: %(found)s") % dict(name=self.addonName(dir), found=addon
             meta = self.addonMeta(package)
             self._install(package, zfile)
         
-        schema = self._manifest_schema
+        schema = self._manifest_schema["properties"]
         manifest_meta = {k: v for k, v in manifest.items()
                          if k in schema and schema[k]["meta"]}
         meta.update(manifest_meta)

requirements.txt

@@ -4,5 +4,6 @@ pyaudio
 requests
 decorator
 markdown
+jsonschema
 psutil; sys_platform == "win32"
 distro; sys_platform != "win32" and sys_platform != "darwin"

tests/test_addons.py

@@ -0,0 +1,83 @@
+import os.path
+from nose.tools import assert_equals
+from mock import MagicMock
+from tempfile import TemporaryDirectory
+from zipfile import ZipFile
+
+from aqt.addons import AddonManager
+
+
+def test_readMinimalManifest():
+    assertReadManifest(
+        '{"package": "yes", "name": "no"}',
+        {"package": "yes", "name": "no"}
+    )
+
+
+def test_readExtraKeys():
+    assertReadManifest(
+        '{"package": "a", "name": "b", "mod": 3, "conflicts": ["d", "e"]}',
+        {"package": "a", "name": "b", "mod": 3, "conflicts": ["d", "e"]}
+    )
+
+
+def test_invalidManifest():
+    assertReadManifest(
+        '{"one": 1}',
+        {}
+    )
+
+
+def test_mustHaveName():
+    assertReadManifest(
+        '{"package": "something"}',
+        {}
+    )
+
+
+def test_mustHavePackage():
+    assertReadManifest(
+        '{"name": "something"}',
+        {}
+    )
+
+
+def test_invalidJson():
+    assertReadManifest(
+        'this is not a JSON dictionary',
+        {}
+    )
+
+
+def test_missingManifest():
+    assertReadManifest(
+        '{"package": "what", "name": "ever"}',
+        {},
+        nameInZip="not-manifest.bin"
+    )
+
+
+def test_ignoreExtraKeys():
+    assertReadManifest(
+        '{"package": "a", "name": "b", "game": "c"}',
+        {"package": "a", "name": "b"}
+    )
+
+
+def test_conflictsMustBeStrings():
+    assertReadManifest(
+        '{"package": "a", "name": "b", "conflicts": ["c", 4, {"d": "e"}]}',
+        {}
+    )
+
+
+def assertReadManifest(contents, expectedManifest, nameInZip="manifest.json"):
+    with TemporaryDirectory() as td:
+        zfn = os.path.join(td, "addon.zip")
+        with ZipFile(zfn, "w") as zfile:
+            zfile.writestr(nameInZip, contents)
+
+        adm = AddonManager(MagicMock())
+
+        with ZipFile(zfn, "r") as zfile:
+            assert_equals(adm.readManifestFile(zfile), expectedManifest)