From 237923e757c956940f6af38bb7d659a2c57d6dde Mon Sep 17 00:00:00 2001 From: Hunter Mellema Date: Fri, 15 Nov 2024 09:04:10 -0700 Subject: [PATCH] Update IDL serializer to write metadata to a separate file to avoid duplication of data --- .../shapes/SmithyIdlModelSerializer.java | 72 ++++++++++--------- .../shapes/SmithyIdlModelSerializerTest.java | 27 +++++-- .../alphabetical/after.smithy | 18 +++++ .../before.smithy} | 14 ++-- .../alphabetical/metadata.smithy | 4 ++ .../output/metadata.smithy | 3 + .../output/ns.primitives.smithy | 2 - .../output/ns.structures.smithy | 2 - .../idl-serialization/out-of-order.smithy | 3 - 9 files changed, 90 insertions(+), 55 deletions(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/after.smithy rename smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/{alphabetical.smithy => alphabetical/before.smithy} (100%) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/metadata.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy 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 index 19a713aff9c..c394db61070 100644 --- 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 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 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 getInlineableShapes( @@ -285,11 +294,10 @@ private String serializeHeader( String namespace, Collection 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> 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> 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 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 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. * - *

Provides a built in $I formatter that formats shape ids, automatically trimming namespace where possible. + *

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/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java index 27dea4cc802..b1412360ae0 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java +++ b/smithy-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 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 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 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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/after.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/after.smithy new file mode 100644 index 00000000000..6b92529ec0f --- /dev/null +++ b/smithy-model/src/test/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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/before.smithy similarity index 100% rename from smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical.smithy rename to smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/before.smithy index bbf96fd5abd..be939f1a540 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical.smithy +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/metadata.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/alphabetical/metadata.smithy new file mode 100644 index 00000000000..80dcac2631e --- /dev/null +++ b/smithy-model/src/test/resources/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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy new file mode 100644 index 00000000000..b9344b1ef7a --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/metadata.smithy @@ -0,0 +1,3 @@ +$version: "2.0" + +metadata shared = true diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.primitives.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.primitives.smithy index ef792507bd1..2b68f6654e4 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.primitives.smithy +++ b/smithy-model/src/test/resources/software/amazon/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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.structures.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.structures.smithy index 6626adf8ebd..4c517ae1219 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/multiple-namespaces/output/ns.structures.smithy +++ b/smithy-model/src/test/resources/software/amazon/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/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/out-of-order.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/out-of-order.smithy index 3947bc3f2d3..d8bd628b149 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/out-of-order.smithy +++ b/smithy-model/src/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