Skip to content

Commit

Permalink
Change of 'nullable' is not detected (#747)
Browse files Browse the repository at this point in the history
Fixes #482
  • Loading branch information
DrSatyr authored Feb 14, 2025
1 parent 42c17be commit 8508eb5
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 2 deletions.
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

0 comments on commit 8508eb5

Please sign in to comment.