From 5879d773df9e5a03696800112df3374d7bc0f8c5 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Mon, 21 Sep 2020 16:41:22 -0700 Subject: [PATCH] Improve information available from diffs This change adds a lot more details to the result of a ModelDiff, including access to the raw Differences between the models, access to the old and new models through the Differences object, access to the old validation events and new validation events, and access to the diff events. Helper methods were added to ModelDiff.Result to detect which validation events were introduced by a change, which validation events were resolved by a change, and if there are breaking changes in the diff (e.g., events that are ERROR or DANGER). Note that any kind of suppression of diff events would need to be applied and managed outside of this helper functionality. Added some missing equals and hashCode implementations. --- .../amazon/smithy/diff/ChangedMetadata.java | 22 +- .../amazon/smithy/diff/ChangedShape.java | 23 +- .../amazon/smithy/diff/Differences.java | 22 +- .../amazon/smithy/diff/ModelDiff.java | 245 +++++++++++++++++- .../amazon/smithy/diff/ModelDiffTest.java | 97 +++++++ 5 files changed, 397 insertions(+), 12 deletions(-) create mode 100644 smithy-diff/src/test/java/software/amazon/smithy/diff/ModelDiffTest.java diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedMetadata.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedMetadata.java index 8136a9f24c3..81e1342d647 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedMetadata.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -15,6 +15,7 @@ package software.amazon.smithy.diff; +import java.util.Objects; import software.amazon.smithy.model.FromSourceLocation; import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.node.Node; @@ -64,4 +65,23 @@ public Node getNewValue() { public SourceLocation getSourceLocation() { return getNewValue().getSourceLocation(); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof ChangedMetadata)) { + return false; + } else { + ChangedMetadata that = (ChangedMetadata) o; + return getKey().equals(that.getKey()) + && Objects.equals(getOldValue(), that.getOldValue()) + && Objects.equals(getNewValue(), that.getNewValue()); + } + } + + @Override + public int hashCode() { + return Objects.hash(getKey(), getOldValue(), getNewValue()); + } } diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java index 0a9d8a1c653..eb74e612fab 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/ChangedShape.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; import software.amazon.smithy.model.FromSourceLocation; @@ -138,6 +139,26 @@ public Map> getTraitDifferences() { return traitDiff; } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof ChangedShape)) { + return false; + } else { + // If the shapes are equal, then the changed traits are equal, so + // there's no need to compare the traitDiff property. + ChangedShape that = (ChangedShape) o; + return Objects.equals(getOldShape(), that.getOldShape()) + && Objects.equals(getNewShape(), that.getNewShape()); + } + } + + @Override + public int hashCode() { + return Objects.hash(getOldShape(), getNewShape()); + } + /** * Finds the trait differences between the old and new shape. * diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java index 6b6b0244340..c2a6d85f98c 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/Differences.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.stream.Stream; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.Node; @@ -161,6 +162,25 @@ public Stream changedMetadata() { return changedMetadata.stream(); } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof Differences)) { + return false; + } else { + // The differences between the models are always equivalent if the + // models are equivalent, so no need to compare them. + Differences that = (Differences) o; + return getOldModel().equals(that.getOldModel()) && getNewModel().equals(that.getNewModel()); + } + } + + @Override + public int hashCode() { + return Objects.hash(getOldModel(), getNewModel()); + } + private static void detectShapeChanges(Model oldModel, Model newModel, Differences differences) { for (Shape oldShape : oldModel.toSet()) { newModel.getShape(oldShape.getId()).ifPresent(newShape -> { diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java index 9d72143db58..3e18a774d95 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/ModelDiff.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -16,22 +16,41 @@ package software.amazon.smithy.diff; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.ServiceLoader; +import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.SmithyBuilder; /** * Computes the difference between two models and any problems that might * occur due to those differences. */ public final class ModelDiff { + private ModelDiff() {} + /** + * Creates a new ModelDiff.Builder that provides in-depth diff analysis. + * + * @return Returns the builder. + */ + public static Builder builder() { + return new Builder(); + } + /** * Evaluates the differences between two models. * + *

Use {@link Builder} directly to get access to additional information. + * * @param oldModel Previous version of the model. * @param newModel New model to compare. * @return Returns the computed validation events. @@ -43,21 +62,229 @@ public static List compare(Model oldModel, Model newModel) { /** * Evaluates the differences between two models. * + *

Use {@link Builder} directly to get access to additional information. + * * @param classLoader ClassLoader used to find {@link DiffEvaluator} service providers. * @param oldModel Previous version of the model. * @param newModel New model to compare. * @return Returns the computed validation events. */ public static List compare(ClassLoader classLoader, Model oldModel, Model newModel) { - List evaluators = new ArrayList<>(); - ServiceLoader.load(DiffEvaluator.class, classLoader).forEach(evaluators::add); - return compare(evaluators, oldModel, newModel); + return builder() + .oldModel(oldModel) + .newModel(newModel) + .classLoader(classLoader) + .compare() + .getDiffEvents(); } - private static List compare(List evaluators, Model oldModel, Model newModel) { - Differences differences = Differences.detect(oldModel, newModel); - return evaluators.parallelStream() - .flatMap(evaluator -> evaluator.evaluate(differences).stream()) - .collect(Collectors.toList()); + /** + * The result of comparing two Smithy models. + */ + public static final class Result { + private final Differences differences; + private final List diffEvents; + private final List oldModelEvents; + private final List newModelEvents; + + public Result( + Differences differences, + List diffEvents, + List oldModelEvents, + List newModelEvents + ) { + this.differences = Objects.requireNonNull(differences); + this.diffEvents = Objects.requireNonNull(diffEvents); + this.oldModelEvents = Objects.requireNonNull(oldModelEvents); + this.newModelEvents = Objects.requireNonNull(newModelEvents); + } + + /** + * Gets a queryable set of differences between two models. + * + * @return Returns the differences. + */ + public Differences getDifferences() { + return differences; + } + + /** + * Gets the diff analysis as a list of {@link ValidationEvent}s. + * + * @return Returns the diff validation events. + */ + public List getDiffEvents() { + return diffEvents; + } + + /** + * Gets the validation events emitted when validating the old model. + * + * @return Returns the old model's validation events. + */ + public List getOldModelEvents() { + return oldModelEvents; + } + + /** + * Gets the validation events emitted when validating the new model. + * + * @return Returns the new model's validation events. + */ + public List getNewModelEvents() { + return newModelEvents; + } + + /** + * Gets the validation events that were present in the old model but + * are no longer an issue in the new model. + * + * @return Returns the resolved validation events. + */ + public Set determineResolvedEvents() { + Set events = new TreeSet<>(getOldModelEvents()); + events.removeAll(getNewModelEvents()); + return events; + } + + /** + * Gets the validation events that were introduced by whatever changes + * were made to the new model. + * + * @return Returns the validation events introduced by the new model. + */ + public Set determineIntroducedEvents() { + Set events = new TreeSet<>(getNewModelEvents()); + events.removeAll(getOldModelEvents()); + return events; + } + + /** + * Determines if the diff events contain any DANGER or ERROR events. + * + * @return Returns true if this diff has breaking changes. + */ + public boolean isDiffBreaking() { + for (ValidationEvent event : getDiffEvents()) { + if (event.getSeverity() == Severity.ERROR || event.getSeverity() == Severity.DANGER) { + return true; + } + } + return false; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof Result)) { + return false; + } + Result result = (Result) o; + return getDifferences().equals(result.getDifferences()) + && getDiffEvents().equals(result.getDiffEvents()) + && getOldModelEvents().equals(result.getOldModelEvents()) + && getNewModelEvents().equals(result.getNewModelEvents()); + } + + @Override + public int hashCode() { + return Objects.hash(getDifferences(), getDiffEvents(), getOldModelEvents(), getNewModelEvents()); + } + } + + /** + * Builder used to construct a diff of two Smithy models. + */ + public static final class Builder { + private Model oldModel; + private Model newModel; + private List oldModelEvents = Collections.emptyList(); + private List newModelEvents = Collections.emptyList(); + private ClassLoader classLoader = ModelDiff.class.getClassLoader(); + + private Builder() {} + + /** + * Sets the ClassLoader used to find {@link DiffEvaluator} service + * providers. + * + * @param classLoader ClassLoader to use. + * @return Returns the builder. + */ + public Builder classLoader(ClassLoader classLoader) { + this.classLoader = Objects.requireNonNull(classLoader); + return this; + } + + /** + * Sets the old model to compare against. + * + * @param oldModel Old version of a model. + * @return Returns the builder. + */ + public Builder oldModel(Model oldModel) { + this.oldModel = Objects.requireNonNull(oldModel); + return this; + } + + /** + * Sets the new model to compare against. + * + * @param newModel New version of a model. + * @return Returns the builder. + */ + public Builder newModel(Model newModel) { + this.newModel = Objects.requireNonNull(newModel); + return this; + } + + /** + * Sets the old model to compare against along with the validation + * events encountered while loading the model. + * + * @param oldModel Old version of a model with events. + * @return Returns the builder. + */ + public Builder oldModel(ValidatedResult oldModel) { + this.oldModel = oldModel.getResult() + .orElseThrow(() -> new IllegalArgumentException("No old model present in ValidatedResult")); + this.oldModelEvents = oldModel.getValidationEvents(); + return this; + } + + /** + * Sets the new model to compare against along with the validation + * events encountered while loading the model. + * + * @param newModel New version of a model with events. + * @return Returns the builder. + */ + public Builder newModel(ValidatedResult newModel) { + this.newModel = newModel.getResult() + .orElseThrow(() -> new IllegalArgumentException("No new model present in ValidatedResult")); + this.newModelEvents = newModel.getValidationEvents(); + return this; + } + + /** + * Performs the diff of the old and new models. + * + * @return Returns the diff {@link Result}. + * @throws IllegalStateException if {@code oldModel} and {@code newModel} are not set. + */ + public Result compare() { + SmithyBuilder.requiredState("oldModel", oldModel); + SmithyBuilder.requiredState("newModel", newModel); + + List evaluators = new ArrayList<>(); + ServiceLoader.load(DiffEvaluator.class, classLoader).forEach(evaluators::add); + Differences differences = Differences.detect(oldModel, newModel); + List diffEvents = evaluators.parallelStream() + .flatMap(evaluator -> evaluator.evaluate(differences).stream()) + .collect(Collectors.toList()); + + return new Result(differences, diffEvents, oldModelEvents, newModelEvents); + } } } diff --git a/smithy-diff/src/test/java/software/amazon/smithy/diff/ModelDiffTest.java b/smithy-diff/src/test/java/software/amazon/smithy/diff/ModelDiffTest.java new file mode 100644 index 00000000000..ab6b72d0f24 --- /dev/null +++ b/smithy-diff/src/test/java/software/amazon/smithy/diff/ModelDiffTest.java @@ -0,0 +1,97 @@ +package software.amazon.smithy.diff; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.StringShape; +import software.amazon.smithy.model.traits.SensitiveTrait; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidatedResult; +import software.amazon.smithy.model.validation.ValidationEvent; + +public class ModelDiffTest { + @Test + public void providesValidationResult() { + Model oldModel = Model.builder().build(); + Model newModel = Model.builder().build(); + List oldEvents = Collections.singletonList( + ValidationEvent.builder() + .id("x") + .severity(Severity.ERROR) + .message("Hello") + .build()); + List newEvents = Collections.singletonList( + ValidationEvent.builder() + .id("y") + .severity(Severity.ERROR) + .message("Hello") + .build()); + + ValidatedResult oldResult = new ValidatedResult<>(oldModel, oldEvents); + ValidatedResult newResult = new ValidatedResult<>(newModel, newEvents); + + ModelDiff.Result result = ModelDiff.builder() + .oldModel(oldResult) + .newModel(newResult) + .compare(); + + assertThat(result.getOldModelEvents(), equalTo(oldEvents)); + assertThat(result.getNewModelEvents(), equalTo(newEvents)); + assertThat(result.getDifferences().addedShapes().count(), is(0L)); + + assertThat(result, equalTo(result)); + assertThat(result.hashCode(), equalTo(result.hashCode())); + + assertThat(result.determineResolvedEvents(), contains(oldEvents.get(0))); + assertThat(result.determineIntroducedEvents(), contains(newEvents.get(0))); + } + + @Test + public void testsEquality() { + Model oldModel = Model.builder() + .putMetadataProperty("foo", Node.from("baz")) + .addShape(StringShape.builder().id("smithy.example#Str").build()) + .build(); + Model newModel = Model.builder() + .putMetadataProperty("foo", Node.from("bar")) + .addShape(StringShape.builder().id("smithy.example#Str").addTrait(new SensitiveTrait()).build()) + .build(); + ModelDiff.Result result1 = ModelDiff.builder().oldModel(oldModel).newModel(newModel).compare(); + ModelDiff.Result result2 = ModelDiff.builder().oldModel(oldModel).newModel(newModel).compare(); + + // Same instance equality. + assertThat(result1, equalTo(result1)); + assertThat(result1.hashCode(), equalTo(result1.hashCode())); + + // .equals equality. + assertThat(result1, equalTo(result2)); + } + + @Test + public void findsBreakingChanges() { + Model oldModel = Model.builder() + .addShape(StringShape.builder().id("smithy.example#Str").build()) + .build(); + Model newModel = Model.builder().build(); + ModelDiff.Result result = ModelDiff.builder().oldModel(oldModel).newModel(newModel).compare(); + + assertThat(result.isDiffBreaking(), is(true)); + } + + @Test + public void detectsWhenNoBreakingChanges() { + Model model = Model.builder() + .addShape(StringShape.builder().id("smithy.example#Str").build()) + .build(); + ModelDiff.Result result = ModelDiff.builder().oldModel(model).newModel(model).compare(); + + assertThat(result.isDiffBreaking(), is(false)); + } +}