diff --git a/.gitignore b/.gitignore index 618c6c87..83d11abc 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ hs_err_pid* swagger-diff.iml testDiff.html testDiff.md +testDiff.json testDeprecatedApi.html testNewApi.html diff --git a/pom.xml b/pom.xml index 27fc64c4..b503e881 100644 --- a/pom.xml +++ b/pom.xml @@ -199,6 +199,27 @@ coveralls-maven-plugin 4.3.0 + + maven-assembly-plugin + + + package + + single + + + + + + jar-with-dependencies + + + + com.deepoove.swagger.diff.cli.CLI + + + + diff --git a/src/main/java/com/deepoove/swagger/diff/compare/ModelDiff.java b/src/main/java/com/deepoove/swagger/diff/compare/ModelDiff.java index 600132f5..0c3a53f9 100644 --- a/src/main/java/com/deepoove/swagger/diff/compare/ModelDiff.java +++ b/src/main/java/com/deepoove/swagger/diff/compare/ModelDiff.java @@ -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 @@ -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)); @@ -122,9 +118,46 @@ 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 leftEnums = enumValues(left); List rightEnums = enumValues(right); + addEnums(diffProperty, leftEnums, rightEnums); + + List 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 leftEnums, List rightEnums) { if (!leftEnums.isEmpty() && !rightEnums.isEmpty()) { ListDiff enumDiff = ListDiff.diff(leftEnums, rightEnums, (t, enumVal) -> { for (String value : t) { @@ -132,10 +165,36 @@ private ElProperty addChangeMetadata(ElProperty diffProperty, Property left, Pro } 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 void addNumericChanges(String description, T left, T right, Function getter, + Function exclusive, List 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 void addChanges(String description, PROP left, PROP right, Function getter, List 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") @@ -146,8 +205,13 @@ private Set copyAndAdd(Set set, T... add) { } private List 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<>(); } diff --git a/src/main/java/com/deepoove/swagger/diff/compare/ParameterDiff.java b/src/main/java/com/deepoove/swagger/diff/compare/ParameterDiff.java index 30e8f3ba..648aed32 100644 --- a/src/main/java/com/deepoove/swagger/diff/compare/ParameterDiff.java +++ b/src/main/java/com/deepoove/swagger/diff/compare/ParameterDiff.java @@ -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; @@ -73,7 +77,6 @@ public ParameterDiff diff(List left, List 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 @@ -84,6 +87,8 @@ public ParameterDiff diff(List left, List right) { ElProperty elProperty = new ElProperty(); elProperty.setEl(rightPara.getName()); elProperty.setProperty(mapToProperty(rightPara)); + + addChangeMetadata(elProperty, (AbstractSerializableParameter) leftPara, (AbstractSerializableParameter) rightPara); changedParameter.setChanged(Lists.newArrayList(elProperty)); } } @@ -109,6 +114,34 @@ public ParameterDiff diff(List left, List right) { return this; } + private ElProperty addChangeMetadata(ElProperty diffProperty, AbstractSerializableParameter left, AbstractSerializableParameter right) { + if(!left.getType().equalsIgnoreCase(right.getType())) { + diffProperty.setNewTypeChange(right.getType()); + } + + List leftEnums = left.getEnum() != null ? left.getEnum() : Collections.emptyList(); + List rightEnums = right.getEnum() != null ? right.getEnum() : Collections.emptyList(); + ModelDiff.addEnums(diffProperty, leftEnums, rightEnums); + + List 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()); diff --git a/src/main/java/com/deepoove/swagger/diff/model/ElProperty.java b/src/main/java/com/deepoove/swagger/diff/model/ElProperty.java index cb2bf282..32fbee84 100644 --- a/src/main/java/com/deepoove/swagger/diff/model/ElProperty.java +++ b/src/main/java/com/deepoove/swagger/diff/model/ElProperty.java @@ -2,6 +2,8 @@ import io.swagger.models.properties.Property; +import java.util.List; + /** * property with expression Language grammar * @@ -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 newEnums; + private List removedEnums; + + private String metadataChanged; public Property getProperty() { return property; @@ -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 getNewEnums() { return newEnums; } - public void setNewEnums(boolean newEnums) { + public void setNewEnums(List newEnums) { this.newEnums = newEnums; } - public boolean isRemovedEnums() { + public void metadataChanged(String changes) { + this.metadataChanged = changes; + } + + public String metadataChanged() { + return this.metadataChanged; + } + + public List getRemovedEnums() { return removedEnums; } - public void setRemovedEnums(boolean removedEnums) { + public void setRemovedEnums(List removedEnums) { this.removedEnums = removedEnums; } } diff --git a/src/main/java/com/deepoove/swagger/diff/output/HtmlRender.java b/src/main/java/com/deepoove/swagger/diff/output/HtmlRender.java index d7cb699c..6fdba4bf 100644 --- a/src/main/java/com/deepoove/swagger/diff/output/HtmlRender.java +++ b/src/main/java/com/deepoove/swagger/diff/output/HtmlRender.java @@ -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.*; @@ -169,15 +172,27 @@ private ContainerTag li_addProp(ElProperty prop) { private ContainerTag li_changedProp(ElProperty prop) { List 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) + ")"; } diff --git a/src/main/java/com/deepoove/swagger/diff/output/MarkdownRender.java b/src/main/java/com/deepoove/swagger/diff/output/MarkdownRender.java index 975e3bea..282aa92e 100644 --- a/src/main/java/com/deepoove/swagger/diff/output/MarkdownRender.java +++ b/src/main/java/com/deepoove/swagger/diff/output/MarkdownRender.java @@ -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 { @@ -63,7 +64,7 @@ private String ol_newEndpoint(List 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(); } @@ -108,7 +109,7 @@ private String ol_changed(List 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); } } diff --git a/src/test/java/com/deepoove/swagger/test/SwaggerDiffTest.java b/src/test/java/com/deepoove/swagger/test/SwaggerDiffTest.java index 6405ab59..cc982cae 100644 --- a/src/test/java/com/deepoove/swagger/test/SwaggerDiffTest.java +++ b/src/test/java/com/deepoove/swagger/test/SwaggerDiffTest.java @@ -10,6 +10,7 @@ import com.google.common.collect.Lists; import io.swagger.models.HttpMethod; import io.swagger.models.parameters.BodyParameter; +import org.apache.commons.lang3.StringUtils; import org.junit.Assert; import org.junit.Test; @@ -275,14 +276,14 @@ public void testChangedPropertyMetadata() { Assert.assertEquals(2, postChgProps.size()); ElProperty orderIdProp = postChgProps.stream().filter(cp -> { return cp.getEl().equalsIgnoreCase("body.id");}).findFirst().get(); - Assert.assertTrue(orderIdProp.isTypeChange()); - Assert.assertFalse(orderIdProp.isNewEnums()); - Assert.assertFalse(orderIdProp.isRemovedEnums()); + Assert.assertTrue(StringUtils.isBlank(orderIdProp.getNewTypeChange())); + Assert.assertNull(orderIdProp.getNewEnums()); + Assert.assertNull(orderIdProp.getRemovedEnums()); ElProperty statusProp = postChgProps.stream().filter(cp -> { return cp.getEl().equalsIgnoreCase("body.status");}).findFirst().get(); - Assert.assertFalse(statusProp.isTypeChange()); - Assert.assertTrue(statusProp.isNewEnums()); - Assert.assertTrue(statusProp.isRemovedEnums()); + Assert.assertTrue(StringUtils.isBlank(orderIdProp.getNewTypeChange())); + Assert.assertNotNull(statusProp.getNewEnums()); + Assert.assertNotNull(statusProp.getRemovedEnums()); Assert.assertTrue("Expecting changed endpoint " + getOrder, changedEndpointMap.containsKey(getOrder)); @@ -293,14 +294,14 @@ public void testChangedPropertyMetadata() { orderIdProp = getChgProps.stream().filter(cp -> { return cp.getEl().equalsIgnoreCase("id");}).findFirst().get(); - Assert.assertTrue(orderIdProp.isTypeChange()); - Assert.assertFalse(orderIdProp.isNewEnums()); - Assert.assertFalse(orderIdProp.isRemovedEnums()); + Assert.assertTrue(StringUtils.isBlank(orderIdProp.getNewTypeChange())); + Assert.assertNull(orderIdProp.getNewEnums()); + Assert.assertNull(orderIdProp.getRemovedEnums()); statusProp = getChgProps.stream().filter(cp -> { return cp.getEl().equalsIgnoreCase("status");}).findFirst().get(); - Assert.assertFalse(statusProp.isTypeChange()); - Assert.assertTrue(statusProp.isNewEnums()); - Assert.assertTrue(statusProp.isRemovedEnums()); + Assert.assertTrue(StringUtils.isBlank(statusProp.getNewTypeChange())); + Assert.assertNotNull(statusProp.getNewEnums()); + Assert.assertNotNull(statusProp.getRemovedEnums()); } private void assertEqual(SwaggerDiff diff) {