Skip to content

Change of 'nullable' is not detected #747

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

Merged
merged 1 commit into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
import org.apache.commons.collections4.CollectionUtils;
import org.openapitools.openapidiff.core.compare.MapKeyDiff;
import org.openapitools.openapidiff.core.compare.OpenApiDiff;
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.model.schema.ChangedOneOfSchema;
import org.openapitools.openapidiff.core.utils.RefPointer;
import org.openapitools.openapidiff.core.utils.RefType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public <V extends Schema<X>, X> DeferredChanged<ChangedSchema> diff(
right.getExclusiveMaximum(),
context))
.setMultipleOf(new ChangedMultipleOf(left.getMultipleOf(), right.getMultipleOf()))
.setNullable(new ChangedNullable(left.getNullable(), right.getNullable()))
.setExamples(new ChangedExamples(left.getExamples(), right.getExamples()))
.setExample(new ChangedExample(left.getExample(), right.getExample()));
builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
import org.openapitools.openapidiff.core.model.schema.ChangedEnum;
import org.openapitools.openapidiff.core.model.schema.ChangedMaxLength;
import org.openapitools.openapidiff.core.model.schema.ChangedMultipleOf;
import org.openapitools.openapidiff.core.model.schema.ChangedNullable;
import org.openapitools.openapidiff.core.model.schema.ChangedNumericRange;
import org.openapitools.openapidiff.core.model.schema.ChangedOneOfSchema;
import org.openapitools.openapidiff.core.model.schema.ChangedReadOnly;
import org.openapitools.openapidiff.core.model.schema.ChangedRequired;
import org.openapitools.openapidiff.core.model.schema.ChangedWriteOnly;
Expand Down Expand Up @@ -39,6 +41,7 @@ public class ChangedSchema implements ComposedChanged {
protected ChangedMaxLength maxLength;
protected ChangedNumericRange numericRange;
protected ChangedMultipleOf multipleOf;
protected ChangedNullable nullable;
protected boolean discriminatorPropertyChanged;
protected ChangedSchema items;
protected ChangedOneOfSchema oneOfSchema;
Expand Down Expand Up @@ -122,6 +125,7 @@ public List<Changed> getChangedElements() {
maxLength,
numericRange,
multipleOf,
nullable,
extensions))
.collect(Collectors.toList());
}
Expand Down Expand Up @@ -285,6 +289,10 @@ public ChangedMultipleOf getMultipleOf() {
return this.multipleOf;
}

public ChangedNullable getNullable() {
return this.nullable;
}

public boolean isDiscriminatorPropertyChanged() {
return this.discriminatorPropertyChanged;
}
Expand Down Expand Up @@ -433,6 +441,12 @@ public ChangedSchema setMultipleOf(final ChangedMultipleOf multipleOf) {
return this;
}

public ChangedSchema setNullable(final ChangedNullable nullable) {
clearChangedCache();
this.nullable = nullable;
return this;
}

public ChangedSchema setDiscriminatorPropertyChanged(final boolean discriminatorPropertyChanged) {
clearChangedCache();
this.discriminatorPropertyChanged = discriminatorPropertyChanged;
Expand Down Expand Up @@ -491,6 +505,7 @@ public boolean equals(Object o) {
&& Objects.equals(maxLength, that.maxLength)
&& Objects.equals(numericRange, that.numericRange)
&& Objects.equals(multipleOf, that.multipleOf)
&& Objects.equals(nullable, that.nullable)
&& Objects.equals(items, that.items)
&& Objects.equals(oneOfSchema, that.oneOfSchema)
&& Objects.equals(addProp, that.addProp)
Expand Down Expand Up @@ -522,6 +537,7 @@ public int hashCode() {
maxLength,
numericRange,
multipleOf,
nullable,
discriminatorPropertyChanged,
items,
oneOfSchema,
Expand Down Expand Up @@ -575,6 +591,8 @@ public java.lang.String toString() {
+ this.getNumericRange()
+ ", multipleOf="
+ this.getMultipleOf()
+ ", nullable="
+ this.getNullable()
+ ", discriminatorPropertyChanged="
+ this.isDiscriminatorPropertyChanged()
+ ", items="
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.openapitools.openapidiff.core.model.schema;

import java.util.Objects;
import org.openapitools.openapidiff.core.model.Changed;
import org.openapitools.openapidiff.core.model.DiffResult;

public class ChangedNullable implements Changed {

private final Boolean left;
private final Boolean right;

public ChangedNullable(Boolean leftNullable, Boolean rightNullable) {
this.left = leftNullable;
this.right = rightNullable;
}

@Override
public DiffResult isChanged() {
boolean leftValue = left != null && left;
boolean rightValue = right != null && right;

if (leftValue != rightValue) {
return DiffResult.COMPATIBLE;
}

return DiffResult.NO_CHANGES;
}

public Boolean getLeft() {
return left;
}

public Boolean getRight() {
return right;
}

@Override
public String toString() {
return "ChangedNullable [left=" + left + ", right=" + right + "]";
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ChangedNullable that = (ChangedNullable) o;
return Objects.equals(left, that.getLeft()) && Objects.equals(right, that.getRight());
}

@Override
public int hashCode() {
return Objects.hash(left, right);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.openapitools.openapidiff.core.model;
package org.openapitools.openapidiff.core.model.schema;

import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.REQUEST_ONEOF_DECREASED;
import static org.openapitools.openapidiff.core.model.BackwardIncompatibleProp.RESPONSE_ONEOF_INCREASED;
Expand All @@ -8,6 +8,11 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.openapitools.openapidiff.core.model.Changed;
import org.openapitools.openapidiff.core.model.ChangedSchema;
import org.openapitools.openapidiff.core.model.ComposedChanged;
import org.openapitools.openapidiff.core.model.DiffContext;
import org.openapitools.openapidiff.core.model.DiffResult;

public class ChangedOneOfSchema implements ComposedChanged {
private final Map<String, String> oldMapping;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.commons.lang3.StringUtils;
import org.openapitools.openapidiff.core.exception.RendererException;
import org.openapitools.openapidiff.core.model.*;
import org.openapitools.openapidiff.core.model.schema.ChangedOneOfSchema;
import org.openapitools.openapidiff.core.utils.RefPointer;
import org.openapitools.openapidiff.core.utils.RefType;
import org.slf4j.Logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,35 @@ public void changeMultipleOfHandling() {
assertThat(props.get("field4").getMultipleOf().getLeft()).isEqualTo(BigDecimal.valueOf(10));
assertThat(props.get("field4").getMultipleOf().getRight()).isNull();
}

@Test // issue #482
public void changeNullabeHandling() {
ChangedOpenApi changedOpenApi =
OpenApiCompare.fromLocations(
"schemaDiff/schema-nullable-diff-1.yaml", "schemaDiff/schema-nullable-diff-2.yaml");
ChangedSchema changedSchema =
getRequestBodyChangedSchema(changedOpenApi, POST, "/schema/nullable", "application/json");

assertThat(changedSchema).isNotNull();
Map<String, ChangedSchema> props = changedSchema.getChangedProperties();
assertThat(props).isNotEmpty();

// Check no changes in nullable
assertThat(props.get("field0")).isNull();

// Check changes in nullable
assertThat(props.get("field1").getNullable().isCompatible()).isTrue();
assertThat(props.get("field1").getNullable().getLeft()).isTrue();
assertThat(props.get("field1").getNullable().getRight()).isFalse();

// Check deletion of nullable
assertThat(props.get("field2").getNullable().isCompatible()).isTrue();
assertThat(props.get("field2").getNullable().getLeft()).isTrue();
assertThat(props.get("field2").getNullable().getRight()).isNull();

// Check addition of nullable
assertThat(props.get("field3").getNullable().isCompatible()).isTrue();
assertThat(props.get("field3").getNullable().getLeft()).isNull();
assertThat(props.get("field3").getNullable().getRight()).isTrue();
}
}
28 changes: 28 additions & 0 deletions core/src/test/resources/schemaDiff/schema-nullable-diff-1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
openapi: 3.0.1
info:
description: Schema diff nullable
title: schema diff nullable
version: 1.0.0
paths:
/schema/nullable:
post:
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/TestDTO'
components:
schemas:
TestDTO:
type: object
properties:
field0:
type: integer
field1:
type: integer
nullable: true
field2:
type: integer
nullable: true
field3:
type: integer
28 changes: 28 additions & 0 deletions core/src/test/resources/schemaDiff/schema-nullable-diff-2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
openapi: 3.0.1
info:
description: Schema diff nullable
title: schema diff nullable
version: 1.0.0
paths:
/schema/nullable:
post:
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/TestDTO'
components:
schemas:
TestDTO:
type: object
properties:
field0:
type: integer
field1:
type: integer
nullable: false
field2:
type: integer
field3:
type: integer
nullable: true