Throw a validation error if there are duplicate operationId in the spec

dan98765 commented 6 years ago

Would resolve #85 We have something like this internally already; adding it to swagger_spec_valiator.

Internally we also assert that there is an operationId defined for each api (endpoint), but https://github.com/OAI/OpenAPI-Specification/issues/1019 makes me think this is controversial so I didn't add that part to this PR; if there's no operationId defined just move on.

coveralls commented 6 years ago

Coverage increased (+0.04%) to 98.228% when pulling 592544c1fcbc9b1dc0ec43a0d9f55446e47ac17d on dan98765:assert_uniqueness_of_operationIds into 40e1cc926775777ff2d56e271fd61697c6235579 on Yelp:master.

dan98765 commented 6 years ago

Updated such that uniqueness is only enforced per-tag. Please check out the added unit test that clarifies the behavior.

monospacesoftware commented 6 years ago

Hi! Our builds started failing today with "swagger_spec_validator.common.SwaggerValidationError: Duplicate operationId: get"...

We use "x-swagger-router-controller" to specify the target class, and "operationId" to specify the function to call on that class. Multiple classes have a "get" function.

    "/api/v2/entities/version": {
      "get": {
        "description": "Query API version",
        "operationId": "get",
        "x-swagger-router-controller": "resources.version.Version",
        "produces": [
          "application/json"
        ], ...

I'm not going to assert this is the best pattern, but I doubt we'll be the only ones effected by this change.

Using x-swagger-router-controller lets us avoid using the full function path as the operationId because it looks quite ugly in the swagger ui and it reveals internal package information about the application to users.

If operationId must be unique now, what is the recommended way to route calls with x-swagger-router-controller, to work around this? Changing function names is a workaround of course, but that potentially breaks a lot of related code. And it implies every function name referenced by swagger has to be unique across a project's entire codebase, regardless of class or package, which seems very unusual.

sjaensch commented 6 years ago

@monospacesoftware well that's what the Swagger 2.0 spec states:

The id MUST be unique among all operations described in the API.

The validation we implemented is actually less strict in that it only requires unique operationIds within the same tag.

Personally I'm not familiar with x-swagger-router-controller. It's not supported by swagger_spec_validator or the bravado libraries.

Yelp / swagger_spec_validator

Throw a validation error if there are duplicate operationId in the spec #93