diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java index 7a453eb65..e3a743455 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java @@ -36,25 +36,41 @@ public Optional diff( final Map left, final Map right) { ChangedPaths changedPaths = new ChangedPaths(left, right); changedPaths.getIncreased().putAll(right); + left.keySet() .forEach( (String url) -> { PathItem leftPath = left.get(url); String template = normalizePath(url); - Optional result = - right.keySet().stream() - .filter(s -> normalizePath(s).equals(template)) - .findFirst(); + Optional> result = + changedPaths.getIncreased().entrySet().stream() + .filter(item -> normalizePath(item.getKey()).equals(template)) + .min((a, b) -> { + if (methodsIntersect(a.getValue(), b.getValue())) { + throw new IllegalArgumentException( + "Two path items have the same signature: " + template); + } + if (a.getKey().equals(url)) { + return -1; + } else if (b.getKey().equals((url))) { + return 1; + } else { + HashSet methodsA = new HashSet<>( + a.getValue().readOperationsMap().keySet()); + methodsA.retainAll(leftPath.readOperationsMap().keySet()); + HashSet methodsB = new HashSet<>( + b.getValue().readOperationsMap().keySet()); + methodsB.retainAll(leftPath.readOperationsMap().keySet()); + return Integer.compare(methodsB.size(), methodsA.size()); + } + }); if (result.isPresent()) { - if (!changedPaths.getIncreased().containsKey(result.get())) { - throw new IllegalArgumentException( - "Two path items have the same signature: " + template); - } - PathItem rightPath = changedPaths.getIncreased().remove(result.get()); + String rightUrl = result.get().getKey(); + PathItem rightPath = changedPaths.getIncreased().remove(rightUrl); Map params = new LinkedHashMap<>(); - if (!url.equals(result.get())) { + if (!url.equals(rightUrl)) { List oldParams = extractParameters(url); - List newParams = extractParameters(result.get()); + List newParams = extractParameters(rightUrl); for (int i = 0; i < oldParams.size(); i++) { params.put(oldParams.get(i), newParams.get(i)); } @@ -65,7 +81,7 @@ public Optional diff( openApiDiff .getPathDiff() .diff(leftPath, rightPath, context) - .ifPresent(path -> changedPaths.getChanged().put(result.get(), path)); + .ifPresent(path -> changedPaths.getChanged().put(rightUrl, path)); } else { changedPaths.getMissing().put(url, leftPath); } @@ -79,4 +95,14 @@ public static Paths valOrEmpty(Paths path) { } return path; } + + private static boolean methodsIntersect(PathItem a, PathItem b) { + Set methodsA = a.readOperationsMap().keySet(); + for (PathItem.HttpMethod method : b.readOperationsMap().keySet()) { + if (methodsA.contains(method)) { + return true; + } + } + return false; + } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java index 71fbc938b..6d136fb25 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java @@ -1,15 +1,18 @@ package org.openapitools.openapidiff.core; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiAreEquals; import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.ChangedOpenApi; public class PathDiffTest { private final String OPENAPI_PATH1 = "path_1.yaml"; private final String OPENAPI_PATH2 = "path_2.yaml"; private final String OPENAPI_PATH3 = "path_3.yaml"; + private final String OPENAPI_PATH4 = "path_4.yaml"; @Test public void testEqual() { @@ -21,4 +24,13 @@ public void testMultiplePathWithSameSignature() { assertThrows( IllegalArgumentException.class, () -> assertOpenApiAreEquals(OPENAPI_PATH3, OPENAPI_PATH3)); } + + @Test + public void testSameTemplateDifferentMethods() { + ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_PATH1, OPENAPI_PATH4); + assertThat(changedOpenApi.getNewEndpoints()) + .hasSize(1) + .satisfiesExactly(endpoint -> assertThat(endpoint.getOperation().getOperationId()).isEqualTo("deletePet")); + assertThat(changedOpenApi.isCompatible()).isTrue(); + } } diff --git a/core/src/test/resources/path_4.yaml b/core/src/test/resources/path_4.yaml new file mode 100644 index 000000000..6d08cb721 --- /dev/null +++ b/core/src/test/resources/path_4.yaml @@ -0,0 +1,52 @@ +openapi: 3.0.0 +servers: + - url: 'http://petstore.swagger.io/v2' +info: + description: >- + This is a sample server Petstore server. You can find out more about + Swagger at [http://swagger.io](http://swagger.io) or on [irc.freenode.net, + #swagger](http://swagger.io/irc/). For this sample, you can use the api key + `special-key` to test the authorization filters. + version: 1.0.0 + title: Swagger Petstore + termsOfService: 'http://swagger.io/terms/' + contact: + email: apiteam@swagger.io + license: + name: Apache 2.0 + url: 'http://www.apache.org/licenses/LICENSE-2.0.html' +paths: + /pet/{petId}: + get: + tags: + - pet + summary: gets a pet by id + description: '' + operationId: updatePetWithForm + parameters: + - name: petId + in: path + description: ID of pet that needs to be updated + required: true + schema: + type: integer + responses: + '405': + description: Invalid input + /pet/{petId2}: + post: + tags: + - pet + summary: deletes a pet + description: '' + operationId: deletePet + parameters: + - name: petId2 + in: path + description: Pet ID to delete + required: true + schema: + type: integer + responses: + '405': + description: Invalid input