-
Notifications
You must be signed in to change notification settings - Fork 218
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add validator for identifiers missing references
This commit introduces a new validator that emits warnings when it detects a member that matches a resource identifier without the relevant `@references` trait configuration.
- Loading branch information
Showing
8 changed files
with
185 additions
and
6 deletions.
There are no files selected for viewing
137 changes: 137 additions & 0 deletions
137
...are/amazon/smithy/model/validation/validators/MemberShouldReferenceResourceValidator.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,137 @@ | ||
| /* | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package software.amazon.smithy.model.validation.validators; | ||
|
|
||
| import static java.lang.String.format; | ||
|
|
||
| import java.util.ArrayList; | ||
| 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.selector.PathFinder; | ||
| import software.amazon.smithy.model.shapes.MemberShape; | ||
| import software.amazon.smithy.model.shapes.ResourceShape; | ||
| import software.amazon.smithy.model.shapes.Shape; | ||
| import software.amazon.smithy.model.shapes.ShapeId; | ||
| import software.amazon.smithy.model.traits.ReferencesTrait; | ||
| import software.amazon.smithy.model.validation.AbstractValidator; | ||
| import software.amazon.smithy.model.validation.ValidationEvent; | ||
| import software.amazon.smithy.model.validation.ValidationUtils; | ||
| import software.amazon.smithy.utils.ListUtils; | ||
|
|
||
| /** | ||
| * Validates if a member matches a resource identifier without the | ||
| * proper configuration of a `@references` trait. | ||
| */ | ||
| public final class MemberShouldReferenceResourceValidator extends AbstractValidator { | ||
| @Override | ||
| public List<ValidationEvent> validate(Model model) { | ||
| // There are usually far fewer resources than members, precompute the identifiers | ||
| // so various short circuits can be added. | ||
| Set<String> identifierNames = getAllIdentifierNames(model); | ||
| // Short circuit validating all the members if we don't have any resources to test. | ||
| if (identifierNames.isEmpty()) { | ||
| return ListUtils.of(); | ||
| } | ||
|
|
||
| // Check every member to see if it's a potential reference. | ||
| List<ValidationEvent> events = new ArrayList<>(); | ||
| for (MemberShape member : model.getMemberShapes()) { | ||
| // Only the known identifier names can match for this, skip names that we don't know. | ||
| if (!identifierNames.contains(member.getMemberName())) { | ||
| continue; | ||
| } | ||
| // Only strings can be identifiers, so skip non-String targets. | ||
| if (!model.expectShape(member.getTarget()).isStringShape()) { | ||
| continue; | ||
| } | ||
|
|
||
| Set<ShapeId> potentialReferences = computePotentialReferences(model, member); | ||
| if (!potentialReferences.isEmpty()) { | ||
| events.add(warning(member, format("This member appears to reference the following resources without " | ||
| + "being included in a `@references` trait: [%s]", | ||
| ValidationUtils.tickedList(potentialReferences)))); | ||
| } | ||
| } | ||
|
|
||
| return events; | ||
| } | ||
|
|
||
| private Set<String> getAllIdentifierNames(Model model) { | ||
| Set<String> identifierNames = new HashSet<>(); | ||
| for (ResourceShape resource : model.getResourceShapes()) { | ||
| identifierNames.addAll(resource.getIdentifiers().keySet()); | ||
| } | ||
| return identifierNames; | ||
| } | ||
|
|
||
| private Set<ShapeId> computePotentialReferences(Model model, MemberShape member) { | ||
| // Exclude any resources already in `@references` on the member or container structure. | ||
| Set<ShapeId> resourcesToIgnore = new HashSet<>(); | ||
| ignoreReferencedResources(member, resourcesToIgnore); | ||
| ignoreReferencedResources(model.expectShape(member.getContainer()), resourcesToIgnore); | ||
|
|
||
| // Check each resource in the model for something missed. | ||
| Set<ShapeId> potentialResources = new HashSet<>(); | ||
| for (ResourceShape resource : model.getResourceShapes()) { | ||
| // We'll want to ignore some resources based on the member -> resource path. | ||
| computeResourcesToIgnore(model, member, resource, resourcesToIgnore); | ||
|
|
||
| // Exclude members bound to resource hierarchies from generating events, | ||
| // including for resources that are within the same hierarchy. | ||
| if (resourcesToIgnore.contains(resource.getId())) { | ||
| continue; | ||
| } | ||
|
|
||
| // This member matches the identifier for the resource we're checking, add it to a list. | ||
| if (isIdentifierMatch(resource, member)) { | ||
| potentialResources.add(resource.getId()); | ||
| } | ||
| } | ||
|
|
||
| // Clean up any resources added through other paths that should be ignored. | ||
| potentialResources.removeAll(resourcesToIgnore); | ||
| return potentialResources; | ||
| } | ||
|
|
||
| private void computeResourcesToIgnore(Model model, MemberShape member, ResourceShape resource, | ||
| Set<ShapeId> resourcesToIgnore) { | ||
| // Exclude actually bound members via searching with a PathFinder. | ||
| List<PathFinder.Path> resourceMemberPaths = PathFinder.create(model) | ||
| .search(resource, ListUtils.of(member)); | ||
| if (!resourceMemberPaths.isEmpty()) { | ||
| // This member is already bound to a resource, so we don't need a references trait for it. | ||
| // In addition, we should not tell users to add a references trait for other resources that | ||
| // are children in that hierarchy - any parent resources or other children of those parents. | ||
| for (PathFinder.Path path : resourceMemberPaths) { | ||
| for (Shape pathShape : path.getShapes()) { | ||
| if (pathShape.isResourceShape()) { | ||
| ResourceShape resourceShape = (ResourceShape) pathShape; | ||
| resourcesToIgnore.add(resourceShape.getId()); | ||
| resourcesToIgnore.addAll(resourceShape.getResources()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private void ignoreReferencedResources(Shape shape, Set<ShapeId> resourcesToIgnore) { | ||
| if (shape.hasTrait(ReferencesTrait.class)) { | ||
| for (ReferencesTrait.Reference reference : shape.expectTrait(ReferencesTrait.class) | ||
| .getReferences()) { | ||
| resourcesToIgnore.add(reference.getResource()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private boolean isIdentifierMatch(ResourceShape resource, MemberShape member) { | ||
| Map<String, ShapeId> identifiers = resource.getIdentifiers(); | ||
| return identifiers.containsKey(member.getMemberName()) | ||
| && identifiers.get(member.getMemberName()).equals(member.getTarget()); | ||
| } | ||
| } |
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
1 change: 1 addition & 0 deletions
1
...are/amazon/smithy/model/errorfiles/validators/references/missing-member-references.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 |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| [WARNING] ns.foo#Operation1Input$fooId: This member appears to reference the following resources without being included in a `@references` trait: [`ns.foo#FooResource`] | MemberShouldReferenceResource |
39 changes: 39 additions & 0 deletions
39
...are/amazon/smithy/model/errorfiles/validators/references/missing-member-references.smithy
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,39 @@ | ||
| $version: "2.0" | ||
|
|
||
| namespace ns.foo | ||
|
|
||
| resource FooResource { | ||
| identifiers: { | ||
| fooId: String | ||
| } | ||
| } | ||
|
|
||
| operation Operation1 { | ||
| input := { | ||
| fooId: String | ||
| } | ||
| } | ||
|
|
||
| operation Operation2 { | ||
| input := @references([{ | ||
| resource: FooResource | ||
| }]) { | ||
| fooId: String | ||
| } | ||
| } | ||
|
|
||
| resource BarResource { | ||
| identifiers: { | ||
| barId: BarId | ||
| } | ||
| } | ||
|
|
||
| operation Operation3 { | ||
| input := @references([{ | ||
| resource: BarResource | ||
| }]) { | ||
| barId: BarId | ||
| } | ||
| } | ||
|
|
||
| string BarId |
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
1 change: 1 addition & 0 deletions
1
...mithy/model/errorfiles/validators/resource-identifiers/detects-misbound-operations.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 +1,2 @@ | ||
| [WARNING] smithy.example#GetFoo: The `smithy.example#GetFoo` operation is bound to the `smithy.example#FooService` service but has members that match identifiers of the following resource shapes: [`smithy.example#Foo`]. It may be more accurately bound to one of them than directly to the service. | ServiceBoundResourceOperation.smithy.example#FooService.GetFoo | ||
| [WARNING] smithy.example#GetFooInput$fooId: This member appears to reference the following resources without being included in a `@references` trait: [`smithy.example#Foo`] | MemberShouldReferenceResource |
2 changes: 1 addition & 1 deletion
2
...are/amazon/smithy/model/errorfiles/validators/resource-operation-binding-validator.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,2 +1,2 @@ | ||
| [ERROR] ns.foo#InvalidResourceOperation1: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `foo` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding | ||
| [ERROR] ns.foo#InvalidResourceOperation1: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `bar` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding | ||
| [ERROR] ns.foo#ValidResourceOperation2: This operation does not form a valid instance operation when bound to resource `ns.foo#ValidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `foo` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding |
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