Merge pull request #5022 from scartmell-arm/mbed-config-json-schema

Add JSON schema based validation to mbed config script

Author: cmonr

requirements.txt

@@ -12,3 +12,4 @@ mbed-greentea>=0.2.24
 beautifulsoup4>=4
 fuzzywuzzy>=0.11
 pyelftools>=0.24
+jsonschema>=2.6

tools/config/__init__.py

@@ -16,6 +16,9 @@
 """
 
 from copy import deepcopy
+from six import moves
+import json
+import six
 import os
 from os.path import dirname, abspath, exists, join, isabs
 import sys
@@ -24,6 +27,7 @@
 from intelhex import IntelHex
 from jinja2 import FileSystemLoader, StrictUndefined
 from jinja2.environment import Environment
+from jsonschema import Draft4Validator, RefResolver
 # Implementation of mbed configuration mechanism
 from tools.utils import json_file_to_dict, intelhex_offset
 from tools.arm_pack_manager import Cache
@@ -341,12 +345,6 @@ def _process_macros(mlist, macros, unit_name, unit_kind):
         macros[macro.macro_name] = macro
 
 
-def check_dict_types(dict, type_dict, dict_loc):
-    for key, value in dict.iteritems():
-        if not isinstance(value, type_dict[key]):
-            raise ConfigException("The value of %s.%s is not of type %s" %
-                                  (dict_loc, key, type_dict[key].__name__))
-
 Region = namedtuple("Region", "name start size active filename")
 
 class Config(object):
@@ -357,17 +355,8 @@ class Config(object):
     __mbed_app_config_name = "mbed_app.json"
     __mbed_lib_config_name = "mbed_lib.json"
 
-    # Allowed keys in configuration dictionaries, and their types
-    # (targets can have any kind of keys, so this validation is not applicable
-    # to them)
-    __allowed_keys = {
-        "library": {"name": str, "config": dict, "target_overrides": dict,
-                    "macros": list, "__config_path": str},
-        "application": {"config": dict, "target_overrides": dict,
-                        "macros": list, "__config_path": str,
-                        "artifact_name": str}
-    }
-
+    __unused_overrides = set(["target.bootloader_img", "target.restrict_size",
+                              "target.mbed_app_start", "target.mbed_app_size"])
 
     # Allowed features in configurations
     __allowed_features = [
@@ -420,15 +409,24 @@ def __init__(self, tgt, top_level_dirs=None, app_config=None):
                 ConfigException("Could not parse mbed app configuration from %s"
                                 % self.app_config_location))
 
-        # Check the keys in the application configuration data
-        unknown_keys = set(self.app_config_data.keys()) - \
-                       set(self.__allowed_keys["application"].keys())
-        if unknown_keys:
-            raise ConfigException("Unknown key(s) '%s' in %s" %
-                                  (",".join(unknown_keys),
-                                   self.__mbed_app_config_name))
-        check_dict_types(self.app_config_data, self.__allowed_keys["application"],
-                         "app-config")
+
+        if self.app_config_location is not None:
+            # Validate the format of the JSON file based on schema_app.json
+            schema_root = os.path.dirname(os.path.abspath(__file__))
+            schema_path = os.path.join(schema_root, "schema_app.json")
+            schema      = json_file_to_dict(schema_path)
+
+            url = moves.urllib.request.pathname2url(schema_path)
+            uri = moves.urllib_parse.urljoin("file://", url)
+
+            resolver = RefResolver(uri, schema)
+            validator = Draft4Validator(schema, resolver=resolver)
+
+            errors = sorted(validator.iter_errors(self.app_config_data))
+
+            if errors:
+                raise ConfigException(",".join(x.message for x in errors))
+
         # Update the list of targets with the ones defined in the application
         # config, if applicable
         self.lib_config_data = {}
@@ -478,11 +476,24 @@ def add_config_files(self, flist):
                 sys.stderr.write(str(exc) + "\n")
                 continue
 
+            # Validate the format of the JSON file based on the schema_lib.json
+            schema_root = os.path.dirname(os.path.abspath(__file__))
+            schema_path = os.path.join(schema_root, "schema_lib.json")
+            schema_file = json_file_to_dict(schema_path)
+
+            url = moves.urllib.request.pathname2url(schema_path)
+            uri = moves.urllib_parse.urljoin("file://", url)
+
+            resolver = RefResolver(uri, schema_file)
+            validator = Draft4Validator(schema_file, resolver=resolver)
+
+            errors = sorted(validator.iter_errors(cfg))
+
+            if errors:
+                raise ConfigException(",".join(x.message for x in errors))
+
             cfg["__config_path"] = full_path
 
-            if "name" not in cfg:
-                raise ConfigException(
-                    "Library configured at %s has no name field." % full_path)
             # If there's already a configuration for a module with the same
             # name, exit with error
             if self.lib_config_data.has_key(cfg["name"]):
@@ -781,12 +792,6 @@ def get_lib_config_data(self):
         """
         all_params, macros = {}, {}
         for lib_name, lib_data in self.lib_config_data.items():
-            unknown_keys = (set(lib_data.keys()) -
-                            set(self.__allowed_keys["library"].keys()))
-            if unknown_keys:
-                raise ConfigException("Unknown key(s) '%s' in %s" %
-                                      (",".join(unknown_keys), lib_name))
-            check_dict_types(lib_data, self.__allowed_keys["library"], lib_name)
             all_params.update(self._process_config_and_overrides(lib_data, {},
                                                                  lib_name,
                                                                  "library"))

tools/config/definitions.json

@@ -0,0 +1,95 @@
+{
+  "name_definition": {
+    "description": "Name of the library",
+    "type": "string",
+    "items": {
+      "type": "string"
+    }
+  },
+  "macro_definition": {
+    "description": "A list of extra macros that will be defined when compiling a project that includes this library.",
+    "type": "array",
+    "items": {
+      "type": "string",
+      "pattern": "(^[\\w]+$|^[\\w]+=.+$)"
+    }
+  },
+  "config_definition": {
+    "description": "List of configuration parameters",
+    "type": "object",
+    "patternProperties": {
+      "^[^ ]+$": {
+        "$ref": "#/config_parameter_base"
+      }
+    },
+    "additionalProperties": false
+  },
+  "target_overrides_definition": {
+    "description": "List of overrides for specific targets",
+    "type": "object",
+    "patternProperties": {
+      "\\*": {
+        "type": "object",
+        "patternProperties": {
+          ".*\\..*": {}
+        },
+        "additionalProperties": false
+      },
+      "^\\S+$": {
+        "$ref": "#/target_override_entry"
+      }
+    },
+    "additionalProperties": false
+  },
+  "config_parameter_long": {
+    "type": "object",
+    "properties": {
+      "help": {
+        "description": "An optional help message that describes the purpose of the parameter",
+        "type": "string"
+      },
+      "value": {
+        "description": "An optional field that defines the value of the parameter",
+        "type": [
+          "integer",
+          "string",
+          "boolean",
+          "null"
+        ]
+      },
+      "required": {
+        "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)",
+        "type": "boolean"
+      },
+      "macro_name": {
+        "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it",
+        "type": "string"
+      }
+    }
+  },
+  "config_parameter_short": {
+    "type": [
+      "string",
+      "integer",
+      "boolean",
+      "null"
+    ]
+  },
+  "config_parameter_base": {
+    "oneOf": [
+      {
+        "$ref": "#/config_parameter_long"
+      },
+      {
+        "$ref": "#/config_parameter_short"
+      }
+    ]
+  },
+  "target_override_entry": {
+    "type": "object",
+    "patternProperties": {
+      "^\\S+$": {}
+    },
+    "additionalProperties": false
+  }
+}

tools/config/schema_app.json

@@ -0,0 +1,24 @@
+{
+  "$schema": "http://json-schema.org/draft-06/schema#",
+  "title": "Mbed Library Schema",
+  "description": "Configuration file for an mbed application",
+  "type": "object",
+  "properties": {
+    "name": {
+      "$ref": "definitions.json#/name_definition"
+    },
+    "config": {
+      "$ref": "definitions.json#/config_definition"
+    },
+    "target_overrides": {
+      "$ref": "definitions.json#/target_overrides_definition"
+    },
+    "macros": {
+      "$ref": "definitions.json#/macro_definition"
+    },
+    "artifact_name": {
+      "type": "string"
+    }
+  },
+  "additionalProperties": false
+}

tools/config/schema_lib.json

@@ -0,0 +1,24 @@
+{
+  "$schema": "http://json-schema.org/draft-06/schema#",
+  "title": "Mbed Library Schema",
+  "description": "Configuration file for an mbed library",
+  "type": "object",
+  "properties": {
+    "name": {
+      "$ref": "definitions.json#/name_definition"
+    },
+    "config": {
+      "$ref": "definitions.json#/config_definition"
+    },
+    "target_overrides": {
+      "$ref": "definitions.json#/target_overrides_definition"
+    },
+    "macros": {
+      "$ref": "definitions.json#/macro_definition"
+    }
+  },
+  "required": [
+    "name"
+  ],
+  "additionalProperties": false
+}

tools/test/config/config_test.py

@@ -108,7 +108,8 @@ def test_init_app_config(target):
 
         config = Config(target, app_config=app_config)
 
-        mock_json_file_to_dict.assert_called_with(app_config)
+        mock_json_file_to_dict.assert_any_call("app_config")
+
         assert config.app_config_data == mock_return
 
 
@@ -149,7 +150,7 @@ def test_init_no_app_config_with_dir(target):
         config = Config(target, [directory])
 
         mock_isfile.assert_called_with(path)
-        mock_json_file_to_dict.assert_called_once_with(path)
+        mock_json_file_to_dict.assert_any_call(path)
         assert config.app_config_data == mock_return
 
 
@@ -171,5 +172,5 @@ def test_init_override_app_config(target):
 
         config = Config(target, [directory], app_config=app_config)
 
-        mock_json_file_to_dict.assert_called_once_with(app_config)
+        mock_json_file_to_dict.assert_any_call(app_config)
         assert config.app_config_data == mock_return

tools/test/config/invalid_key/test_data.json

@@ -1,5 +1,5 @@
 {
     "K64F": {
-        "exception_msg": "Unknown key(s)"
+        "exception_msg": "Additional properties are not allowed ('unknown_key' was unexpected)"
     }
 }

tools/test/config/invalid_key_lib/test_data.json

@@ -1,5 +1,5 @@
 {
     "K64F": {
-        "exception_msg": "Unknown key(s)"
+        "exception_msg": "Additional properties are not allowed ('unknown_key' was unexpected)"
     }
 }