diff --git a/README.md b/README.md index 019cca0b..ddaf3a1f 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 (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.: + +```java +OpenApiDiffOptions options = OpenApiDiffOptions + .builder() + .pathMatcher(new MyCustomPathMatcher()) + .build(); + +ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec, null, options); +``` + ### Render difference --- 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 23f4e1c3..b21fc1f0 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 8b4f459b..0352e017 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 00000000..c354049e --- /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 00000000..339bf021 --- /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); +}