Update IDL serializer to write metadata to a separate file to avoid d…

…uplication of data
smithy-lang · Nov 15, 2024 · bf32a87 · bf32a87
1 parent 449da56
commit bf32a87
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 55 deletions.
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java
@@ -52,10 +52,10 @@
 import software.amazon.smithy.model.traits.Trait;
 import software.amazon.smithy.model.traits.UnitTypeTrait;
 import software.amazon.smithy.utils.AbstractCodeWriter;
-import software.amazon.smithy.utils.CodeWriter;
 import software.amazon.smithy.utils.FunctionalUtils;
 import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.Pair;
+import software.amazon.smithy.utils.SimpleCodeWriter;
 import software.amazon.smithy.utils.SmithyBuilder;
 import software.amazon.smithy.utils.StringUtils;
 
@@ -154,13 +154,22 @@ public Map<Path, String> serialize(Model model) {
                 .filter(shapeFilter)
                 .collect(Collectors.groupingBy(shapePlacer)).entrySet().stream()
                 .collect(Collectors.toMap(Map.Entry::getKey, entry -> serialize(model, entry.getValue())));
+        // If there is no metadata, do not create metadata file
+        if (model.getMetadata().isEmpty()) {
+            return result;
+        }
+
+        // Add metadata file to the list of output files
+        Path metadataPath = Paths.get("metadata.smithy");
+        if (basePath != null) {
+            metadataPath = basePath.resolve(metadataPath);
+        }
         if (result.isEmpty()) {
-            Path path = Paths.get("metadata.smithy");
-            if (basePath != null) {
-                path = basePath.resolve(path);
-            }
-            return Collections.singletonMap(path, serializeHeader(
-                    model, null, Collections.emptySet(), inlineInputSuffix, inlineOutputSuffix));
+            return Collections.singletonMap(metadataPath, serializeHeader(
+                    model, null, Collections.emptySet(), inlineInputSuffix, inlineOutputSuffix, true));
+        } else {
+            result.put(metadataPath, serializeHeader(
+                    model, null, Collections.emptySet(), inlineInputSuffix, inlineOutputSuffix, true));
         }
         return result;
     }
@@ -191,8 +200,8 @@ private String serialize(Model fullModel, Collection<Shape> shapes) {
                 .forEach(shape -> shape.accept(shapeSerializer));
 
         String header = serializeHeader(
-                fullModel, namespace, shapes, inlineSuffixes.getLeft(), inlineSuffixes.getRight());
-        return header + codeWriter.toString();
+                fullModel, namespace, shapes, inlineSuffixes.getLeft(), inlineSuffixes.getRight(), false);
+        return header + codeWriter;
     }
 
     private Set<ShapeId> getInlineableShapes(
@@ -285,11 +294,10 @@ private String serializeHeader(
             String namespace,
             Collection<Shape> shapes,
             String inputSuffix,
-            String outputSuffix
+            String outputSuffix,
+            boolean addMetadata
     ) {
         SmithyCodeWriter codeWriter = new SmithyCodeWriter(null, fullModel);
-        NodeSerializer nodeSerializer = new NodeSerializer(codeWriter, fullModel);
-
         codeWriter.write("$$version: \"$L\"", Model.MODEL_VERSION);
 
         if (shapes.stream().anyMatch(Shape::isOperationShape)) {
@@ -304,23 +312,19 @@ private String serializeHeader(
 
         codeWriter.write("");
 
-        Comparator<Map.Entry<String, Node>> comparator = componentOrder.metadataComparator();
-
-        // Write the full metadata into every output. When loaded back together the conflicts will be ignored,
-        // but if they're separated out then each file will still have all the context.
-        fullModel.getMetadata().entrySet().stream()
-                .filter(entry -> metadataFilter.test(entry.getKey()))
-                .sorted(comparator)
-                .forEach(entry -> {
-                    codeWriter.trimTrailingSpaces(false)
-                            .writeInline("metadata $K = ", entry.getKey())
-                            .trimTrailingSpaces();
-                    nodeSerializer.serialize(entry.getValue());
-                    codeWriter.write("");
-                });
-
-        if (!fullModel.getMetadata().isEmpty()) {
-            codeWriter.write("");
+        if (addMetadata) {
+            NodeSerializer nodeSerializer = new NodeSerializer(codeWriter, fullModel);
+            Comparator<Map.Entry<String, Node>> comparator = componentOrder.metadataComparator();
+            fullModel.getMetadata().entrySet().stream()
+                    .filter(entry -> metadataFilter.test(entry.getKey()))
+                    .sorted(comparator)
+                    .forEach(entry -> {
+                        codeWriter.trimTrailingSpaces(false)
+                                .writeInline("metadata $K = ", entry.getKey())
+                                .trimTrailingSpaces();
+                        nodeSerializer.serialize(entry.getValue());
+                        codeWriter.write("");
+                    });
         }
 
         if (namespace != null) {
@@ -606,7 +610,7 @@ private void writeMixins(Shape shape) {
 
         private void writeShapeMembers(Collection<MemberShape> members) {
             if (members.isEmpty()) {
-                // When the are no members to write, put "{}" on the same line.
+                // When there are no members to write, put "{}" on the same line.
                 codeWriter.writeInline("{}").write("");
             } else {
                 codeWriter.openBlock("{", "}", () -> {
@@ -1028,7 +1032,7 @@ private void serializeKeyValuePairs(ObjectNode node, Shape shape) {
             }
 
             // If we're looking at a structure or union shape, we'll need to get the member shape based on the
-            // node key. Here we pre-compute a mapping so we don't have to traverse the member list every time.
+            // node key. Here we pre-compute a mapping, so we don't have to traverse the member list every time.
             Map<String, MemberShape> members;
             if (shape == null) {
                 members = Collections.emptyMap();
@@ -1058,11 +1062,11 @@ private void serializeKeyValuePairs(ObjectNode node, Shape shape) {
     }
 
     /**
-     * Extension of {@link CodeWriter} that provides additional convenience methods.
+     * Extension of {@link SimpleCodeWriter} that provides additional convenience methods.
      *
-     * <p>Provides a built in $I formatter that formats shape ids, automatically trimming namespace where possible.
+     * <p>Provides a built-in $I formatter that formats shape ids, automatically trimming namespace where possible.
      */
-    private static final class SmithyCodeWriter extends CodeWriter {
+    private static final class SmithyCodeWriter extends SimpleCodeWriter {
         private static final Pattern UNQUOTED_KEY_STRING = Pattern.compile("[a-zA-Z_][a-zA-Z_0-9]*");
         private final String namespace;
         private final Model model;

diff --git a/...model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java b/...model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
@@ -11,12 +11,15 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.io.IOException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
+import java.util.Objects;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.DynamicTest;
@@ -33,10 +36,11 @@
 import software.amazon.smithy.utils.MapUtils;
 
 public class SmithyIdlModelSerializerTest {
+    private static final URL TEST_FILE_URL = Objects.requireNonNull(SmithyIdlModelSerializer.class.getResource("idl-serialization/cases"));
+
     @TestFactory
     public Stream<DynamicTest> generateTests() throws IOException, URISyntaxException {
-        return Files.list(Paths.get(
-                SmithyIdlModelSerializer.class.getResource("idl-serialization/cases").toURI()))
+        return Files.list(Paths.get(TEST_FILE_URL.toURI()))
                 .map(path -> DynamicTest.dynamicTest(path.getFileName().toString(), () -> testConversion(path)));
     }
 
@@ -79,10 +83,13 @@ public void filtersShapes() {
                 .build();
         Map<Path, String> serialized = serializer.serialize(model);
 
-        assertThat(serialized, aMapWithSize(1));
+        assertThat(serialized, aMapWithSize(2));
         assertThat(serialized, hasKey(Paths.get("ns.structures.smithy")));
         assertThat(serialized.get(Paths.get("ns.structures.smithy")),
                 containsString("namespace ns.structures"));
+        assertThat(serialized, hasKey(Paths.get("metadata.smithy")));
+        assertThat(serialized.get(Paths.get("metadata.smithy")),
+                containsString("metadata shared = true"));
         assertThat(serialized, not(hasKey(Paths.get("smithy.api.smithy"))));
     }
 
@@ -236,15 +243,21 @@ public void usesOriginalSourceLocation() {
 
     @Test
     public void sortsAlphabetically() {
-        URL resource = getClass().getResource("idl-serialization/alphabetical.smithy");
-        Model model = Model.assembler().addImport(resource).assemble().unwrap();
+        URL before = getClass().getResource("idl-serialization/alphabetical/before.smithy");
+        URL after = getClass().getResource("idl-serialization/alphabetical/after.smithy");
+        URL metadata = getClass().getResource("idl-serialization/alphabetical/metadata.smithy");
+
+        Model model = Model.assembler().addImport(before).assemble().unwrap();
         Map<Path, String> reserialized = SmithyIdlModelSerializer.builder()
                 .componentOrder(SmithyIdlComponentOrder.ALPHA_NUMERIC)
                 .build()
                 .serialize(model);
-        String modelResult = reserialized.values().iterator().next().replace("\r\n", "\n");
 
-        assertThat(modelResult, equalTo(IoUtils.readUtf8Url(resource).replace("\r\n", "\n")));
+        String modelResult = reserialized.get(Paths.get("com.example.smithy")).replace("\r\n", "\n");
+        String metadataResult = reserialized.get(Paths.get("metadata.smithy")).replace("\r\n", "\n");
+
+        assertThat(modelResult, equalTo(IoUtils.readUtf8Url(after).replace("\r\n", "\n")));
+        assertThat(metadataResult, equalTo(IoUtils.readUtf8Url(metadata).replace("\r\n", "\n")));
     }
 
     @Test

diff --git a/...resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/after.smithy b/...resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/after.smithy
@@ -0,0 +1,18 @@
+$version: "2.0"
+
+namespace com.example
+
+structure Abc {
+    bar: String
+    @length(
+        min: 1
+    )
+    @required
+    baz: String
+}
+
+string Def
+
+service Hij {
+    version: "2006-03-01"
+}
diff --git a/...pes/idl-serialization/alphabetical.smithy → ...-serialization/alphabetical/before.smithy b/...pes/idl-serialization/alphabetical.smithy → ...-serialization/alphabetical/before.smithy
@@ -1,10 +1,16 @@
 $version: "2.0"
 
-metadata foo = "hi"
 metadata zoo = "test"
+metadata foo = "hi"
 
 namespace com.example
 
+service Hij {
+    version: "2006-03-01"
+}
+
+string Def
+
 structure Abc {
     bar: String
     @length(
@@ -13,9 +19,3 @@ structure Abc {
     @required
     baz: String
 }
-
-string Def
-
-service Hij {
-    version: "2006-03-01"
-}
diff --git a/...ources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/metadata.smithy b/...ources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/metadata.smithy
@@ -0,0 +1,4 @@
+$version: "2.0"
+
+metadata foo = "hi"
+metadata zoo = "test"
diff --git a/...e/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy b/...e/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy
@@ -0,0 +1,3 @@
+$version: "2.0"
+
+metadata shared = true
diff --git a/...zon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.primitives.smithy b/...zon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.primitives.smithy
@@ -1,7 +1,5 @@
 $version: "2.0"
 
-metadata shared = true
-
 namespace ns.primitives
 
 list StringList {

diff --git a/...zon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.structures.smithy b/...zon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.structures.smithy
@@ -1,7 +1,5 @@
 $version: "2.0"
 
-metadata shared = true
-
 namespace ns.structures
 
 use ns.primitives#StringList

diff --git a/.../test/resources/software/amazon/smithy/model/shapes/idl-serialization/out-of-order.smithy b/.../test/resources/software/amazon/smithy/model/shapes/idl-serialization/out-of-order.smithy
@@ -1,8 +1,5 @@
 $version: "2.0"
 
-metadata zoo = "test"
-metadata foo = "hi"
-
 namespace com.example
 
 string MyString