Allow paths resolving to same template but with different methods (#224)

Fixes #170

Author: Trundle

core/src/main/java/org/openapitools/openapidiff/core/compare/PathsDiff.java

@@ -36,25 +36,41 @@ public Optional<ChangedPaths> diff(
       final Map<String, PathItem> left, final Map<String, PathItem> 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<String> result =
-                  right.keySet().stream()
-                      .filter(s -> normalizePath(s).equals(template))
-                      .findFirst();
+              Optional<Map.Entry<String, PathItem>> 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<PathItem.HttpMethod> methodsA = new HashSet<>(
+                              a.getValue().readOperationsMap().keySet());
+                          methodsA.retainAll(leftPath.readOperationsMap().keySet());
+                          HashSet<PathItem.HttpMethod> 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<String, String> params = new LinkedHashMap<>();
-                if (!url.equals(result.get())) {
+                if (!url.equals(rightUrl)) {
                   List<String> oldParams = extractParameters(url);
-                  List<String> newParams = extractParameters(result.get());
+                  List<String> 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<ChangedPaths> 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<PathItem.HttpMethod> methodsA = a.readOperationsMap().keySet();
+    for (PathItem.HttpMethod method : b.readOperationsMap().keySet()) {
+      if (methodsA.contains(method)) {
+        return true;
+      }
+    }
+    return false;
+  }
 }

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();
+  }
 }

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