Repository: smithy-lang/smithy
Pull request #2412 (2024-10-01 22:10:37)

Fix JSON Schema pointer conflicts with trait definitions
This commit updates the DeconflictingStrategy to scrub trait definitions from the model used to pre-compute JSON Schema pointers. These shapes wouldn't have made their way into converted schemas anyways, and existing behavior for non-computed pointers is maintained.

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
index ec4cf34c5..09f84243f 100644
--- a/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
+++ b/smithy-jsonschema/src/main/java/software/amazon/smithy/jsonschema/DeconflictingStrategy.java
@@ -29,6 +29,7 @@ import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.SimpleShape;
 import software.amazon.smithy.model.traits.EnumTrait;
 import software.amazon.smithy.model.traits.PrivateTrait;
+import software.amazon.smithy.model.transform.ModelTransformer;
 import software.amazon.smithy.utils.FunctionalUtils;
 import software.amazon.smithy.utils.StringUtils;
 
@@ -56,8 +57,14 @@ final class DeconflictingStrategy implements RefStrategy {
         this.delegate = delegate;
 
         // Pre-compute a map of all converted shape refs. Sort the shapes
-        // to make the result deterministic.
-        model.shapes().filter(shapePredicate.and(FunctionalUtils.not(this::isIgnoredShape))).sorted().forEach(shape -> {
+        // to make the result deterministic. Eliminate trait definitions
+        // ahead of that computation to eliminate potential conflicts with
+        // shapes that won't go into JSON Schema anyway.
+        Model scrubbedModel = ModelTransformer.create().scrubTraitDefinitions(model);
+        scrubbedModel.shapes()
+                .filter(shapePredicate.and(FunctionalUtils.not(this::isIgnoredShape)))
+                .sorted()
+                .forEach(shape -> {
             String pointer = delegate.toPointer(shape.getId());
             if (!reversePointers.containsKey(pointer)) {
                 pointers.put(shape.getId(), pointer);


Test patch:
diff --git a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/DeconflictingStrategyTest.java b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/DeconflictingStrategyTest.java
index 2927c9be2..a870b9b70 100644
--- a/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/DeconflictingStrategyTest.java
+++ b/smithy-jsonschema/src/test/java/software/amazon/smithy/jsonschema/DeconflictingStrategyTest.java
@@ -13,6 +13,8 @@ import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StringShape;
 import software.amazon.smithy.model.shapes.StructureShape;
+import software.amazon.smithy.model.traits.PrivateTrait;
+import software.amazon.smithy.model.traits.TraitDefinition;
 
 public class DeconflictingStrategyTest {
     @Test
@@ -97,4 +99,27 @@ public class DeconflictingStrategyTest {
             .createDefaultStrategy(model, new JsonSchemaConfig(), propertyNamingStrategy, alwaysTrue());
         assertThat(strategy.toPointer(a.getId()), equalTo("#/definitions/Severity"));
     }
+
+    @Test
+    public void excludesTraitDefinitions() {
+        StringShape member = StringShape.builder().id("com.foo#String").build();
+        StructureShape matcher = StructureShape.builder()
+                .id("com.foo#Matcher")
+                .addMember("member", member.getId())
+                .build();
+        StructureShape matcherForTrait = StructureShape.builder()
+                .id("com.bar#Matcher")
+                .addTrait(new PrivateTrait())
+                .build();
+        StructureShape trait = StructureShape.builder()
+                .id("com.bar#Trait")
+                .addTrait(TraitDefinition.builder().build())
+                .addMember("matcher", matcherForTrait.toShapeId())
+                .build();
+        Model model = Model.assembler().addShapes(trait, matcherForTrait, matcher, member).assemble().unwrap();
+        PropertyNamingStrategy propertyNamingStrategy = PropertyNamingStrategy.createDefaultStrategy();
+        RefStrategy strategy = RefStrategy
+                .createDefaultStrategy(model, new JsonSchemaConfig(), propertyNamingStrategy, alwaysTrue());
+        assertThat(strategy.toPointer(matcher.getId()), equalTo("#/definitions/Matcher"));
+    }
 }