Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add httpQueryParams trait #735

Merged
merged 22 commits into from
Mar 15, 2021
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Improve HttpQueryParams validator message
Chase Coalwell committed Mar 11, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 7004468b1b661df70d7f7b592fa424caf68dde37
Original file line number Diff line number Diff line change
@@ -21,10 +21,13 @@
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.traits.HttpQueryParamsTrait;
import software.amazon.smithy.model.traits.HttpQueryTrait;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidationUtils;

/**
* When the `httpQueryParams` trait is used, this validator emits a NOTE when another member of the container shape
@@ -47,15 +50,31 @@ private List<ValidationEvent> validateQueryTraitUsage(Model model) {
shape.asMemberShape().flatMap(member -> model.getShape(member.getContainer())
.flatMap(Shape::asStructureShape))
.ifPresent(structure -> {
for (MemberShape memberShape : structure.members()) {
if (memberShape.hasTrait(HttpQueryTrait.class)) {
events.add(note(shape, String.format("Trait `httpQueryParams` may potentially conflict with"
+ " `httpQuery` trait applied to `%s`.", memberShape.toShapeId())));
}
// Gather the names of member shapes, as strings, that apply HttpQuery traits
List<String> queryShapes = getMembersWithTrait(structure, HttpQueryTrait.class);
if (queryShapes.size() > 0) {
events.add(createNote(structure, shape.toShapeId().getMember().get(), queryShapes));
}
});
}

return events;
}

private List<String> getMembersWithTrait(StructureShape structure, Class<? extends Trait> trait) {
List<String> members = new ArrayList<>();
for (MemberShape member : structure.members()) {
if (member.hasTrait(trait)) {
members.add(member.getMemberName());
}
}
return members;
}

private ValidationEvent createNote(Shape target, String queryParamsShape, List<String> queryShapes) {
return note(target, String.format("Operation input `%s` has an `httpQueryParams` trait applied to the `%s` "
+ "member and `httpQuery` traits applied to the following members: %s. This can cause confusion when "
+ "keys from the `httpQueryParams` trait conflict with those defined directly by `httpQuery` traits",
target.toShapeId(), queryParamsShape, ValidationUtils.tickedList(queryShapes)));
}
}
Original file line number Diff line number Diff line change
@@ -29,4 +29,4 @@
[ERROR] ns.foo#HInput$a: Trait `httpHeader` cannot be applied to `ns.foo#HInput$a`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, collection > member > :test(boolean, number, string, timestamp))) | TraitTarget
[ERROR] ns.foo#EInput$label2: Trait `httpLabel` cannot be applied to `ns.foo#EInput$label2`. This trait may only be applied to shapes that match the following selector: structure > member[trait|required] :test(> :test(string, number, boolean, timestamp)) | TraitTarget
[ERROR] ns.foo#OInput$a: Trait `httpLabel` cannot be applied to `ns.foo#OInput$a`. This trait may only be applied to shapes that match the following selector: structure > member[trait|required] :test(> :test(string, number, boolean, timestamp)) | TraitTarget
[NOTE] ns.foo#QueryParamsInput$queryParams: Trait `httpQueryParams` may potentially conflict with `httpQuery` trait applied to `ns.foo#QueryParamsInput$namedQuery`. | HttpQueryParamsTrait
[NOTE] ns.foo#QueryParamsInput: Operation input `ns.foo#QueryParamsInput` has an `httpQueryParams` trait applied to the `queryParams` member and `httpQuery` traits applied to the following members: `namedQuery`, `otherNamedQuery`. This can cause confusion when keys from the `httpQueryParams` trait conflict with those defined directly by `httpQuery` traits | HttpQueryParamsTrait
Original file line number Diff line number Diff line change
@@ -754,6 +754,12 @@
"smithy.api#httpQuery": "named"
}
},
"otherNamedQuery": {
"target": "ns.foo#String",
"traits": {
"smithy.api#httpQuery": "otherNamed"
}
},
"queryParams": {
"target": "ns.foo#MapOfString",
"traits": {