Merge branch '471-lint-check-unknown-keys' into 'master'

check for unknown app fields and build flags when parsing yml

Closes #471

See merge request fdroid/fdroidserver!554

fdroidserver/metadata.py

@@ -3,6 +3,7 @@
 # metadata.py - part of the FDroid server tools
 # Copyright (C) 2013, Ciaran Gultnieks, ciaran@ciarang.com
 # Copyright (C) 2013-2014 Daniel Martí <mvdan@mvdan.cc>
+# Copyright (C) 2017-2018 Michael Pöhn <michael.poehn@fsfe.org>
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Affero General Public License as published by
@@ -99,6 +100,57 @@ app_fields = set([
     'builds',    # For formats that do builds as a list
 ])
 
+yaml_app_field_order = [
+    'Disabled',
+    'AntiFeatures',
+    'Provides',
+    'Categories',
+    'License',
+    'AuthorName',
+    'AuthorEmail',
+    'AuthorWebSite',
+    'WebSite',
+    'SourceCode',
+    'IssueTracker',
+    'Translation',
+    'Changelog',
+    'Donate',
+    'FlattrID',
+    'LiberapayID',
+    'Bitcoin',
+    'Litecoin',
+    '\n',
+    'Name',
+    'AutoName',
+    'Summary',
+    'Description',
+    '\n',
+    'RequiresRoot',
+    '\n',
+    'RepoType',
+    'Repo',
+    'Binaries',
+    '\n',
+    'Builds',
+    '\n',
+    'MaintainerNotes',
+    '\n',
+    'ArchivePolicy',
+    'AutoUpdateMode',
+    'UpdateCheckMode',
+    'UpdateCheckIgnore',
+    'VercodeOperation',
+    'UpdateCheckName',
+    'UpdateCheckData',
+    'CurrentVersion',
+    'CurrentVersionCode',
+    '\n',
+    'NoSourceSince',
+]
+
+
+yaml_app_fields = [x for x in yaml_app_field_order if x != '\n']
+
 
 class App(dict):
 
@@ -1022,7 +1074,26 @@ def parse_json_metadata(mf, app):
 
 def parse_yaml_metadata(mf, app):
     yamldata = yaml.load(mf, Loader=YamlLoader)
+
     if yamldata:
+        for field in yamldata:
+            if field not in yaml_app_fields:
+                warn_or_exception(_("Unrecognised app field '{fieldname}' "
+                                    "in '{path}'").format(fieldname=field,
+                                                          path=mf.name))
+        if yamldata.get('Builds', None):
+            for build in yamldata.get('Builds', []):
+                # put all build flag keywords into a set to avoid
+                # excessive looping action
+                build_flag_set = set()
+                for build_flag in build.keys():
+                    build_flag_set.add(build_flag)
+                for build_flag in build_flag_set:
+                    if build_flag not in build_flags:
+                        warn_or_exception(
+                            _("Unrecognised build flag '{build_flag}' "
+                              "in '{path}'").format(build_flag=build_flag,
+                                                    path=mf.name))
         app.update(yamldata)
     return app
 
@@ -1133,54 +1204,6 @@ def write_yaml(mf, app):
 
         return builds
 
-    yaml_app_field_order = [
-        'Disabled',
-        'AntiFeatures',
-        'Provides',
-        'Categories',
-        'License',
-        'AuthorName',
-        'AuthorEmail',
-        'AuthorWebSite',
-        'WebSite',
-        'SourceCode',
-        'IssueTracker',
-        'Translation',
-        'Changelog',
-        'Donate',
-        'FlattrID',
-        'LiberapayID',
-        'Bitcoin',
-        'Litecoin',
-        '\n',
-        'Name',
-        'AutoName',
-        'Summary',
-        'Description',
-        '\n',
-        'RequiresRoot',
-        '\n',
-        'RepoType',
-        'Repo',
-        'Binaries',
-        '\n',
-        'Builds',
-        '\n',
-        'MaintainerNotes',
-        '\n',
-        'ArchivePolicy',
-        'AutoUpdateMode',
-        'UpdateCheckMode',
-        'UpdateCheckIgnore',
-        'VercodeOperation',
-        'UpdateCheckName',
-        'UpdateCheckData',
-        'CurrentVersion',
-        'CurrentVersionCode',
-        '\n',
-        'NoSourceSince',
-    ]
-
     yaml_app = _app_to_yaml(app)
     ruamel.yaml.round_trip_dump(yaml_app, mf, indent=4, block_seq_indent=2)
 

tests/metadata.TestCase

@@ -2,6 +2,7 @@
 
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
+import io
 import glob
 import inspect
 import logging
@@ -13,6 +14,8 @@ import sys
 import unittest
 import yaml
 import tempfile
+import textwrap
+from unittest import mock
 
 localmodule = os.path.realpath(
     os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..'))
@@ -22,6 +25,7 @@ if localmodule not in sys.path:
 
 import fdroidserver.common
 import fdroidserver.metadata
+from fdroidserver.exception import MetaDataException
 
 
 class MetadataTest(unittest.TestCase):
@@ -139,6 +143,28 @@ class MetadataTest(unittest.TestCase):
             allappids.append(appid)
         self.assertEqual(randomlist, allappids)
 
+    def test_parse_yaml_metadata_unknown_app_field(self):
+        mf = io.StringIO(textwrap.dedent("""\
+            AutoName: F-Droid
+            RepoType: git
+            Builds: []
+            bad: value"""))
+        mf.name = 'mock_filename.yaml'
+        with mock.patch('fdroidserver.metadata.warnings_action', 'error'):
+            with self.assertRaises(MetaDataException):
+                fdroidserver.metadata.parse_yaml_metadata(mf, {})
+
+    def test_parse_yaml_metadata_unknown_build_flag(self):
+        mf = io.StringIO(textwrap.dedent("""\
+            AutoName: F-Droid
+            RepoType: git
+            Builds:
+              - bad: value"""))
+        mf.name = 'mock_filename.yaml'
+        with mock.patch('fdroidserver.metadata.warnings_action', 'error'):
+            with self.assertRaises(MetaDataException):
+                fdroidserver.metadata.parse_yaml_metadata(mf, {})
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))

tests/metadata/org.videolan.vlc.yml

@@ -17,7 +17,7 @@ Description: |
 RepoType: git
 Repo: git://git.videolan.org/vlc-ports/android.git
 
-builds:
+Builds:
   - versionName: 0.0.11-ARMv7
     versionCode: 110
     commit: 0.0.11