Skip to content

Include JS OpenAPI from app.json in /api responses#1874

Merged
eddyashton merged 14 commits into
microsoft:masterfrom
eddyashton:js_dynamic_openapi
Nov 12, 2020
Merged

Include JS OpenAPI from app.json in /api responses#1874
eddyashton merged 14 commits into
microsoft:masterfrom
eddyashton:js_dynamic_openapi

Conversation

@eddyashton

Copy link
Copy Markdown
Member

The endpoints table now stores the openapi elements defined for each endpoint in app.json, and merges these into the resulting document during build_api.

The modules.py test retrieves the OpenAPI docs for the tested JS apps, and checks that they're valid OpenAPI. This required a few minor changes to the existing app.json files.

To support this I've updated the Lua JSON converters, so they can correctly roundtrip an empty object or empty array. This is done by adding a magic field to the metatable on the pushed table. Its an ugly solution, but does what we need for now, while we're still reliant on Lua.

@eddyashton eddyashton requested a review from a team as a code owner November 11, 2020 13:35
@ghost

ghost commented Nov 11, 2020

Copy link
Copy Markdown

js_dynamic_openapi@15316 aka 20201111.20 vs master ewma over 50 builds from 14780 to 15305
images

@eddyashton eddyashton merged commit adc5ff5 into microsoft:master Nov 12, 2020
@eddyashton eddyashton deleted the js_dynamic_openapi branch November 12, 2020 09:12
Comment thread tests/npm-app/app.json
"application/json": {
"schema": {
"items": {
"items": {},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validator we're using doesn't like this empty object - it doesn't think this matches the schema for a JSON schema object. So we say the same thing ("the items have no further type restriction") by omitting the field entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants