Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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);
}
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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();
}
}
52 changes: 52 additions & 0 deletions core/src/test/resources/path_4.yaml
Original file line number Diff line number Diff line change
@@ -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: [email protected]
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