OperationId Existence Check Fails for Multiple Operations with Different HTTP Methods at Single Path (JAX-RS)

[fess0009]

Background
The JAX-RS Reader assigns default 'operationId' values for all identified operations based upon their method name. It does so if the corresponding @ApiOperation annotation does not provide the 'nickname' value used to specify an 'operationId'.

Problem Statement
It attempts to guarantee uniqueness of this value across the specification by checking whether the generated default has already been used in previously processed operations. However, the logic associated with the existence check incorrectly assumes that only a single operation with a single HTTP method can exist at a given path.

This results in duplicate 'operationId' values in the following scenario:

• Multiple non-GET endpoints exist with matching method names (GET endpoints are immune to this issue as they are the handled first in the existence check logic)

AND

• Additional endpoint(s) exist at the same path as one of the above endpoints with an HTTP method being handled earlier in the existence check logic (see Details below)

Example
The following resources

@Path("/first")
public class FirstResource {
    @GET
    public Response read() { return Response.noContent().build(); }

    @PUT
    public Response update() { return Response.noContent().build(); }
}

@Path("/second")
public class SecondResource {
    @GET
    public Response read() { return Response.noContent().build(); }

    @PUT
    public Response update() { return Response.noContent().build(); }
}

generate the following specification - note the duplicate 'operationId' values for the PUT endpoints

{
  "openapi" : "3.0.1",
  "info" : {
    "title" : "Example API",
    "version" : "1.0.0"
  },
  "paths" : {
    "/example/first" : {
      "get" : {
        "operationId" : "read",
        "responses" : {
          "default" : {
            "description" : "default response",
            "content" : {
              "*/*" : { }
            }
          }
        }
      },
      "put" : {
        "operationId" : "update",
        "responses" : {
          "default" : {
            "description" : "default response",
            "content" : {
              "*/*" : { }
            }
          }
        }
      }
    },
    "/example/second" : {
      "get" : {
        "operationId" : "read_1",
        "responses" : {
          "default" : {
            "description" : "default response",
            "content" : {
              "*/*" : { }
            }
          }
        }
      },
      "put" : {
        "operationId" : "update",
        "responses" : {
          "default" : {
            "description" : "default response",
            "content" : {
              "*/*" : { }
            }
          }
        }
      }
    }
  }
}

Details
This issue is caused by the java.io.swagger.v3.jaxrs2.Reader class' 'extractOperationIdFromPathItem' method definition below. This logic incorrectly assumes that a PathItem will only contain a single HTTP method / operation and uses the id of the first operation on the path that is encountered.

private String extractOperationIdFromPathItem(PathItem path) {
	if (path.getGet() != null) {
		return path.getGet().getOperationId();
	} else if (path.getPost() != null) {
		return path.getPost().getOperationId();
	} else if (path.getPut() != null) {
		return path.getPut().getOperationId();
	} else if (path.getDelete() != null) {
		return path.getDelete().getOperationId();
	} else if (path.getOptions() != null) {
		return path.getOptions().getOperationId();
	} else if (path.getHead() != null) {
		return path.getHead().getOperationId();
	} else if (path.getPatch() != null) {
		return path.getPatch().getOperationId();
	}
	return "";
}

In the example above, when this method is called during the processing of the second PUT operation, this logic locates the GET operation on the same path as the first PUT operation and returns the GET 'operationId' value. This 'operationId' does not match the default value for the second PUT operation and the caller ('existOperationId' method) returns false. This false during the existence check permits the default method name-based 'operationId' value to be used for both PUT operations.

Proposed Solution
The 'existOperationId' method should check all paths for all operations to ensure there is not a conflict.

[frantuma]

fixed in #2988