Add tests for DowngradeToV1

This commit adds the following test cases for 1.0 roundtrip
serialization for `@enum` and `@box` traits (refactor test
factories).

Also, added a string shape with the `@enum` trait for the 2.0
enum roundtrip test case.

Updated removing unnecessary defaults to fix `MemberShape`
cases with comments to explain each case.

smithy-model/src/main/java/software/amazon/smithy/model/transform/DowngradeToV1.java

@@ -100,22 +100,38 @@ private Model removeUnnecessaryDefaults(ModelTransformer transformer, Model mode
         }
 
         for (Shape shape : model.getShapesWithTrait(DefaultTrait.class)) {
-            DefaultTrait trait = shape.expectTrait(DefaultTrait.class);
-            // Members with a null default are considered boxed. Keep the trait to retain consistency with other
-            // indexes and checks.
-            if (!trait.toNode().isNullNode()) {
-                if (!NullableIndex.isDefaultZeroValueOfTypeInV1(trait.toNode(), shape.getType())) {
-                    updates.add(Shape.shapeToBuilder(shape)
-                                        .removeTrait(DefaultTrait.ID)
-                                        .removeTrait(AddedDefaultTrait.ID)
-                                        .build());
-                }
+            if (removeDefaultFromShape(shape, model)) {
+                updates.add(Shape.shapeToBuilder(shape)
+                        .removeTrait(DefaultTrait.ID)
+                        .removeTrait(AddedDefaultTrait.ID)
+                        .build());
             }
         }
 
         return transformer.replaceShapes(model, updates);
     }
 
+    private boolean removeDefaultFromShape(Shape shape, Model model) {
+        DefaultTrait trait = shape.expectTrait(DefaultTrait.class);
+
+        // Members with a null default are considered boxed. Keep the trait to retain consistency with other
+        // indexes and checks.
+        if (trait.toNode().isNullNode()) {
+            return false;
+        }
+
+        Shape target = model.expectShape(shape.asMemberShape().map(MemberShape::getTarget).orElse(shape.getId()));
+        DefaultTrait targetDefault = target.getTrait(DefaultTrait.class).orElse(null);
+
+        // If the target shape has no default trait or it isn't equal to the default trait of the member, then
+        // the default value has no representation in Smithy 1.0 models.
+        if (targetDefault == null || !targetDefault.toNode().equals(trait.toNode())) {
+            return true;
+        }
+
+        return !NullableIndex.isDefaultZeroValueOfTypeInV1(trait.toNode(), target.getType());
+    }
+
     private Model removeOtherV2Traits(ModelTransformer transformer, Model model) {
         Set<Shape> updates = new HashSet<>();
 

smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java

@@ -641,10 +641,10 @@ public Model addClientOptional(Model model, boolean applyWhenNoDefaultValue) {
      * Removes Smithy IDL 2.0 features from a model that are not strictly necessary to keep for consistency with the
      * rest of Smithy.
      *
-     * <p>This transformer converts enum shapes to string shapes with the enum trait, intEnum shapes to integer shapes,
-     * flattens and removes mixins, removes properties from resources, and removes default traits that have no impact
-     * on IDL 1.0 semantics (i.e., default traits on structure members set to something other than null, or default
-     * traits on any other shape that are not the zero value of the shape of a 1.0 model).
+     * <p>This transformer is lossy, and converts enum shapes to string shapes with the enum trait, intEnum shapes to
+     * integer shapes, flattens and removes mixins, removes properties from resources, and removes default traits that
+     * have no impact on IDL 1.0 semantics (i.e., default traits on structure members set to something other than null,
+     * or default traits on any other shape that are not the zero value of the shape of a 1.0 model).
      *
      * @param model Model to downgrade.
      * @return Returns the downgraded model.

smithy-model/src/test/java/software/amazon/smithy/model/shapes/ModelSerializerTest.java

@@ -49,24 +49,43 @@
 
 public class ModelSerializerTest {
     @TestFactory
-    public Stream<DynamicTest> generateTests() throws IOException, URISyntaxException {
+    public Stream<DynamicTest> generateV2RoundTripTests() throws IOException, URISyntaxException {
         return Files.list(Paths.get(
-                        SmithyIdlModelSerializer.class.getResource("ast-serialization/cases").toURI()))
+                        SmithyIdlModelSerializer.class.getResource("ast-serialization/cases/v2").toURI()))
                 .filter(path -> !path.toString().endsWith(".1.0.json"))
-                .map(path -> DynamicTest.dynamicTest(path.getFileName().toString(), () -> testRoundTrip(path)));
+                .map(path -> DynamicTest.dynamicTest(path.getFileName().toString(), () -> testRoundTripV2(path)));
     }
 
-    public void testRoundTrip(Path path) {
+    private void testRoundTripV2(Path path) {
+        testV2Serialization(path, path);
+        testV1DowngradeSerialization(path, Paths.get(path.toString().replace(".json", ".1.0.json")));
+    }
+
+    @TestFactory
+    public Stream<DynamicTest> generateV1RoundTripTests() throws IOException, URISyntaxException {
+        return Files.list(Paths.get(
+                        SmithyIdlModelSerializer.class.getResource("ast-serialization/cases/v1").toURI()))
+                .filter(path -> !path.toString().endsWith(".2.0.json"))
+                .map(path -> DynamicTest.dynamicTest(path.getFileName().toString(), () -> testRoundTripV1(path)));
+    }
+
+    private void testRoundTripV1(Path path) {
+        testV2Serialization(path, Paths.get(path.toString().replace(".json", ".2.0.json")));
+        testV1DowngradeSerialization(path, path);
+    }
+
+    private void testV2Serialization(Path path, Path expectedV2Path) {
         Model model = Model.assembler().addImport(path).assemble().unwrap();
         ModelSerializer serializer = ModelSerializer.builder().build();
         ObjectNode actual = serializer.serialize(model);
-        ObjectNode expected = Node.parse(IoUtils.readUtf8File(path)).expectObjectNode();
+        ObjectNode expected = Node.parse(IoUtils.readUtf8File(expectedV2Path)).expectObjectNode();
 
         Node.assertEquals(actual, expected);
+    }
 
-        // Now validate the file is serialized correctly when downgraded to 1.0.
-        Path downgradeFile = Paths.get(path.toString().replace(".json", ".1.0.json"));
-        ObjectNode expectedDowngrade = Node.parse(IoUtils.readUtf8File(downgradeFile)).expectObjectNode();
+    private void testV1DowngradeSerialization(Path path, Path expectedV1Path) {
+        Model model = Model.assembler().addImport(path).assemble().unwrap();
+        ObjectNode expectedDowngrade = Node.parse(IoUtils.readUtf8File(expectedV1Path)).expectObjectNode();
         ModelSerializer serializer1 = ModelSerializer.builder().version("1.0").build();
         ObjectNode model1 = serializer1.serialize(model);
 

smithy-model/src/test/java/software/amazon/smithy/model/transform/DowngradeToV1Test.java

@@ -120,15 +120,48 @@ public void removesResourceProperties() {
     public void removesUnnecessaryDefaults() {
         String stringModel = "$version: \"2.0\"\n"
                 + "namespace smithy.example\n"
+                // A Root level shape with a 1.0 non-zero value drops the default value and is boxed
                 + "@default(10)\n"
                 + "integer MyInteger\n"
+                // A Root level shape with a 1.0 zero value keeps the default value
+                + "@default(0)\n"
+                + "integer ZeroInteger\n"
+                // A Root level shape with no default value is boxed
+                + "integer BoxedInteger\n"
+                // Omitted Test Case: [ERROR] The @default trait can be set to null only on members
+                + "// @default(null)\n"
+                + "// integer ExplicitlyBoxedInteger\n"
+                // StructureShape still exists
                 + "structure Struct {\n"
+                // A Member that targets a shape with a 1.0 non-zero value drops the default value and is not boxed
                 + "  @default(10)\n"
                 + "  foo: MyInteger\n"
+                // Omitted Test Case: [ERROR] Member defines a default value that differs from the default value of the target shape
+                + "  // @default(5)\n"
+                + "  // fooFive: MyInteger\n"
+                // A Member that targets a shape with a matching default 1.0 zero value keeps the default value
+                + "  zeroTargetZeroMember: ZeroInteger = 0\n"
+                // Omitted Test Case: [ERROR] Member defines a default value that differs from the default value of the target shape
+                + "  // zeroTargetNonzeroMember: ZeroInteger = 1\n"
+                // A Member that has a default value of null keeps the default value of null and is boxed
+                + "  zeroTargetBoxedMember: ZeroInteger = null\n"
+                // Omitted Test Case: [ERROR] Member defines a default value that differs from the default value of the target shape
+                + "  // zeroTargetImplicitBoxedMember: ZeroInteger\n"
+                // A Member that has a target shape with no default value drops the default value
+                + "  boxedTargetZeroMember: BoxedInteger = 0\n"
+                // A Member that has a target shape with no default value drops the default value
+                + "  boxedTargetNonzeroMember: BoxedInteger = 1\n"
+                // A Member that has a default value of null keeps the default value of null and is boxed
+                + "  boxedTargetBoxedMember: BoxedInteger = null\n"
+                // A Member that has no default value has no default trait and the member is not boxed
+                + "  boxedTargetImplicitBoxedMember: BoxedInteger\n"
+                // A Member that has a default value of null keeps the default value of null and is boxed
                 + "  baz: PrimitiveInteger = null\n"
+                // A Member with the addedDefault trait drops both the default and addedDefault trait
                 + "  @default(\"hi\")\n"
                 + "  @addedDefault\n"
                 + "  bar: String\n"
+                // A Member keeps the required trait and drops the clientOptional trait
                 + "  @required\n"
                 + "  @clientOptional\n"
                 + "  bam: String\n"
@@ -141,19 +174,91 @@ public void removesUnnecessaryDefaults() {
 
         Model downgraded = ModelTransformer.create().downgradeToV1(model);
 
+        Model.assembler()
+                .addModel(downgraded)
+                .assemble()
+                .unwrap();
+
+        // A Root level shape with a 1.0 non-zero value drops the default value and is boxed
         ShapeId integerShape = ShapeId.from("smithy.example#MyInteger");
         assertThat(downgraded.expectShape(integerShape).hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(downgraded.expectShape(integerShape).hasTrait(AddedDefaultTrait.class), Matchers.is(false));
         assertThat(downgraded.expectShape(integerShape).hasTrait(BoxTrait.class), Matchers.is(true));
 
+        // A Root level shape with a 1.0 zero value keeps the default value
+        ShapeId zeroIntegerShape = ShapeId.from("smithy.example#ZeroInteger");
+        assertThat(downgraded.expectShape(zeroIntegerShape).hasTrait(DefaultTrait.class), Matchers.is(true));
+        assertThat(downgraded.expectShape(zeroIntegerShape).expectTrait(DefaultTrait.class).toNode()
+                        .expectNumberNode().getValue(),
+                Matchers.is(0L));
+        assertThat(downgraded.expectShape(zeroIntegerShape).hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(downgraded.expectShape(zeroIntegerShape).hasTrait(BoxTrait.class), Matchers.is(false));
+
+        // A Root level shape with no default value is boxed
+        ShapeId boxedIntegerShape = ShapeId.from("smithy.example#BoxedInteger");
+        assertThat(downgraded.expectShape(boxedIntegerShape).hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(downgraded.expectShape(boxedIntegerShape).hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(downgraded.expectShape(boxedIntegerShape).hasTrait(BoxTrait.class), Matchers.is(true));
+
+        // StructureShape still exists
         StructureShape dStruct = downgraded.expectShape(ShapeId.from("smithy.example#Struct"), StructureShape.class);
+
+        // A Member that targets a shape with a 1.0 non-zero value drops the default value and is not boxed
         assertThat(dStruct.getAllMembers().get("foo").hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("foo").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("foo").hasTrait(BoxTrait.class), Matchers.is(false));
+
+        // A Member that targets a shape with a matching default 1.0 zero value keeps the default value
+        assertThat(dStruct.getAllMembers().get("zeroTargetZeroMember").hasTrait(DefaultTrait.class), Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("zeroTargetZeroMember").expectTrait(DefaultTrait.class).toNode()
+                        .expectNumberNode().getValue(),
+                Matchers.is(0L));
+        assertThat(dStruct.getAllMembers().get("zeroTargetZeroMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("zeroTargetZeroMember").hasTrait(BoxTrait.class), Matchers.is(false));
+
+        // A Member that has a default value of null keeps the default value of null and is boxed
+        assertThat(dStruct.getAllMembers().get("zeroTargetBoxedMember").hasTrait(DefaultTrait.class), Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("zeroTargetBoxedMember").expectTrait(DefaultTrait.class).toNode().isNullNode(),
+                Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("zeroTargetBoxedMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("zeroTargetBoxedMember").hasTrait(BoxTrait.class), Matchers.is(true));
+
+        // A Member that has a target shape with no default value drops the default value
+        assertThat(dStruct.getAllMembers().get("boxedTargetZeroMember").hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetZeroMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetZeroMember").hasTrait(BoxTrait.class), Matchers.is(false));
+
+        // A Member that has a target shape with no default value drops the default value
+        assertThat(dStruct.getAllMembers().get("boxedTargetNonzeroMember").hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetNonzeroMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetNonzeroMember").hasTrait(BoxTrait.class), Matchers.is(false));
+
+        // A Member that has a default value of null keeps the default value of null and is boxed
+        assertThat(dStruct.getAllMembers().get("boxedTargetBoxedMember").hasTrait(DefaultTrait.class), Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("boxedTargetBoxedMember").expectTrait(DefaultTrait.class).toNode().isNullNode(),
+                Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("boxedTargetBoxedMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetBoxedMember").hasTrait(BoxTrait.class), Matchers.is(true));
+
+        // A Member that has no default value has no default trait and the member is not boxed
+        assertThat(dStruct.getAllMembers().get("boxedTargetImplicitBoxedMember").hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetImplicitBoxedMember").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("boxedTargetImplicitBoxedMember").hasTrait(BoxTrait.class), Matchers.is(false));
 
+        // A Member that has a default value of null keeps the default value of null and is boxed
         assertThat(dStruct.getAllMembers().get("baz").hasTrait(DefaultTrait.class), Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("baz").expectTrait(DefaultTrait.class).toNode().isNullNode(),
+                Matchers.is(true));
+        assertThat(dStruct.getAllMembers().get("baz").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
         assertThat(dStruct.getAllMembers().get("baz").hasTrait(BoxTrait.class), Matchers.is(true));
 
+        // A Member with the addedDefault trait drops both the default and addedDefault trait
         assertThat(dStruct.getAllMembers().get("bar").hasTrait(DefaultTrait.class), Matchers.is(false));
         assertThat(dStruct.getAllMembers().get("bar").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
 
+        // A Member keeps the required trait and drops the clientOptional trait
+        assertThat(dStruct.getAllMembers().get("bam").hasTrait(DefaultTrait.class), Matchers.is(false));
+        assertThat(dStruct.getAllMembers().get("bam").hasTrait(AddedDefaultTrait.class), Matchers.is(false));
         assertThat(dStruct.getAllMembers().get("bam").hasTrait(RequiredTrait.class), Matchers.is(true));
         assertThat(dStruct.getAllMembers().get("bam").hasTrait(ClientOptionalTrait.class), Matchers.is(false));
     }