civisanalytics/swagger-diff

swagger-diff confused with two operations with same number of parameters

Closed this issue · 3 comments

I am getting this error after running swagger-diff against an unchanged resource

API specification for configurator service has changed in an incompatible way: - incompatible request     params
  - get /employers/{}
  - new required request param: key
  - missing request param: id (in: path, type: integer) 

I believe it's because this resource has two GET methods each with a single parameter:

@GET
@Path("{id: [1-9]\\d*}")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "If successful, returns an employee with the given id.",
              response = Employer.class)
public Employer getEmployerById(@PathParam("id") long id ){
    return employerRepository.findById(id);
}

@GET
@Path("{key: \\D[\\w]+}")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "If successful, returns an employee with the given key.",
              response = Employer.class)
public Employer getEmployerByKey(@PathParam("key") String key ){
    return employerRepository.findByKey(key);
}

The swagger JSON looks like this:

"/employers/{key}": {
  "get": {
    "responses": {
      "200": {
        "schema": {
          "$ref": "#/definitions/Employer"
        },
        "description": "successful operation"
      }
    },
    "parameters": [
      {
        "pattern": "\\D[\\w]+",
        "type": "string",
        "required": true,
        "in": "path",
        "name": "key"
      }
    ],
    "produces": [
      "application/json"
    ],
    "operationId": "getEmployerByKey",
    "description": "",
    "summary": "If successful, returns an employee with the given key.",
    "tags": [
      "employers"
    ]
  }
},
"/employers/{id}": {
  "get": {
    "responses": {
      "200": {
        "schema": {
          "$ref": "#/definitions/Employer"
        },
        "description": "successful operation"
      }
    },
    "parameters": [
      {
        "format": "int64",
        "pattern": "[1-9]\\d*",
        "type": "integer",
        "required": true,
        "in": "path",
        "name": "id"
      }
    ],
    "produces": [
      "application/json"
    ],
    "operationId": "getEmployerById",
    "description": "",
    "summary": "If successful, returns an employee with the given id.",
    "tags": [
      "employers"
    ]
  }
},

While this validates using Swagger's Validator Badge, I don't think this is actually a valid specification. Check out the discussion on apigee-127/sway#32. The people commenting that the paths are the same/invalid are all affiliated with Swagger, and the discussion led to an issue to document that this is unsupported.

Ooh, I didn't know about Validator Badge.

Thanks for the thread, it does make sense. Looks like the workaround is to
merge the two methods into one with two optional parameters, and we'll use
the id if it's present otherwise we'll use the key.

On Tue, Dec 1, 2015 at 12:19 PM jeffreyc notifications@github.com wrote:

While this validates using Swagger's Validator Badge
https://github.com/swagger-api/validator-badge, I don't think this is
actually a valid specification. Check out the discussion on
apigee-127/sway#32 apigee-127/sway#32. The
people commenting that the paths are the same/invalid are all affiliated
with Swagger, and the discussion led to an issue to document that this is
unsupported.


Reply to this email directly or view it on GitHub
#19 (comment)
.

Closing the issue, it appears to be invalid from Swagger's perspective, even though swagger-core is emitting it without error.