Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public void testShapesGenerationWithoutOrder() {
runner.run();

assertThat(testDirected.generatedShapes,
contains(
containsInAnyOrder(
ShapeId.from("smithy.example#FooOperation"),
ShapeId.from("smithy.example#FooOperationOutput"),
ShapeId.from("smithy.example#A"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.neighbor.NeighborProvider;
Expand Down Expand Up @@ -56,8 +56,8 @@ public static TopDownIndex of(Model model) {
}

private void findContained(ShapeId container, Collection<Shape> shapes) {
Set<ResourceShape> containedResources = new TreeSet<>();
Set<OperationShape> containedOperations = new TreeSet<>();
Set<ResourceShape> containedResources = new LinkedHashSet<>();
Set<OperationShape> containedOperations = new LinkedHashSet<>();

for (Shape shape : shapes) {
if (!shape.getId().equals(container)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,15 @@ public boolean hasNext() {
while (!stack.isEmpty()) {
// Every relationship is returned, even if the same shape is pointed
// to multiple times from a single shape.
Relationship relationship = stack.pop();

// Use removeLast to retrieve relationships in their defined order rather
// than the reverse of the defined order. Note that this only preserves
// the order of a particular relationship type, not the order of all
// relationship types. So a resources `operation` relationships will be
// resolved in the order of that list, but the resources's `resource`
// relationships will nevertheless always appear first because that is
// simply the order that the NeighborVisitor checks them in.
Relationship relationship = stack.removeLast();

// Only traverse this relationship if the shape it points to hasn't
// already been traversed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
import static org.hamcrest.Matchers.containsInAnyOrder;
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.OperationShape;
import software.amazon.smithy.model.shapes.ResourceShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;

public class TopDownIndexTest {
Expand Down Expand Up @@ -67,4 +70,59 @@ public void findsAllChildren() {

assertThat(childIndex.getContainedResources(ShapeId.from("ns.foo#NotThere")), empty());
}

@Test
public void preservesModeledOrder() {
Model model = Model.assembler()
.addImport(TopDownIndexTest.class.getResource("top-down-order.smithy"))
.assemble()
.unwrap();

TopDownIndex index = TopDownIndex.of(model);
List<ShapeId> serviceOperations = index
.getContainedOperations(ShapeId.from("com.example#Service"))
.stream()
.map(Shape::toShapeId)
.collect(Collectors.toList());
assertThat(serviceOperations,
contains(
ShapeId.from("com.example#OperationB"),
ShapeId.from("com.example#OperationA"),
ShapeId.from("com.example#OperationC"),
ShapeId.from("com.example#OperationD"),
ShapeId.from("com.example#OperationO"),
ShapeId.from("com.example#OperationG")));

List<ShapeId> resourceOperations = index
.getContainedOperations(ShapeId.from("com.example#ResourceA"))
.stream()
.map(Shape::toShapeId)
.collect(Collectors.toList());
assertThat(resourceOperations,
contains(
ShapeId.from("com.example#OperationD"),
ShapeId.from("com.example#OperationO"),
ShapeId.from("com.example#OperationG")));

List<ShapeId> serviceResources = index
.getContainedResources(ShapeId.from("com.example#Service"))
.stream()
.map(Shape::toShapeId)
.collect(Collectors.toList());
assertThat(serviceResources,
contains(
ShapeId.from("com.example#ResourceA"),
ShapeId.from("com.example#ResourceC"),
ShapeId.from("com.example#ResourceB")));

List<ShapeId> resourceResources = index
.getContainedResources(ShapeId.from("com.example#ResourceA"))
.stream()
.map(Shape::toShapeId)
.collect(Collectors.toList());
assertThat(resourceResources,
contains(
ShapeId.from("com.example#ResourceC"),
ShapeId.from("com.example#ResourceB")));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
$version: "2.0"

namespace com.example


service Service {
operations: [
OperationB
OperationA
OperationC
]
resources: [
ResourceA
]
}

operation OperationB { }

operation OperationC {}

operation OperationA {}

resource ResourceA {
operations: [
OperationD
OperationO
OperationG
]
resources: [
ResourceC
ResourceB
]
}

resource ResourceB {}

resource ResourceC {}

operation OperationD {}

operation OperationG {}

operation OperationO {}
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,18 @@ public List<ValidationEvent> validate(Model model) {

List<ValidationEvent> errors = new ArrayList<>();
for (ServiceShape serviceShape : model.getServiceShapesWithTrait(EndpointRuleSetTrait.class)) {
// This validator is sensitive to order, and with the change to topdownindex returning
// things in modeled order, the actual errors returned changed. So This reverses that order
// to side-step the issue.
// TODO: Make the validator not sensitive to order
List<OperationShape> operations = new ArrayList<>(topDownIndex.getContainedOperations(serviceShape));
operations = operations.reversed();
// Pull all the parameters used in this service related to endpoints, validating that
// they are of matching types across the traits that can define them.
Pair<List<ValidationEvent>, Map<String, Parameter>> errorsParamsPair = validateAndExtractParameters(
model,
serviceShape,
topDownIndex.getContainedOperations(serviceShape));
operations);
errors.addAll(errorsParamsPair.getLeft());

// Make sure parameters align across Params <-> RuleSet transitions.
Expand All @@ -79,7 +85,7 @@ public List<ValidationEvent> validate(Model model) {
private Pair<List<ValidationEvent>, Map<String, Parameter>> validateAndExtractParameters(
Model model,
ServiceShape serviceShape,
Set<OperationShape> containedOperations
List<OperationShape> containedOperations
) {
List<ValidationEvent> errors = new ArrayList<>();
Map<String, Parameter> endpointParams = new HashMap<>();
Expand Down
Loading