From f9de97dea6f5b0c86f92161332ad7a9e5e82e404 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Wed, 3 Sep 2025 14:32:59 +0200 Subject: [PATCH 1/3] Fix for #823 --- .../core/compare/OpenApiDiffOptions.java | 12 ++ .../openapidiff/core/compare/PathsDiff.java | 122 ++++-------------- .../compare/matchers/DefaultPathMatcher.java | 82 ++++++++++++ .../core/compare/matchers/PathMatcher.java | 19 +++ 4 files changed, 136 insertions(+), 99 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/DefaultPathMatcher.java create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/PathMatcher.java diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java index 23f4e1c37..b21fc1f03 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiffOptions.java @@ -6,9 +6,12 @@ import org.apache.commons.configuration2.CompositeConfiguration; import org.apache.commons.configuration2.YAMLConfiguration; import org.apache.commons.configuration2.ex.ConfigurationException; +import org.openapitools.openapidiff.core.compare.matchers.DefaultPathMatcher; +import org.openapitools.openapidiff.core.compare.matchers.PathMatcher; public class OpenApiDiffOptions { private final CompositeConfiguration config; + private PathMatcher pathMatcher; private OpenApiDiffOptions(CompositeConfiguration config) { this.config = config; @@ -18,6 +21,10 @@ public CompositeConfiguration getConfig() { return config; } + public PathMatcher getPathMatcher() { + return pathMatcher != null ? pathMatcher : new DefaultPathMatcher(); + } + public static Builder builder() { return new Builder(); } @@ -45,5 +52,10 @@ public Builder configProperty(String propKey, String propVal) { public OpenApiDiffOptions build() { return built; } + + public Builder pathMatcher(PathMatcher matcher) { + built.pathMatcher = matcher; + return this; + } } } 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 8b4f459b8..0352e017f 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 @@ -1,13 +1,10 @@ package org.openapitools.openapidiff.core.compare; -import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.oas.models.Paths; -import io.swagger.v3.oas.models.parameters.Parameter; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.IntStream; import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedPaths; import org.openapitools.openapidiff.core.model.DiffContext; @@ -22,20 +19,6 @@ public PathsDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - private static String normalizePath(String path) { - return path.replaceAll(REGEX_PATH, "{}"); - } - - private static List extractParameters(String path) { - ArrayList params = new ArrayList<>(); - Pattern pattern = Pattern.compile(REGEX_PATH); - Matcher matcher = pattern.matcher(path); - while (matcher.find()) { - params.add(matcher.group(1)); - } - return params; - } - public DeferredChanged diff( final Map left, final Map right) { DeferredBuilder builder = new DeferredBuilder<>(); @@ -43,114 +26,55 @@ public DeferredChanged diff( ChangedPaths changedPaths = new ChangedPaths(left, right, openApiDiff.getOptions()); changedPaths.getIncreased().putAll(right); - left.keySet() + left.entrySet() .forEach( - (String url) -> { - PathItem leftPath = left.get(url); - String template = normalizePath(url); + pathEntry -> { + String leftUrl = pathEntry.getKey(); + PathItem leftPath = pathEntry.getValue(); Optional> result = - changedPaths.getIncreased().entrySet().stream() - .filter(item -> normalizePath(item.getKey()).equals(template)) - .min( - (a, b) -> { - if (methodsAndParametersIntersect(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()); - } - }); + openApiDiff + .getOptions() + .getPathMatcher() + .find(pathEntry, changedPaths.getIncreased()); if (result.isPresent()) { String rightUrl = result.get().getKey(); PathItem rightPath = changedPaths.getIncreased().remove(rightUrl); Map params = new LinkedHashMap<>(); - if (!url.equals(rightUrl)) { - List oldParams = extractParameters(url); + if (!leftUrl.equals(rightUrl)) { + List oldParams = extractParameters(leftUrl); List newParams = extractParameters(rightUrl); for (int i = 0; i < oldParams.size(); i++) { params.put(oldParams.get(i), newParams.get(i)); } } DiffContext context = new DiffContext(openApiDiff.getOptions()); - context.setUrl(url); + context.setUrl(leftUrl); context.setParameters(params); - context.setLeftAndRightUrls(url, rightUrl); + context.setLeftAndRightUrls(leftUrl, rightUrl); builder .with(openApiDiff.getPathDiff().diff(leftPath, rightPath, context)) .ifPresent(path -> changedPaths.getChanged().put(rightUrl, path)); } else { - changedPaths.getMissing().put(url, leftPath); + changedPaths.getMissing().put(leftUrl, leftPath); } }); return builder.buildIsChanged(changedPaths); } + private List extractParameters(String path) { + ArrayList params = new ArrayList<>(); + Pattern pattern = Pattern.compile(REGEX_PATH); + Matcher matcher = pattern.matcher(path); + while (matcher.find()) { + params.add(matcher.group(1)); + } + return params; + } + public static Paths valOrEmpty(Paths path) { if (path == null) { path = new Paths(); } return path; } - - /** - * @param a a path form the open api spec - * @param b another path from the same open api spec - * @return true in case both paths are of the same method AND their templated - * parameters are of the same type; false otherwise - */ - private static boolean methodsAndParametersIntersect(PathItem a, PathItem b) { - Set methodsA = a.readOperationsMap().keySet(); - for (PathItem.HttpMethod method : b.readOperationsMap().keySet()) { - if (methodsA.contains(method)) { - Operation left = a.readOperationsMap().get(method); - Operation right = b.readOperationsMap().get(method); - if (left.getParameters().size() == right.getParameters().size()) { - return parametersIntersect(left.getParameters(), right.getParameters()); - } - return false; - } - } - return false; - } - - /** - * Checks if provided parameter pairs are equal by type and format - * - * @param left parameters from the first compared method - * @param right parameters from the second compared method - * @return true in case each parameter pair is of the same type; false - * otherwise - */ - private static boolean parametersIntersect(List left, List right) { - int parametersSize = left.size(); - long intersectedParameters = - IntStream.range(0, left.size()) - .filter(i -> parametersTypeEquals(left.get(i), right.get(i))) - .count(); - return parametersSize == intersectedParameters; - } - - /** - * Checks if provided parameter pair is equal by type and format - * - * @param left parameter from the first compared method - * @param right parameter from the second compared method - * @return true in case parameter pair is of the same type; false - * otherwise - */ - private static boolean parametersTypeEquals(Parameter left, Parameter right) { - return Objects.equals(left.getSchema().getType(), right.getSchema().getType()) - && Objects.equals(left.getSchema().getFormat(), right.getSchema().getFormat()); - } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/DefaultPathMatcher.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/DefaultPathMatcher.java new file mode 100644 index 000000000..c354049e9 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/DefaultPathMatcher.java @@ -0,0 +1,82 @@ +package org.openapitools.openapidiff.core.compare.matchers; + +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.parameters.Parameter; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.IntStream; + +/** Default implementation of PathMatcher */ +public class DefaultPathMatcher implements PathMatcher { + + private static final String REGEX_PATH = "\\{([^/{}]+)}"; + + @Override + public Optional> find( + Map.Entry what, Map candidates) { + String leftUrl = what.getKey(); + PathItem leftPath = what.getValue(); + + final String template = normalizePath(leftUrl); + return candidates.entrySet().stream() + .filter(item -> normalizePath(item.getKey()).equals(template)) + .min( + (a, b) -> { + if (methodsAndParametersIntersect(a.getValue(), b.getValue())) { + throw new IllegalArgumentException( + "Two path items have the same signature: " + template); + } + if (a.getKey().equals(leftUrl)) { + return -1; + } else if (b.getKey().equals(leftUrl)) { + 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()); + } + }); + } + + private static String normalizePath(String path) { + return path.replaceAll(REGEX_PATH, "{}"); + } + + private static boolean methodsAndParametersIntersect(PathItem a, PathItem b) { + Set methodsA = a.readOperationsMap().keySet(); + for (PathItem.HttpMethod method : b.readOperationsMap().keySet()) { + if (methodsA.contains(method)) { + Operation left = a.readOperationsMap().get(method); + Operation right = b.readOperationsMap().get(method); + if (left.getParameters().size() == right.getParameters().size()) { + return parametersIntersect(left.getParameters(), right.getParameters()); + } + return false; + } + } + return false; + } + + private static boolean parametersIntersect(List left, List right) { + int parametersSize = left.size(); + long intersectedParameters = + IntStream.range(0, left.size()) + .filter(i -> parametersTypeEquals(left.get(i), right.get(i))) + .count(); + return parametersSize == intersectedParameters; + } + + private static boolean parametersTypeEquals(Parameter left, Parameter right) { + return Objects.equals(left.getSchema().getType(), right.getSchema().getType()) + && Objects.equals(left.getSchema().getFormat(), right.getSchema().getFormat()); + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/PathMatcher.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/PathMatcher.java new file mode 100644 index 000000000..339bf0212 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/matchers/PathMatcher.java @@ -0,0 +1,19 @@ +package org.openapitools.openapidiff.core.compare.matchers; + +import io.swagger.v3.oas.models.PathItem; +import java.util.Map; +import java.util.Optional; + +/** Strategy to find a matching path. */ +public interface PathMatcher { + + /** + * Finds a matching path entry. + * + * @param what entry of the path to find + * @param candidates map of right spec paths to search in + * @return Optional entry of the matching right path + */ + Optional> find( + Map.Entry what, Map candidates); +} From 5c0f4d30a1a468871e11c1a6c8fbeec7cb945411 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Wed, 3 Sep 2025 14:41:07 +0200 Subject: [PATCH 2/3] Fixes #823 --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index 019cca0b5..4db6c98f2 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,21 @@ public class Main { } ``` +### Path Matching while comparing two OpenAPI paths + +Path matching controls how paths from the old and new specs are paired during comparison. The default matcher (DefaultPathMatcher) normalizes templated segments so that, for example, `/users/{id}` matches `/users/{userId}`. Fails on ambiguous signatures. + +You can plug in a custom matcher via `OpenApiDiffOptions` implementing the `PathMatcher` interface.: + +```java +OpenApiDiffOptions options = OpenApiDiffOptions + .builder() + .pathMatcher(new MyCustomPathMatcher()) + .build(); + +ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec, null, options); +``` + ### Render difference --- From c312bfc8af5409aa1adbfd42b64bcde46bf4d849 Mon Sep 17 00:00:00 2001 From: Aliaksandr Pinchuk Date: Wed, 3 Sep 2025 14:47:11 +0200 Subject: [PATCH 3/3] docs: enhance README.md with detailed explanation of path matching behavior --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4db6c98f2..ddaf3a1f8 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ public class Main { ### Path Matching while comparing two OpenAPI paths -Path matching controls how paths from the old and new specs are paired during comparison. The default matcher (DefaultPathMatcher) normalizes templated segments so that, for example, `/users/{id}` matches `/users/{userId}`. Fails on ambiguous signatures. +Path matching controls how paths from the old and new specs are paired during comparison (PathsDiff.java). The default matcher (DefaultPathMatcher) obfuscates path parameter names, meaning `/users/{id}` matches `/users/{userId}`. Default matcher fails on ambiguous signatures if spec contains few paths semantically identical. In case this behaviour is not fitting your use case, you can implement your own matching strategy. You can plug in a custom matcher via `OpenApiDiffOptions` implementing the `PathMatcher` interface.: