diff --git a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java index 318a00a2f03..d9d06cbfe24 100644 --- a/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java +++ b/smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedEnumTrait.java @@ -39,9 +39,9 @@ * list of existing values. */ public final class ChangedEnumTrait extends AbstractDiffEvaluator { - private static final String ORDER_CHANGED = ".OrderChanged"; - private static final String NAME_CHANGED = ".NameChanged"; - private static final String REMOVED = ".Removed"; + private static final String ORDER_CHANGED = ".OrderChanged."; + private static final String NAME_CHANGED = ".NameChanged."; + private static final String REMOVED = ".Removed."; @Override public List evaluate(Differences differences) { @@ -73,7 +73,8 @@ private List validateEnum(ChangedShape change, Pair events = new ArrayList<>(); int oldEndPosition = oldTrait.getValues().size() - 1; - for (EnumDefinition definition : oldTrait.getValues()) { + for (int enumIndex = 0; enumIndex < oldTrait.getValues().size(); enumIndex++) { + EnumDefinition definition = oldTrait.getValues().get(enumIndex); Optional maybeNewValue = newTrait.getValues().stream() .filter(d -> d.getValue().equals(definition.getValue())) .findFirst(); @@ -84,7 +85,7 @@ private List validateEnum(ChangedShape change, Pair validateEnum(ChangedShape change, Pair validateEnum(ChangedShape change, Pair events = ModelDiff.compare(modelA, modelB); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(2)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").get(0).getSeverity(), + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").get(0).getSeverity(), equalTo(Severity.ERROR)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").get(1).getSeverity(), equalTo(Severity.NOTE)); @@ -116,6 +116,7 @@ public void detectsRemovedEnums() { .addTrait(EnumTrait.builder() .addEnum(EnumDefinition.builder().value("foo").build()) .addEnum(EnumDefinition.builder().value("baz").build()) + .addEnum(EnumDefinition.builder().value("bat").build()) .build()) .build(); StringShape s2 = StringShape.builder() @@ -128,8 +129,9 @@ public void detectsRemovedEnums() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.2").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(2)); } @Test @@ -153,8 +155,8 @@ public void detectsRemovedEnumsEnumTraitNoNameToEnumShape() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); } @@ -182,8 +184,8 @@ public void detectsRemovedEnumsEnumTraitWithNameToEnumShape() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").stream() + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); } @@ -192,21 +194,24 @@ public void detectsRenamedEnums() { StringShape s1 = StringShape.builder() .id("foo.baz#Baz") .addTrait(EnumTrait.builder() - .addEnum(EnumDefinition.builder().value("foo").name("OLD").build()) + .addEnum(EnumDefinition.builder().value("foo").name("OLD1").build()) + .addEnum(EnumDefinition.builder().value("baz").name("OLD2").build()) .build()) .build(); StringShape s2 = StringShape.builder() .id("foo.baz#Baz") .addTrait(EnumTrait.builder() - .addEnum(EnumDefinition.builder().value("foo").name("NEW").build()) + .addEnum(EnumDefinition.builder().value("foo").name("NEW1").build()) + .addEnum(EnumDefinition.builder().value("baz").name("NEW2").build()) .build()) .build(); Model modelA = Model.assembler().addShape(s1).assemble().unwrap(); Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(2)); } @Test @@ -227,8 +232,8 @@ public void detectsRenamedEnumsEnumTraitNoNameToEnumShape() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").get(0).getSeverity(), + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").get(0).getSeverity(), equalTo(Severity.ERROR)); } @@ -251,8 +256,8 @@ public void detectsRenamedEnumsEnumTraitWithNameToEnumShape() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").get(0).getSeverity(), + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").get(0).getSeverity(), equalTo(Severity.ERROR)); } @@ -268,6 +273,7 @@ public void detectsInsertedEnums() { .id("foo.baz#Baz") .addTrait(EnumTrait.builder() .addEnum(EnumDefinition.builder().value("baz").build()) + .addEnum(EnumDefinition.builder().value("bat").build()) .addEnum(EnumDefinition.builder().value("foo").build()) .build()) .build(); @@ -275,8 +281,35 @@ public void detectsInsertedEnums() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged").get(0).getSeverity(), equalTo(Severity.ERROR)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(2)); + } + + @Test + public void detectsInsertedEnumsBeforeAppendedEnums() { + StringShape s1 = StringShape.builder() + .id("foo.baz#Baz") + .addTrait(EnumTrait.builder() + .addEnum(EnumDefinition.builder().value("foo").build()) + .build()) + .build(); + StringShape s2 = StringShape.builder() + .id("foo.baz#Baz") + .addTrait(EnumTrait.builder() + .addEnum(EnumDefinition.builder().value("baz").build()) + .addEnum(EnumDefinition.builder().value("bat").build()) + .addEnum(EnumDefinition.builder().value("foo").build()) + .addEnum(EnumDefinition.builder().value("bar").build()) + .build()) + .build(); + Model modelA = Model.assembler().addShape(s1).assemble().unwrap(); + Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); + List events = ModelDiff.compare(modelA, modelB); + + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, Severity.ERROR).size(), equalTo(2)); } @Test @@ -299,7 +332,7 @@ public void detectsInsertedEnumsEnumTraitNoNameToEnumShape() { List events = ModelDiff.compare(modelA, modelB); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(2)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.0").size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); } @@ -324,8 +357,8 @@ public void detectsInsertedEnumsEnumTraitWithNameToEnumShape() { Model modelB = Model.assembler().addShape(s2).assemble().unwrap(); List events = ModelDiff.compare(modelA, modelB); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged").stream() + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.OrderChanged.0").stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); } @@ -353,7 +386,7 @@ public void detectsAppendedEnumsAfterRemovedEnums() { List changeEvents = TestHelper.findEvents(allEvents, "ChangedEnumTrait"); assertThat(changeEvents.size(), equalTo(2)); - assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed").size(), equalTo(1)); + assertThat(TestHelper.findEvents(changeEvents, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); ValidationEvent removedEvent = changeEvents.get(0); assertThat(removedEvent.getSeverity(), equalTo(Severity.ERROR)); @@ -391,8 +424,9 @@ public void detectsAppendedEnumsAfterRemovedEnumsEnumTraitNoNameToEnumShape() { List events = ModelDiff.compare(modelA, modelB); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(4)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged").size(), equalTo(2)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.0").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.NameChanged.2").size(), equalTo(1)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(0, 3).stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(3, 4).stream() @@ -429,8 +463,8 @@ public void detectsAppendedEnumsAfterRemovedEnumsEnumTraitWithNameToEnumShape() List events = ModelDiff.compare(modelA, modelB); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").size(), equalTo(2)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").size(), equalTo(1)); - assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed").stream() + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").size(), equalTo(1)); + assertThat(TestHelper.findEvents(events, "ChangedEnumTrait.Removed.1").stream() .allMatch(e -> e.getSeverity() == Severity.ERROR), equalTo(true)); assertThat(TestHelper.findEvents(events, "ChangedEnumTrait").subList(1, 2).stream() .allMatch(e -> e.getSeverity() == Severity.NOTE), equalTo(true));