De-depude model components by location and value

smithy-model/src/main/java/software/amazon/smithy/model/loader/MetadataContainer.java

@@ -56,22 +56,34 @@ final class MetadataContainer {
      * @param value Value to set.
      */
     void putMetadata(String key, Node value) {
-        if (!data.containsKey(key)) {
-            data.put(key, value);
-        } else if (data.get(key).isArrayNode() && value.isArrayNode()) {
-            ArrayNode previous = data.get(key).expectArrayNode();
-            List<Node> merged = new ArrayList<>(previous.getElements());
+        Node previous = data.putIfAbsent(key, value);
+
+        if (previous == null) {
+            return;
+        }
+
+        if (LoaderUtils.isSameLocation(value, previous) && value.equals(previous)) {
+            // The assumption here is that if the metadata value is exactly the
+            // same and from the same location, then the same model file was
+            // included more than once in a way that side-steps file and URL
+            // de-duplication. For example, this can occur when a Model is assembled
+            // through a ModelAssembler using model discovery, then the Model is
+            // added to a subsequent ModelAssembler, and then model discovery is
+            // performed again using the same classpath.
+            LOGGER.finer(() -> "Ignoring duplicate metadata key from same exact location: " + key);
+        } else if (previous.isArrayNode() && value.isArrayNode()) {
+            ArrayNode previousArray = previous.expectArrayNode();
+            List<Node> merged = new ArrayList<>(previousArray.getElements());
             merged.addAll(value.expectArrayNode().getElements());
-            ArrayNode mergedArray = new ArrayNode(merged, value.getSourceLocation());
-            data.put(key, mergedArray);
-        } else if (!data.get(key).equals(value)) {
+            data.put(key, new ArrayNode(merged, value.getSourceLocation()));
+        } else if (!previous.equals(value)) {
             events.add(ValidationEvent.builder()
                     .id(Validator.MODEL_ERROR)
                     .severity(Severity.ERROR)
                     .sourceLocation(value)
                     .message(format(
                             "Metadata conflict for key `%s`. Defined in both `%s` and `%s`",
-                            key, value.getSourceLocation(), data.get(key).getSourceLocation()))
+                            key, value.getSourceLocation(), previous.getSourceLocation()))
                     .build());
         } else {
             LOGGER.fine(() -> "Ignoring duplicate metadata definition of " + key);

smithy-model/src/main/java/software/amazon/smithy/model/loader/TraitContainer.java

@@ -146,6 +146,20 @@ public void onTrait(ShapeId target, Trait value) {
 
             if (traits.containsKey(traitId)) {
                 Trait previousTrait = traits.get(traitId);
+
+                if (LoaderUtils.isSameLocation(previousTrait, value) && previousTrait.equals(value)) {
+                    // The assumption here is that if the trait value is exactly the
+                    // same and from the same location, then the same model file was
+                    // included more than once in a way that side-steps file and URL
+                    // de-duplication. For example, this can occur when a Model is assembled
+                    // through a ModelAssembler using model discovery, then the Model is
+                    // added to a subsequent ModelAssembler, and then model discovery is
+                    // performed again using the same classpath.
+                    LOGGER.finest(() -> String.format("Ignoring duplicate %s trait value on %s at same exact location",
+                                                      traitId, target));
+                    return;
+                }
+
                 Node previous = previousTrait.toNode();
                 Node updated = value.toNode();
 
@@ -157,8 +171,7 @@ public void onTrait(ShapeId target, Trait value) {
                         return;
                     }
                 } else if (previous.equals(updated)) {
-                    LOGGER.fine(() -> String.format(
-                            "Ignoring duplicate %s trait value on %s", traitId, target));
+                    LOGGER.fine(() -> String.format("Ignoring duplicate %s trait value on %s", traitId, target));
                     return;
                 } else {
                     events.add(ValidationEvent.builder()

smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java

@@ -51,6 +51,7 @@
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.node.StringNode;
+import software.amazon.smithy.model.shapes.ModelSerializer;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.ShapeType;
@@ -616,4 +617,19 @@ public void createsDynamicTraitWhenTraitFactoryReturnsEmpty() {
         assertTrue(model.expectShape(id).findTrait(traitId).isPresent());
         assertThat(model.expectShape(id).findTrait(traitId).get(), instanceOf(DynamicTrait.class));
     }
+
+    @Test
+    public void dedupesExactSameTraitsAndMetadataFromSameLocation() {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("dedupe-models.smithy"))
+                .assemble()
+                .unwrap();
+        Model model2 = Model.assembler()
+                .addModel(model)
+                .addImport(getClass().getResource("dedupe-models.smithy"))
+                .assemble()
+                .unwrap();
+
+        assertThat(model, equalTo(model2));
+    }
 }

smithy-model/src/test/resources/software/amazon/smithy/model/loader/dedupe-models.smithy

@@ -0,0 +1,6 @@
+metadata abc = [1]
+
+namespace smithy.example
+
+@tags(["a", "b"])
+string MyString