Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

[Node.js Server] Tags and x-swagger-router-controller #325

Closed
Cooker-Monster opened this issue Jan 20, 2016 · 11 comments
Closed

[Node.js Server] Tags and x-swagger-router-controller #325

Cooker-Monster opened this issue Jan 20, 2016 · 11 comments

Comments

@Cooker-Monster
Copy link

Hi, let start with following swagger.yaml fragment :

/status:
get:
x-swagger-router-controller: Commons
operationId: getStatus
tags:
- commons
- admin

I undestood that x-swagger-router-controller extension and operationId are mandatory for node controller routing.

IMHO specific extension todo that is not so nice in an agnostic specification ...

The other thing is that there is a getStatus function in each controller for each tag.
So, in the example, getStatus() will exist in Commons & Admin generated controllers
Am I wrong ?

... if not, I'm wondering if the following default rule could be adopted (my 2 cents !):

-1- The function defined by the operationId value would be generated only for the first specified Tag
-2- the routing will be automatically set for the couple first tag + operation Id
-3- and so, no specific extension x-swagger-router-controller is needed

maybe a naïve proposal ...

@Cooker-Monster
Copy link
Author

Sorry, maybe better placed here : swagger-node ?

@whitlockjc
Copy link
Member

Swagger is agnostic but x-swagger-router-controller is specific to swagger-tools/swagger-node. Not sure how else you'd do this. In Swagger 1.2, we overloaded the nickname field for this and we could do the same for operationId but it felt right to use vendor extensions for this. So while you might say it's "not nice", I'm not sure how else you can do this.

There is support to do routing using the operationId only and so by default, you don't need x-swagger-router-controller. Using it just allows you to disambiguate getStatus in Admin from the one in Commons. As for tags, tags do nothing for routing right now and never have been requested but I could see how you might want to set the x-swagger-router-controller in a tag and then have operations using the tag to inherit the x-swagger-router-controller from it. But..how would you handle disambiguation if you used multiple tags with the same x-swagger-router-controller?

What's interesting is that Swagger actually requires unique operationId values. (See #219) The reason swagger-tools doesn't do this is that it wasn't enforced prior so many swagger-tools deployments, and apigee-127/swagger-node deployments, would suddenly fail. So we've slated that fix for 1.0.0.

All of this being said, I've actually already proposed that we remove x-swagger-router-controller in #221 but we can't do this until 1.0.0 but for different reasons. You want them removed because you don't like using vendor extensions for x-swagger-router-controller, which is arguably why vendor extensions exist. I want to remove them because in all honesty, if we were validating unique operationId values, disambiguation wouldn't be required.

I'm closing this because at best it is a duplicate of #221.

@Cooker-Monster
Copy link
Author

Sorry but I can't figure how to manage a many tagged operation without x-swagger-router-controller extension, even with an operationId setted.

As long as there is a generated function foreach generated controler foreach tag there is a need to set the route ... or I definitely miss something !

PS: My "not so nice" comment was not aggressive, it was just a point of view enforced by the so new "Open Api" generic position ;-)

@whitlockjc
Copy link
Member

I didn't see it that way, I'm just saying that using vendor extensions for something like this is pretty much what they were created for.

As for your issue, if you're running into issues with routing not working, you can run it via DEBUG=swagger-tools* swagger start to see why the misses might be happening. (Debugging swagger-tools is documented here: https://github.com/apigee-127/swagger-tools/blob/master/docs/Middleware.md#swagger-middleware-debugging) Also, as I mentioned above, tags do not play a role in the swagger-router middleware shipped with swagger-tools so I'm not sure why they are being mentioned.

How swagger-router works, and is intended to be used, is documented here: https://github.com/apigee-127/swagger-tools/blob/master/docs/Middleware.md#how-to-use The idea is that you can either use operationId alone and/or x-swagger-router-controller combined with operationId. (You can also omit operationId and then the operation's method will be used but this approach would likely require x-swagger-router-controller to remove ambiguity.)

@Cooker-Monster
Copy link
Author

Yes, I see, thx.
Tags behaviour is more a topic concerning node generator (swagger-node project) as I mentionned above.
I must go deeply in it and switch to the right place ... obviously.

@whitlockjc
Copy link
Member

I do think that tag-based routing could be a real thing but since they are not one-to-one, you'd still have collisions which would require something like x-swagger-router-controller to sort out. Since swagger-node uses swagger-tools for its middleware, and CLI validation, it too plays by the same rules in that tags play no special role.

@Cooker-Monster
Copy link
Author

Tags play role in that node generator generates a controller for each one ... and so multiply function named by operationId. That's why the idea behind the implicit rule "take only the first tag as the controller name" avoids to much "noisy code" (it's not an agressive expression at all ;-)
but yes, complex combinations could still occur ... I guess

@whitlockjc
Copy link
Member

Ah, I didn't know that swagger-codegen associated tags with controllers. swagger-codegen and swagger-tools are not directly related and it was up to the swagger-codegen author to do this. In swagger-node or swagger-tools, tags play no special role. I realize you're saying the the code generated as an association between controllers and tags but swagger-node, which uses swagger-tools for its middleware, is not the one creating the association and it does not honor/use the association as well because swagger-tools does not treat tags specially. I hope this is clear but at least now I know where this came from.

@Cooker-Monster
Copy link
Author

Yes, firstly I should double check the main project involved by the topic !
but exchanges are fructuous and you gave me many tips.
thx

@whitlockjc
Copy link
Member

Glad to help. Sometimes it's hard to see what project is doing what since a number of them play together. Let me know if I can help further.

@Cooker-Monster
Copy link
Author

Yes, Swagger ecosystem is very rich ! and very powerful !
many initiatives, many projects and many guys who want to use them !
A true success story in fact :-)
Issue entries are sometime boring for authors but it's also the good way to reach the mens who best know what's inside the engine !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants