Lengthy resolution / validation on large / complex OAS 2.0 definitions

[MikeRalphson]

As promised, when resolving / validating this definition https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json

using Sway 2.0.5 from npm and this snippet

const SwaggerApi = require('sway');
const util = require('util');
const yaml = require('js-yaml');
const fs = require('fs');

const input = process.argv[2];

SwaggerApi.create({definition: input, jsonRefs: {resolveCirculars:true} })
  .then(function (api) {
     let drr = api.definitionRemotesResolved;
     fs.writeFileSync('./resolved.yaml',yaml.safeDump(drr),'utf8');
     //console.log(util.inspect(api));
     let val = api.validate();
     console.log(util.inspect(val.errors));
  }, function (err) {
     console.error(err.stack);
});

I get the following result

RangeError: Maximum call stack size exceeded
    at doWalk (/home/mike/nodejs/testsway/node_modules/sway/lib/helpers.js:599:19)
    at /home/mike/nodejs/testsway/node_modules/sway/lib/helpers.js:610:11
    at /home/mike/nodejs/testsway/node_modules/lodash/lodash.js:4911:15
    at baseForOwn (/home/mike/nodejs/testsway/node_modules/lodash/lodash.js:2996:24)
    at /home/mike/nodejs/testsway/node_modules/lodash/lodash.js:4880:18
    at Function.forEach (/home/mike/nodejs/testsway/node_modules/lodash/lodash.js:9344:14)
    at doWalk (/home/mike/nodejs/testsway/node_modules/sway/lib/helpers.js:609:11)
    at /home/mike/nodejs/testsway/node_modules/sway/lib/helpers.js:610:11
    at /home/mike/nodejs/testsway/node_modules/lodash/lodash.js:4911:15
    at baseForOwn (/home/mike/nodejs/testsway/node_modules/lodash/lodash.js:2996:24)

with the following timings

real    0m59.217s
user    1m3.064s
sys 0m3.795s

Impact: in APIs.guru's Travis CI setup, we're having to skip validation of this definition, as otherwise the job doesn't complete in time.

[MikeRalphson]

Again, as requested, comparative time for oas-kit to fetch, resolve, convert and validate:

time node oas-validate.js --verbose --verbose --resolve https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json
Gathering...
GET https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json
Finished resolution! 
Finished resolution! 
https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json
#/paths/~1Forum~1GetTopicsPaged~1{page}~1{pageSize}~1{group}~1{sort}~1{quickDate}~1{categoryFilter}~1/get/parameters/6/schema
Invalid type integer for format byte
  Bungie.Net API 2.3.2
  www.bungie.net 

Failures:
https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json

Tests: 0 passing, 1 failing, 0 warnings

real    0m2.567s
user    0m2.899s
sys 0m0.143s

[whitlockjc]

If you remove options.jsonRefs.resolveCircular, it looks a good bit more reasonable: 6.77s user 0.19s system 104% cpu 6.665 total options.jsonRefs.resolveCircular shouldn't be necessary, but it also shouldn't be breaking things either. Even once that is fixed, the performance difference is noticeable but I'm not 100% sure where the cost is coming. From a resolution perspective, we do resolve references twice, but at different levels, and that is a likely culprit. (The idea here was to have a way to resolve all remotes first to get a locals-only representation for things like swagger-ui or similar, and one fully-resolved.) From a validation perspective, #160 has been around for a while and that could play a role in the speed.

Thanks for the findings.

[MikeRalphson]

Thanks. I think in apis.guru we use the same options for all definitions, and it's likely some other ones need resolveCircular. I'll look at making it optional. We also only need a "locals only" rendering, not every $ref resolved.

[whitlockjc]

I wonder what the detail is in resolution detail/scope between json-refs (sway) and oas-kit. I hope we're comparing apples to apples.

[MikeRalphson]

Yes oas-kit only resolves external references, leaving "locals only". It also bails on the first validation error, unlike Sway. But then, it is also doing the work of converting OAS 2.0 to 3.0.0. Definitely some fruit disparity here though :smile:

[MikeRalphson]

I'm going to try and get another of our dependencies, api-spec-converter to update to Sway ^2.0.5 as that seems to be the main culprit now.

[whitlockjc]

sway isn't without it's warts and I'd love to fix them, like #160. Thanks again for taking the time.