forked from smithy-lang/smithy
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Check references via idRef in UnreferencedShapes (smithy-lang#2105)
Applications of traits with `@idRef` members don't create relationships between shapes, so removing unreferenced shapes from the following: ``` service FooService { ... operations: [GetFoo] } operation GetFoo { output := { foo: Foo } } @trait @idref(failWhenMissing: true, selector: "*") string myIdRef @myIdRef(Referenced) structure Foo {} stucture Referenced {} ``` will remove `Referenced` from the model. This commit changes UnreferencedShapes so that `Referenced` is considered connected to `FooService` because it is referenced by an `@idRef` on a shape connected to the service. This commit adds a new kind of shape relationship, ID_REF, from a shape to any shapes referenced by `@idRef`'s within the shape's trait values. This relationship is created by doing the following: 1. Get all trait definition shapes. 2. Find all paths from those trait definition shapes to shapes with the idRef trait. Each path found says that applications of the trait definition may have a shape reference. 3. For each path, get all applications of the trait and search their node values using the path to the shape that had idRef. 4. Check the node value - if it is a shape id, create the ID_REF relationship between the shape that originally had the trait applied, and the found shape id. NeighborProvider was updated to allow wrapping an existing provider in a new one with ID_REF relationships, and UnreferencedShapes to use the wrapped provider. UnreferencedShapes will now traverse the ID_REF relationships, so any idRef'd shapes will be considered connected. Other changes: - A few test cases that expected to receive an UnreferencedShape event were updated - An `asShapeId` method was added to StringNode
- Loading branch information
1 parent
671066a
commit c0de593
Showing
16 changed files
with
919 additions
and
6 deletions.
There are no files selected for viewing
1 change: 0 additions & 1 deletion
1
...ware/amazon/smithy/aws/cloudformation/traits/errorfiles/additionalschemas-conflict.errors
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,2 @@ | ||
[ERROR] smithy.example#AdditionalSchemasConflictResource: The `bar` property of the generated `AdditionalSchemasConflictResource` CloudFormation resource targets multiple shapes: [smithy.api#Boolean, smithy.api#String]. Reusing member names that target different shapes can cause confusion for users of the API. This target discrepancy must either be resolved in the model or one of the members must be excluded from the conversion. | CfnResourceProperty | ||
[NOTE] smithy.example#AdditionalSchemasConflictProperties: The structure shape is not connected to from any service shape. | UnreferencedShape | ||
[WARNING] smithy.example#AdditionalSchemasConflictResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait |
1 change: 0 additions & 1 deletion
1
...oftware/amazon/smithy/aws/cloudformation/traits/errorfiles/deconflict-by-excluding.errors
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,2 @@ | ||
[NOTE] smithy.example#AdditionalSchemasDeconflictedProperties: The structure shape is not connected to from any service shape. | UnreferencedShape | ||
[WARNING] smithy.example#CreateAdditionalSchemasDeconflictedResourceRequest$bar: This shape applies a trait that is unstable: aws.cloudformation#cfnExcludeProperty | UnstableTrait | ||
[WARNING] smithy.example#AdditionalSchemasDeconflictedResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait |
1 change: 0 additions & 1 deletion
1
...mazon/smithy/aws/cloudformation/traits/errorfiles/invalid-additional-schemas-shape.errors
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,2 @@ | ||
[ERROR] smithy.example#InvalidAdditionalSchemasShapeResource: Error validating trait `aws.cloudformation#cfnResource`.additionalSchemas.0: Shape ID `smithy.example#ListShape` does not match selector `structure` | TraitValue | ||
[NOTE] smithy.example#ListShape: The list shape is not connected to from any service shape. | UnreferencedShape | ||
[WARNING] smithy.example#InvalidAdditionalSchemasShapeResource: This shape applies a trait that is unstable: aws.cloudformation#cfnResource | UnstableTrait |
108 changes: 108 additions & 0 deletions
108
...hy-model/src/main/java/software/amazon/smithy/model/neighbor/IdRefShapeRelationships.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
/* | ||
* Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.model.neighbor; | ||
|
||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import software.amazon.smithy.model.Model; | ||
import software.amazon.smithy.model.node.Node; | ||
import software.amazon.smithy.model.node.StringNode; | ||
import software.amazon.smithy.model.selector.PathFinder; | ||
import software.amazon.smithy.model.selector.Selector; | ||
import software.amazon.smithy.model.shapes.MemberShape; | ||
import software.amazon.smithy.model.shapes.Shape; | ||
import software.amazon.smithy.model.shapes.ShapeId; | ||
import software.amazon.smithy.model.traits.IdRefTrait; | ||
import software.amazon.smithy.model.traits.Trait; | ||
import software.amazon.smithy.model.traits.TraitDefinition; | ||
|
||
/** | ||
* Finds all {@link RelationshipType#ID_REF} relationships in a model. | ||
* | ||
* <p>This works by finding all paths from {@link TraitDefinition} shapes | ||
* to shapes with {@link IdRefTrait}, then using those paths to search | ||
* the node values of each application of the trait to extract the {@link ShapeId} | ||
* value. Because we don't have a fixed set of traits known to potentially have | ||
* idRef members, this has to be done dynamically. | ||
*/ | ||
final class IdRefShapeRelationships { | ||
private static final Selector WITH_ID_REF = Selector.parse("[trait|idRef]"); | ||
|
||
private final Model model; | ||
private final Map<ShapeId, Set<Relationship>> relationships = new HashMap<>(); | ||
|
||
IdRefShapeRelationships(Model model) { | ||
this.model = model; | ||
} | ||
|
||
Map<ShapeId, Set<Relationship>> getRelationships() { | ||
PathFinder finder = PathFinder.create(model); | ||
for (Shape traitDef : model.getShapesWithTrait(TraitDefinition.class)) { | ||
if (traitDef.hasTrait(IdRefTrait.class)) { | ||
// PathFinder doesn't handle the case where the trait def has the idRef | ||
NodeQuery query = new NodeQuery().self(); | ||
addRelationships(traitDef, query); | ||
continue; | ||
} | ||
List<PathFinder.Path> paths = finder.search(traitDef, WITH_ID_REF); | ||
if (!paths.isEmpty()) { | ||
for (PathFinder.Path path : paths) { | ||
NodeQuery query = buildNodeQuery(path); | ||
addRelationships(traitDef, query); | ||
} | ||
} | ||
} | ||
return relationships; | ||
} | ||
|
||
private void addRelationships(Shape traitDef, NodeQuery query) { | ||
model.getShapesWithTrait(traitDef.getId()).forEach(shape -> { | ||
Trait trait = shape.findTrait(traitDef.getId()).get(); // We already know the shape has the trait. | ||
Node node = trait.toNode(); | ||
// Invalid shape ids are handled by the idRef trait validator, so ignore them here. | ||
query.execute(node).forEach(n -> n.asStringNode() | ||
.flatMap(StringNode::asShapeId) | ||
.flatMap(model::getShape) | ||
.map(referenced -> Relationship.create(shape, RelationshipType.ID_REF, referenced)) | ||
.ifPresent(rel -> relationships | ||
.computeIfAbsent(rel.getShape().getId(), id -> new HashSet<>()).add(rel))); | ||
}); | ||
} | ||
|
||
private static NodeQuery buildNodeQuery(PathFinder.Path path) { | ||
NodeQuery query = new NodeQuery(); | ||
// The path goes from trait definition -> shape with idRef | ||
for (Relationship relationship : path) { | ||
if (!relationship.getNeighborShape().isPresent()) { | ||
break; | ||
} | ||
switch (relationship.getRelationshipType()) { | ||
case MAP_KEY: | ||
query.anyMemberName(); | ||
break; | ||
case MAP_VALUE: | ||
query.anyMember(); | ||
break; | ||
case LIST_MEMBER: | ||
case SET_MEMBER: | ||
query.anyElement(); | ||
break; | ||
case UNION_MEMBER: | ||
case STRUCTURE_MEMBER: | ||
MemberShape member = (MemberShape) relationship.getNeighborShape().get(); | ||
query.member(member.getMemberName()); | ||
break; | ||
default: | ||
// Other relationship types don't produce meaningful edges to search the node. | ||
break; | ||
} | ||
} | ||
return query; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
102 changes: 102 additions & 0 deletions
102
smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NodeQuery.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/* | ||
* Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.model.neighbor; | ||
|
||
import java.util.ArrayDeque; | ||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.Queue; | ||
import software.amazon.smithy.model.node.Node; | ||
|
||
/** | ||
* Searches {@link Node}s to find matching children. Each search | ||
* condition is executed on the result of the previous search, | ||
* and the results are aggregated. | ||
*/ | ||
final class NodeQuery { | ||
private static final Query SELF = (node, result) -> result.add(node); | ||
|
||
private static final Query ANY_MEMBER = (node, result) -> { | ||
if (node == null || !node.isObjectNode()) { | ||
return; | ||
} | ||
result.addAll(node.expectObjectNode().getMembers().values()); | ||
}; | ||
|
||
private static final Query ANY_ELEMENT = (node, result) -> { | ||
if (node == null || !node.isArrayNode()) { | ||
return; | ||
} | ||
result.addAll(node.expectArrayNode().getElements()); | ||
}; | ||
|
||
private static final Query ANY_MEMBER_NAME = (node, result) -> { | ||
if (node == null || !node.isObjectNode()) { | ||
return; | ||
} | ||
result.addAll(node.expectObjectNode().getMembers().keySet()); | ||
}; | ||
|
||
private final List<Query> queries = new ArrayList<>(); | ||
|
||
NodeQuery() { | ||
} | ||
|
||
NodeQuery self() { | ||
queries.add(SELF); | ||
return this; | ||
} | ||
|
||
NodeQuery member(String name) { | ||
queries.add((node, result) -> { | ||
if (node == null || !node.isObjectNode()) { | ||
return; | ||
} | ||
node.expectObjectNode().getMember(name).ifPresent(result::add); | ||
}); | ||
return this; | ||
} | ||
|
||
NodeQuery anyMember() { | ||
queries.add(ANY_MEMBER); | ||
return this; | ||
} | ||
|
||
NodeQuery anyElement() { | ||
queries.add(ANY_ELEMENT); | ||
return this; | ||
} | ||
|
||
NodeQuery anyMemberName() { | ||
queries.add(ANY_MEMBER_NAME); | ||
return this; | ||
} | ||
|
||
Collection<Node> execute(Node node) { | ||
Queue<Node> previousResult = new ArrayDeque<>(); | ||
|
||
if (queries.isEmpty()) { | ||
return previousResult; | ||
} | ||
|
||
previousResult.add(node); | ||
for (Query query : queries) { | ||
// Each time a query runs, it adds to the queue, but we only want it to | ||
// run on the nodes added by the previous query. | ||
for (int i = previousResult.size(); i > 0; i--) { | ||
query.run(previousResult.poll(), previousResult); | ||
} | ||
} | ||
|
||
return previousResult; | ||
} | ||
|
||
@FunctionalInterface | ||
interface Query { | ||
void run(Node node, Queue<Node> result); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.