diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java index 5097a633490..8108ac92685 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java @@ -16,6 +16,7 @@ package software.amazon.smithy.model.neighbor; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.EntityShape; @@ -33,7 +34,6 @@ import software.amazon.smithy.model.shapes.ShapeVisitor; import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.shapes.UnionShape; -import software.amazon.smithy.utils.ListUtils; /** * Finds all neighbors of a shape, returning them as a list of @@ -59,12 +59,32 @@ public List getNeighbors(Shape shape) { @Override public List getDefault(Shape shape) { - return ListUtils.of(); + return shape.getMixins().isEmpty() + ? Collections.emptyList() + : initializeRelationships(shape, 0); + } + + private List initializeRelationships(Shape shape, int knownMemberCount) { + if (shape.isMemberShape()) { + // Members have mixins but shouldn't contribute a relationship. + return new ArrayList<>(knownMemberCount); + } else { + knownMemberCount += shape.getMixins().size(); + List result = new ArrayList<>(knownMemberCount); + for (ShapeId mixin : shape.getMixins()) { + result.add(relationship(shape, RelationshipType.MIXIN, mixin)); + } + return result; + } } @Override public List serviceShape(ServiceShape shape) { - List result = new ArrayList<>(); + int operationAndResourceRelationships = 2 * (shape.getAllOperations().size() + shape.getResources().size()); + int errorSize = shape.getErrors().size(); + int neededSize = operationAndResourceRelationships + errorSize; + List result = initializeRelationships(shape, neededSize); + // Add OPERATION from service -> operation. Add BINDING from operation -> service. for (ShapeId operation : shape.getOperations()) { addBinding(result, shape, operation, RelationshipType.OPERATION); @@ -92,7 +112,12 @@ private void addBound(List result, Shape container, ShapeId bindin @Override public List resourceShape(ResourceShape shape) { - List result = new ArrayList<>(); + // This is a rough estimate for the number of needed relationships. + int operationAndResourceRelationships = 2 * (shape.getAllOperations().size() + shape.getResources().size()); + int identifierSize = shape.getIdentifiers().size(); + int neededSize = (operationAndResourceRelationships * 2) + identifierSize; + List result = initializeRelationships(shape, neededSize); + // Add IDENTIFIER relationships. shape.getIdentifiers().forEach((k, v) -> result.add(relationship(shape, RelationshipType.IDENTIFIER, v))); // Add RESOURCE from resourceA -> resourceB and BOUND from resourceB -> resourceA @@ -154,10 +179,22 @@ private void addServiceAndResourceBindings(List result, ResourceSh @Override public List operationShape(OperationShape shape) { - List result = new ArrayList<>(); + ShapeId input = shape.getInput().orElse(null); + ShapeId output = shape.getOutput().orElse(null); + + // Calculate the number of relationships up front. + int assumedRelationshipCount = shape.getErrors().size() + + (input == null ? 0 : 1) + + (output == null ? 0 : 1); + List result = initializeRelationships(shape, assumedRelationshipCount); - shape.getInput().ifPresent(input -> result.add(relationship(shape, RelationshipType.INPUT, input))); - shape.getOutput().ifPresent(output -> result.add(relationship(shape, RelationshipType.OUTPUT, output))); + if (input != null) { + result.add(relationship(shape, RelationshipType.INPUT, input)); + } + + if (output != null) { + result.add(relationship(shape, RelationshipType.OUTPUT, output)); + } for (ShapeId errorId : shape.getErrors()) { result.add(relationship(shape, RelationshipType.ERROR, errorId)); @@ -167,7 +204,7 @@ public List operationShape(OperationShape shape) { @Override public List memberShape(MemberShape shape) { - List result = new ArrayList<>(2); + List result = initializeRelationships(shape, 2); result.add(relationship(shape, RelationshipType.MEMBER_CONTAINER, shape.getContainer())); result.add(relationship(shape, RelationshipType.MEMBER_TARGET, shape.getTarget())); return result; @@ -175,7 +212,7 @@ public List memberShape(MemberShape shape) { @Override public List enumShape(EnumShape shape) { - List result = new ArrayList<>(); + List result = initializeRelationships(shape, shape.getAllMembers().size()); for (MemberShape member : shape.getAllMembers().values()) { result.add(relationship(shape, RelationshipType.ENUM_MEMBER, member)); } @@ -184,7 +221,7 @@ public List enumShape(EnumShape shape) { @Override public List intEnumShape(IntEnumShape shape) { - List result = new ArrayList<>(); + List result = initializeRelationships(shape, shape.getAllMembers().size()); for (MemberShape member : shape.getAllMembers().values()) { result.add(relationship(shape, RelationshipType.INT_ENUM_MEMBER, member)); } @@ -193,17 +230,21 @@ public List intEnumShape(IntEnumShape shape) { @Override public List listShape(ListShape shape) { - return ListUtils.of(relationship(shape, RelationshipType.LIST_MEMBER, shape.getMember())); + List result = initializeRelationships(shape, 1); + result.add(relationship(shape, RelationshipType.LIST_MEMBER, shape.getMember())); + return result; } @Override public List setShape(SetShape shape) { - return ListUtils.of(relationship(shape, RelationshipType.SET_MEMBER, shape.getMember())); + List result = initializeRelationships(shape, 1); + result.add(relationship(shape, RelationshipType.SET_MEMBER, shape.getMember())); + return result; } @Override public List mapShape(MapShape shape) { - List result = new ArrayList<>(2); + List result = initializeRelationships(shape, 2); result.add(relationship(shape, RelationshipType.MAP_KEY, shape.getKey())); result.add(relationship(shape, RelationshipType.MAP_VALUE, shape.getValue())); return result; @@ -211,10 +252,7 @@ public List mapShape(MapShape shape) { @Override public List structureShape(StructureShape shape) { - List result = new ArrayList<>(); - for (ShapeId mixin : shape.getMixins()) { - result.add(relationship(shape, RelationshipType.MIXIN, mixin)); - } + List result = initializeRelationships(shape, shape.getAllMembers().size()); for (MemberShape member : shape.getAllMembers().values()) { result.add(Relationship.create(shape, RelationshipType.STRUCTURE_MEMBER, member)); } @@ -223,10 +261,7 @@ public List structureShape(StructureShape shape) { @Override public List unionShape(UnionShape shape) { - List result = new ArrayList<>(); - for (ShapeId mixin : shape.getMixins()) { - result.add(relationship(shape, RelationshipType.MIXIN, mixin)); - } + List result = initializeRelationships(shape, shape.getAllMembers().size()); for (MemberShape member : shape.getAllMembers().values()) { result.add(Relationship.create(shape, RelationshipType.UNION_MEMBER, member)); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java index 156db099dd8..0fe1ac4b96e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java @@ -102,6 +102,7 @@ private Optional validateTarget(Model model, Shape shape, Shape return Optional.of(error(shape, format( "Members cannot target %s shapes, but found %s", target.getType(), target))); } + return Optional.empty(); case MAP_KEY: return target.asMemberShape().flatMap(m -> validateMapKey(shape, m.getTarget(), model)); case RESOURCE: diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java index 4ecaccfbcf5..f34f99fa8b3 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java @@ -21,6 +21,7 @@ import static org.hamcrest.Matchers.empty; import java.util.List; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.BlobShape; @@ -58,6 +59,26 @@ public void blobShape() { assertThat(relationships, empty()); } + @Test + public void findsMixinsOnThingsOtherThanStructAndUnion() { + Shape blobMixin = BlobShape.builder().id("smithy.example#BlobMixin") + .addTrait(MixinTrait.builder().build()) + .build(); + Shape shape = BlobShape.builder() + .id("ns.foo#name") + .addMixin(blobMixin) + .build(); + Model model = Model.builder().addShapes(shape, blobMixin).build(); + NeighborVisitor neighborVisitor = new NeighborVisitor(model); + List relationships = shape.accept(neighborVisitor); + + List types = relationships.stream() + .map(Relationship::getRelationshipType) + .collect(Collectors.toList()); + + assertThat(types, contains(RelationshipType.MIXIN)); + } + @Test public void booleanShape() { Shape shape = BooleanShape.builder().id("ns.foo#name").build();