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

3scale import openapi command fails on same operationId at different routes #222

Closed
eladmotola opened this issue Oct 30, 2019 · 5 comments
Closed

Comments

@eladmotola
Copy link

eladmotola commented Oct 30, 2019

When running 3scale import openapi with a swagger file that contains the same method name on different route, I got an error of Failed to create method because it already been created by the first route. for example:

{{ "swagger": "2.0", "info": { "version": "v1", "title": "WebApplication1" }, "host": "localhost:3873", "schemes": ["http"], "paths": { "/api/User": { "get": { "tags": ["User"], "operationId": "User_Get", "consumes": [], "produces": ["application/json", "text/json", "application/xml", "text/xml"], "responses": { "200": { "description": "OK", "schema": { "type": "array", "items": { "type": "string" } } } } } }, "/api/User/{id}": { "get": { "tags": ["User"], "operationId": "User_Get", "consumes": [], "produces": ["application/json", "text/json", "application/xml", "text/xml"], "parameters": [{ "name": "id", "in": "path", "required": true, "type": "integer", "format": "int32" } ], "responses": { "200": { "description": "OK", "schema": { "type": "string" } } } } } }, "definitions": {} }}

First, the first route /api/User will create the User_Get method. After that, /api/User/{id} will try to create User_Get method too but it fails.
When I ran the command again, the import will succeed, because the method is already created and its in"update" mode.

It leads to 2 problems as I can see right now:

  1. There is inconsistency between insert and update an API with toolbox
  2. There is 2 routes for 1 methods

After more digging, I found out that if I remove the operationId field, the method name is being generated by the path name. So I propose it will the default always.

happens with toolbox v0.13.0

@eguzki
Copy link
Member

eguzki commented Oct 31, 2019

Swagger spec does not allow same operationId on different operations.

In Operation Object reference, about operationId field

Unique string used to identify the operation. 
The id MUST be unique among all operations described in the API. 
Tools and libraries MAY use the operationId to uniquely identify an operation, therefore, it is recommended to follow common programming naming conventions.

Therefore, the WebApplication1 swagger spec described above is invalid.

The toolbox uses as method unique identifier:

  • operationId if exists
  • #{verb}#{path} otherwise

According to the swagger spec, I think that is correct, as the toolbox can assume unique identifier among all operations.

Feel free to reopen if there are further comments or unexpected behaviors

@eguzki eguzki closed this as completed Oct 31, 2019
@eguzki
Copy link
Member

eguzki commented Oct 31, 2019

We will review our swagger validator, as it should have rejected it before processing

Thanks for the feedback

@eguzki
Copy link
Member

eguzki commented Oct 31, 2019

Apicurio should catch duplicated operationId https://www.apicur.io/

@eladmotola
Copy link
Author

eladmotola commented Oct 31, 2019

We will review our swagger validator, as it should have rejected it before processing

Thanks for the feedback

@eguzki So should we open this issue again to track that?

@eguzki
Copy link
Member

eguzki commented Nov 4, 2019

I have done it for you #223

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

2 participants