Skip to content

Commit

Permalink
Fix issues validating rule/test params
Browse files Browse the repository at this point in the history
  • Loading branch information
kstich committed Sep 5, 2023
1 parent 980e30c commit 169ec5e
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public Provider() {

@Override
public Trait createTrait(ShapeId target, Node value) {
EndpointRuleSetTrait trait = builder()
EndpointRuleSetTrait trait = builder().sourceLocation(value)
.ruleSet(value)
.build();
trait.setNodeCache(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.SmithyUnstableApi;

Expand Down Expand Up @@ -51,7 +52,19 @@ public List<ValidationEvent> validate(Model model) {
if (operationNameMap.containsKey(operationName)) {
StructureShape inputShape = model.expectShape(
operationNameMap.get(operationName).getInputShape(), StructureShape.class);
events.addAll(validateOperationInput(model, serviceShape, inputShape, testOperationInput));

List<ValidationEvent> operationInputEvents = validateOperationInput(model, serviceShape,
inputShape, testOperationInput);

// Error test cases may use invalid inputs as the mechanism to trigger their error,
// so lower the severity before emitting.
if (testCase.getExpect().getError().isPresent()) {
for (ValidationEvent event : operationInputEvents) {
events.add(event.toBuilder().severity(Severity.WARNING).build());
}
} else {
events.addAll(operationInputEvents);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,18 @@ private List<ValidationEvent> validateParametersMatching(
List<ValidationEvent> errors = new ArrayList<>();
Set<String> matchedParams = new HashSet<>();
for (Parameter parameter : ruleSetParams) {
// Don't worry about checking built-in parameters.
String name = parameter.getName().toString();

// Don't worry about checking built-in based parameters.
if (parameter.isBuiltIn()) {
matchedParams.add(name);
continue;
}

String name = parameter.getName().toString();
if (!modelParams.containsKey(name)) {
errors.add(parameterError(serviceShape, parameter, "RuleSet.UnmatchedName",
String.format("Parameter `%s` exists in ruleset but not in service model", name)));
String.format("Parameter `%s` exists in ruleset but not in service model, existing params: %s",
name, String.join(", ", modelParams.keySet()))));
} else {
matchedParams.add(name);
if (parameter.getType() != modelParams.get(name).getType()) {
Expand All @@ -167,8 +170,10 @@ private List<ValidationEvent> validateParametersMatching(

for (Map.Entry<String, Parameter> entry : modelParams.entrySet()) {
if (!matchedParams.contains(entry.getKey())) {
errors.add(parameterError(serviceShape, entry.getValue(), "RuleSet.UnmatchedName",
String.format("Parameter `%s` exists in service model but not in ruleset", entry.getKey())));
errors.add(parameterError(serviceShape, serviceShape.expectTrait(EndpointRuleSetTrait.class),
"RuleSet.UnmatchedName",
String.format("Parameter `%s` exists in service model but not in ruleset, existing params: %s",
entry.getKey(), matchedParams)));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ use smithy.rules#staticContextParams
"ExtraParameter": {
"type": "string",
"documentation": "docs"
},
"ExtraBuiltIn": {
"type": "string",
"documentation": "docs",
"builtIn": "SDK::Endpoint"
}
},
"rules": []
})
@clientContextParams(
Region: {type: "string", documentation: "docs"}
Region: {type: "string", documentation: "docs"},
ExtraBuiltIn: {type: "string", documentation: "docs"}
)
service FizzBuzz {
operations: [GetResource, GetAnotherResource]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[WARNING] smithy.example#InvalidService: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait
[ERROR] smithy.example#InvalidService: The operationInput value for an endpoint test does not match the operation's input shape: Missing required structure member `fizz` for `smithy.example#GetThingInput` | EndpointTestsTrait
[WARNING] smithy.example#InvalidService: The operationInput value for an endpoint test does not match the operation's input shape: Missing required structure member `fizz` for `smithy.example#GetThingInput` | EndpointTestsTrait

0 comments on commit 169ec5e

Please sign in to comment.