Support external schema reference about open-api

[MugeSo]

For SRP, local file loading should be omitted.
Just requires passing {id : SchemaObject} hash to jsonschema via express-openapi-validation or express-openapi-response-validation.
And also, of course, I want to pass it via initialize argument of express-openapi.

[jsdevel]

Can you try this?

$ref: 'http://json-schema.org/draft-04/schema#'

[MugeSo]

I got the same error 😢

[jsdevel]

@MugeSo this will likely take a decent amount of work. Have a look at https://github.com/tdegrunt/jsonschema#dereferencing-schemas. As you can see, we'd need to update each module that uses jsonschema.

Given that, one of the designs of openapi is to be a limited subset of jsonschema. I'm curious to know what intentions you have in referencing http://json-schema.org/schema?

[MugeSo]

@jsdevel
At first, i know i should use 'http://json-schema.org/draft-04/schema#', 'http://json-schema.org/schema' is my mistake.

The reason why I reference it is to contain external defined schema in api-doc is nonsense.
When api-doc contains them, it get large wastefully.

And as i said in #17 (comment) , i don't need dynamic loading.
So, deferencing-schemas is not needed. just call v.addSchema.

Like:

Object.keys(externalSchemas).forEach(function(key) {
    v.addSchema(schemas[key], key);
});

If you want, I send PR to each modules 😉

[jsdevel]

PRs are welcomed!

[MugeSo]

@jsdevel
I fond $ref of parameters object are also unresolved.
e.g:

parameters = [
   { $ref: '#/parameters/myparameter' }
]

And I considered about it and this again.

Now I think it's better to use json-refs or something to resolve any references(not only schema) in api doc.

How about?

[jsdevel]

So $ref actually isn't listed as a valid property in parameters. See the spec.

[jsdevel]

I usually use code to overcome this shortcoming of openapi/swagger

parameters: [
  myParameterModule.myParameter
]

myParameterModule looks like this:

  myParameter: {
    name: 'foo',
    type: 'string',
    in: 'query'
  }

[MugeSo]

$ref is valid.

See parameters property.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#operationObject
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#path-item-object

https://github.com/OAI/OpenAPI-Specification/blob/master/schemas/v2.0/schema.json#L1482

[jsdevel]

I see it: https://github.com/OAI/OpenAPI-Specification/blob/master/schemas/v2.0/schema.json#L1482

Then it should work. Have you tried this?

parameters = [
   { $ref: '#/definitions/myparameter' }
]

[MugeSo]

It's wrong. parameters property of Swagger Object is for define parameters.
See:
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#fixed-fields
https://github.com/OAI/OpenAPI-Specification/blob/master/schemas/v2.0/schema.json#L186

[MugeSo]

And it doesn't work because openapi-jsonschema-parameters doesn't handle that.

[jsdevel]

@MugeSo can you open a PR to address this? This is a good find.

[MugeSo]

Also resposes property of `Operation Object'.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#responses-object

can you open a PR to address this? This is a good find.

I'll try it.
But it takes some time because it seems complex.
(And it's 3:25am here)

[jsdevel]

(And it's 3:25am here)

Go to bed! lol

[jsdevel]

Yea, I feel this would be a major addition to the framework. Perhaps we should track responses and parameters separately?

[jsdevel]

json-ref-lite looks pretty cool too.

[MugeSo]

I've seen json-ref-lite.
It' nice except that it resolve reference start with http: only through network.
This causes problem when schema host server is down.

However, unfortunately, other library i found is async.
If we async one, openapi.initialize get async too :(

[MugeSo]

OTTOMH

var jsonRef = requrie('json-ref-lite');

var externalSchemas = args.externalSchemas || [];
// create id based reference map
var ids = jsonRef.findId(externalSchemas.concat(apiDoc.definitions));
var resolved = copy(methodDoc);// json-ref-lite is destractive
jsonRef.replace(resolved, ids, apiDoc);

[MugeSo]

The reason why use only apiDoc.definitions is Reference Object supports only canonical reference.

schema properties are JSON Schema Object and it has own $ref definition in spec.

$ref - As a JSON Reference

[MugeSo]

Sorry, in the previous code, ids include all ids in externalSchemas.
And external thing not only schema.

var jsonRef = requrie('json-ref-lite');

// id -> json object map
var externalJsons = copy(args.externalJsons) || {};

// create id based reference map
var ids = shallowMerge(jsonRef.findId(apiDoc.definitions), externalSchemas);
var resolvedMethodDoc = jsonRef.replace(copy(methodDoc), ids, apiDoc); // json-ref-lite is destractive

[MugeSo]

Finally:

// id -> json object map
jsonRef.cache = copy(args.externalJsons) || {};

var resolvedMethodDoc = jsonRef.replace(
  copy(methodDoc), // json-ref-lite is destractive
 jsonRef.findId(apiDoc.definitions),
 apiDoc
);

OK, I send PR soon.

[MugeSo]

Unfortunately, current implementation works with invalid reference like:

{
  responses: {
    200: {
      $ref: '#/definitions/User'
    },
    default: {
      $ref: '#/definitions/Error'
    }
  }
};

This should be:

{
  responses: {
    200: {
      $ref: '#/responses/User'
    },
    default: {
      $ref: '#/responses/Error'
    }
  }
};

// with apiDoc
{
  responses: {
    User: {
        description: "Response for user representation",
        schema: { $ref: "#/definitions/User" }
    },
    Error: {
        description: "Generic error response",
        schema: { $ref: "#/definitions/Error" }
  }
}

See: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#responses-object

So, what should I do? Can I cause BC break?
If so, it takes more time because i need fix some tests existing.

[jsdevel]

Let me take a crack at this. After thinking about it some more, I feel I have a simple solution.

[MugeSo]

I found simple solution too.

function wrongReferenceCorrected(responses) {
  var fixed = copy(responses);
  for(statusCode in responses) {
    if (responses[statusCode].$ref && /^#\/definitions\//.testr(esponses[statusCode].$ref) {
        responses[statusCode] = {
            description: statusCode + " Response",
            schema: responses[statusCode]
        };
        console.warn(loggingKey, 'Correct wrong refernce. methodDoc.responses.statusCode.$ref can be only "#/responses/responseName"');
    }
  }
}

[jsdevel]

@MugeSo can you review #20 ?

[jsdevel]

@MugeSo with version v0.16.0, parameters and responses have support for local $refs whereas before they didn't. If you can submit a PR for external refs then that would be great. Thanks for all your contributions!

[jsdevel]

@MugeSo after looking into json-ref-lite a bit more, I have a few things about it that I'm not sure would be good for express-openapi:

• It's relying on spawnSync which is available in the latest versions of node, but isn't in older versions.
• When spawnSync isn't available, it tries to install a native add on. I'd really like to avoid native add ons if at all possible.
• Initialization time will increase with the added latency.

It may be better to update express-openapi-validation and express-openapi-response-validation to use jsonschema's async loading features for external schemas.

[MugeSo]

spawnSync is used in sync-request right?
But it's just optional.
No network access is needed when jsonRef.cache has every external JSONs.

My solution do that.

[MugeSo]

After coderofsalvation/json-ref-lite#2 is merged, i'll send PR ;)

[jsdevel]

Closed by #30 .