From 63f08bace61b01d629bcdc3beaaf8870c7bfd993 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 18 Sep 2015 14:40:48 +0100 Subject: [PATCH 1/9] Fix the examples in the swagger API documentation to be valid JSON --- api/client-server/v1/presence.yaml | 6 +++--- api/client-server/v1/sync.yaml | 2 +- templating/matrix_templates/units.py | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/api/client-server/v1/presence.yaml b/api/client-server/v1/presence.yaml index 717e9c47..5684398b 100644 --- a/api/client-server/v1/presence.yaml +++ b/api/client-server/v1/presence.yaml @@ -101,7 +101,7 @@ paths: The length of time in milliseconds since an action was performed by this user. status_msg: - type: string + type: [string, "null"] description: The state message for this user if one was set. 404: description: |- @@ -185,7 +185,7 @@ paths: "last_active_ago": 395, "presence": "offline", "user_id": "@alice:matrix.org" - } + }, "type": "m.presence" }, { @@ -195,7 +195,7 @@ paths: "last_active_ago": 16874, "presence": "online", "user_id": "@marisa:matrix.org" - } + }, "type": "m.presence" } ] diff --git a/api/client-server/v1/sync.yaml b/api/client-server/v1/sync.yaml index 27c7073c..833c425a 100644 --- a/api/client-server/v1/sync.yaml +++ b/api/client-server/v1/sync.yaml @@ -343,7 +343,7 @@ paths: "body": "Hello world!", "msgtype": "m.text" }, - "room_id:" "!wfgy43Sg4a:matrix.org", + "room_id:": "!wfgy43Sg4a:matrix.org", "user_id": "@bob:matrix.org", "event_id": "$asfDuShaf7Gafaw:matrix.org", "type": "m.room.message" diff --git a/templating/matrix_templates/units.py b/templating/matrix_templates/units.py index 8e803d3b..3083a51e 100644 --- a/templating/matrix_templates/units.py +++ b/templating/matrix_templates/units.py @@ -87,6 +87,8 @@ def get_json_schema_object_fields(obj, enforce_title=False): desc += ( " Must be '%s'." % props[key_name]["enum"][0] ) + if isinstance(value_type, list): + value_type = " or ".join(value_type) fields["rows"].append({ "key": key_name, From 299a4356d49e9b694c1fb6340cda2669e8e30377 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 18 Sep 2015 16:10:21 +0100 Subject: [PATCH 2/9] Add script to check that the example responses in the swagger matches the examples. --- api/check_examples.py | 75 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 api/check_examples.py diff --git a/api/check_examples.py b/api/check_examples.py new file mode 100755 index 00000000..cec8ac09 --- /dev/null +++ b/api/check_examples.py @@ -0,0 +1,75 @@ +#! /usr/bin/env python + +import sys + +def import_error(module, package, debian, error): + sys.stderr.write(( + "Error importing %(module)s: %(error)r\n" + "To install %(module)s run:\n" + " pip install %(package)s\n" + "or on Debian run:\n" + " sudo apt-get install python-%(debian)s\n" + ) % locals()) + if __name__=='__main__': + sys.exit(1) + +try: + import jsonschema +except ImportError as e: + import_error("jsonschema", "jsonschema", "jsonschema", e) + raise + +try: + import yaml +except ImportError as e: + import_error("yaml", "PyYAML", "yaml", e) + raise + +import json +import os + +def check_response(filepath, request, code, response): + try: + example = json.loads( + response.get('examples', {}).get('application/json', "null") + ) + except Exception as e: + raise ValueError("Error parsing JSON example response for %r %r" % ( + request, code + ), e) + schema = response.get('schema') + fileurl = "file://" + os.path.abspath(filepath) + if example and schema: + try: + print ("Checking schema for: %r %r %r" % (filepath, request, code)) + # Setting the 'id' tells jsonschema where the file is so that it + # can correctly resolve relative $ref references in the schema + schema['id'] = fileurl + jsonschema.validate(example, schema) + except Exception as e: + raise ValueError("Error validating JSON schema for %r %r" %( + request, code + ), e) + + +def check_swagger_file(filepath): + with open(filepath) as f: + swagger = yaml.load(f) + + for path, path_api in swagger['paths'].items(): + for method, request_api in path_api.items(): + request = "%s %s" % (method.upper(), path) + try: + responses = request_api['responses'] + except KeyError: + raise ValueError("No responses for %r" % (request,)) + for code, response in responses.items(): + check_response(filepath, request, code, response) + + +if __name__=='__main__': + for path in sys.argv[1:]: + try: + check_swagger_file(path) + except Exception as e: + raise ValueError("Error checking file %r" % (path,), e) From 9896f98e2b63fd6204482bd9c7b8ba699487576d Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 18 Sep 2015 16:21:48 +0100 Subject: [PATCH 3/9] Search for yaml swagger files if check_examples.py is run without arguments. --- api/check_examples.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/api/check_examples.py b/api/check_examples.py index cec8ac09..9b2f1fe9 100755 --- a/api/check_examples.py +++ b/api/check_examples.py @@ -1,6 +1,9 @@ #! /usr/bin/env python import sys +import json +import os + def import_error(module, package, debian, error): sys.stderr.write(( @@ -10,7 +13,7 @@ def import_error(module, package, debian, error): "or on Debian run:\n" " sudo apt-get install python-%(debian)s\n" ) % locals()) - if __name__=='__main__': + if __name__ == '__main__': sys.exit(1) try: @@ -25,8 +28,6 @@ except ImportError as e: import_error("yaml", "PyYAML", "yaml", e) raise -import json -import os def check_response(filepath, request, code, response): try: @@ -47,7 +48,7 @@ def check_response(filepath, request, code, response): schema['id'] = fileurl jsonschema.validate(example, schema) except Exception as e: - raise ValueError("Error validating JSON schema for %r %r" %( + raise ValueError("Error validating JSON schema for %r %r" % ( request, code ), e) @@ -56,7 +57,7 @@ def check_swagger_file(filepath): with open(filepath) as f: swagger = yaml.load(f) - for path, path_api in swagger['paths'].items(): + for path, path_api in swagger.get('paths', {}).items(): for method, request_api in path_api.items(): request = "%s %s" % (method.upper(), path) try: @@ -67,8 +68,15 @@ def check_swagger_file(filepath): check_response(filepath, request, code, response) -if __name__=='__main__': - for path in sys.argv[1:]: +if __name__ == '__main__': + paths = sys.argv[1:] + if not paths: + paths = [] + for (root, dirs, files) in os.walk(os.curdir): + for filename in files: + if filename.endswith(".yaml"): + paths.append(os.path.join(root, filename)) + for path in paths: try: check_swagger_file(path) except Exception as e: From f827765ba1999073bc9357695b27bcc3e7e0c30c Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 18 Sep 2015 16:35:27 +0100 Subject: [PATCH 4/9] Make to code to skip checking swagger responses which don't have an application/json example clearer. --- api/check_examples.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/check_examples.py b/api/check_examples.py index 9b2f1fe9..00e75263 100755 --- a/api/check_examples.py +++ b/api/check_examples.py @@ -30,10 +30,11 @@ except ImportError as e: def check_response(filepath, request, code, response): + example = None try: - example = json.loads( - response.get('examples', {}).get('application/json', "null") - ) + example_json = response.get('examples', {}).get('application/json') + if example_json: + example = json.loads(example_json) except Exception as e: raise ValueError("Error parsing JSON example response for %r %r" % ( request, code From 6b5b8432b3761484f3e36910575f5fd844c09400 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 18 Sep 2015 17:26:10 +0100 Subject: [PATCH 5/9] Turn on code highlighting for HTTP api responses and add a code highlighting stylesheet for the specification. --- scripts/codehighlight.css | 6 ++++++ scripts/gendoc.py | 2 +- templating/matrix_templates/templates/http-api.tmpl | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 scripts/codehighlight.css diff --git a/scripts/codehighlight.css b/scripts/codehighlight.css new file mode 100644 index 00000000..5c9b0c36 --- /dev/null +++ b/scripts/codehighlight.css @@ -0,0 +1,6 @@ +pre.code .comment, code .comment { color: green } +pre.code .keyword, code .keyword { color: darkred; font-weight: bold } +pre.code .name.builtin, code .name.builtin { color: darkred; font-weight: bold } +pre.code .literal.number, code .literal.number { color: blue } +pre.code .name.tag, code .name.tag { color: darkgreen } +pre.code .literal.string, code .literal.string { color: darkblue } diff --git a/scripts/gendoc.py b/scripts/gendoc.py index 3521efed..a821aea7 100755 --- a/scripts/gendoc.py +++ b/scripts/gendoc.py @@ -12,7 +12,7 @@ import sys os.chdir(os.path.dirname(os.path.abspath(__file__))) stylesheets = { - "stylesheet_path": ["basic.css", "nature.css"] + "stylesheet_path": ["basic.css", "nature.css", "codehighlight.css"] } title_style_matchers = { diff --git a/templating/matrix_templates/templates/http-api.tmpl b/templating/matrix_templates/templates/http-api.tmpl index a0b25924..a03ffa7d 100644 --- a/templating/matrix_templates/templates/http-api.tmpl +++ b/templating/matrix_templates/templates/http-api.tmpl @@ -62,7 +62,9 @@ Response{{"s" if endpoint.example.responses|length > 1 else "" }}: {{res["description"]}} -Example:: +Example + +.. code:: json {{res["example"] | indent_block(2)}} From 52640eb2053e155c7f33468c0e6c74512b062b0b Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 21 Sep 2015 13:02:37 +0100 Subject: [PATCH 6/9] Add a python script for checking that the examples match the event schema. Does the same checks as check.sh, but is a *lot* faster making it suitable for using as a pre-commit hook. I don't suggest replacing check.sh since it's good to check that the schema works with multiple implementations of jsonschema. --- event-schemas/check_examples.py | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100755 event-schemas/check_examples.py diff --git a/event-schemas/check_examples.py b/event-schemas/check_examples.py new file mode 100755 index 00000000..5fc08f4a --- /dev/null +++ b/event-schemas/check_examples.py @@ -0,0 +1,62 @@ +#! /usr/bin/env python + +import sys +import json +import os + + +def import_error(module, package, debian, error): + sys.stderr.write(( + "Error importing %(module)s: %(error)r\n" + "To install %(module)s run:\n" + " pip install %(package)s\n" + "or on Debian run:\n" + " sudo apt-get install python-%(debian)s\n" + ) % locals()) + if __name__ == '__main__': + sys.exit(1) + +try: + import jsonschema +except ImportError as e: + import_error("jsonschema", "jsonschema", "jsonschema", e) + raise + +try: + import yaml +except ImportError as e: + import_error("yaml", "PyYAML", "yaml", e) + raise + + +def check_example_file(examplepath, schemapath): + with open(examplepath) as f: + example = yaml.load(f) + + with open(schemapath) as f: + schema = yaml.load(f) + + fileurl = "file://" + os.path.abspath(schemapath) + + print ("Checking schema for: %r %r" % (examplepath, schemapath)) + # Setting the 'id' tells jsonschema where the file is so that it + # can correctly resolve relative $ref references in the schema + schema['id'] = fileurl + try: + jsonschema.validate(example, schema) + except: + raise ValueError("Error validating JSON schema for %r %r" % ( + examplepath, schemapath + ), e) + + +def check_example_dir(exampledir, schemadir): + for root, dirs, files in os.walk(exampledir): + for filename in files: + examplepath = os.path.join(root, filename) + schemapath = examplepath.replace(exampledir, schemadir) + check_example_file(examplepath, schemapath) + + +if __name__ == '__main__': + check_example_dir("examples", "schema") From 6ba9b29b3b4ffe9d19c30302e36813b1835eb117 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 21 Sep 2015 15:04:03 +0100 Subject: [PATCH 7/9] Report all the errors in schemas/check_examples, not just the first error. --- event-schemas/check_examples.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/event-schemas/check_examples.py b/event-schemas/check_examples.py index 5fc08f4a..8675396e 100755 --- a/event-schemas/check_examples.py +++ b/event-schemas/check_examples.py @@ -3,6 +3,7 @@ import sys import json import os +import traceback def import_error(module, package, debian, error): @@ -44,19 +45,29 @@ def check_example_file(examplepath, schemapath): schema['id'] = fileurl try: jsonschema.validate(example, schema) - except: + except Exception as e: raise ValueError("Error validating JSON schema for %r %r" % ( examplepath, schemapath ), e) def check_example_dir(exampledir, schemadir): + errors = [] for root, dirs, files in os.walk(exampledir): for filename in files: examplepath = os.path.join(root, filename) schemapath = examplepath.replace(exampledir, schemadir) - check_example_file(examplepath, schemapath) - + try: + check_example_file(examplepath, schemapath) + except Exception as e: + errors.append(sys.exc_info()) + for (exc_type, exc_value, exc_trace) in errors: + traceback.print_exception(exc_type, exc_value, exc_trace) + if errors: + raise ValueError("Error validating examples") if __name__ == '__main__': - check_example_dir("examples", "schema") + try: + check_example_dir("examples", "schema") + except: + sys.exit(1) From 8974b2b67bbc2acf0c482c646be6b07b230a1a15 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 21 Sep 2015 15:05:10 +0100 Subject: [PATCH 8/9] Skip files that start with ".", e.g. vim swp files. --- event-schemas/check_examples.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/event-schemas/check_examples.py b/event-schemas/check_examples.py index 8675396e..e54d3a1c 100755 --- a/event-schemas/check_examples.py +++ b/event-schemas/check_examples.py @@ -55,6 +55,9 @@ def check_example_dir(exampledir, schemadir): errors = [] for root, dirs, files in os.walk(exampledir): for filename in files: + if filename.startswith("."): + # Skip over any vim .swp files. + continue examplepath = os.path.join(root, filename) schemapath = examplepath.replace(exampledir, schemadir) try: From 2c31731262b3258307dd7e5eb8364d71817c4749 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 21 Sep 2015 15:16:02 +0100 Subject: [PATCH 9/9] Add the jenkins command to source control so that we can update it without having to fiddle with the jenkins UI. It also allow us to move files without breaking the CI since we won't be hard coding the locations of scripts in the jenkins UI. --- jenkins.sh | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100755 jenkins.sh diff --git a/jenkins.sh b/jenkins.sh new file mode 100755 index 00000000..e1043644 --- /dev/null +++ b/jenkins.sh @@ -0,0 +1,9 @@ +#! /bin/bash + +set -ex + +(cd event-schemas/ && ./check_examples.py) +(cd api && ./check_examples.py) +(cd scripts && ./gendoc.py) +(cd api && npm install && node validator.js -s "client-server/v1") +(cd event-schemas/ && ./check.sh)