Handling $ref-s to external files (canonical dereferencing)

[bardzusny]

Related: #20 .

This is feature request for being able to split swagger spec into several (or many...) files.

Right now we're able to reference only parameters available in the main (same) swagger spec file:

{
  "$ref": "#/definitions/myCar",
  "$ref": "#/parameters/myEngine"
}

What's lacking is possibility to reference external files, per http://json-schema.org/latest/json-schema-core.html#anchor30 :

{
  "$ref": "definitions.json#/myCar",
  "$ref": "parameters.json#/myEngine"
}

This would /greatly/ help to prevent main spec file from getting huge/hard to navigate.

Am I the only one who would want this? I already have some code laying around doing this, but outside of swaggerize-builder - basically just preprocessing .json file before it's passed on to it. I'm more than willing to try to package it into PR (probably extending current refresolver function in validator), but was also thinking about using some ready-made parsing library (couldn't find a good one though). Any ideas will be welcome.

[tlivings]

This should already be possible using the schemas option passed to builder.

[bardzusny]

@tlivings: yes, that seems to be what I'm after - and a nicer solution at that!

Only that I cannot get it to work. It's quite possible that I'm missing something, but it looks to me like enjoi issue/bug.

It seems that as long as any subSchemas are provided, enjoi fails to de-reference inline references in the main schema.

Here's the key part (lib/index.js, lines 52-54):

enjoi(require('./schema/swagger-spec/schemas/v2.0/schema.json'), schemas).validate(options.api, function (error) {
    assert.ifError(error);
});

As long as schemas is not undefined (any sub-schema is provided), inline references in schema.json are not resolved, and it throws up an obvious error:

AssertionError: Can not find schema reference: #/definitions/info.

I think that's it for this issue, but could you confirm that this looks like a material for another one at enjoi? I noticed this case (inline references in the main schema when subSchemas are provided) is not covered by tests, and looks very much like unwanted behavior (or at the very least makes it impossible to use the schemas option with the builder).

[tlivings]

Yeah that definitely doesn't sound right. Please open an issue and let's keep this one open as well until resolved.

[tlivings]

So there are two tests for this:

https://github.com/tlivings/enjoi/blob/master/test/test-enjoi.js#L64

and

https://github.com/tlivings/enjoi/blob/master/test/test-enjoi.js#L87

Which indicate this shouldn't be an issue in enjoi.

In swagaerize-builder we build a schemas object to pass that contains the main api schema + any sub schemas.

But you are right! There is a bug. Basically the sub schemas should contain the swagger schema for that particular validation call under a key # (similar to this: https://github.com/krakenjs/swaggerize-builder/blob/master/lib/validator.js#L14).

[bardzusny]

Should it really be needed for provided subSchemas to contain main schema (under # key?) for inline references to be resolved correctly? Isn't it just a workaround? Feel free to correct me. I submitted PR to enjoi that I think illustrates what I mean pretty clearly (including test), did you happen to take a look - https://github.com/tlivings/enjoi/pull/6 ?

Getting back to the issue at hand - schemas option passed to the builder is taken into account only during validation. What I was keen on, originally, was de-referencing api-docs before they're served, making it easy and straightforward to keep the spec divided into many files.

But, actually, the more I read, the more it seems like a bad idea. Right now Swagger UI still needs some things in the spec to be referenced, i.e. not accepting inline schema definitions ( https://github.com/swagger-api/swagger-js/issues/189 ). It's not handling multi-level references either (my observation: #/definitions/Cat would work, while #/definitions/Animals/Cat would not). Possibly other things - allowed in the spec, but not really handled correctly by the UI.

So maybe we should skip such pre-processing of api-docs, at least for now. Even then I'm getting doubtful such functionality falls within the scope of this project.

[tlivings]

This should now be resolved after the last couple of published to enjoi. Closing?

[jharmn]

@tlivings You might want to cross-check against recent updates to specs/examples/swagger-js:

This is en route for swagger-js: https://github.com/swagger-api/swagger-js/issues/417

Already updated in the Swagger spec: https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#referenceObject ...and examples https://github.com/swagger-api/swagger-spec/tree/master/examples/v2.0/yaml/petstore-separate

[tlivings]

It's sort of strange that they are using file names in $ref. That places requirements in validators to load files during validation.

[jharmn]

Right, but if you try to scale out microservices, you have to. So far, they've only supported http:// or inline refs. http:// is clearly problematic for different environments and forks, this solves that problem.

I'm also working on adding a command-line option to https://github.com/BigstickCarpet/swagger-parser ...but you can use that to do all of the resolution stuff (it already supports relative file resolution quite nicely).

[tlivings]

Actually $ref should only be an id followed by a path. Uris are simply used as ids typically.

Some validators might pre-resolve refs for validation afterwards, but it depends on the validator.

Placing an assumption that the validator will read a file or fetch a url is non standard imo.

[tlivings]

I should probably open an issue on swagger to discuss this.

[jharmn]

Reuse is already pretty clearly defined at: https://github.com/swagger-api/swagger-spec/blob/master/guidelines/REUSE.md

Also, Swagger specifies JSON Reference, which is very well defined (used as the supporting material for the addition).

Swagger-Parser, for instance, already supports all of this.

There's an open ticket (at request of @webron) to update this in the Swagger spec: https://github.com/swagger-api/swagger-spec/issues/388

[tlivings]

:+1: