Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Removal of 'deprecated' not detected #743

Merged
merged 11 commits into from
Feb 11, 2025
Merged
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