Skip to content

Fix comparing submodels to take into account the correct property #46

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix comparing submodels to take into account the correct property #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c710546
Fix comparing submodels to take into account the correct property
May 7, 2022
54be1ed
Add packaging jar with all dependencies
May 7, 2022
61015c4
Don't display null for lack of endpoint description
May 12, 2022
e9eeae2
Add better explanation for api changes
Aug 8, 2022
f96adfa
Fix tests
Aug 8, 2022
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ hs_err_pid*
swagger-diff.iml
testDiff.html
testDiff.md
testDiff.json
testDeprecatedApi.html
testNewApi.html

21 changes: 21 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,27 @@
<artifactId>coveralls-maven-plugin</artifactId>
<version>4.3.0</version>
</plugin>
<plugin>
<artifactId>maven-assembly-plugin</artifactId>
<executions>
<execution>
<phase>package</phase>
<goals>
<goal>single</goal>
</goals>
</execution>
</executions>
<configuration>
<descriptorRefs>
<descriptorRef>jar-with-dependencies</descriptorRef>
</descriptorRefs>
<archive>
<manifest>
<mainClass>com.deepoove.swagger.diff.cli.CLI</mainClass>
</manifest>
</archive>
</configuration>
</plugin>
</plugins>
</build>
</project>
100 changes: 82 additions & 18 deletions src/main/java/com/deepoove/swagger/diff/compare/ModelDiff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
package com.deepoove.swagger.diff.compare;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.math.BigDecimal;
import java.util.*;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;

import com.deepoove.swagger.diff.model.ElProperty;

import com.google.common.base.Joiner;
import io.swagger.models.ArrayModel;
import io.swagger.models.Model;
import io.swagger.models.RefModel;
import io.swagger.models.properties.ArrayProperty;
import io.swagger.models.properties.Property;
import io.swagger.models.properties.RefProperty;
import io.swagger.models.properties.StringProperty;
import io.swagger.models.properties.*;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;

/**
* compare two model
Expand Down Expand Up @@ -85,7 +81,7 @@ private ModelDiff diff(Model leftInputModel, Model rightInputModel, String paren
Property left = leftProperties.get(key);
Property right = rightProperties.get(key);
Model leftSubModel = findModel(left, oldDedinitions);
Model rightSubModel = findModel(left, newDedinitions);
Model rightSubModel = findModel(right, newDedinitions);
if (leftSubModel != null || rightSubModel != null) {
diff(leftSubModel, rightSubModel, buildElString(parentEl, key),
copyAndAdd(visited, leftModel, rightModel));
Expand Down Expand Up @@ -122,20 +118,83 @@ private ElProperty convert2ElProperty(String propName, String parentEl, Property
}

private ElProperty addChangeMetadata(ElProperty diffProperty, Property left, Property right) {
diffProperty.setTypeChange(!left.getType().equalsIgnoreCase(right.getType()));
if(!left.getType().equalsIgnoreCase(right.getType())) {
diffProperty.setNewTypeChange(right.getType());
}

List<String> leftEnums = enumValues(left);
List<String> rightEnums = enumValues(right);
addEnums(diffProperty, leftEnums, rightEnums);

List<String> changes = new ArrayList<>();
if(!StringUtils.equals(left.getType(), right.getType())) {
return diffProperty;
} else if (left instanceof AbstractNumericProperty) {
AbstractNumericProperty leftP = (AbstractNumericProperty) left;
AbstractNumericProperty rightP = (AbstractNumericProperty) right;


addNumericChanges("maximum", leftP, rightP, AbstractNumericProperty::getMaximum, AbstractNumericProperty::getExclusiveMaximum, changes);
addNumericChanges("minimum", leftP, rightP, AbstractNumericProperty::getMinimum, AbstractNumericProperty::getExclusiveMinimum, changes);
} else if(left instanceof StringProperty) {
StringProperty leftP = (StringProperty) left;
StringProperty rightP = (StringProperty) right;

addChanges("maximum length", leftP, rightP, StringProperty::getMaxLength, changes);
addChanges("minimum length", leftP, rightP, StringProperty::getMinLength, changes);
addChanges("default", leftP, rightP, StringProperty::getDefault, changes);
addChanges("pattern", leftP, rightP, StringProperty::getPattern, changes);
}

addChanges("format", left, right, Property::getFormat, changes);
addChanges("example", left, right, Property::getExample, changes);
addChanges("allow empty value", left, right, Property::getAllowEmptyValue, changes);
addChanges("readonly", left, right, Property::getReadOnly, changes);
addChanges("required", left, right, Property::getRequired, changes);
addChanges("description", left, right, Property::getDescription, changes);

diffProperty.metadataChanged(Joiner.on(". ").join(changes));
return diffProperty;
}

public static void addEnums(ElProperty diffProperty, List<String> leftEnums, List<String> rightEnums) {
if (!leftEnums.isEmpty() && !rightEnums.isEmpty()) {
ListDiff<String> enumDiff = ListDiff.diff(leftEnums, rightEnums, (t, enumVal) -> {
for (String value : t) {
if (enumVal.equalsIgnoreCase(value)) { return value; }
}
return null;
});
diffProperty.setNewEnums(enumDiff.getIncreased() != null && !enumDiff.getIncreased().isEmpty());
diffProperty.setRemovedEnums(enumDiff.getMissing() != null && !enumDiff.getMissing().isEmpty());
diffProperty.setNewEnums(enumDiff.getIncreased());
diffProperty.setRemovedEnums(enumDiff.getMissing());
}
}

public static <T> void addNumericChanges(String description, T left, T right, Function<T, BigDecimal> getter,
Function<T, Boolean> exclusive, List<String> changes) {
if(!Objects.equals(getter.apply(left), getter.apply(right))) {
if(getter.apply(left) == null) {
changes.add(String.format("Added %s %s %s", BooleanUtils.isTrue(exclusive.apply(right)) ? "exclusive" : "nonexclusive", description, getter.apply(right)));
} else if (getter.apply(right) == null) {
changes.add(String.format("Removed %s", description));
} else {
changes.add(String.format("Changed %s from '%s' -> '%s'", description, getter.apply(left), getter.apply(right)));
}
} else if(!Objects.equals(exclusive.apply(left), exclusive.apply(right))) {
changes.add(String.format("Changed %s to %s", description, BooleanUtils.isTrue(exclusive.apply(right)) ? "exclusive" : "nonexlusive"));
}
}

public static <PROP> void addChanges(String description, PROP left, PROP right, Function<PROP, Object> getter, List<String> changes) {
if(!Objects.equals(getter.apply(left), getter.apply(right))) {
if (getter.apply(left) == null) {
changes.add(String.format("Added %s %s", description, getter.apply(right)));
} else if (getter.apply(right) == null) {
changes.add(String.format("Removed %s", description));
} else {
changes.add(String.format("Changed %s from '%s' -> '%s'", description, getter.apply(left), getter.apply(right)));
}
}
return diffProperty;
}

@SuppressWarnings("unchecked")
Expand All @@ -146,8 +205,13 @@ private <T> Set<T> copyAndAdd(Set<T> set, T... add) {
}

private List<String> enumValues(Property prop) {
if (prop instanceof StringProperty && ((StringProperty) prop).getEnum() != null) {
return ((StringProperty) prop).getEnum();
Property finalProp = prop;
if(prop instanceof ArrayProperty) {
finalProp = ((ArrayProperty) prop).getItems();
}

if (finalProp instanceof StringProperty && ((StringProperty) finalProp).getEnum() != null) {
return ((StringProperty) finalProp).getEnum();
} else {
return new ArrayList<>();
}
Expand Down
43 changes: 38 additions & 5 deletions src/main/java/com/deepoove/swagger/diff/compare/ParameterDiff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package com.deepoove.swagger.diff.compare;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import java.math.BigDecimal;
import java.util.*;
import java.util.function.Function;

import com.google.common.base.Joiner;
import io.swagger.models.properties.AbstractNumericProperty;
import io.swagger.models.properties.ArrayProperty;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;

import com.deepoove.swagger.diff.model.ChangedParameter;
Expand Down Expand Up @@ -73,7 +77,6 @@ public ParameterDiff diff(List<Parameter> left, List<Parameter> right) {
changedParameter.setIncreased(diff.getIncreased());
changedParameter.setMissing(diff.getMissing());
changedParameter.setChanged(diff.getChanged());

}

// Let's handle the case where the new API has fx changed the type
Expand All @@ -84,6 +87,8 @@ public ParameterDiff diff(List<Parameter> left, List<Parameter> right) {
ElProperty elProperty = new ElProperty();
elProperty.setEl(rightPara.getName());
elProperty.setProperty(mapToProperty(rightPara));

addChangeMetadata(elProperty, (AbstractSerializableParameter) leftPara, (AbstractSerializableParameter) rightPara);
changedParameter.setChanged(Lists.newArrayList(elProperty));
}
}
Expand All @@ -109,6 +114,34 @@ public ParameterDiff diff(List<Parameter> left, List<Parameter> right) {
return this;
}

private ElProperty addChangeMetadata(ElProperty diffProperty, AbstractSerializableParameter left, AbstractSerializableParameter right) {
if(!left.getType().equalsIgnoreCase(right.getType())) {
diffProperty.setNewTypeChange(right.getType());
}

List<String> leftEnums = left.getEnum() != null ? left.getEnum() : Collections.emptyList();
List<String> rightEnums = right.getEnum() != null ? right.getEnum() : Collections.emptyList();
ModelDiff.addEnums(diffProperty, leftEnums, rightEnums);

List<String> changes = new ArrayList<>();
ModelDiff.addNumericChanges("maximum", left, right, AbstractSerializableParameter::getMaximum, AbstractSerializableParameter::isExclusiveMaximum, changes);
ModelDiff.addNumericChanges("minimum", left, right, AbstractSerializableParameter::getMinimum, AbstractSerializableParameter::isExclusiveMinimum, changes);

ModelDiff.addChanges("format", left, right, AbstractSerializableParameter::getFormat, changes);
ModelDiff.addChanges("pattern", left, right, AbstractSerializableParameter::getPattern, changes);
ModelDiff.addChanges("example", left, right, AbstractSerializableParameter::getExample, changes);
ModelDiff.addChanges("allow empty value", left, right, AbstractSerializableParameter::getAllowEmptyValue, changes);
ModelDiff.addChanges("required", left, right, AbstractSerializableParameter::getRequired, changes);
ModelDiff.addChanges("default", left, right, AbstractSerializableParameter::getDefaultValue, changes);
ModelDiff.addChanges("minLength", left, right, AbstractSerializableParameter::getMinLength, changes);
ModelDiff.addChanges("maxLength", left, right, AbstractSerializableParameter::getMaxLength, changes);
ModelDiff.addChanges("minItems", left, right, AbstractSerializableParameter::getMinItems, changes);
ModelDiff.addChanges("maxItems", left, right, AbstractSerializableParameter::getMaxItems, changes);

diffProperty.metadataChanged(Joiner.on(". ").join(changes));
return diffProperty;
}

private Property mapToProperty(Parameter rightPara) {
Property prop = new StringProperty();
prop.setAccess(rightPara.getAccess());
Expand Down
34 changes: 23 additions & 11 deletions src/main/java/com/deepoove/swagger/diff/model/ElProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import io.swagger.models.properties.Property;

import java.util.List;

/**
* property with expression Language grammar
*
Expand All @@ -15,9 +17,11 @@ public class ElProperty {
private Property property;

// optional change metadata
private boolean isTypeChange;
private boolean newEnums;
private boolean removedEnums;
private String newTypeChange;
private List<String> newEnums;
private List<String> removedEnums;

private String metadataChanged;

public Property getProperty() {
return property;
Expand All @@ -35,27 +39,35 @@ public void setEl(String el) {
this.el = el;
}

public boolean isTypeChange() {
return isTypeChange;
public String getNewTypeChange() {
return newTypeChange;
}

public void setTypeChange(boolean typeChange) {
isTypeChange = typeChange;
public void setNewTypeChange(String typeChange) {
newTypeChange = newTypeChange;
}

public boolean isNewEnums() {
public List<String> getNewEnums() {
return newEnums;
}

public void setNewEnums(boolean newEnums) {
public void setNewEnums(List<String> newEnums) {
this.newEnums = newEnums;
}

public boolean isRemovedEnums() {
public void metadataChanged(String changes) {
this.metadataChanged = changes;
}

public String metadataChanged() {
return this.metadataChanged;
}

public List<String> getRemovedEnums() {
return removedEnums;
}

public void setRemovedEnums(boolean removedEnums) {
public void setRemovedEnums(List<String> removedEnums) {
this.removedEnums = removedEnums;
}
}
27 changes: 21 additions & 6 deletions src/main/java/com/deepoove/swagger/diff/output/HtmlRender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

import com.deepoove.swagger.diff.SwaggerDiff;
import com.deepoove.swagger.diff.model.*;
import com.google.common.collect.Collections2;
import io.swagger.models.HttpMethod;
import io.swagger.models.parameters.Parameter;
import io.swagger.models.properties.Property;
import j2html.tags.ContainerTag;
import org.apache.commons.lang3.StringUtils;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Collectors;

import static j2html.TagCreator.*;

Expand Down Expand Up @@ -169,15 +172,27 @@ private ContainerTag li_addProp(ElProperty prop) {
private ContainerTag li_changedProp(ElProperty prop) {
List<String> changeDetails = new ArrayList<>();
String changeDetailsHeading = "";
if (prop.isTypeChange()) {
changeDetails.add("Data Type");
if (!StringUtils.isBlank(prop.getNewTypeChange())) {
changeDetails.add(String.format("Changed data type to %s", prop.getNewTypeChange()));
}
if (prop.isNewEnums()) {
changeDetails.add("Added Enum");
if (prop.getNewEnums() != null && !prop.getNewEnums().isEmpty()) {
changeDetails.add(String.format("Added enum value %s", prop.getNewEnums()
.stream()
.map(val -> String.format("'%s'", val))
.collect(Collectors.joining(", "))
));
}
if (prop.isRemovedEnums()) {
changeDetails.add("Removed Enum");
if (prop.getRemovedEnums() != null && !prop.getRemovedEnums().isEmpty()) {
changeDetails.add(String.format("Removed enum value %s", prop.getRemovedEnums()
.stream()
.map(val -> String.format("'%s'", val))
.collect(Collectors.joining(", "))
));
}
if(!StringUtils.isBlank(prop.metadataChanged())) {
changeDetails.add(prop.metadataChanged());
}

if (! changeDetails.isEmpty()) {
changeDetailsHeading = " (" + String.join(", ", changeDetails) + ")";
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/deepoove/swagger/diff/output/MarkdownRender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.swagger.models.HttpMethod;
import io.swagger.models.parameters.Parameter;
import io.swagger.models.properties.Property;
import org.apache.commons.lang3.StringUtils;

public class MarkdownRender implements Render {

Expand Down Expand Up @@ -63,7 +64,7 @@ private String ol_newEndpoint(List<Endpoint> endpoints) {
private String li_newEndpoint(String method, String path, String desc) {
StringBuffer sb = new StringBuffer();
sb.append(LI).append(CODE).append(method).append(CODE)
.append(" " + path).append(" " + desc + "\n");
.append(" " + path).append(" " + StringUtils.trimToEmpty(desc) + "\n");
return sb.toString();
}

Expand Down Expand Up @@ -108,7 +109,7 @@ private String ol_changed(List<ChangedEndpoint> changedEndpoints) {
.append(ul_consume(changedOperation));
}
sb.append(CODE).append(method).append(CODE)
.append(" " + pathUrl).append(" " + desc + " \n")
.append(" " + pathUrl).append(" " + StringUtils.trimToEmpty(desc) + " \n")
.append(ul_detail);
}
}
Expand Down
Loading