Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3] Option to move referenced definitions/responses/etc. from other files into the relevant keys in the main document #16

Closed
glen-84 opened this issue Aug 7, 2015 · 31 comments

Comments

@glen-84
Copy link

glen-84 commented Aug 7, 2015

I'm using v3 to dereference my Swagger spec which is broken out into many separate files. It works to create a single file, however the file is unnecessarily large as everything is inlined. It would be really great if you could configure it to move referenced definitions/responses, etc. to the relevant container keys in the main document, instead of inlining everything.

In other words, rewrite the references to point to local keys in the main document, to avoid the repetition.

@diogeneshamilton
Copy link

+1

@JamesMessinger
Copy link
Member

Yep, this feature has been requested by several people - including myself :) - so it's definitely going to be in v3.0.0. I'm working on it this week, as a matter of fact.

@glen-84
Copy link
Author

glen-84 commented Aug 10, 2015

Sweet! 🍠

@glen-84
Copy link
Author

glen-84 commented Sep 8, 2015

@BigstickCarpet Did you make any progress on this?

@JamesMessinger
Copy link
Member

Sorry for the silence. I got swamped for a couple of weeks with a work project, so I just resumed working on the Swagger stuff this past weekend. I've stubbed the bundle() method, but right now it just throws an error. It's not much, but it's a start :)

@glen-84
Copy link
Author

glen-84 commented Sep 9, 2015

No worries, thanks for the update. 😃

@JamesMessinger
Copy link
Member

Good news! The the bundle() method has been implemented! Install Swagger Parser v3.0.0-alpha.9 and let me know what you think :)

@glen-84
Copy link
Author

glen-84 commented Sep 21, 2015

Well, the good news is that the file size went from 208 KB to 157 KB, the bad news is, I get errors (in Swagger editor).

Our setup is like this:

  1. swagger.yaml references operations in separate files like this:

    paths:
        /videos:
            $ref: "operations/videos/videos.yaml#/Videos"
    
  2. videos.yaml references model definitions in separate files like this:

    Videos:
        get:
            summary: Get a list of videos
            tags:
                - Videos
            responses:
                200:
                    description: Success
                    schema:
                        type: object
                        properties:
                            data:
                                type: array
                                items:
                                    $ref: "../../definitions/models/videos/video.yaml#/Video"
    

This doesn't seem to be working when using bundle().

@JamesMessinger
Copy link
Member

Ah, crap. That's a scenario I hadn't accounted for. You're not actually referencing the entire videos.yaml file anywhere, only parts of it. The way I wrote the bundle() method relies on the entire file being referenced at least once somewhere in the API. :-/

I'll add a test to my test suite to cover this. It might be a few days before I get it fixed though. This is a busy week for me. In the meantime, you can workaround the bug by referencing the entire videos.yaml file somewhere:

definitions:
  videos-paths:
    $ref: operations/videos/videos.yaml

@glen-84
Copy link
Author

glen-84 commented Sep 21, 2015

No rush, thanks James.

@glen-84
Copy link
Author

glen-84 commented Oct 29, 2015

Any news here?

@JamesMessinger
Copy link
Member

Sorry, I was away on holiday for most of October (I'm actually sitting in the airport right now, catching-up on a month's worth of emails). I had hoped to get this issue fixed before I left for the month, but that didn't happen. :(

The next week or two are going to be really hectic for me, as I'm catching-up on a month's worth of stuff and also starting a new job (at Postman) tomorrow! So I might not get this fixed for a few more weeks. Sorry it's taking so long. :(

In the meantime, if you want to take a stab at fixing it and submit a PR, I'd be glad to review and merge it. I don't think it will be a very significant code change, though I could be mistaken.

@glen-84
Copy link
Author

glen-84 commented Nov 8, 2015

No worries.

I wouldn't even know where to start, so I think that it's best if I wait for you. Would it be too pushy if I pinged you again in 4-6 weeks? 😟

Good luck at your new job!

@JamesMessinger
Copy link
Member

Yes, please feel free to ping me in 4-6 weeks. I promise I'll do my best to have it done by then, but I can't predict what might come up. :)

@glen-84
Copy link
Author

glen-84 commented Dec 28, 2015

Ping =)

@JamesMessinger
Copy link
Member

Good news! I finally got around to fixing this! It's fixed in the latest version of JSON Schema $Ref Parser, but I had to make some breaking changes, so I still need to update Swagger Parser to work with it. I should be able to knock that out tomorrow.

@JamesMessinger JamesMessinger reopened this Jan 4, 2016
@tlusk
Copy link

tlusk commented Jan 4, 2016

That's great news, I just started using this parser tonight and ran into this issue. Glad to hear it should be fixed very soon!

@glen-84
Copy link
Author

glen-84 commented Jan 4, 2016

Awesome!

@JamesMessinger
Copy link
Member

In case you're wondering why this isn't done yet, I'm working on another breaking change in JSON Schema $Ref Parser, so it makes sense to wait to get that in too, rather than releasing two major versions of Swagger Parser back-to-back.

@glen-84
Copy link
Author

glen-84 commented Jan 6, 2016

Thanks for the update. =)

@glen-84
Copy link
Author

glen-84 commented Mar 28, 2016

Any news here?

@JamesMessinger
Copy link
Member

Yep. Just resumed working on it yesterday. I was completely swamped for the past three months and haven't been able to work on any of my open-source projects (except the ones that I work on as part of my job). But things are finally back to normal, so I'm trying to get caught-up now.

@JamesMessinger
Copy link
Member

Swagger Parser v4.0 beta is now available, and includes the fix for this issue.
You can install the beta using npm install swagger-parser@beta.

@glen-84
Copy link
Author

glen-84 commented Apr 13, 2016

Unfortunately I'm seeing errors like this:

Object
code: "UNRESOLVABLE_REFERENCE"
message: "Reference could not be resolved: #/paths/~1blogs~1posts~1%7Bid%7D/patch/responses/200/schema/properties/data"
path: Array [10]
0: "paths"
1: "/blogs/{id}/posts"
2: "get"
3: "responses"
4: "200"
5: "schema"
6: "properties"
7: "data"
8: "items"
9: "$ref"
level: 900
type: "Swagger Error"
description: "Reference could not be resolved: #/paths/~1blogs~1posts~1%7Bid%7D/patch/responses/200/schema/properties/data"
lineNumber: 0

Do you have any ideas?

@JamesMessinger
Copy link
Member

@glen-84 Can you post a sample schema that reproduces the error? I'll dig into it.

@glen-84
Copy link
Author

glen-84 commented Apr 13, 2016

It seems to be affecting the dereference method as well. Please try this:

I hope I've included everything. If not, please let me know.

Note: It's possible that this was affecting more recent versions of the 3.x series as well.

@JamesMessinger
Copy link
Member

JamesMessinger commented Apr 15, 2016

@glen-84 Are you sure you ran the 4.0.0-beta-1 version of Swagger Parser? I ran your example above, and it bundled and dereferenced fine on Node 0.12, 4.0, and 5.0. Here's the code I ran. You can run it yourself to verify.

swagger-parser-test.zip

@glen-84
Copy link
Author

glen-84 commented Apr 15, 2016

@BigstickCarpet Now copy the contents of bundled.json into the Swagger Editor here, and you'll see the Reference could not be resolved error.

@JamesMessinger
Copy link
Member

@glen-84 - Ah, I didn't realize the errors you were getting were from Swagger Editor, not from Swagger Parser. This appears to be a bug in Swagger Editor. I've opened an issue for it.

@JamesMessinger
Copy link
Member

@glen-84 - As a temporary workaround though... you could move the definitions to the definitions property and $reference them from there, instead of $referencing them under the paths property

@glen-84
Copy link
Author

glen-84 commented Apr 15, 2016

@BigstickCarpet Thanks! I'll keep an eye on that issue.

@glen-84 glen-84 closed this as completed May 7, 2021
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

No branches or pull requests

4 participants