Skip to content

Commit

Permalink
Fix JSON Schema pointer conflicts with trait definitions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kstich committed Oct 1, 2024
1 parent 193a262 commit 864dfd7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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;

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
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
Expand Down Expand Up @@ -97,4 +99,27 @@ public void excludesPrivatePreludeShapes() {
.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"));
}
}

0 comments on commit 864dfd7

Please sign in to comment.