apigee-127 / swagger-tools

A Node.js and browser module that provides tooling around Swagger.
MIT License
701 stars 374 forks source link

Supporting $ref to relative path #227

Open hhp21 opened 9 years ago

hhp21 commented 9 years ago

I have a Swagger 2.0 spec api/spec.json and inside there I want to have a reference to a local file api/defs.json for my definitions. So in spec.json I'm doing something like:

    "/message": {
      "post": {
        "parameters": [{
          "name": "message",
          "in": "body",
          "schema": {
            "$ref": "defs.json"
          }

where defs.json looks like

{
    "properties": {
      "text": {
        "type": "string"
      }
    },
    "required": ["text"]
}

But I just get #/paths/~1message/post/parameters/0/schema/$ref: Not a valid JSON Reference on init

I've tried several variations like:

"$ref" : "./defs.json"

This starts but fails with #: Reference could not be resolved: ./defs.json when a request is sent.

I have read that $ref supports internal and remote (with an absolute URL), but not been able to find any documentation on relative paths.

Here is a mention that this might be possible already: https://github.com/swagger-api/swagger-spec/issues/275#issuecomment-95232922 Here is a mention that it has just been implemented in swagger-js: https://github.com/swagger-api/swagger-js/issues/417

Are relative paths supported at all?

whitlockjc commented 9 years ago

JSON Reference resolution is handled by json-refs and there is a known issue for supporting this: whitlockjc/json-refs/issues/11. I will keep this open in here until this issue is resolved in json-refs.

whitlockjc commented 9 years ago

This should be fixed but there is currently a bug in json-refs/issues/24 that you need to be careful of. No circular remote/relative references? Things will work like you want.

glen-84 commented 9 years ago

@whitlockjc,

It's not working for me. Am I doing something wrong?

http://pastie.org/private/0ozh5wm8li9qrrifl9cbua (swagger.yaml) http://pastie.org/private/6coezsiq2d9bmawl4yygg (definitions/models/videos/video.yaml)

whitlockjc commented 9 years ago

How are you loading swagger.yaml?

glen-84 commented 9 years ago

With Swagger UI.

Also, it seems to request the main file 8 times when I reference the video file, but it doesn't request the video file itself.

whitlockjc commented 9 years ago

swagger-ui does not support relative references.

whitlockjc commented 9 years ago

I'm going to reopen the issue until I have unit tests showing it works but swagger-ui is not related to swagger-tools.

glen-84 commented 9 years ago

Thanks @whitlockjc, I guess it's this one: https://github.com/swagger-api/swagger-ui/issues/1456.

oliverhr commented 9 years ago

It would be great to be able to run this example:

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

whitlockjc commented 9 years ago

The problem we have right now with supporting this is that swagger-tools does not know where the original document was loaded from so doing relative reference resolution isn't possible without providing that context. swagger-tools uses path-loader and json-refs which both support relative references but they do it based on process.cwd on Node.js and window.location in the browser. Whenever the location of the loaded document is not known and does not conform to the aforementioned defaults, the related json-refs/path-loader APIs have an options.location option to fill in the gap.

That being said, to support this I would need to update all APIs in swagger-tools to support passing this option to json-refs/path-loader. It's not impossible but it would change the APIs a bit. I need to think about this a little more.

coconitro commented 9 years ago

I'm having a little tough time following, is this only an issue only with swagger-ui? I'm unable to get it to work using the swagger-node module loading swagger.yaml (completely in node, no browser in the chain at all)

Relevant snippet

swagger.yaml (snipped)

        default:
          description: Error
          schema:
            $ref: "error.yaml#/ErrorResponse"

error.yaml

ErrorResponse:
  required:
    - errors
  properties:
    errors:
      type: array
      items:
        $ref: "#Error"

Then when running swagger-node's validate I get this:

swagger validate api/swagger/swagger.yaml 

Project Errors
--------------
#/paths/~1events/get/responses/default/schema/$ref: Not a valid JSON Reference
Results: 1 errors, 0 warnings
whitlockjc commented 9 years ago

Your #Error reference is wrong. It should be #/definitions/Error.

coconitro commented 9 years ago

Good catch although it appears to be tangential. I changed errors.yaml as a test to be:

ErrorResponse:
  required:
    - errors
  properties:
    errors:
      type: string
whitlockjc commented 9 years ago

What's the error now? I doubt swagger-node supports relative references just yet since it uses swagger-tools which is known not to support relative references except under certain conditions.

coconitro commented 9 years ago

Sorry I should have been more clear. Its the same error as before.

#/paths/~1events/get/responses/default/schema/$ref: Not a valid JSON Reference

Under what conditions does swagger-tools support relative references? I could maybe force them or look thru swagger-node to figure out how to get them to work.

whitlockjc commented 9 years ago

The paths must be relative to process.cwd which is not ideal of course. So for swagger-node, your relative references would start with ./api/swagger. So if you had a users.yaml in api/swagger, you'd have something like this:

# ...
  $ref: './api/swagger/users.yaml#/definitions/User'
# ...
glen-84 commented 9 years ago

Not sure if it's relevant, but what we're doing is using swagger-parser (3.0.0-alpha.5) to dereference our swagger.yaml file (with relative paths), and then we save it as swagger.json and serve that.

whitlockjc commented 9 years ago

swagger-tools dereferences references too but relative references weren't a thing until recently. When we started work on breaking up swagger-tools, we started swagger-core-api and it does work with relative references. In face, swagger-core-api does better validation than swagger-tools at this point. The reason I bring this up is swagger-tools will be deprecated in the future and will be replaced by swagger-core-api and per-server middlewares/plugins that build on top of swagger-core-api.

diogeneshamilton commented 9 years ago

@whitlockjc I use swagger-editor currently, and it seems that I must wait for it to be updated to use swagger-core-api in order to be usable with relative URLs. Am I getting that right?

whitlockjc commented 9 years ago

Yes. We are in the process of migrating swagger-editor to swagger-core-api. Also, supporting relative references for the swagger-editor will likely involve some other work since it's intended right now to be a single-file UI but this could change that.

dkhanal commented 9 years ago

I'm currently in the process of designing a fairly large set of APIs with various widgets/types reused.

I'd like the Swagger Spec itself to be decomposed into one spec file per path referenced through $ref in YAML. Then the referenced spec file itself reference JSON schema definitions all through HTTP-based relative $ref's -- since the hostname would be tier/deployment-specific:

So (in pseudo-code):

myApiDoc.yaml:
    /my/resource/:
        $ref: "swagger-sppecs/myResource.json"

------

Schema definition in swagger-sppecs/myResource.json:
{
    "properties": {
        "propertyA": {
            "$ref":"../schema/propertyA.json"
        }
    }
}

------

Schema definition in schema/propertyA.json:
{
    "properties": {
        "propertyB": {
            "$ref":"../schema/propertyB.json"
        }
    }
}

My questions are:

  1. With Swagger-Tools, is it possible to decompose/refactor YAML into specs (it looks like it is possible with the new changes)
  2. With Swagger-Tools, is it possible to refactor the referenced JSON schemas themselves to utilize relative ref's in composite widgets and types? Ideally, I would like the schema library to be hosted externally, since the schema definitions could be utilized across technologies and applications.

Any thoughts on the current/upcoming capabilities or recommended patterns around this?

whitlockjc commented 9 years ago

The underlying library swagger-tools uses for JSON Reference resolution (json-refs) supports relative references but to use it requires swagger-tools to know where the loaded Swagger document came from, which it doesn't, and it needs a way to pass this information to json-refs. To support this we need to alter all APIs to take an options object so that we can pass the location where the Swagger document was loaded from to json-refs. There is one situation where this can work now but it is not something work documenting: If your relative references are relative to process.cwd() in Node.js or window.location in the browser.

dkhanal commented 9 years ago

Thank you for the response. I was able to get the Node side to work by making sure the relative refs were off-of process.cwd() like you suggested. However, neither Swagger UI nor Swagger Editor is able to expand the ref'ed schemas. Swagger UI just doesn't work, while Swagger Editor only shows the referenced relative URL for the model.

Is there any way to get Swagger Editor (or UI) to expand the schemas into a more human-friendly view (like it would if the definitions were local and not ref'ed)?

Regarding your comment on window.location, does that imply that the docs would have to served essentially off of process.cwd() for both node and Swagger Editor/UI to work?

whitlockjc commented 9 years ago

swagger-ui is its own thing and they do their own stuff. swagger-editor uses swagger-tools but remember that references then become relative to window.location and I'm sure that the swagger-editor doesn't serve up any documents other than the one. swagger-tools supporting relative references is just part of this as the whole tooling ecosystem needs to support it properly.

I'll work on getting swagger-tools to have an API for specifying the document root so relative references work universally. Then it's up to swagger-editor and others to use the new support.

dkhanal commented 9 years ago

Thanks again for the response. So is your recommendation for now (until all tools in the ecosystem support the doc root) to just go with a single specification document with all references local to the doc?

Having one large YAML with thousands of lines of specification sounds rather monolithic to me -- so as a newcomer to swagger/tools, I would appreciate your insight on what the prevalent industry patterns are for composing a large set of APIs (essentially around reuse and physical refactoring of the specification document).

whitlockjc commented 9 years ago

What server environment are you using?

dkhanal commented 9 years ago

node + express

whitlockjc commented 9 years ago

So just Express and swagger-tools or something else? How does swagger-editor and swagger-ui get into the mix?

dkhanal commented 9 years ago

I dropped Swagger UI for its inability to reference externally ref'ed schemas.

I have a locally built Swagger Editor that I am hosting on a static server (different than where API would be hosted) -- just to use the UI portion of the editor. The editor portion is not used.

Swagger Editor worked great as long as the ref'd URIs were absolute. But I need them to be relative to support multiple deployment environments, and that's kind of where I'm hitting a roadblock.

The Node+Express side (even mocks) works just fine (even with relative refs).

Only alternative that seems to work with Swagger Editor/UI is to make the YAML spec a monolithic doc with no external or relative refs for schemas.

glen-84 commented 9 years ago

@dkhanal Did you see my comment above?

Here is our Gulp file: https://gist.github.com/glen-84/f1bdda0bbe9c32da0d3f

whitlockjc commented 9 years ago

Yeah, the Node.js side of things should work because you got the resolution of references working due to changing your references to be relative to process.cwd(). Everything on the Node.js side should work. If you use the swagger-ui middleware in swagger-tools, it will serve the fully resolved document and a working swagger-ui for it. @glen-84 has a working recipe as well to work around while we work on doing this in swagger-tools.

dkhanal commented 9 years ago

@glen-84 Thank you. I will play with this workaround.

@whitlockjc Thanks. Did you mean that swagger-ui middleware would serve a fully resolved document (even when there's relative refs) currently, or it will in the future?

whitlockjc commented 9 years ago

It does already, or it should.

dkhanal commented 9 years ago

@whitlockjc - No it doesn't appear to be working. It looks like the $ref's are expected to be relative to process.cwd() on the server (and the Node side works), but Swagger UI (served by the middleware) also needs the referenced schemas right off the root accessible from the browser. Not really a clean workaround, IMO. I wanted to express.static(..) the schema folder so that the browser could access it, but not sure how to get express.static(..) to work with swagger-express-mw. Looks like routes are overwritten by the middleware.

dkhanal commented 9 years ago

@glen-84 I used the swagger-parser tool and it finds the referenced files and everything but complains about a schema issue:

SyntaxError: Data does not match any schemas from "oneOf"
Data path: "/paths/~1user~1login/get/responses/200"
Schema path: "/properties/paths/patternProperties/^~1/properties/get/properties/responses/patternProperties/^([0-9]{3})$|^(default)$/oneOf"
]

Here's the YAML:

paths:
  /user/login:
    get:
      x-swagger-router-controller: "userApi_v1"
      consumes:
        - "application/json"
      produces:
        - "application/json"
        - "text/html"
        - "text/plain"
      description: "Authenticates the specified credentials."
      operationId: "login"
      parameters:
        - name: "username"
          in: "query"
          description: "Username to login as."
          required: true
          type: "string"
        - name: "password"
          in: "query"
          description: "Password to login with."
          required: true
          type: "string"
          format: "password"
        - name: "persistent"
          in: "query"
          description: "Indicates if the client chooses to have a persistent cookie."
          required: false
          type: "boolean"
      responses:
        200:
          description: "Success"
          schema:
            $ref: "schema/json/user/v1/responses/loginResponse.json"
        default:
          description: "Error"
          schema:
            $ref: "schema/json/common/v1/standardError.json"

Here's the referenced loginResponse.json:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "id": "https://myCompany.com/schema/json/user/v1/responses/loginResponse",
  "type": "string",
  "additionalProperties": false,
  "title": "Login Response",
  "name": "/"
}
whitlockjc commented 9 years ago

If you are willing to use swagger-parser, might as well use swagger-core-api since that's the successor to swagger-tools' API. It supports relative references.

dkhanal commented 9 years ago

@glen-84 It looks like I now know why I'm getting the validation error. The $schema ref's apparently can't be full-blown JSON Schema Draft 4 objects. If I remove $schema, id, additionalProperties, and name from the schema definition, then the tool works. The validation is probably done against the Swager 2.0 definition of 'schema', which doesn't support all of the JSON Draft 4 attributes.

@whitlockjc Thanks. Any ideas when swagger-core-api will be officially released? Is it near production?

whitlockjc commented 9 years ago

Within the next day or two.

dkhanal commented 9 years ago

Ok, sounds good. Can I use it with swagger-express-mw? Please let me know (or point me to doc) how it integrates with or replaces SwaggerExpress.create(...)

whitlockjc commented 9 years ago

No reason why you couldn't. Here is an example:

function configureServer (swaggerDoc) {
  // Do your server initialization here
}

require('swagger-core-api').create({
  definition: swaggerDocPath
}).then(function (swaggerApi) {
  // Configure server
  configureServer(swaggerApi.resolved);
}, function (err) {
  console.error(err.stack);
  process.exit(1);
});
dkhanal commented 9 years ago

Just tried to npm install (npm install swagger-core-api --save). It picked up a package.json and an essentially empty index.js. Nothing else.

whitlockjc commented 9 years ago

npm install https://github.com/apigee-127/swagger-core-api.git --save. Until I do the next release, the current NPM package is broken as it was created as a temporary placeholder. I'll do an official release, probably with project rename to make it more intuitive.

dkhanal commented 9 years ago

Thanks... Installing now. Does it completely replace swagger-express-mw and swagger-tools? Is the CLI that generates the boilerplate going to use this for Node+Express? If so, should I completely do away with my SwaggerExpress.create() and use this instead?

whitlockjc commented 9 years ago

No, this is just an API for doing Swagger integrations. It provides Swagger document loading (local/remote), reference resolution (local, remote, relative, etc.), document normalization (security requirements, path-level parameters, etc.) and APIs for interacting with the document. For you, it would be used to load your document to work around swagger-tools not exposing what's necessary to handle relative references.

Pretty soon, after we release the project, we'll start rewriting the middlewares and exposing them individually with more support than just Connect and Connect-based/compatible servers. Implementations should be much simpler this way. I'll be writing a blog entry on where we're at and where we're going.

dkhanal commented 9 years ago

@whitlockjc Good stuff -- thanks!

I tried the following. The UI is served with the YAML expanded/resolved to 1 level of depth, but ref's that are deeper (i.e. those present inside the referenced JSON schemas) are not resolved.

Is there a way to get swagger-core-api to fully expand the document to the leaf level?

 SwaggerApi.create({definition: "./api/swagger/swagger.yaml"})
        .then(function (swaggerApi) {
            // Add swagger-ui (This must be before swaggerExpress.register)
            app.use(new SwaggerUi(swaggerApi.resolved));
        }, function (err) {
            console.error(err.stack);
            process.exit(1);
    });
whitlockjc commented 9 years ago

I assume you're talking about relative references? With swagger-core-api, relative references work as the should so make sure they are relative to the documents loading them, not process.cwd() like you tried for swagger-tools'.

dkhanal commented 9 years ago

@whitlockjc Thanks, Jeremy -- I think I figured it out. Looks like the nested $ref's have to be relative to the file where they are referenced.

whitlockjc commented 9 years ago

That is correct.

dkhanal commented 9 years ago

@whitlockjc Jeremy -- You guys are working on very useful stuff. I'm up and running with swagger-core-api to serve fully resolved docs. Many thanks!

whitlockjc commented 9 years ago

Glad to help.