Skip to content

Commit

Permalink
Removal of 'deprecated' not detected (#743)
Browse files Browse the repository at this point in the history
* Fix issue #485 - Removal of 'deprecated' is not detected on schema, operation, parameter, and header level.
* Reorganize tests structure
* Extend test coverage for CchemaDiff, OperationDiff, ParameterDiff, HeaderDiff
  • Loading branch information
DrSatyr authored Feb 11, 2025
1 parent 1987575 commit 3afafef
Show file tree
Hide file tree
Showing 61 changed files with 1,015 additions and 1,103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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.ChangedExample;
import org.openapitools.openapidiff.core.model.ChangedExamples;
Expand Down Expand Up @@ -46,12 +45,10 @@ protected DeferredChanged<ChangedHeader> computeDiff(
DeferredBuilder<Changed> builder = new DeferredBuilder<>();
ChangedHeader changedHeader =
new ChangedHeader(left, right, context)
.setRequired(getBooleanDiff(left.getRequired(), right.getRequired()))
.setDeprecated(
!Boolean.TRUE.equals(left.getDeprecated())
&& Boolean.TRUE.equals(right.getDeprecated()))
.setRequired(!Objects.equals(left.getRequired(), right.getRequired()))
.setDeprecated(!Objects.equals(left.getDeprecated(), right.getDeprecated()))
.setStyle(!Objects.equals(left.getStyle(), right.getStyle()))
.setExplode(getBooleanDiff(left.getExplode(), right.getExplode()))
.setExplode(!Objects.equals(left.getExplode(), right.getExplode()))
.setExamples(new ChangedExamples(left.getExamples(), right.getExamples()))
.setExample(new ChangedExample(left.getExample(), right.getExample()));
builder
Expand All @@ -77,10 +74,4 @@ protected DeferredChanged<ChangedHeader> computeDiff(
.ifPresent(changedHeader::setExtensions);
return builder.buildIsChanged(changedHeader);
}

private boolean getBooleanDiff(Boolean left, Boolean right) {
boolean leftRequired = Optional.ofNullable(left).orElse(Boolean.FALSE);
boolean rightRequired = Optional.ofNullable(right).orElse(Boolean.FALSE);
return leftRequired != rightRequired;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ public DeferredChanged<ChangedOperation> diff(
.diff(oldOperation.getOperationId(), newOperation.getOperationId(), context))
.ifPresent(changedOperation::setOperationId);
changedOperation.setDeprecated(
!Boolean.TRUE.equals(oldOperation.getDeprecated())
&& Boolean.TRUE.equals(newOperation.getDeprecated()));
!Objects.equals(oldOperation.getDeprecated(), newOperation.getDeprecated()));

if (oldOperation.getRequestBody() != null || newOperation.getRequestBody() != null) {
builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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.ChangedExample;
import org.openapitools.openapidiff.core.model.ChangedExamples;
Expand Down Expand Up @@ -51,14 +50,12 @@ protected DeferredChanged<ChangedParameter> computeDiff(
new ChangedParameter(right.getName(), right.getIn(), context)
.setOldParameter(left)
.setNewParameter(right)
.setChangeRequired(getBooleanDiff(left.getRequired(), right.getRequired()))
.setDeprecated(
!Boolean.TRUE.equals(left.getDeprecated())
&& Boolean.TRUE.equals(right.getDeprecated()))
.setChangeRequired(!Objects.equals(left.getRequired(), right.getRequired()))
.setDeprecated(!Objects.equals(left.getDeprecated(), right.getDeprecated()))
.setChangeAllowEmptyValue(
getBooleanDiff(left.getAllowEmptyValue(), right.getAllowEmptyValue()))
!Objects.equals(left.getAllowEmptyValue(), right.getAllowEmptyValue()))
.setChangeStyle(!Objects.equals(left.getStyle(), right.getStyle()))
.setChangeExplode(getBooleanDiff(left.getExplode(), right.getExplode()))
.setChangeExplode(!Objects.equals(left.getExplode(), right.getExplode()))
.setExamples(new ChangedExamples(left.getExamples(), right.getExamples()))
.setExample(new ChangedExample(left.getExample(), right.getExample()));
builder
Expand All @@ -84,10 +81,4 @@ protected DeferredChanged<ChangedParameter> computeDiff(
.ifPresent(changedParameter::setExtensions);
return builder.buildIsChanged(changedParameter);
}

private boolean getBooleanDiff(Boolean left, Boolean right) {
boolean leftRequired = Optional.ofNullable(left).orElse(Boolean.FALSE);
boolean rightRequired = Optional.ofNullable(right).orElse(Boolean.FALSE);
return leftRequired != rightRequired;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ public <V extends Schema<X>, X> DeferredChanged<ChangedSchema> diff(
.setContext(context)
.setOldSchema(left)
.setNewSchema(right)
.setChangeDeprecated(
!Boolean.TRUE.equals(left.getDeprecated())
&& Boolean.TRUE.equals(right.getDeprecated()))
.setChangeDeprecated(!Objects.equals(left.getDeprecated(), right.getDeprecated()))
.setChangeTitle(!Objects.equals(left.getTitle(), right.getTitle()))
.setRequired(
ListDiff.diff(new ChangedRequired(left.getRequired(), right.getRequired(), context)))
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package org.openapitools.openapidiff.core;

import io.swagger.v3.oas.models.PathItem.HttpMethod;
import java.util.Optional;
import javax.annotation.Nullable;
import org.openapitools.openapidiff.core.model.ChangedApiResponse;
import org.openapitools.openapidiff.core.model.ChangedContent;
import org.openapitools.openapidiff.core.model.ChangedHeaders;
import org.openapitools.openapidiff.core.model.ChangedMediaType;
import org.openapitools.openapidiff.core.model.ChangedOpenApi;
import org.openapitools.openapidiff.core.model.ChangedOperation;
import org.openapitools.openapidiff.core.model.ChangedParameter;
import org.openapitools.openapidiff.core.model.ChangedParameters;
import org.openapitools.openapidiff.core.model.ChangedRequestBody;
import org.openapitools.openapidiff.core.model.ChangedResponse;
import org.openapitools.openapidiff.core.model.ChangedSchema;

public class ChangesResolver {

@Nullable
public static ChangedOperation getChangedOperation(
ChangedOpenApi changedOpenApi, HttpMethod method, String path) {
return changedOpenApi.getChangedOperations().stream()
.filter(
operation ->
operation.getHttpMethod().equals(method) && operation.getPathUrl().equals(path))
.findFirst()
.orElse(null);
}

@Nullable
public static ChangedParameter getChangedParameter(
ChangedOpenApi changedOpenApi, HttpMethod method, String path, String parameterName) {
ChangedOperation changedOperation = getChangedOperation(changedOpenApi, method, path);

if (changedOperation == null) {
return null;
}

return Optional.ofNullable(changedOperation.getParameters())
.map(ChangedParameters::getChanged)
.flatMap(
changedParameters ->
changedParameters.stream()
.filter(changedParameter -> changedParameter.getName().equals(parameterName))
.findFirst())
.orElse(null);
}

@Nullable
public static ChangedHeaders getChangedResponseHeaders(
ChangedOpenApi changedOpenApi, HttpMethod method, String path, String responseCode) {
return Optional.ofNullable(getChangedOperation(changedOpenApi, method, path))
.map(ChangedOperation::getApiResponses)
.map(ChangedApiResponse::getChanged)
.map(responses -> responses.get(responseCode))
.map(ChangedResponse::getHeaders)
.orElse(null);
}

@Nullable
public static ChangedSchema getRequestBodyChangedSchema(
ChangedOperation changedOperation, String mediaType) {
return Optional.ofNullable(changedOperation)
.map(ChangedOperation::getRequestBody)
.map(ChangedRequestBody::getContent)
.map(ChangedContent::getChanged)
.map(changedMediaTypes -> changedMediaTypes.get(mediaType))
.map(ChangedMediaType::getSchema)
.orElse(null);
}
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,69 @@
package org.openapitools.openapidiff.core;

import static io.swagger.v3.oas.models.PathItem.HttpMethod.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.openapitools.openapidiff.core.ChangesResolver.getChangedOperation;

import org.junit.jupiter.api.Test;
import org.openapitools.openapidiff.core.model.ChangedOpenApi;
import org.openapitools.openapidiff.core.model.ChangedOperation;
import org.openapitools.openapidiff.core.model.DiffResult;

public class OperationDiffTest {

private final String OPENAPI_DOC1 = "operation_diff_1.yaml";
private final String OPENAPI_DOC2 = "operation_diff_2.yaml";
private final String OPENAPI_DOC1 = "operationDiff/operation_diff_1.yaml";
private final String OPENAPI_DOC2 = "operationDiff/operation_diff_2.yaml";

@Test
public void testContentDiffWithOneEmptyMediaType() {
public void testOperationIdChanged() {
ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_DOC1, OPENAPI_DOC2);
assertThat(changedOpenApi.isChanged()).isEqualTo(DiffResult.METADATA);
assertThat(changedOpenApi.isDifferent()).isTrue();
assertThat(changedOpenApi.getChangedOperations().size()).isEqualTo(1);
assertThat(changedOpenApi.getChangedOperations().get(0).getOperationId().isDifferent())
.isTrue();
ChangedOperation changedOperation =
getChangedOperation(changedOpenApi, GET, "/operation/operation-id");

assertThat(changedOperation).isNotNull();
assertThat(changedOperation.isChanged()).isEqualTo(DiffResult.METADATA);
assertThat(changedOperation.getOperationId().getRight()).isEqualTo("changed");
}

@Test
public void testOperationSummaryChanged() {
ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_DOC1, OPENAPI_DOC2);
ChangedOperation changedOperation =
getChangedOperation(changedOpenApi, GET, "/operation/summary");

assertThat(changedOperation).isNotNull();
assertThat(changedOperation.isChanged()).isEqualTo(DiffResult.METADATA);
assertThat(changedOperation.getSummary().getRight()).isEqualTo("changed");
}

@Test
public void testOperationDescriptionChanged() {
ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_DOC1, OPENAPI_DOC2);
ChangedOperation changedOperation =
getChangedOperation(changedOpenApi, GET, "/operation/description");

assertThat(changedOperation).isNotNull();
assertThat(changedOperation.isChanged()).isEqualTo(DiffResult.METADATA);
assertThat(changedOperation.getDescription().getRight()).isEqualTo("changed");
}

@Test
public void testOperationBecomesDeprecated() {
ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_DOC1, OPENAPI_DOC2);
ChangedOperation changedOperation =
getChangedOperation(changedOpenApi, GET, "/operation/becomes-deprecated");

assertThat(changedOperation).isNotNull();
assertThat(changedOperation.isDeprecated()).isTrue();
}

@Test
public void testOperationBecomesNotDeprecated() {
ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(OPENAPI_DOC1, OPENAPI_DOC2);
ChangedOperation changedOperation =
getChangedOperation(changedOpenApi, GET, "/operation/becomes-not-deprecated");

assertThat(changedOperation).isNotNull();
assertThat(changedOperation.isDeprecated()).isTrue();
}
}
Loading

0 comments on commit 3afafef

Please sign in to comment.