diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java index 60d749ea631..eed4e08f590 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java @@ -35,6 +35,7 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Supplier; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -547,29 +548,34 @@ public ValidatedResult assemble() { } Model processedModel = processor.buildModel(); + Model transformed; + + // Do the 1.0 -> 2.0 transform before full-model validation. + try { + transformed = new ModelInteropTransformer(processedModel, events, processor::getShapeVersion) + .transform(); + } catch (SourceException e) { + // The transformation shouldn't throw, but if it does, return here with the original model. + LOGGER.log(Level.SEVERE, "Error in ModelInteropTransformer: ", e); + events.add(ValidationEvent.fromSourceException(e)); + return new ValidatedResult<>(processedModel, events); + } // If ERROR validation events occur while loading, then performing more // granular semantic validation will only obscure the root cause of errors. if (LoaderUtils.containsErrorEvents(events)) { - return returnOnlyErrors(processedModel, events); + return returnOnlyErrors(transformed, events); } - // Do the 1.0 -> 2.0 transform before full-model validation. - ValidatedResult transformed = new ModelInteropTransformer(processedModel, events, - processor::getShapeVersion).transform(); - - if (disableValidation - || !transformed.getResult().isPresent() - || LoaderUtils.containsErrorEvents(transformed.getValidationEvents())) { - // Don't continue to validate the model if the upgrade raised ERROR events. - return transformed; + if (disableValidation) { + return new ValidatedResult<>(transformed, events); } try { - return validate(transformed.getResult().get(), transformed.getValidationEvents()); + return validate(transformed, events); } catch (SourceException e) { events.add(ValidationEvent.fromSourceException(e)); - return new ValidatedResult<>(transformed.getResult().get(), events); + return new ValidatedResult<>(transformed, events); } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java index 253b7278d72..907904bbd77 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java @@ -41,7 +41,6 @@ import software.amazon.smithy.model.traits.StreamingTrait; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.transform.ModelTransformer; -import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; /** @@ -74,13 +73,13 @@ final class ModelInteropTransformer { private final Function fileToVersion; private final List shapeUpgrades = new ArrayList<>(); - ModelInteropTransformer(Model model, List events, Function fileToVersion) { + ModelInteropTransformer(Model model, List mutableEvents, Function fileToVersion) { this.model = model; - this.events = events; + this.events = mutableEvents; this.fileToVersion = fileToVersion; } - ValidatedResult transform() { + Model transform() { // TODO: can these transforms be more efficiently moved into the loader? for (StructureShape struct : model.getStructureShapes()) { if (!Prelude.isPreludeShape(struct)) { @@ -106,7 +105,7 @@ ValidatedResult transform() { } } - return new ValidatedResult<>(ModelTransformer.create().replaceShapes(model, shapeUpgrades), events); + return ModelTransformer.create().replaceShapes(model, shapeUpgrades); } private void upgradeV1Member(MemberShape member, Shape target) { diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java index 0db515a4c0c..17ec073312a 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java @@ -1108,4 +1108,32 @@ public void appliesBoxTraitsToMixinsToo() { assertThat(model1, equalTo(model2)); } + + @Test + public void versionTransformsAreAlwaysApplied() { + ValidatedResult result = Model.assembler() + .addImport(getClass().getResource("invalid-1.0-model-upgraded.smithy")) + .assemble(); + + // Ensure that the invalid trait caused the model to have errors. + assertThat(result.getValidationEvents(Severity.ERROR), not(empty())); + + // Ensure that the model was created. + assertThat(result.getResult().isPresent(), is(true)); + + Model model = result.getResult().get(); + + // Ensure that the model upgrade transformations happened. + Shape myInteger = model.expectShape(ShapeId.from("smithy.example#MyInteger")); + Shape fooBaz = model.expectShape(ShapeId.from("smithy.example#Foo$baz")); + Shape fooBam = model.expectShape(ShapeId.from("smithy.example#Foo$bam")); + + // These shapes have a default zero value so have a default trait for 2.0. + assertThat(myInteger.getAllTraits(), hasKey(DefaultTrait.ID)); + assertThat(fooBaz.getAllTraits(), hasKey(DefaultTrait.ID)); + + // These members are considered boxed in 1.0. + assertThat(fooBam.getAllTraits(), hasKey(BoxTrait.ID)); + assertThat(fooBam.expectTrait(DefaultTrait.class).toNode(), equalTo(Node.nullNode())); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid-1.0-model-upgraded.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid-1.0-model-upgraded.smithy new file mode 100644 index 00000000000..bbd4588cc7a --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/invalid-1.0-model-upgraded.smithy @@ -0,0 +1,20 @@ +$version: "1.0" + +namespace smithy.example + +integer MyInteger + +structure Foo { + bar: Integer, + baz: MyInteger, + + @box + bam: PrimitiveInteger, + + // An invalid trait doesn't break the loading process or prevent upgrades. + @thisTraitDoesNotExist + boo: String, + + // An invalid target doesn't break the loading process or prevent upgrades. + bux: InvalidTarget +}