Skip to content

Commit 7813acb

Browse files
committed
Add validator for service bound resoruce operations
This commit introduces a new validator that emits warnings when it detects operations that are bound directly to a service when a resource bound to that service may be a better match.
1 parent 75d5a75 commit 7813acb

File tree

4 files changed

+138
-0
lines changed

4 files changed

+138
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package software.amazon.smithy.model.validation.validators;
7+
8+
import static java.lang.String.format;
9+
10+
import java.util.ArrayList;
11+
import java.util.HashMap;
12+
import java.util.HashSet;
13+
import java.util.List;
14+
import java.util.Map;
15+
import java.util.Set;
16+
import software.amazon.smithy.model.Model;
17+
import software.amazon.smithy.model.knowledge.OperationIndex;
18+
import software.amazon.smithy.model.knowledge.TopDownIndex;
19+
import software.amazon.smithy.model.shapes.MemberShape;
20+
import software.amazon.smithy.model.shapes.OperationShape;
21+
import software.amazon.smithy.model.shapes.ResourceShape;
22+
import software.amazon.smithy.model.shapes.ServiceShape;
23+
import software.amazon.smithy.model.shapes.ShapeId;
24+
import software.amazon.smithy.model.traits.RequiredTrait;
25+
import software.amazon.smithy.model.validation.AbstractValidator;
26+
import software.amazon.smithy.model.validation.ValidationEvent;
27+
import software.amazon.smithy.model.validation.ValidationUtils;
28+
29+
/**
30+
* Validates if operations that are bound directly to a service may
31+
* be more accurately bound to a resource bound to the same service.
32+
*/
33+
public final class ServiceBoundResourceOperationValidator extends AbstractValidator {
34+
@Override
35+
public List<ValidationEvent> validate(Model model) {
36+
List<ValidationEvent> events = new ArrayList<>();
37+
OperationIndex operationIndex = OperationIndex.of(model);
38+
TopDownIndex topDownIndex = TopDownIndex.of(model);
39+
40+
// Check every service operation to see if it should be bound to a resource instead.
41+
for (ServiceShape service : model.getServiceShapes()) {
42+
// Store potential targets to emit one event per operation.
43+
Map<OperationShape, Set<ShapeId>> potentiallyBetterBindings = new HashMap<>();
44+
for (ShapeId operationId : service.getOperations()) {
45+
OperationShape operation = model.expectShape(operationId, OperationShape.class);
46+
// Check the resources of the containing service to test for input/output attachment.
47+
for (ResourceShape resource : topDownIndex.getContainedResources(service)) {
48+
// Check the operation members to see if they are implicit matches for resource identifiers.
49+
for (MemberShape member : operationIndex.getInputMembers(operation).values()) {
50+
if (isImplicitIdentifierBinding(member, resource)) {
51+
potentiallyBetterBindings.computeIfAbsent(operation, k -> new HashSet<>())
52+
.add(resource.getId());
53+
}
54+
}
55+
for (MemberShape member : operationIndex.getOutputMembers(operation).values()) {
56+
if (isImplicitIdentifierBinding(member, resource)) {
57+
potentiallyBetterBindings.computeIfAbsent(operation, k -> new HashSet<>())
58+
.add(resource.getId());
59+
}
60+
}
61+
}
62+
}
63+
64+
// Emit events per service that's present with a potentially bad binding.
65+
for (Map.Entry<OperationShape, Set<ShapeId>> entry : potentiallyBetterBindings.entrySet()) {
66+
events.add(warning(entry.getKey(), service, format(
67+
"The `%s` operation is bound to the `%s` service but has members that match identifiers "
68+
+ "of the following resource shapes: [%s]. It may be more accurately bound to one "
69+
+ "of them than directly to the service.",
70+
entry.getKey().getId(), service.getId(), ValidationUtils.tickedList(entry.getValue())),
71+
service.getId().toString(), entry.getKey().getId().getName()));
72+
}
73+
}
74+
75+
return events;
76+
}
77+
78+
private boolean isImplicitIdentifierBinding(MemberShape member, ResourceShape resource) {
79+
return resource.getIdentifiers().containsKey(member.getMemberName())
80+
&& member.getTrait(RequiredTrait.class).isPresent()
81+
&& member.getTarget().equals(resource.getIdentifiers().get(member.getMemberName()));
82+
}
83+
}

smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ software.amazon.smithy.model.validation.validators.ResourceIdentifierValidator
4040
software.amazon.smithy.model.validation.validators.ResourceLifecycleValidator
4141
software.amazon.smithy.model.validation.validators.ResourceOperationInputOutputValidator
4242
software.amazon.smithy.model.validation.validators.ServiceAuthDefinitionsValidator
43+
software.amazon.smithy.model.validation.validators.ServiceBoundResourceOperationValidator
4344
software.amazon.smithy.model.validation.validators.ServiceValidator
4445
software.amazon.smithy.model.validation.validators.SetValidator
4546
software.amazon.smithy.model.validation.validators.ShapeIdConflictValidator
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[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
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
$version: "2.0"
2+
3+
namespace smithy.example
4+
5+
service FooService {
6+
version: "2020-07-02"
7+
operations: [GetFoo]
8+
resources: [Foo, BoundNoMatch]
9+
}
10+
11+
resource Foo {
12+
identifiers: {
13+
fooId: String
14+
}
15+
create: CreateFoo
16+
}
17+
18+
operation CreateFoo {
19+
input := {
20+
@required
21+
something: String
22+
}
23+
output := {
24+
@required
25+
fooId: String
26+
27+
@required
28+
something: String
29+
}
30+
}
31+
32+
operation GetFoo {
33+
input := {
34+
@required
35+
fooId: String
36+
}
37+
output := {
38+
@required
39+
something: String
40+
}
41+
}
42+
43+
resource Unbound {
44+
identifiers: {
45+
id: String
46+
}
47+
}
48+
49+
resource BoundNoMatch {
50+
identifiers: {
51+
id: String
52+
}
53+
}

0 commit comments

Comments
 (0)