diff --git a/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java b/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java index 6736a5fd7..f9d24d490 100644 --- a/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java +++ b/cli/src/main/java/org/openapitools/openapidiff/cli/Main.java @@ -1,14 +1,11 @@ package org.openapitools.openapidiff.cli; +import io.swagger.v3.parser.core.models.AuthorizationValue; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; - -import io.swagger.v3.parser.core.models.AuthorizationValue; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.DefaultParser; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ApiResponseDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ApiResponseDiff.java index 669a4fded..5c5a0bde7 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ApiResponseDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ApiResponseDiff.java @@ -1,18 +1,17 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.responses.ApiResponse; import io.swagger.v3.oas.models.responses.ApiResponses; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Optional; +import javax.annotation.Nullable; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedApiResponse; import org.openapitools.openapidiff.core.model.ChangedResponse; import org.openapitools.openapidiff.core.model.DiffContext; - -import javax.annotation.Nullable; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; /** Created by adarsh.sharma on 04/01/18. */ public class ApiResponseDiff { @@ -22,14 +21,22 @@ public ApiResponseDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - public Optional diff(@Nullable ApiResponses left, @Nullable ApiResponses right, DiffContext context) { + public DeferredChanged diff( + @Nullable ApiResponses left, @Nullable ApiResponses right, DiffContext context) { MapKeyDiff responseMapKeyDiff = MapKeyDiff.diff(left, right); List sharedResponseCodes = responseMapKeyDiff.getSharedKey(); Map resps = new LinkedHashMap<>(); + DeferredBuilder builder = new DeferredBuilder<>(); + for (String responseCode : sharedResponseCodes) { - openApiDiff - .getResponseDiff() - .diff(left != null ? left.get(responseCode) : null, right != null ? right.get(responseCode) : null, context) + builder + .with( + openApiDiff + .getResponseDiff() + .diff( + left != null ? left.get(responseCode) : null, + right != null ? right.get(responseCode) : null, + context)) .ifPresent(changedResponse -> resps.put(responseCode, changedResponse)); } ChangedApiResponse changedApiResponse = @@ -37,10 +44,15 @@ public Optional diff(@Nullable ApiResponses left, @Nullable .setIncreased(responseMapKeyDiff.getIncreased()) .setMissing(responseMapKeyDiff.getMissing()) .setChanged(resps); - openApiDiff - .getExtensionsDiff() - .diff(left != null ? left.getExtensions() : null, right != null ? right.getExtensions() : null, context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff( + left != null ? left.getExtensions() : null, + right != null ? right.getExtensions() : null, + context)) .ifPresent(changedApiResponse::setExtensions); - return isChanged(changedApiResponse); + return builder.buildIsChanged(changedApiResponse); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ContentDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ContentDiff.java index 0c08f9e49..9193238ac 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ContentDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ContentDiff.java @@ -6,9 +6,12 @@ import io.swagger.v3.oas.models.media.Content; import io.swagger.v3.oas.models.media.MediaType; import java.util.*; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedContent; import org.openapitools.openapidiff.core.model.ChangedMediaType; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; public class ContentDiff { @@ -18,32 +21,47 @@ public ContentDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - public Optional diff(Content left, Content right, DiffContext context) { + public DeferredChanged diff(Content left, Content right, DiffContext context) { + DeferredBuilder builder = new DeferredBuilder(); MapKeyDiff mediaTypeDiff = MapKeyDiff.diff(left, right); List sharedMediaTypes = mediaTypeDiff.getSharedKey(); Map changedMediaTypes = new LinkedHashMap<>(); - for (String mediaTypeKey : sharedMediaTypes) { - MediaType oldMediaType = left.get(mediaTypeKey); - MediaType newMediaType = right.get(mediaTypeKey); - ChangedMediaType changedMediaType = - new ChangedMediaType(oldMediaType.getSchema(), newMediaType.getSchema(), context); - openApiDiff - .getSchemaDiff() - .diff( - new HashSet<>(), - oldMediaType.getSchema(), - newMediaType.getSchema(), - context.copyWithRequired(true)) - .ifPresent(changedMediaType::setSchema); - if (!isUnchanged(changedMediaType)) { - changedMediaTypes.put(mediaTypeKey, changedMediaType); - } - } - return isChanged( - new ChangedContent(left, right, context) - .setIncreased(mediaTypeDiff.getIncreased()) - .setMissing(mediaTypeDiff.getMissing()) - .setChanged(changedMediaTypes)); + + sharedMediaTypes.stream() + .forEach( + (mediaTypeKey) -> { + MediaType oldMediaType = left.get(mediaTypeKey); + MediaType newMediaType = right.get(mediaTypeKey); + + ChangedMediaType changedMediaType = + new ChangedMediaType(oldMediaType.getSchema(), newMediaType.getSchema(), context); + + builder + .with( + openApiDiff + .getSchemaDiff() + .diff( + oldMediaType.getSchema(), + newMediaType.getSchema(), + context.copyWithRequired(true))) + .ifPresent( + (value) -> { + changedMediaType.setSchema(value); + if (!isUnchanged(changedMediaType)) { + changedMediaTypes.put(mediaTypeKey, changedMediaType); + } + }); + }); + + return builder + .build() + .mapOptional( + (value) -> + isChanged( + new ChangedContent(left, right, context) + .setIncreased(mediaTypeDiff.getIncreased()) + .setMissing(mediaTypeDiff.getMissing()) + .setChanged(changedMediaTypes))); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/HeaderDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/HeaderDiff.java index 343d044f1..e21f028f4 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/HeaderDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/HeaderDiff.java @@ -1,14 +1,15 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.headers.Header; import java.util.HashSet; import java.util.Objects; import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedHeader; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -31,16 +32,17 @@ public HeaderDiff(OpenApiDiff openApiDiff) { : null; } - public Optional diff(Header left, Header right, DiffContext context) { + public DeferredChanged diff(Header left, Header right, DiffContext context) { return cachedDiff(new HashSet<>(), left, right, left.get$ref(), right.get$ref(), context); } @Override - protected Optional computeDiff( + protected DeferredChanged computeDiff( HashSet refSet, Header left, Header right, DiffContext context) { left = refPointer.resolveRef(leftComponents, left, left.get$ref()); right = refPointer.resolveRef(rightComponents, right, right.get$ref()); + DeferredBuilder builder = new DeferredBuilder(); ChangedHeader changedHeader = new ChangedHeader(left, right, context) .setRequired(getBooleanDiff(left.getRequired(), right.getRequired())) @@ -49,23 +51,28 @@ protected Optional computeDiff( && Boolean.TRUE.equals(right.getDeprecated())) .setStyle(!Objects.equals(left.getStyle(), right.getStyle())) .setExplode(getBooleanDiff(left.getExplode(), right.getExplode())); - openApiDiff - .getMetadataDiff() - .diff(left.getDescription(), right.getDescription(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(left.getDescription(), right.getDescription(), context)) .ifPresent(changedHeader::setDescription); - openApiDiff - .getSchemaDiff() - .diff(new HashSet<>(), left.getSchema(), right.getSchema(), context.copyWithRequired(true)) + builder + .with( + openApiDiff + .getSchemaDiff() + .diff(left.getSchema(), right.getSchema(), context.copyWithRequired(true))) .ifPresent(changedHeader::setSchema); - openApiDiff - .getContentDiff() - .diff(left.getContent(), right.getContent(), context) + builder + .with(openApiDiff.getContentDiff().diff(left.getContent(), right.getContent(), context)) .ifPresent(changedHeader::setContent); - openApiDiff - .getExtensionsDiff() - .diff(left.getExtensions(), right.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(left.getExtensions(), right.getExtensions(), context)) .ifPresent(changedHeader::setExtensions); - return isChanged(changedHeader); + return builder.buildIsChanged(changedHeader); } private boolean getBooleanDiff(Boolean left, Boolean right) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/HeadersDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/HeadersDiff.java index f915f634d..44c8d3085 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/HeadersDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/HeadersDiff.java @@ -6,10 +6,11 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import org.openapitools.openapidiff.core.model.ChangedHeader; import org.openapitools.openapidiff.core.model.ChangedHeaders; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; /** Created by adarsh.sharma on 28/12/17. */ public class HeadersDiff { @@ -19,24 +20,28 @@ public HeadersDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - public Optional diff( + public DeferredChanged diff( Map left, Map right, DiffContext context) { MapKeyDiff headerMapDiff = MapKeyDiff.diff(left, right); List sharedHeaderKeys = headerMapDiff.getSharedKey(); Map changed = new LinkedHashMap<>(); + DeferredBuilder builder = new DeferredBuilder<>(); for (String headerKey : sharedHeaderKeys) { Header oldHeader = left.get(headerKey); Header newHeader = right.get(headerKey); - openApiDiff - .getHeaderDiff() - .diff(oldHeader, newHeader, context) + builder + .with(openApiDiff.getHeaderDiff().diff(oldHeader, newHeader, context)) .ifPresent(changedHeader -> changed.put(headerKey, changedHeader)); } - return isChanged( - new ChangedHeaders(left, right, context) - .setIncreased(headerMapDiff.getIncreased()) - .setMissing(headerMapDiff.getMissing()) - .setChanged(changed)); + return builder + .build() + .mapOptional( + (value) -> + isChanged( + new ChangedHeaders(left, right, context) + .setIncreased(headerMapDiff.getIncreased()) + .setMissing(headerMapDiff.getMissing()) + .setChanged(changed))); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java index 7b110769d..9fb292156 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/OpenApiDiff.java @@ -8,14 +8,10 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; -import org.openapitools.openapidiff.core.model.ChangedExtensions; -import org.openapitools.openapidiff.core.model.ChangedOpenApi; -import org.openapitools.openapidiff.core.model.ChangedOperation; -import org.openapitools.openapidiff.core.model.ChangedPath; -import org.openapitools.openapidiff.core.model.ChangedPaths; -import org.openapitools.openapidiff.core.model.Endpoint; +import org.openapitools.openapidiff.core.model.*; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.DeferredSchemaCache; import org.openapitools.openapidiff.core.utils.EndpointUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,6 +44,7 @@ public class OpenApiDiff { private List missingEndpoints; private List changedOperations; private ChangedExtensions changedExtensions; + private DeferredSchemaCache deferredSchemaCache; /* * @param oldSpecOpenApi @@ -86,17 +83,25 @@ private void initializeFields() { this.oAuthFlowDiff = new OAuthFlowDiff(this); this.extensionsDiff = new ExtensionsDiff(this); this.metadataDiff = new MetadataDiff(this); + this.deferredSchemaCache = new DeferredSchemaCache(this); } private ChangedOpenApi compare() { preProcess(oldSpecOpenApi); preProcess(newSpecOpenApi); - Optional paths = + + // 1st pass scans paths to collect all schemas + DeferredChanged paths = this.pathsDiff.diff( valOrEmpty(oldSpecOpenApi.getPaths()), valOrEmpty(newSpecOpenApi.getPaths())); + + // 2nd pass processes deferred schemas + deferredSchemaCache.process(); + this.newEndpoints = new ArrayList<>(); this.missingEndpoints = new ArrayList<>(); this.changedOperations = new ArrayList<>(); + paths.ifPresent( changedPaths -> { this.newEndpoints = EndpointUtils.convert2EndpointList(changedPaths.getIncreased()); @@ -117,6 +122,7 @@ private ChangedOpenApi compare() { getExtensionsDiff() .diff(oldSpecOpenApi.getExtensions(), newSpecOpenApi.getExtensions()) .ifPresent(this::setChangedExtension); + return getChangedOpenApi(); } @@ -162,7 +168,12 @@ private ChangedOpenApi getChangedOpenApi() { .setNewSpecOpenApi(newSpecOpenApi) .setOldSpecOpenApi(oldSpecOpenApi) .setChangedOperations(changedOperations) - .setChangedExtensions(changedExtensions); + .setChangedExtensions(changedExtensions) + .setChangedSchemas(deferredSchemaCache.getChangedSchemas()); + } + + public DeferredSchemaCache getDeferredSchemaCache() { + return deferredSchemaCache; } public PathsDiff getPathsDiff() { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/OperationDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/OperationDiff.java index 3a9f0f656..fb7f94742 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/OperationDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/OperationDiff.java @@ -7,50 +7,75 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedOperation; import org.openapitools.openapidiff.core.model.ChangedParameters; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Created by adarsh.sharma on 04/01/18. */ public class OperationDiff { + private static final Logger log = LoggerFactory.getLogger(OperationDiff.class); + private final OpenApiDiff openApiDiff; public OperationDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - public Optional diff( + public DeferredChanged diff( Operation oldOperation, Operation newOperation, DiffContext context) { + + DeferredBuilder builder = new DeferredBuilder<>(); ChangedOperation changedOperation = new ChangedOperation(context.getUrl(), context.getMethod(), oldOperation, newOperation); - openApiDiff - .getMetadataDiff() - .diff(oldOperation.getSummary(), newOperation.getSummary(), context) + log.debug( + "Diff operation {} {}", changedOperation.getPathUrl(), changedOperation.getHttpMethod()); + + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(oldOperation.getSummary(), newOperation.getSummary(), context)) .ifPresent(changedOperation::setSummary); - openApiDiff - .getMetadataDiff() - .diff(oldOperation.getDescription(), newOperation.getDescription(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(oldOperation.getDescription(), newOperation.getDescription(), context)) .ifPresent(changedOperation::setDescription); - openApiDiff - .getMetadataDiff() - .diff(oldOperation.getOperationId(), newOperation.getOperationId(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(oldOperation.getOperationId(), newOperation.getOperationId(), context)) .ifPresent(changedOperation::setOperationId); changedOperation.setDeprecated( !Boolean.TRUE.equals(oldOperation.getDeprecated()) && Boolean.TRUE.equals(newOperation.getDeprecated())); if (oldOperation.getRequestBody() != null || newOperation.getRequestBody() != null) { - openApiDiff - .getRequestBodyDiff() - .diff( - oldOperation.getRequestBody(), newOperation.getRequestBody(), context.copyAsRequest()) + builder + .with( + openApiDiff + .getRequestBodyDiff() + .diff( + oldOperation.getRequestBody(), + newOperation.getRequestBody(), + context.copyAsRequest())) .ifPresent(changedOperation::setRequestBody); } - openApiDiff - .getParametersDiff() - .diff(oldOperation.getParameters(), newOperation.getParameters(), context) + builder + .with( + openApiDiff + .getParametersDiff() + .diff(oldOperation.getParameters(), newOperation.getParameters(), context)) .ifPresent( params -> { removePathParameters(context.getParameters(), params); @@ -58,25 +83,62 @@ public Optional diff( }); if (oldOperation.getResponses() != null || newOperation.getResponses() != null) { - openApiDiff - .getApiResponseDiff() - .diff(oldOperation.getResponses(), newOperation.getResponses(), context.copyAsResponse()) - .ifPresent(changedOperation::setApiResponses); + builder + .with( + openApiDiff + .getApiResponseDiff() + .diff( + oldOperation.getResponses(), + newOperation.getResponses(), + context.copyAsResponse())) + .ifPresent( + (responses) -> { + log.debug( + "operation " + + changedOperation.getPathUrl() + + " " + + changedOperation.getHttpMethod() + + " setting api responses " + + responses.getChangedElements().stream() + .filter(c -> c != null) + .map(c -> c.isChanged()) + .filter(c -> c != null) + .map(c -> c.toString()) + .collect(Collectors.joining(","))); + changedOperation.setApiResponses(responses); + }); } if (oldOperation.getSecurity() != null || newOperation.getSecurity() != null) { - openApiDiff - .getSecurityRequirementsDiff() - .diff(oldOperation.getSecurity(), newOperation.getSecurity(), context) + builder + .with( + openApiDiff + .getSecurityRequirementsDiff() + .diff(oldOperation.getSecurity(), newOperation.getSecurity(), context)) .ifPresent(changedOperation::setSecurityRequirements); } - openApiDiff - .getExtensionsDiff() - .diff(oldOperation.getExtensions(), newOperation.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(oldOperation.getExtensions(), newOperation.getExtensions(), context)) .ifPresent(changedOperation::setExtensions); - return isChanged(changedOperation); + return builder + .build() + .mapOptional( + (value) -> { + Optional changed = isChanged(changedOperation); + log.debug( + "Is changed operation " + + changedOperation.getPathUrl() + + " " + + changedOperation.getHttpMethod() + + " changed: " + + changed.map(c -> c.isChanged()).orElse(null)); + return changed; + }); } public void removePathParameters(Map pathParameters, ChangedParameters params) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ParameterDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ParameterDiff.java index 37d769f1a..6652b0bc4 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ParameterDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ParameterDiff.java @@ -1,14 +1,15 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.parameters.Parameter; import java.util.HashSet; import java.util.Objects; import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedParameter; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -31,16 +32,19 @@ public ParameterDiff(OpenApiDiff openApiDiff) { : null; } - public Optional diff(Parameter left, Parameter right, DiffContext context) { + public DeferredChanged diff( + Parameter left, Parameter right, DiffContext context) { return cachedDiff(new HashSet<>(), left, right, left.get$ref(), right.get$ref(), context); } @Override - protected Optional computeDiff( + protected DeferredChanged computeDiff( HashSet refSet, Parameter left, Parameter right, DiffContext context) { left = refPointer.resolveRef(this.leftComponents, left, left.get$ref()); right = refPointer.resolveRef(this.rightComponents, right, right.get$ref()); + DeferredBuilder builder = new DeferredBuilder<>(); + ChangedParameter changedParameter = new ChangedParameter(right.getName(), right.getIn(), context) .setOldParameter(left) @@ -53,23 +57,28 @@ protected Optional computeDiff( getBooleanDiff(left.getAllowEmptyValue(), right.getAllowEmptyValue())) .setChangeStyle(!Objects.equals(left.getStyle(), right.getStyle())) .setChangeExplode(getBooleanDiff(left.getExplode(), right.getExplode())); - openApiDiff - .getSchemaDiff() - .diff(refSet, left.getSchema(), right.getSchema(), context.copyWithRequired(true)) + builder + .with( + openApiDiff + .getSchemaDiff() + .diff(left.getSchema(), right.getSchema(), context.copyWithRequired(true))) .ifPresent(changedParameter::setSchema); - openApiDiff - .getMetadataDiff() - .diff(left.getDescription(), right.getDescription(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(left.getDescription(), right.getDescription(), context)) .ifPresent(changedParameter::setDescription); - openApiDiff - .getContentDiff() - .diff(left.getContent(), right.getContent(), context) + builder + .with(openApiDiff.getContentDiff().diff(left.getContent(), right.getContent(), context)) .ifPresent(changedParameter::setContent); - openApiDiff - .getExtensionsDiff() - .diff(left.getExtensions(), right.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(left.getExtensions(), right.getExtensions(), context)) .ifPresent(changedParameter::setExtensions); - return isChanged(changedParameter); + return builder.buildIsChanged(changedParameter); } private boolean getBooleanDiff(Boolean left, Boolean right) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ParametersDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ParametersDiff.java index f7fa573b2..5271f363e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ParametersDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ParametersDiff.java @@ -1,15 +1,16 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.parameters.Parameter; import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedParameters; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -49,8 +50,10 @@ public static boolean same(Parameter left, Parameter right) { && Objects.equals(left.getIn(), right.getIn()); } - public Optional diff( + public DeferredChanged diff( List left, List right, DiffContext context) { + + DeferredBuilder builder = new DeferredBuilder(); ChangedParameters changedParameters = new ChangedParameters(left, right != null ? new ArrayList<>(right) : null, context); if (null == left) left = new ArrayList<>(); @@ -65,14 +68,13 @@ public Optional diff( } else { Parameter rightPara = rightParam.get(); right.remove(rightPara); - openApiDiff - .getParameterDiff() - .diff(leftPara, rightPara, context) + builder + .with(openApiDiff.getParameterDiff().diff(leftPara, rightPara, context)) .ifPresent(changedParameters.getChanged()::add); } } changedParameters.getIncreased().addAll(right); - return isChanged(changedParameters); + return builder.buildIsChanged(changedParameters); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathDiff.java index 1562dfbb4..87494521e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/PathDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/PathDiff.java @@ -1,14 +1,14 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; import java.util.List; import java.util.Map; -import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedPath; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; public class PathDiff { private final OpenApiDiff openApiDiff; @@ -17,12 +17,14 @@ public PathDiff(OpenApiDiff openApiDiff) { this.openApiDiff = openApiDiff; } - public Optional diff(PathItem left, PathItem right, DiffContext context) { + public DeferredChanged diff(PathItem left, PathItem right, DiffContext context) { Map oldOperationMap = left.readOperationsMap(); Map newOperationMap = right.readOperationsMap(); MapKeyDiff operationsDiff = MapKeyDiff.diff(oldOperationMap, newOperationMap); List sharedMethods = operationsDiff.getSharedKey(); + DeferredBuilder builder = new DeferredBuilder(); + ChangedPath changedPath = new ChangedPath(context.getUrl(), left, right, context) .setIncreased(operationsDiff.getIncreased()) @@ -30,15 +32,20 @@ public Optional diff(PathItem left, PathItem right, DiffContext con for (PathItem.HttpMethod method : sharedMethods) { Operation oldOperation = oldOperationMap.get(method); Operation newOperation = newOperationMap.get(method); - openApiDiff - .getOperationDiff() - .diff(oldOperation, newOperation, context.copyWithMethod(method)) + builder + .with( + openApiDiff + .getOperationDiff() + .diff(oldOperation, newOperation, context.copyWithMethod(method))) .ifPresent(changedPath.getChanged()::add); } - openApiDiff - .getExtensionsDiff() - .diff(left.getExtensions(), right.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(left.getExtensions(), right.getExtensions(), context)) .ifPresent(changedPath::setExtensions); - return isChanged(changedPath); + + return builder.buildIsChanged(changedPath); } } 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 e3a743455..7e977b6cd 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,14 +1,15 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.oas.models.Paths; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedPaths; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; public class PathsDiff { private static final String REGEX_PATH = "\\{([^/]+)\\}"; @@ -32,8 +33,10 @@ private static List extractParameters(String path) { return params; } - public Optional diff( + public DeferredChanged diff( final Map left, final Map right) { + DeferredBuilder builder = new DeferredBuilder(); + ChangedPaths changedPaths = new ChangedPaths(left, right); changedPaths.getIncreased().putAll(right); @@ -45,25 +48,26 @@ public Optional diff( 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( + .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 (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()) { String rightUrl = result.get().getKey(); PathItem rightPath = changedPaths.getIncreased().remove(rightUrl); @@ -78,15 +82,14 @@ public Optional diff( DiffContext context = new DiffContext(); context.setUrl(url); context.setParameters(params); - openApiDiff - .getPathDiff() - .diff(leftPath, rightPath, context) + builder + .with(openApiDiff.getPathDiff().diff(leftPath, rightPath, context)) .ifPresent(path -> changedPaths.getChanged().put(rightUrl, path)); } else { changedPaths.getMissing().put(url, leftPath); } }); - return isChanged(changedPaths); + return builder.buildIsChanged(changedPaths); } public static Paths valOrEmpty(Paths path) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ReferenceDiffCache.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ReferenceDiffCache.java index befe8cff9..bc20f8fa5 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ReferenceDiffCache.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ReferenceDiffCache.java @@ -3,26 +3,28 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; /** Created by adarsh.sharma on 07/01/18. */ -public abstract class ReferenceDiffCache { - private final Map refDiffMap; +public abstract class ReferenceDiffCache { + private final Map> refDiffMap; public ReferenceDiffCache() { this.refDiffMap = new HashMap<>(); } - private Optional getFromCache(CacheKey cacheKey) { - return Optional.ofNullable(refDiffMap.get(cacheKey)); + private DeferredChanged getFromCache(CacheKey cacheKey) { + return refDiffMap.get(cacheKey); } - private void addToCache(CacheKey cacheKey, D changed) { + private void addToCache(CacheKey cacheKey, DeferredChanged changed) { refDiffMap.put(cacheKey, changed); } - public Optional cachedDiff( + public DeferredChanged cachedDiff( HashSet refSet, C left, C right, @@ -32,17 +34,17 @@ public Optional cachedDiff( boolean areBothRefParameters = leftRef != null && rightRef != null; if (areBothRefParameters) { CacheKey key = new CacheKey(leftRef, rightRef, context); - Optional changedFromRef = getFromCache(key); - if (changedFromRef.isPresent()) { + DeferredChanged changedFromRef = getFromCache(key); + if (changedFromRef != null) { return changedFromRef; } else { String refKey = getRefKey(leftRef, rightRef); if (refSet.contains(refKey)) { - return Optional.empty(); + return RealizedChanged.empty(); } else { refSet.add(refKey); - Optional changed = computeDiff(refSet, left, right, context); - addToCache(key, changed.orElse(null)); + DeferredChanged changed = computeDiff(refSet, left, right, context); + addToCache(key, changed); refSet.remove(refKey); return changed; } @@ -56,6 +58,6 @@ protected String getRefKey(String leftRef, String rightRef) { return leftRef + ":" + rightRef; } - protected abstract Optional computeDiff( + protected abstract DeferredChanged computeDiff( HashSet refSet, C left, C right, DiffContext context); } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/RequestBodyDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/RequestBodyDiff.java index 0a1d8c4a0..22d1fe119 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/RequestBodyDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/RequestBodyDiff.java @@ -1,15 +1,16 @@ package org.openapitools.openapidiff.core.compare; import static java.util.Optional.ofNullable; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; import io.swagger.v3.oas.models.media.Content; import io.swagger.v3.oas.models.parameters.RequestBody; import java.util.HashSet; import java.util.Map; -import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedRequestBody; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -27,7 +28,7 @@ private static Map getExtensions(RequestBody body) { return ofNullable(body).map(RequestBody::getExtensions).orElse(null); } - public Optional diff( + public DeferredChanged diff( RequestBody left, RequestBody right, DiffContext context) { String leftRef = left != null ? left.get$ref() : null, rightRef = right != null ? right.get$ref() : null; @@ -35,8 +36,10 @@ public Optional diff( } @Override - protected Optional computeDiff( + protected DeferredChanged computeDiff( HashSet refSet, RequestBody left, RequestBody right, DiffContext context) { + DeferredBuilder builder = new DeferredBuilder(); + Content oldRequestContent = new Content(); Content newRequestContent = new Content(); RequestBody oldRequestBody = null; @@ -65,22 +68,25 @@ protected Optional computeDiff( ChangedRequestBody changedRequestBody = new ChangedRequestBody(oldRequestBody, newRequestBody, context) .setChangeRequired(leftRequired != rightRequired); - openApiDiff - .getMetadataDiff() - .diff( - oldRequestBody != null ? oldRequestBody.getDescription() : null, - newRequestBody != null ? newRequestBody.getDescription() : null, - context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff( + oldRequestBody != null ? oldRequestBody.getDescription() : null, + newRequestBody != null ? newRequestBody.getDescription() : null, + context)) .ifPresent(changedRequestBody::setDescription); - openApiDiff - .getContentDiff() - .diff(oldRequestContent, newRequestContent, context) + builder + .with(openApiDiff.getContentDiff().diff(oldRequestContent, newRequestContent, context)) .ifPresent(changedRequestBody::setContent); - openApiDiff - .getExtensionsDiff() - .diff(getExtensions(left), getExtensions(right), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(getExtensions(left), getExtensions(right), context)) .ifPresent(changedRequestBody::setExtensions); - return isChanged(changedRequestBody); + return builder.buildIsChanged(changedRequestBody); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/ResponseDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/ResponseDiff.java index 0a84de744..b18b77f4a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/ResponseDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/ResponseDiff.java @@ -1,13 +1,15 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.responses.ApiResponse; import java.util.HashSet; import java.util.Optional; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedResponse; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -30,33 +32,44 @@ public ResponseDiff(OpenApiDiff openApiDiff) { : null; } - public Optional diff(ApiResponse left, ApiResponse right, DiffContext context) { + public DeferredChanged diff( + ApiResponse left, ApiResponse right, DiffContext context) { + if (left == null && right == null) { + return new RealizedChanged(Optional.empty()); + } + if ((left == null && right != null) || (left != null && right == null)) { + return new RealizedChanged(Optional.of(new ChangedResponse(left, right, context))); + } return cachedDiff(new HashSet<>(), left, right, left.get$ref(), right.get$ref(), context); } @Override - protected Optional computeDiff( + protected DeferredChanged computeDiff( HashSet refSet, ApiResponse left, ApiResponse right, DiffContext context) { left = refPointer.resolveRef(leftComponents, left, left.get$ref()); right = refPointer.resolveRef(rightComponents, right, right.get$ref()); + DeferredBuilder builder = new DeferredBuilder<>(); ChangedResponse changedResponse = new ChangedResponse(left, right, context); - openApiDiff - .getMetadataDiff() - .diff(left.getDescription(), right.getDescription(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(left.getDescription(), right.getDescription(), context)) .ifPresent(changedResponse::setDescription); - openApiDiff - .getContentDiff() - .diff(left.getContent(), right.getContent(), context) + builder + .with(openApiDiff.getContentDiff().diff(left.getContent(), right.getContent(), context)) .ifPresent(changedResponse::setContent); - openApiDiff - .getHeadersDiff() - .diff(left.getHeaders(), right.getHeaders(), context) + builder + .with(openApiDiff.getHeadersDiff().diff(left.getHeaders(), right.getHeaders(), context)) .ifPresent(changedResponse::setHeaders); - openApiDiff - .getExtensionsDiff() - .diff(left.getExtensions(), right.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(left.getExtensions(), right.getExtensions(), context)) .ifPresent(changedResponse::setExtensions); - return isChanged(changedResponse); + + return builder.buildIsChanged(changedResponse); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java index 0c25509c5..990029026 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java @@ -21,10 +21,13 @@ import org.openapitools.openapidiff.core.compare.schemadiffresult.SchemaDiffResult; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; +import org.openapitools.openapidiff.core.model.deferred.RecursiveSchemaSet; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; -public class SchemaDiff extends ReferenceDiffCache { +public class SchemaDiff { private static final RefPointer refPointer = new RefPointer<>(RefType.SCHEMAS); private static final Map, Class> @@ -274,17 +277,27 @@ private static String getSchemaRef(Schema schema) { return ofNullable(schema).map(Schema::get$ref).orElse(null); } - public Optional diff( + @Deprecated + public DeferredChanged diff( HashSet refSet, Schema left, Schema right, DiffContext context) { + return this.diff(new RecursiveSchemaSet(), left, right, context); + } + + public DeferredChanged diff(Schema left, Schema right, DiffContext context) { + return this.diff(new RecursiveSchemaSet(), left, right, context); + } + + public DeferredChanged diff( + RecursiveSchemaSet refSet, Schema left, Schema right, DiffContext context) { if (left == null && right == null) { - return Optional.empty(); + return new RealizedChanged<>(Optional.empty()); } - return cachedDiff(refSet, left, right, getSchemaRef(left), getSchemaRef(right), context); + return computeDeferredDiff(refSet, left, right, context); } - public Optional getTypeChangedSchema( + public DeferredChanged getTypeChangedSchema( Schema left, Schema right, DiffContext context) { - return Optional.of( + return new RealizedChanged( SchemaDiff.getSchemaDiffResult(openApiDiff) .getChangedSchema() .setOldSchema(left) @@ -293,9 +306,20 @@ public Optional getTypeChangedSchema( .setContext(context)); } - @Override - protected Optional computeDiff( - HashSet refSet, Schema left, Schema right, DiffContext context) { + protected DeferredChanged computeDeferredDiff( + RecursiveSchemaSet refSet, Schema left, Schema right, DiffContext context) { + + CacheKey key = new CacheKey(getSchemaRef(left), getSchemaRef(right), context); + if (key.getLeft() != null && key.getRight() != null) { + return openApiDiff.getDeferredSchemaCache().getOrAddSchema(refSet, key, left, right); + } else { + return computeDiffForReal(refSet, left, right, context); + } + } + + public DeferredChanged computeDiffForReal( + RecursiveSchemaSet refSet, Schema left, Schema right, DiffContext context) { + left = refPointer.resolveRef(this.leftComponents, left, getSchemaRef(left)); right = refPointer.resolveRef(this.rightComponents, right, getSchemaRef(right)); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java index 6c058ab07..4e41f077a 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementDiff.java @@ -1,13 +1,14 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.security.SecurityRequirement; import io.swagger.v3.oas.models.security.SecurityScheme; import java.util.*; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedSecurityRequirement; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; /** Created by adarsh.sharma on 07/01/18. */ public class SecurityRequirementDiff { @@ -61,8 +62,10 @@ private LinkedHashMap> contains( return found; } - public Optional diff( + public DeferredChanged diff( SecurityRequirement left, SecurityRequirement right, DiffContext context) { + DeferredBuilder builder = new DeferredBuilder(); + ChangedSecurityRequirement changedSecurityRequirement = new ChangedSecurityRequirement(left, right != null ? getCopy(right) : null); @@ -76,22 +79,23 @@ public Optional diff( } else { Optional rightSchemeRef = rightSec.keySet().stream().findFirst(); rightSchemeRef.ifPresent(rightRequirement::remove); - rightSchemeRef - .flatMap( - rightScheme -> - openApiDiff - .getSecuritySchemeDiff() - .diff( - leftEntry.getKey(), - leftEntry.getValue(), - rightScheme, - rightSec.get(rightScheme), - context)) - .ifPresent(changedSecurityRequirement::addChanged); + rightSchemeRef.ifPresent( + rightScheme -> + builder + .with( + openApiDiff + .getSecuritySchemeDiff() + .diff( + leftEntry.getKey(), + leftEntry.getValue(), + rightScheme, + rightSec.get(rightScheme), + context)) + .ifPresent(changedSecurityRequirement::addChanged)); } } rightRequirement.forEach(changedSecurityRequirement::addIncreased); - return isChanged(changedSecurityRequirement); + return builder.buildIsChanged(changedSecurityRequirement); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java index 30b99a542..6ba68fdfa 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecurityRequirementsDiff.java @@ -1,7 +1,5 @@ package org.openapitools.openapidiff.core.compare; -import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; - import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.security.SecurityRequirement; import io.swagger.v3.oas.models.security.SecurityScheme; @@ -13,9 +11,11 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; -import org.openapitools.openapidiff.core.model.ChangedSecurityRequirement; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedSecurityRequirements; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; /** Created by adarsh.sharma on 07/01/18. */ public class SecurityRequirementsDiff { @@ -77,8 +77,11 @@ private Pair getPair(SecurityScheme secu return new ImmutablePair<>(securityScheme.getType(), securityScheme.getIn()); } - protected Optional diff( + protected DeferredChanged diff( List left, List right, DiffContext context) { + + DeferredBuilder builder = new DeferredBuilder(); + left = left == null ? new ArrayList<>() : left; right = right == null ? new ArrayList<>() : getCopy(right); @@ -92,14 +95,14 @@ protected Optional diff( } else { SecurityRequirement rightSec = rightSecOpt.get(); right.remove(rightSec); - Optional diff = - openApiDiff.getSecurityRequirementDiff().diff(leftSecurity, rightSec, context); - diff.ifPresent(changedSecurityRequirements::addChanged); + builder + .with(openApiDiff.getSecurityRequirementDiff().diff(leftSecurity, rightSec, context)) + .ifPresent(changedSecurityRequirements::addChanged); } } right.forEach(changedSecurityRequirements::addIncreased); - return isChanged(changedSecurityRequirements); + return builder.buildIsChanged(changedSecurityRequirements); } private List getCopy(List right) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java index cf11b0d12..ad011f1fb 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/SecuritySchemeDiff.java @@ -7,10 +7,11 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Optional; import org.openapitools.openapidiff.core.model.ChangedSecurityScheme; import org.openapitools.openapidiff.core.model.ChangedSecuritySchemeScopes; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; /** Created by adarsh.sharma on 11/01/18. */ public class SecuritySchemeDiff extends ReferenceDiffCache { @@ -30,7 +31,7 @@ public SecuritySchemeDiff(OpenApiDiff openApiDiff) { : null; } - public Optional diff( + public DeferredChanged diff( String leftSchemeRef, List leftScopes, String rightSchemeRef, @@ -38,7 +39,7 @@ public Optional diff( DiffContext context) { SecurityScheme leftSecurityScheme = leftComponents.getSecuritySchemes().get(leftSchemeRef); SecurityScheme rightSecurityScheme = rightComponents.getSecuritySchemes().get(rightSchemeRef); - Optional changedSecuritySchemeOpt = + DeferredChanged changedSecuritySchemeOpt = cachedDiff( new HashSet<>(), leftSecurityScheme, @@ -46,22 +47,26 @@ public Optional diff( leftSchemeRef, rightSchemeRef, context); - ChangedSecurityScheme changedSecurityScheme = - changedSecuritySchemeOpt.orElse( - new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme)); - changedSecurityScheme = getCopyWithoutScopes(changedSecurityScheme); - - if (changedSecurityScheme != null - && leftSecurityScheme.getType() == SecurityScheme.Type.OAUTH2) { - isChanged(ListDiff.diff(new ChangedSecuritySchemeScopes(leftScopes, rightScopes))) - .ifPresent(changedSecurityScheme::setChangedScopes); - } - return isChanged(changedSecurityScheme); + return changedSecuritySchemeOpt.map( + (changedSecuritySchemeOptional) -> { + ChangedSecurityScheme changedSecurityScheme = + changedSecuritySchemeOptional.orElse( + new ChangedSecurityScheme(leftSecurityScheme, rightSecurityScheme)); + changedSecurityScheme = getCopyWithoutScopes(changedSecurityScheme); + + if (changedSecurityScheme != null + && leftSecurityScheme.getType() == SecurityScheme.Type.OAUTH2) { + isChanged(ListDiff.diff(new ChangedSecuritySchemeScopes(leftScopes, rightScopes))) + .ifPresent(changedSecurityScheme::setChangedScopes); + } + + return changedSecurityScheme; + }); } @Override - protected Optional computeDiff( + protected DeferredChanged computeDiff( HashSet refSet, SecurityScheme leftSecurityScheme, SecurityScheme rightSecurityScheme, @@ -107,7 +112,7 @@ protected Optional computeDiff( .diff(leftSecurityScheme.getExtensions(), rightSecurityScheme.getExtensions(), context) .ifPresent(changedSecurityScheme::setExtensions); - return Optional.of(changedSecurityScheme); + return new RealizedChanged(changedSecurityScheme); } private ChangedSecurityScheme getCopyWithoutScopes(ChangedSecurityScheme original) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ArraySchemaDiffResult.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ArraySchemaDiffResult.java index cefa18e3f..8c6357ab7 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ArraySchemaDiffResult.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ArraySchemaDiffResult.java @@ -3,11 +3,11 @@ import io.swagger.v3.oas.models.Components; import io.swagger.v3.oas.models.media.ArraySchema; import io.swagger.v3.oas.models.media.Schema; -import java.util.HashSet; -import java.util.Optional; import org.openapitools.openapidiff.core.compare.OpenApiDiff; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RecursiveSchemaSet; /** Created by adarsh.sharma on 18/12/17. */ public class ArraySchemaDiffResult extends SchemaDiffResult { @@ -16,8 +16,8 @@ public ArraySchemaDiffResult(OpenApiDiff openApiDiff) { } @Override - public , X> Optional diff( - HashSet refSet, + public , X> DeferredChanged diff( + RecursiveSchemaSet refSet, Components leftComponents, Components rightComponents, T left, @@ -25,15 +25,23 @@ public , X> Optional diff( DiffContext context) { ArraySchema leftArraySchema = (ArraySchema) left; ArraySchema rightArraySchema = (ArraySchema) right; - super.diff(refSet, leftComponents, rightComponents, left, right, context); - openApiDiff - .getSchemaDiff() - .diff( - refSet, - leftArraySchema.getItems(), - rightArraySchema.getItems(), - context.copyWithRequired(true)) - .ifPresent(changedSchema::setItems); - return isApplicable(context); + + DeferredChanged superSchemaDiff = + super.diff(refSet, leftComponents, rightComponents, left, right, context) + .flatMap( + (changeSchemaOptional) -> { + DeferredChanged itemsDiff = + openApiDiff + .getSchemaDiff() + .diff( + refSet, + leftArraySchema.getItems(), + rightArraySchema.getItems(), + context.copyWithRequired(true)); + itemsDiff.ifPresent(changedSchema::setItems); + return itemsDiff; + }); + + return superSchemaDiff.mapOptional(schemaOptional -> isApplicable(context)); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java index ea681db13..d55e0271d 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/ComposedSchemaDiffResult.java @@ -4,7 +4,6 @@ import io.swagger.v3.oas.models.media.ComposedSchema; import io.swagger.v3.oas.models.media.Discriminator; import io.swagger.v3.oas.models.media.Schema; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -16,6 +15,10 @@ import org.openapitools.openapidiff.core.model.ChangedOneOfSchema; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RealizedChanged; +import org.openapitools.openapidiff.core.model.deferred.RecursiveSchemaSet; import org.openapitools.openapidiff.core.utils.RefPointer; import org.openapitools.openapidiff.core.utils.RefType; @@ -28,16 +31,19 @@ public ComposedSchemaDiffResult(OpenApiDiff openApiDiff) { } @Override - public , X> Optional diff( - HashSet refSet, + public , X> DeferredChanged diff( + RecursiveSchemaSet refSet, Components leftComponents, Components rightComponents, T left, T right, DiffContext context) { + if (left instanceof ComposedSchema) { ComposedSchema leftComposedSchema = (ComposedSchema) left; ComposedSchema rightComposedSchema = (ComposedSchema) right; + DeferredBuilder discriminatorChangedBuilder = new DeferredBuilder<>(); + if (CollectionUtils.isNotEmpty(leftComposedSchema.getOneOf()) || CollectionUtils.isNotEmpty(rightComposedSchema.getOneOf())) { @@ -56,7 +62,7 @@ public , X> Optional diff( changedSchema.setNewSchema(right); changedSchema.setDiscriminatorPropertyChanged(true); changedSchema.setContext(context); - return Optional.of(changedSchema); + return new RealizedChanged<>(Optional.of(changedSchema)); } Map leftMapping = getMapping(leftComposedSchema); @@ -72,25 +78,36 @@ public , X> Optional diff( leftSchema.set$ref(leftMapping.get(key)); Schema rightSchema = new Schema(); rightSchema.set$ref(rightMapping.get(key)); - Optional changedSchema = - openApiDiff - .getSchemaDiff() - .diff(refSet, leftSchema, rightSchema, context.copyWithRequired(true)); - changedSchema.ifPresent(schema -> changedMapping.put(key, schema)); + discriminatorChangedBuilder + .with( + openApiDiff + .getSchemaDiff() + .diff(refSet, leftSchema, rightSchema, context.copyWithRequired(true))) + .ifPresent(schema -> changedMapping.put(key, schema)); } - changedSchema.setOneOfSchema( - new ChangedOneOfSchema(leftMapping, rightMapping, context) - .setIncreased(mappingDiff.getIncreased()) - .setMissing(mappingDiff.getMissing()) - .setChanged(changedMapping)); + + discriminatorChangedBuilder.whenSet( + (composedSchemas) -> { + changedSchema.setOneOfSchema( + new ChangedOneOfSchema(leftMapping, rightMapping, context) + .setIncreased(mappingDiff.getIncreased()) + .setMissing(mappingDiff.getMissing()) + .setChanged(changedMapping)); + }); } - return super.diff(refSet, leftComponents, rightComponents, left, right, context); + + return discriminatorChangedBuilder + .build() + .flatMap( + (values) -> + super.diff(refSet, leftComponents, rightComponents, left, right, context)); } else { return openApiDiff.getSchemaDiff().getTypeChangedSchema(left, right, context); } } - private Map getSchema(Components components, Map mapping, ComposedSchema composedSchema) { + private Map getSchema( + Components components, Map mapping, ComposedSchema composedSchema) { Map result = new LinkedHashMap<>(); mapping.forEach( (key, value) -> result.put(key, refPointer.resolveRef(components, new Schema(), value))); @@ -106,7 +123,7 @@ private Map getMapping(ComposedSchema composedSchema) { for (Schema schema : composedSchema.getOneOf()) { String ref = schema.get$ref(); if (ref == null) { - continue; + continue; } String schemaName = refPointer.getRefName(ref); if (schemaName == null) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java index 24f178c65..d2be3725e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/compare/schemadiffresult/SchemaDiffResult.java @@ -10,8 +10,12 @@ import org.openapitools.openapidiff.core.compare.MapKeyDiff; import org.openapitools.openapidiff.core.compare.OpenApiDiff; import org.openapitools.openapidiff.core.model.Change; +import org.openapitools.openapidiff.core.model.Changed; import org.openapitools.openapidiff.core.model.ChangedSchema; import org.openapitools.openapidiff.core.model.DiffContext; +import org.openapitools.openapidiff.core.model.deferred.DeferredBuilder; +import org.openapitools.openapidiff.core.model.deferred.DeferredChanged; +import org.openapitools.openapidiff.core.model.deferred.RecursiveSchemaSet; import org.openapitools.openapidiff.core.model.schema.*; public class SchemaDiffResult { @@ -28,13 +32,16 @@ public SchemaDiffResult(String type, OpenApiDiff openApiDiff) { this.changedSchema.setType(type); } - public , X> Optional diff( - HashSet refSet, + public , X> DeferredChanged diff( + RecursiveSchemaSet refSet, Components leftComponents, Components rightComponents, V left, V right, DiffContext context) { + + DeferredBuilder builder = new DeferredBuilder<>(); + ChangedEnum changedEnum = ListDiff.diff(new ChangedEnum<>(left.getEnum(), right.getEnum(), context)); changedSchema @@ -53,36 +60,42 @@ public , X> Optional diff( .setReadOnly(new ChangedReadOnly(left.getReadOnly(), right.getReadOnly(), context)) .setWriteOnly(new ChangedWriteOnly(left.getWriteOnly(), right.getWriteOnly(), context)) .setMaxLength(new ChangedMaxLength(left.getMaxLength(), right.getMaxLength(), context)); - openApiDiff - .getExtensionsDiff() - .diff(left.getExtensions(), right.getExtensions(), context) + builder + .with( + openApiDiff + .getExtensionsDiff() + .diff(left.getExtensions(), right.getExtensions(), context)) .ifPresent(changedSchema::setExtensions); - openApiDiff - .getMetadataDiff() - .diff(left.getDescription(), right.getDescription(), context) + builder + .with( + openApiDiff + .getMetadataDiff() + .diff(left.getDescription(), right.getDescription(), context)) .ifPresent(changedSchema::setDescription); Map leftProperties = left.getProperties(); Map rightProperties = right.getProperties(); MapKeyDiff propertyDiff = MapKeyDiff.diff(leftProperties, rightProperties); for (String key : propertyDiff.getSharedKey()) { - openApiDiff - .getSchemaDiff() - .diff( - refSet, - leftProperties.get(key), - rightProperties.get(key), - required(context, key, right.getRequired())) + builder + .with( + openApiDiff + .getSchemaDiff() + .diff( + refSet, + leftProperties.get(key), + rightProperties.get(key), + required(context, key, right.getRequired()))) .ifPresent( changedSchema1 -> changedSchema.getChangedProperties().put(key, changedSchema1)); } - compareAdditionalProperties(refSet, left, right, context); + compareAdditionalProperties(refSet, left, right, context, builder); changedSchema .getIncreasedProperties() .putAll(filterProperties(Change.Type.ADDED, propertyDiff.getIncreased(), context)); changedSchema .getMissingProperties() .putAll(filterProperties(Change.Type.REMOVED, propertyDiff.getMissing(), context)); - return isApplicable(context); + return builder.build().mapOptional((values) -> isApplicable(context)); } protected Optional isApplicable(DiffContext context) { @@ -125,7 +138,11 @@ private DiffContext required(DiffContext context, String key, List requi } private void compareAdditionalProperties( - HashSet refSet, Schema leftSchema, Schema rightSchema, DiffContext context) { + RecursiveSchemaSet refSet, + Schema leftSchema, + Schema rightSchema, + DiffContext context, + DeferredBuilder builder) { Object left = leftSchema.getAdditionalProperties(); Object right = rightSchema.getAdditionalProperties(); if (left instanceof Schema || right instanceof Schema) { @@ -137,7 +154,7 @@ private void compareAdditionalProperties( .setOldSchema(leftAdditionalSchema) .setNewSchema(rightAdditionalSchema); if (left != null && right != null) { - Optional addPropChangedSchemaOP = + DeferredChanged addPropChangedSchemaOP = openApiDiff .getSchemaDiff() .diff( @@ -145,9 +162,16 @@ private void compareAdditionalProperties( leftAdditionalSchema, rightAdditionalSchema, context.copyWithRequired(false)); - apChangedSchema = addPropChangedSchemaOP.orElse(apChangedSchema); + builder + .with(addPropChangedSchemaOP) + .whenSet( + (optional) -> { + ChangedSchema apc = optional.orElse(apChangedSchema); + isChanged(apc).ifPresent(changedSchema::setAddProp); + }); + } else { + isChanged(apChangedSchema).ifPresent(changedSchema::setAddProp); } - isChanged(apChangedSchema).ifPresent(changedSchema::setAddProp); } } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/Change.java b/core/src/main/java/org/openapitools/openapidiff/core/model/Change.java index 604aaffe9..a688336e6 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/Change.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/Change.java @@ -22,7 +22,7 @@ public static Change removed(T oldValue) { public enum Type { ADDED, CHANGED, - REMOVED; + REMOVED } public Change(final T oldValue, final T newValue, final Type type) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java index 99c6a387d..dca98a5f9 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedOpenApi.java @@ -13,6 +13,7 @@ public class ChangedOpenApi implements ComposedChanged { private List newEndpoints; private List missingEndpoints; private List changedOperations; + private List changedSchemas; private ChangedExtensions changedExtensions; public List getDeprecatedEndpoints() { @@ -27,7 +28,9 @@ public List getDeprecatedEndpoints() { @Override public List getChangedElements() { - return Stream.concat(changedOperations.stream(), Stream.of(changedExtensions)) + return Stream.concat( + Stream.concat(changedOperations.stream(), Stream.of(changedExtensions)), + changedSchemas.stream()) .collect(Collectors.toList()); } @@ -68,6 +71,10 @@ public ChangedExtensions getChangedExtensions() { return this.changedExtensions; } + public List getChangedSchemas() { + return changedSchemas; + } + public ChangedOpenApi setOldSpecOpenApi(final OpenAPI oldSpecOpenApi) { this.oldSpecOpenApi = oldSpecOpenApi; return this; @@ -98,6 +105,11 @@ public ChangedOpenApi setChangedExtensions(final ChangedExtensions changedExtens return this; } + public ChangedOpenApi setChangedSchemas(final List changedSchemas) { + this.changedSchemas = changedSchemas; + return this; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java index 0dfd49ae5..c353e84a9 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSchema.java @@ -2,10 +2,7 @@ import io.swagger.v3.oas.models.PathItem; import io.swagger.v3.oas.models.media.Schema; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; import org.openapitools.openapidiff.core.model.schema.ChangedEnum; @@ -39,32 +36,98 @@ public class ChangedSchema implements ComposedChanged { protected ChangedSchema addProp; private ChangedExtensions extensions; + // Flags to avoid recursive calls to isChanged() and getChangedElements() + private boolean gettingChangedElements = false; + private boolean gettingIsChanged = false; + + // cached results for isChanged() + private DiffResult changed; + private DiffResult coreChanged; + + // cached results for getChangedElements() + private List changedElements; + public ChangedSchema() { increasedProperties = new LinkedHashMap<>(); missingProperties = new LinkedHashMap<>(); changedProperties = new LinkedHashMap<>(); } + private void clearChangedCache() { + this.changed = null; + this.coreChanged = null; + this.changedElements = null; + } + + @Override + public DiffResult isChanged() { + if (gettingIsChanged) { + return DiffResult.NO_CHANGES; + } + + gettingIsChanged = true; + if (this.changed == null) { + DiffResult elementsResult = + DiffResult.fromWeight( + getChangedElements().stream() + .filter(Objects::nonNull) + .map(Changed::isChanged) + .mapToInt(DiffResult::getWeight) + .max() + .orElse(0)); + DiffResult result; + if (isCoreChanged().getWeight() > elementsResult.getWeight()) { + result = isCoreChanged(); + } else { + result = elementsResult; + } + this.changed = result; + } + gettingIsChanged = false; + + return this.changed; + } + @Override public List getChangedElements() { - return Stream.concat( - changedProperties.values().stream(), - Stream.of( - description, - readOnly, - writeOnly, - items, - oneOfSchema, - addProp, - enumeration, - required, - maxLength, - extensions)) - .collect(Collectors.toList()); + if (gettingChangedElements) { + return Collections.emptyList(); + } + + gettingChangedElements = true; + if (changedElements == null) { + List changed = + Stream.concat( + changedProperties.values().stream(), + Stream.of( + description, + readOnly, + writeOnly, + items, + oneOfSchema, + addProp, + enumeration, + required, + maxLength, + extensions)) + .collect(Collectors.toList()); + this.changedElements = changed; + } + gettingChangedElements = false; + + return this.changedElements; } @Override public DiffResult isCoreChanged() { + if (this.coreChanged == null) { + this.coreChanged = calculateCoreChanged(); + } + + return this.coreChanged; + } + + private DiffResult calculateCoreChanged() { if (!changedType && (oldSchema == null && newSchema == null || oldSchema != null && newSchema != null) && !changeFormat @@ -208,96 +271,115 @@ public ChangedSchema setType(final String type) { } public ChangedSchema setChangedProperties(final Map changedProperties) { + clearChangedCache(); this.changedProperties = changedProperties; return this; } public ChangedSchema setIncreasedProperties(final Map increasedProperties) { + clearChangedCache(); this.increasedProperties = increasedProperties; return this; } public ChangedSchema setMissingProperties(final Map missingProperties) { + clearChangedCache(); this.missingProperties = missingProperties; return this; } public ChangedSchema setChangeDeprecated(final boolean changeDeprecated) { + clearChangedCache(); this.changeDeprecated = changeDeprecated; return this; } public ChangedSchema setDescription(final ChangedMetadata description) { + clearChangedCache(); this.description = description; return this; } public ChangedSchema setChangeTitle(final boolean changeTitle) { + clearChangedCache(); this.changeTitle = changeTitle; return this; } public ChangedSchema setRequired(final ChangedRequired required) { + clearChangedCache(); this.required = required; return this; } public ChangedSchema setChangeDefault(final boolean changeDefault) { + clearChangedCache(); this.changeDefault = changeDefault; return this; } public ChangedSchema setEnumeration(final ChangedEnum enumeration) { + clearChangedCache(); this.enumeration = enumeration; return this; } public ChangedSchema setChangeFormat(final boolean changeFormat) { + clearChangedCache(); this.changeFormat = changeFormat; return this; } public ChangedSchema setReadOnly(final ChangedReadOnly readOnly) { + clearChangedCache(); this.readOnly = readOnly; return this; } public ChangedSchema setWriteOnly(final ChangedWriteOnly writeOnly) { + clearChangedCache(); this.writeOnly = writeOnly; return this; } public ChangedSchema setChangedType(final boolean changedType) { + clearChangedCache(); this.changedType = changedType; return this; } public ChangedSchema setMaxLength(final ChangedMaxLength maxLength) { + clearChangedCache(); this.maxLength = maxLength; return this; } public ChangedSchema setDiscriminatorPropertyChanged(final boolean discriminatorPropertyChanged) { + clearChangedCache(); this.discriminatorPropertyChanged = discriminatorPropertyChanged; return this; } public ChangedSchema setItems(final ChangedSchema items) { + clearChangedCache(); this.items = items; return this; } public ChangedSchema setOneOfSchema(final ChangedOneOfSchema oneOfSchema) { + clearChangedCache(); this.oneOfSchema = oneOfSchema; return this; } public ChangedSchema setAddProp(final ChangedSchema addProp) { + clearChangedCache(); this.addProp = addProp; return this; } public ChangedSchema setExtensions(final ChangedExtensions extensions) { + clearChangedCache(); this.extensions = extensions; return this; } diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilder.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilder.java new file mode 100644 index 000000000..959e7625e --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilder.java @@ -0,0 +1,106 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import static org.openapitools.openapidiff.core.utils.ChangedUtils.isChanged; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import org.openapitools.openapidiff.core.model.Changed; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DeferredBuilder implements Consumer> { + private static final Logger log = LoggerFactory.getLogger(DeferredBuilder.class); + + private final List> deferredValues = new ArrayList<>(); + private final List>>>> whenSet = new ArrayList<>(); + + public Optional with(Optional value) { + return value; + } + + public DeferredChanged with(DeferredChanged value) { + deferredValues.add(value); + return value; + } + + public DeferredBuilder add(DeferredChanged value) { + deferredValues.add(value); + return this; + } + + public DeferredBuilder addAll(List> values) { + deferredValues.addAll(values); + return this; + } + + public DeferredBuilder addAll(Stream> values) { + deferredValues.addAll(values.collect(Collectors.toList())); + return this; + } + + public DeferredBuilder whenSet(Consumer>>> consumer) { + whenSet.add(consumer); + return this; + } + + @Override + public void accept(DeferredChanged value) { + deferredValues.add(value); + } + + public DeferredChanged>> build() { + if (deferredValues.isEmpty()) { + return DeferredChanged.empty(); + } + + log.debug("Building collected deferred {}", DeferredLogger.logValue(deferredValues)); + + final PendingChanged>> changed = new PendingChanged(); + whenSet.forEach(changed::whenSet); + + Optional[] values = new Optional[deferredValues.size()]; + + IntStream.range(0, deferredValues.size()) + .forEach( + (i) -> { + DeferredChanged deferredItem = deferredValues.get(i); + deferredItem.whenSet( + (value) -> { + values[i] = value; + log.debug( + "Collected deferred item set this={}, item={}, values = {}", + this, + DeferredLogger.logValue(value), + DeferredLogger.logValue(values)); + if (isFull(values)) { + log.debug( + "Collected deferred triggering complete this={}, values = {}", + this, + DeferredLogger.logValue(values)); + changed.setValue(Optional.of(Arrays.asList(values))); + } + }); + }); + + return changed; + } + + public DeferredChanged buildIsChanged(V changed) { + return build().flatMap((values) -> (DeferredChanged) DeferredChanged.of(isChanged(changed))); + } + + private static boolean isFull(Object[] values) { + for (int i = 0; i < values.length; i++) { + if (values[i] == null) { + return false; + } + } + return true; + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredChanged.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredChanged.java new file mode 100644 index 000000000..c2aecf979 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredChanged.java @@ -0,0 +1,38 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Function; +import javax.annotation.Nullable; +import org.openapitools.openapidiff.core.model.ChangedResponse; + +public interface DeferredChanged { + + void ifPresent(Consumer consumer); + + void whenSet(Consumer> consumer); + + DeferredChanged map(Function, Q> consumer); + + DeferredChanged mapOptional(Function, Optional> consumer); + + DeferredChanged flatMap(Function, DeferredChanged> consumer); + + static DeferredChanged empty() { + return RealizedChanged.empty(); + } + + static DeferredChanged ofNullable(@Nullable T value) { + return new RealizedChanged(value); + } + + static DeferredChanged of(Optional changed) { + return new RealizedChanged(changed); + } + + boolean isPresent(); + + boolean isValueSet(); + + T get(); +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredLogger.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredLogger.java new file mode 100644 index 000000000..90020188e --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredLogger.java @@ -0,0 +1,71 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.openapitools.openapidiff.core.model.Changed; +import org.openapitools.openapidiff.core.model.ComposedChanged; + +public class DeferredLogger { + + public static Object logValue(Object value) { + return new Object() { + public String toString() { + return valueToString(value); + } + }; + } + + public static String optionalToString(Optional value) { + if (value == null) { + return "null"; + } else { + return value.map((v) -> "Optional[" + valueToString(v) + "]").orElse("Optional[empty]"); + } + } + + public static String changedToString(Changed value) { + if (value instanceof ComposedChanged) { + return "Changed: " + value.getClass() + " (composed) "; + } else { + return "Changed: " + value.getClass() + " " + value.isChanged(); + } + } + + public static String streamToString(Stream values) { + return "[" + values.map(v -> valueToString(v)).collect(Collectors.joining(", ")) + "]"; + } + + public static String deferredChangeToString(DeferredChanged deferredChanged) { + if (deferredChanged.isValueSet()) { + if (deferredChanged.isPresent()) { + Object value = deferredChanged.get(); + return valueToString(value); + } else { + return deferredChanged.toString(); + } + } else { + return deferredChanged.toString(); + } + } + + public static String valueToString(Object value) { + if (value == null) { + return "null"; + } else if (value instanceof Changed) { + return changedToString((Changed) value); + } else if (value instanceof Optional) { + return optionalToString((Optional) value); + } else if (value instanceof DeferredChanged) { + return deferredChangeToString((DeferredChanged) value); + } else if (value.getClass().isArray()) { + return streamToString(Arrays.stream((Object[]) value)); + } else if (value instanceof Collection) { + return streamToString(((Collection) value).stream()); + } else { + return value.toString(); + } + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredSchemaCache.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredSchemaCache.java new file mode 100644 index 000000000..2033e0cf5 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/DeferredSchemaCache.java @@ -0,0 +1,122 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import io.swagger.v3.oas.models.media.Schema; +import java.util.*; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import org.openapitools.openapidiff.core.compare.CacheKey; +import org.openapitools.openapidiff.core.compare.OpenApiDiff; +import org.openapitools.openapidiff.core.model.Changed; +import org.openapitools.openapidiff.core.model.ChangedSchema; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Holds schema created by jonathan.hess@gmail.com */ +public class DeferredSchemaCache { + private static final Logger log = LoggerFactory.getLogger(DeferredSchemaCache.class); + + private Map cache = new LinkedHashMap<>(); + private Queue processingQueue = new ArrayDeque<>(); + // private Queue deferredOperations = new ArrayDeque<>(); + + private final OpenApiDiff openApiDiff; + + public DeferredSchemaCache(OpenApiDiff openApiDiff) { + this.openApiDiff = openApiDiff; + } + + public SchemaDiffOperation addSchema( + RecursiveSchemaSet refSet, CacheKey key, Schema left, Schema right) { + if (!cache.containsKey(key)) { + log.debug("Added schema cache {}", key); + SchemaDiffOperation operation = + new SchemaDiffOperation(openApiDiff, refSet, key, left, right); + cache.put(key, operation); + processingQueue.add(key); + return operation; + } else { + return cache.get(key); + } + } + + public DeferredChanged getOrAddSchema( + RecursiveSchemaSet refSet, CacheKey key, Schema left, Schema right) { + // don't allow recursive references to schemas + if (refSet.contains(key)) { + log.debug("getOrAddSchema recursive call aborted {} ", key); + return DeferredChanged.empty(); + } + + refSet.put(key); + SchemaDiffOperation operation; + if (cache.containsKey(key)) { + operation = cache.get(key); + log.debug("getOrAddSchema cached {} {}", key, operation.diffResult); + } else { + operation = addSchema(refSet, key, left, right); + log.debug("getOrAddSchema added {} {}", key, operation.diffResult); + } + return operation.diffResult; + } + + public void process() { + processSchemaQueue(); + // while(! deferredOperations.isEmpty()) { + // processSchemaQueue(); + // DeferredOperation op = deferredOperations.poll(); + // if(op != null) { + // log.debug("Processing deferred {}", op); + // op.process(); + // } + // } + } + + public void processSchemaQueue() { + PendingChanged.logResolved(); + while (!processingQueue.isEmpty()) { + CacheKey key = processingQueue.poll(); + if (key != null) { + log.debug("Processing schema {}", key); + SchemaDiffOperation operation = cache.get(key); + DeferredChanged realValue = + operation + .openApiDiff + .getSchemaDiff() + .computeDiffForReal( + operation.refSet, operation.left, operation.right, key.getContext()); + operation.processed = true; + realValue.whenSet( + (value) -> { + log.debug("Schema processed {} {}", key, DeferredLogger.logValue(value)); + operation.diffResult.setValue(value); + }); + log.debug("Processing schema started {}", key); + } + PendingChanged.logResolved(); + } + } + + public Collection getOperations() { + return cache.values(); + } + + public List getChangedSchemas() { + return cache.values().stream() + .filter(op -> op.processed && op.diffResult.isPresent()) + .map(op -> op.diffResult.get()) + .collect(Collectors.toList()); + } + + private static class DeferredOperation { + private PendingChanged pending = new PendingChanged<>(); + private final Supplier> supplier; + + public DeferredOperation(Supplier> supplier) { + this.supplier = supplier; + } + + public void process() { + pending.setValue(supplier.get()); + } + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/PendingChanged.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/PendingChanged.java new file mode 100644 index 000000000..1881f2ee2 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/PendingChanged.java @@ -0,0 +1,169 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; +import java.util.function.Function; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PendingChanged implements DeferredChanged { + private static final Logger log = LoggerFactory.getLogger(PendingChanged.class); + + private List> ifPresentConsumers = new ArrayList<>(); + private List>> whenSetConsumers = new ArrayList<>(); + + @Nullable private T value; + private Optional valueOptional = Optional.empty(); + private boolean valueIsPresent; + private boolean valueSet; + + private static final AtomicInteger deferredCounter = new AtomicInteger(); + private static final AtomicInteger resolvedCounter = new AtomicInteger(); + + @Override + public void ifPresent(Consumer consumer) { + if (valueSet) { + if (valueIsPresent) { + consumer.accept(value); + } + } else { + ifPresentConsumers.add(consumer); + } + } + + public void setValue(Optional value) { + if (!valueSet) { + this.valueSet = true; + this.valueIsPresent = value.isPresent(); + this.value = value.orElse(null); + this.valueOptional = value; + + log.debug("set {}", DeferredLogger.logValue(this.value)); + + if (this.valueIsPresent) { + ifPresentConsumers.forEach(c -> c.accept(this.value)); + } + + whenSetConsumers.forEach(c -> c.accept(this.valueOptional)); + + } else { + throw new IllegalStateException( + "PendingChanged may not be set more than once. Value was already set."); + } + } + + public boolean isPresent() { + return valueSet && valueIsPresent; + } + + public T get() { + return valueOptional.get(); + } + + public boolean isValueSet() { + return valueSet; + } + + public void whenSet(Consumer> consumer) { + if (valueSet) { + consumer.accept(valueOptional); + } else { + whenSetConsumers.add(consumer); + } + } + + public DeferredChanged map(Function, Q> function) { + return mapOptional((v) -> Optional.ofNullable(function.apply(v))); + } + + public DeferredChanged mapOptional(Function, Optional> function) { + if (valueSet) { + Optional result = function.apply(this.valueOptional); + log.debug( + "map resolved {} {} -> {}", + function, + DeferredLogger.logValue(this.value), + DeferredLogger.logValue(result)); + return new RealizedChanged(result); + } else { + final PendingChanged mappedChanged = new PendingChanged(); + log.debug("map deferred {} ? -> ?", function); + deferredCounter.incrementAndGet(); + whenSet( + (value) -> { + Optional result = function.apply(this.valueOptional); + log.debug( + "map resolved {} {} -> {}", + function, + DeferredLogger.logValue(this.value), + DeferredLogger.logValue(result)); + resolvedCounter.incrementAndGet(); + mappedChanged.setValue(result); + }); + return mappedChanged; + } + } + + public DeferredChanged flatMap(Function, DeferredChanged> function) { + if (valueSet) { + DeferredChanged nextDeferred = function.apply(this.valueOptional); + log.debug("flat map deferred {} {} -> ?", function, DeferredLogger.logValue(this.value)); + deferredCounter.incrementAndGet(); + nextDeferred.whenSet( + (nextValue) -> { + log.debug( + "flat map resolved {} {} -> {}", + function, + DeferredLogger.logValue(this.value), + DeferredLogger.logValue(nextValue)); + resolvedCounter.incrementAndGet(); + }); + return nextDeferred; + } else { + final PendingChanged mappedChanged = new PendingChanged<>(); + log.debug("flat map deferred {} ? -> ?", function); + deferredCounter.incrementAndGet(); + whenSet( + (value) -> { + DeferredChanged nextDeferred = function.apply(value); + nextDeferred.whenSet( + (nextValue) -> { + log.debug( + "flat map deferred {} {} -> {}", + function, + DeferredLogger.logValue(this.value), + DeferredLogger.logValue(nextValue)); + resolvedCounter.incrementAndGet(); + mappedChanged.setValue(nextValue); + }); + log.debug("flat map resolved {} {} -> ?", function, DeferredLogger.logValue(value)); + }); + return mappedChanged; + } + } + + @Override + public String toString() { + return "PendingChanged{" + + "value=" + + DeferredLogger.logValue(value) + + ", valueSet=" + + valueSet + + ", ifPresentConsumers.size=" + + ifPresentConsumers.size() + + ", whenSetConsumers.size=" + + whenSetConsumers.size() + + '}'; + } + + public static void logResolved() { + int deferred = deferredCounter.get(); + int resolved = resolvedCounter.get(); + log.debug( + "Outstanding: {} Deferred: {} Resolved {}", deferred - resolved, deferred, resolved); + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RealizedChanged.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RealizedChanged.java new file mode 100644 index 000000000..fccf66846 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RealizedChanged.java @@ -0,0 +1,64 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Function; + +public class RealizedChanged implements DeferredChanged { + private final Optional value; + + public RealizedChanged(T value) { + this.value = Optional.ofNullable(value); + } + + public RealizedChanged(Optional value) { + this.value = value; + } + + @Override + public void ifPresent(Consumer consumer) { + value.ifPresent(consumer); + } + + @Override + public void whenSet(Consumer> consumer) { + consumer.accept(value); + } + + public static RealizedChanged empty() { + return new RealizedChanged(Optional.empty()); + } + + @Override + public boolean isPresent() { + return value.isPresent(); + } + + @Override + public boolean isValueSet() { + return true; + } + + @Override + public T get() { + return value.get(); + } + + public DeferredChanged map(Function, Q> function) { + return new RealizedChanged(function.apply(this.value)); + } + + @Override + public DeferredChanged mapOptional(Function, Optional> consumer) { + return new RealizedChanged<>(consumer.apply(this.value)); + } + + public DeferredChanged flatMap(Function, DeferredChanged> function) { + return function.apply(this.value); + } + + @Override + public String toString() { + return "RealizedChanged{" + "value=" + value + '}'; + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java new file mode 100644 index 000000000..079139db2 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/RecursiveSchemaSet.java @@ -0,0 +1,18 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.HashSet; +import org.openapitools.openapidiff.core.compare.CacheKey; + +public class RecursiveSchemaSet { + HashSet leftKeys = new HashSet<>(); + HashSet rightKeys = new HashSet<>(); + + public boolean contains(CacheKey key) { + return leftKeys.contains(key.getLeft()) || rightKeys.contains(key.getRight()); + } + + public void put(CacheKey key) { + leftKeys.add(key.getLeft()); + leftKeys.add(key.getRight()); + } +} diff --git a/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/SchemaDiffOperation.java b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/SchemaDiffOperation.java new file mode 100644 index 000000000..512b47a43 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/model/deferred/SchemaDiffOperation.java @@ -0,0 +1,26 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import io.swagger.v3.oas.models.media.Schema; +import org.openapitools.openapidiff.core.compare.CacheKey; +import org.openapitools.openapidiff.core.compare.OpenApiDiff; +import org.openapitools.openapidiff.core.model.ChangedSchema; + +public class SchemaDiffOperation { + final OpenApiDiff openApiDiff; + final RecursiveSchemaSet refSet; + final CacheKey key; + final Schema left; + final Schema right; + boolean processed; + + PendingChanged diffResult = new PendingChanged<>(); + + SchemaDiffOperation( + OpenApiDiff openApiDiff, RecursiveSchemaSet refSet, CacheKey key, Schema left, Schema right) { + this.openApiDiff = openApiDiff; + this.refSet = refSet; + this.key = key; + this.left = left; + this.right = right; + } +} diff --git a/core/src/test/java/org/openapitools/openapidiff/core/LargeSchemaTest.java b/core/src/test/java/org/openapitools/openapidiff/core/LargeSchemaTest.java new file mode 100644 index 000000000..2086b016d --- /dev/null +++ b/core/src/test/java/org/openapitools/openapidiff/core/LargeSchemaTest.java @@ -0,0 +1,163 @@ +package org.openapitools.openapidiff.core; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiAreEquals; +import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; +import static org.slf4j.LoggerFactory.getLogger; + +import io.swagger.v3.core.util.Yaml; +import io.swagger.v3.oas.models.Components; +import io.swagger.v3.oas.models.OpenAPI; +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.media.ArraySchema; +import io.swagger.v3.oas.models.media.Content; +import io.swagger.v3.oas.models.media.MediaType; +import io.swagger.v3.oas.models.media.ObjectSchema; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.media.StringSchema; +import io.swagger.v3.oas.models.parameters.RequestBody; +import io.swagger.v3.oas.models.responses.ApiResponse; +import io.swagger.v3.oas.models.responses.ApiResponses; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Random; +import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.ChangedOpenApi; +import org.slf4j.Logger; + +/** Created by jonathan.hess on 2020-07-16. */ +public class LargeSchemaTest { + public static final Logger LOG = getLogger(LargeSchemaTest.class); + + @Test + public void testGeneratedApiSame() { + OpenAPI generated = largeGeneratedApi(); + assertOpenApiAreEquals(generated, generated); + } + + @Test + public void testGeneratedApiDifferent() { + OpenAPI generated = largeGeneratedApi(); + OpenAPI generated2 = largeGeneratedApi(); + assertOpenApiBackwardIncompatible(generated, generated2); + } + + public static void assertOpenApiAreEquals(OpenAPI oldSpec, OpenAPI newSpec) { + ChangedOpenApi changedOpenApi = OpenApiCompare.fromSpecifications(oldSpec, newSpec); + LOG.info("Result: {}", changedOpenApi.isChanged().getValue()); + assertThat(changedOpenApi.getNewEndpoints()).isEmpty(); + assertThat(changedOpenApi.getMissingEndpoints()).isEmpty(); + assertThat(changedOpenApi.getChangedOperations()).isEmpty(); + } + + public static void assertOpenApiBackwardIncompatible(OpenAPI oldSpec, OpenAPI newSpec) { + ChangedOpenApi changedOpenApi = OpenApiCompare.fromSpecifications(oldSpec, newSpec); + LOG.info("Result: {}", changedOpenApi.isChanged().getValue()); + assertThat(changedOpenApi.isDifferent()).isTrue(); + assertThat(changedOpenApi.isIncompatible()).isTrue(); + } + + public OpenAPI largeGeneratedApi() { + final int modelCount = 200; + final int arrayCount = 50; + final int refPerModelCount = 5; + final int endpointCount = 20; + + Random random = new Random(); + + OpenAPI api = new OpenAPI(); + api.setPaths(new Paths()); + api.setComponents(new Components()); + + // create 200 model schemas + Map schemas = new LinkedHashMap<>(); + api.getComponents().setSchemas(schemas); + for (int i = 0; i < modelCount; i++) { + ObjectSchema schema = new ObjectSchema(); + schema.setProperties(new LinkedHashMap<>()); + schema.getProperties().put("name", new StringSchema()); + schema.getProperties().put("description", new StringSchema()); + List required = new ArrayList<>(); + required.add("name"); + schema.setRequired(required); + schemas.put(modelName(i), schema); + } + + // create 50 array schemas + for (int i = modelCount; i < modelCount + arrayCount; i++) { + ArraySchema arraySchema = new ArraySchema(); + arraySchema.setItems(refSchema(i)); + schemas.put(modelName(i), arraySchema); + } + + // list of schema names + + // Create cyclic properties on schemas, make the refs required + schemas.values().stream() + .filter(schema -> schema instanceof ObjectSchema) + .map(schema -> (ObjectSchema) schema) + .forEach( + schema -> { + for (int i = 0; i < refPerModelCount; i++) { + int schemaNumber = random.nextInt(modelCount + arrayCount); + String propertyName = "refTo" + schemaNumber; + schema.getProperties().put(propertyName, refSchema(schemaNumber)); + schema.getRequired().add(propertyName); + } + }); + + // generated endpoints + for (int i = 0; i < endpointCount; i++) { + String path = "/endpoint" + i; + PathItem pathItem = new PathItem(); + Operation operation = new Operation(); + pathItem.post(operation); + + operation.setRequestBody( + new RequestBody() + .content( + new Content() + .addMediaType( + "application/json", new MediaType().schema(refSchema(i % modelCount))))); + ApiResponse responseOk = + new ApiResponses() + .put( + "200", + new ApiResponse() + .content( + new Content() + .addMediaType( + "application/json", + new MediaType().schema(refSchema(i % modelCount))))); + ApiResponses responses = new ApiResponses(); + responses.put("200", responseOk); + operation.setResponses(responses); + api.getPaths().put(path, pathItem); + } + + try { + LOG.info("Printing schema to target/large-api.yaml"); + Yaml.pretty().writeValue(new File("target/large-api.yaml"), api); + } catch (IOException e) { + throw new RuntimeException(e); + } + return api; + } + + private Schema refSchema(final int i) { + String itemModelName = modelName(i); + Schema refSchema = new Schema(); + refSchema.set$ref("#/components/schemas/" + itemModelName); + return refSchema; + } + + private String modelName(final int i) { + return String.format("Model%03d", i); + } +} diff --git a/core/src/test/java/org/openapitools/openapidiff/core/OneOfDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/OneOfDiffTest.java index cf893c588..af9506cb4 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/OneOfDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/OneOfDiffTest.java @@ -44,6 +44,7 @@ public void testComposedSchema() { public void testComposedSchemaDiff() { assertOpenApiAreEquals(OPENAPI_DOC10, OPENAPI_DOC10); } + @Test public void testOneOfDiscrimitatorChanged() { // The oneOf 'discriminator' changed: 'realtype' -> 'othertype': 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 6d136fb25..1c8168c41 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/PathDiffTest.java @@ -30,7 +30,9 @@ public void testSameTemplateDifferentMethods() { ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_PATH1, OPENAPI_PATH4); assertThat(changedOpenApi.getNewEndpoints()) .hasSize(1) - .satisfiesExactly(endpoint -> assertThat(endpoint.getOperation().getOperationId()).isEqualTo("deletePet")); + .satisfiesExactly( + endpoint -> + assertThat(endpoint.getOperation().getOperationId()).isEqualTo("deletePet")); assertThat(changedOpenApi.isCompatible()).isTrue(); } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java index 5d93c61dc..0f6d003ac 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/RecursiveSchemaTest.java @@ -10,12 +10,18 @@ public class RecursiveSchemaTest { private final String OPENAPI_DOC1 = "recursive_model_1.yaml"; private final String OPENAPI_DOC2 = "recursive_model_2.yaml"; + private final String OPENAPI_DOC3 = "recursive_model_3.yaml"; @Test public void testDiffSame() { assertOpenApiAreEquals(OPENAPI_DOC1, OPENAPI_DOC1); } + @Test + public void testDiffDifferentCyclic() { + assertOpenApiBackwardIncompatible(OPENAPI_DOC1, OPENAPI_DOC3); + } + @Test public void testDiffDifferent() { assertOpenApiBackwardIncompatible(OPENAPI_DOC1, OPENAPI_DOC2); diff --git a/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java index 056b92319..71d89f4ec 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/SecurityDiffTest.java @@ -91,7 +91,7 @@ public void testWithUnknownSecurityScheme() { IllegalArgumentException.class, () -> OpenApiCompare.fromLocations(OPENAPI_DOC3, OPENAPI_DOC3)); assertThrows( - IllegalArgumentException.class, - () -> OpenApiCompare.fromLocations(OPENAPI_DOC4, OPENAPI_DOC4)); + IllegalArgumentException.class, + () -> OpenApiCompare.fromLocations(OPENAPI_DOC4, OPENAPI_DOC4)); } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/compare/ApiResponseDiffTest.java b/core/src/test/java/org/openapitools/openapidiff/core/compare/ApiResponseDiffTest.java index 59ad6cac7..97b8ab094 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/compare/ApiResponseDiffTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/compare/ApiResponseDiffTest.java @@ -1,27 +1,27 @@ package org.openapitools.openapidiff.core.compare; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardCompatible; import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; class ApiResponseDiffTest extends Assertions { - /*** - * This is a regression test - when no responses were set, we would get an exception - * since the OpenAPI object has a `null` ApiResponses field. - */ - @Test - public void testNoResponseInPrevious() { - // The previous API had no response, so adding a response shape is still compatible. - assertOpenApiBackwardCompatible( - "backwardCompatibility/apiResponse_diff_1.yaml", - "backwardCompatibility/apiResponse_diff_2.yaml", true); + /** + * * This is a regression test - when no responses were set, we would get an exception since the + * OpenAPI object has a `null` ApiResponses field. + */ + @Test + public void testNoResponseInPrevious() { + // The previous API had no response, so adding a response shape is still compatible. + assertOpenApiBackwardCompatible( + "backwardCompatibility/apiResponse_diff_1.yaml", + "backwardCompatibility/apiResponse_diff_2.yaml", + true); - // Removing the response shape is backwards incompatible. - assertOpenApiBackwardIncompatible( - "backwardCompatibility/apiResponse_diff_2.yaml", - "backwardCompatibility/apiResponse_diff_1.yaml"); - } -} \ No newline at end of file + // Removing the response shape is backwards incompatible. + assertOpenApiBackwardIncompatible( + "backwardCompatibility/apiResponse_diff_2.yaml", + "backwardCompatibility/apiResponse_diff_1.yaml"); + } +} diff --git a/core/src/test/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilderChangedTest.java b/core/src/test/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilderChangedTest.java new file mode 100644 index 000000000..2168e367c --- /dev/null +++ b/core/src/test/java/org/openapitools/openapidiff/core/model/deferred/DeferredBuilderChangedTest.java @@ -0,0 +1,175 @@ +package org.openapitools.openapidiff.core.model.deferred; + +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openapitools.openapidiff.core.model.schema.ChangedReadOnly; +import org.openapitools.openapidiff.core.model.schema.ChangedWriteOnly; + +public class DeferredBuilderChangedTest { + + private PendingChanged changed; + private Optional whenSet; + private ChangedWriteOnly mappedValue; + private Optional chainedValue; + + @BeforeEach + public void beforeEach() { + whenSet = Optional.empty(); + mappedValue = null; + chainedValue = Optional.empty(); + changed = new PendingChanged<>(); + } + + @Test + public void testPendingChangedValueSetBeforeListeners() { + PendingChanged changed = new PendingChanged(); + changed.setValue(Optional.of("Hello")); + ChangedAssertion assertion = new ChangedAssertion(changed); + assertion.assertSet(true); + } + + @Test + public void testPendingChangedValueSetAfterListeners() { + PendingChanged changed = new PendingChanged(); + ChangedAssertion assertion = new ChangedAssertion(changed); + changed.setValue(Optional.of("Hello")); + assertion.assertSet(true); + } + + @Test + public void testPendingChangedValueEMpty() { + PendingChanged changed = new PendingChanged(); + ChangedAssertion assertion = new ChangedAssertion(changed); + changed.setValue(Optional.empty()); + assertion.assertSet(false); + } + + @Test + public void testRealizedChange() { + RealizedChanged changed = new RealizedChanged<>("hello"); + ChangedAssertion assertion = new ChangedAssertion(changed); + assertion.assertSet(true); + } + + @Test + public void testRealizedChangeEmpty() { + RealizedChanged changed = new RealizedChanged<>(Optional.empty()); + ChangedAssertion assertion = new ChangedAssertion(changed); + assertion.assertSet(false); + } + + private static class ChangedAssertion { + AtomicBoolean map = new AtomicBoolean(false); + AtomicBoolean flatMap = new AtomicBoolean(false); + AtomicBoolean mapOptional = new AtomicBoolean(false); + AtomicBoolean whenSet = new AtomicBoolean(false); + AtomicBoolean ifPresent = new AtomicBoolean(false); + + public ChangedAssertion(DeferredChanged changed) { + changed.mapOptional( + (value) -> { + mapOptional.set(true); + return Optional.empty(); + }); + changed.map( + (value) -> { + map.set(true); + return Optional.empty(); + }); + changed.flatMap( + (value) -> { + flatMap.set(true); + return DeferredChanged.empty(); + }); + changed.whenSet( + (value) -> { + whenSet.set(true); + }); + changed.ifPresent( + (value) -> { + ifPresent.set(true); + }); + } + + public void assertSet(boolean expectedIfPresent) { + Assertions.assertTrue(mapOptional.get(), "mapOptional"); + Assertions.assertTrue(map.get(), "map"); + Assertions.assertTrue(flatMap.get(), "flatMap"); + Assertions.assertTrue(whenSet.get(), "whenSet"); + Assertions.assertEquals(expectedIfPresent, ifPresent.get(), "ifPresent"); + } + + public void assertNotSet() { + Assertions.assertFalse(mapOptional.get(), "mapOptional"); + Assertions.assertFalse(map.get(), "map"); + Assertions.assertFalse(flatMap.get(), "flatMap"); + Assertions.assertFalse(whenSet.get(), "whenSet"); + Assertions.assertFalse(ifPresent.get(), "ifPresent"); + } + } + + @Test + public void testFlatMap() { + PendingChanged flatMapPending = new PendingChanged<>(); + + changed.whenSet((value) -> this.whenSet = value); + + DeferredChanged chainedChanged = + changed.flatMap( + (value) -> { + System.out.println("Flatmap called"); + return flatMapPending; + }); + chainedChanged.whenSet(value -> chainedValue = value); + + DeferredChanged mappedDeferred = + changed.map(value -> new ChangedWriteOnly(false, true, null)); + mappedDeferred.ifPresent(v -> mappedValue = v); + + Assertions.assertFalse(whenSet.isPresent()); + Assertions.assertFalse(chainedValue.isPresent()); + Assertions.assertNull(mappedValue); + + changed.setValue(Optional.of(new ChangedReadOnly(false, true, null))); + Assertions.assertTrue(whenSet.isPresent()); + Assertions.assertNotNull(mappedValue); + Assertions.assertFalse(chainedValue.isPresent()); + + flatMapPending.setValue(Optional.of(new ChangedWriteOnly(false, true, null))); + Assertions.assertTrue(whenSet.isPresent()); + Assertions.assertTrue(chainedValue.isPresent()); + } + + @Test + public void testDeferredBuilderEmpty() { + DeferredBuilder builder = new DeferredBuilder(); + ChangedAssertion builderAssertion = new ChangedAssertion(builder.build()); + builderAssertion.assertSet(false); + } + + @Test + public void testDeferredBuilderAllRealized() { + DeferredBuilder builder = new DeferredBuilder(); + builder.add(new RealizedChanged("hello")); + builder.add(new RealizedChanged("bye")); + ChangedAssertion builderAssertion = new ChangedAssertion(builder.build()); + builderAssertion.assertSet(true); + } + + @Test + public void testDeferredBuilderPending() { + PendingChanged changed = new PendingChanged(); + + DeferredBuilder builder = new DeferredBuilder(); + builder.add(new RealizedChanged("hello")); + builder.add(changed); + ChangedAssertion builderAssertion = new ChangedAssertion(builder.build()); + builderAssertion.assertNotSet(); + + changed.setValue(Optional.of("hello")); + builderAssertion.assertSet(true); + } +} diff --git a/core/src/test/resources/recursive_model_3.yaml b/core/src/test/resources/recursive_model_3.yaml new file mode 100644 index 000000000..eea603965 --- /dev/null +++ b/core/src/test/resources/recursive_model_3.yaml @@ -0,0 +1,37 @@ +openapi: 3.0.1 +info: + title: recursive test + version: '1.0' +servers: + - url: 'http://localhost:8000/' +paths: + /ping: + get: + operationId: ping + responses: + '200': + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/B' +components: + schemas: + B: + type: object + properties: + message: + type: string + details: + type: array + items: + $ref: '#/components/schemas/C' + C: + type: object + properties: + message: + type: string + details: + type: array + items: + $ref: '#/components/schemas/B'