Skip to content

Commit

Permalink
Validate that query literal do not conflict with query params (#1786)
Browse files Browse the repository at this point in the history
* Validate that query literal do not conflict with query params

* Address PR comments
  • Loading branch information
sugmanue authored and mtdowling committed Jun 2, 2023
1 parent 29ee927 commit b7e600a
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,5 @@ public boolean equals(Object other) {
public int hashCode() {
return super.hashCode() + queryLiterals.hashCode();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@
import java.util.Map;
import java.util.Set;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.OperationIndex;
import software.amazon.smithy.model.pattern.UriPattern;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.traits.HttpQueryTrait;
import software.amazon.smithy.model.traits.HttpTrait;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidationUtils;
Expand All @@ -41,7 +45,7 @@ public List<ValidationEvent> validate(Model model) {
if (!model.isTraitApplied(HttpQueryTrait.class)) {
return Collections.emptyList();
} else {
return validateBindings(getQueryBindings(model));
return validateBindings(getQueryBindings(model), getStructureToOperations(model));
}
}

Expand All @@ -64,7 +68,10 @@ private Map<StructureShape, Map<String, Set<String>>> getQueryBindings(Model mod
return queryBindings;
}

private List<ValidationEvent> validateBindings(Map<StructureShape, Map<String, Set<String>>> queryBindings) {
private List<ValidationEvent> validateBindings(
Map<StructureShape, Map<String, Set<String>>> queryBindings,
Map<StructureShape, List<OperationShape>> structureToOperations
) {
List<ValidationEvent> events = new ArrayList<>();

for (Map.Entry<StructureShape, Map<String, Set<String>>> entry : queryBindings.entrySet()) {
Expand All @@ -77,8 +84,34 @@ private List<ValidationEvent> validateBindings(Map<StructureShape, Map<String, S
paramsToMembers.getKey(), ValidationUtils.tickedList(paramsToMembers.getValue()))));
}
}

List<OperationShape> operations = structureToOperations.getOrDefault(entry.getKey(),
Collections.emptyList());
for (OperationShape operation : operations) {
UriPattern pattern = operation.expectTrait(HttpTrait.class).getUri();
for (Map.Entry<String, String> literalEntry : pattern.getQueryLiterals().entrySet()) {
String literalKey = literalEntry.getKey();
if (entry.getValue().containsKey(literalKey)) {
events.add(error(entry.getKey(), String.format(
"`httpQuery` name `%s` conflicts with the `http` trait of the `%s` operation: `%s`",
literalKey, operation.getId(), pattern)));
}
}
}
}

return events;
}

private Map<StructureShape, List<OperationShape>> getStructureToOperations(Model model) {
OperationIndex index = OperationIndex.of(model);
Map<StructureShape, List<OperationShape>> structureToOperations = new HashMap<>();
for (OperationShape operation : model.getOperationShapesWithTrait(HttpTrait.class)) {
index.getInput(operation)
.ifPresent(structure -> structureToOperations
.computeIfAbsent(structure, key -> new ArrayList<>())
.add(operation));
}
return structureToOperations;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationOne` operation: `/example?code` | HttpQueryTrait
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
$version: "2"
namespace smithy.example

// OperationOne defines a query literal `code` that conflicts with the
// query param defined in the input structure.

service SmithyExample {
operations: [
OperationOne
]
}

@http(code: 200, method: "GET", uri: "/example?code")
@readonly
operation OperationOne {
input: OperationOneInput
output: OperationOneOutput
}

structure OperationOneInput {
@httpQuery("code")
code: String

@httpQuery("state")
state: String
}

structure OperationOneOutput {
@required
@httpHeader("Content-Type")
contentType: String

@required
@httpPayload
content: Blob
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationOne` operation: `/example?code=bar` | HttpQueryTrait
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
$version: "2"
namespace smithy.example

// OperationOne defines a query literal `code` with value `bar` that
// conflicts with the query param defined in the input structure.

service SmithyExample {
operations: [
OperationOne
]
}

@http(code: 200, method: "GET", uri: "/example?code=bar")
@readonly
operation OperationOne {
input: OperationOneInput
output: OperationOneOutput
}

structure OperationOneInput {
@httpQuery("code")
code: String

@httpQuery("state")
state: String
}

structure OperationOneOutput {
@required
@httpHeader("Content-Type")
contentType: String

@required
@httpPayload
content: Blob
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationOne` operation: `/one?code` | HttpQueryTrait
[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationTwo` operation: `/two?code=bar` | HttpQueryTrait

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
$version: "2"
namespace smithy.example

// OperationOne and OperationTwo define a query literal `code` and
// `code=bar` that conflicts with the query param defined in the input
// structure.

service SmithyExample {
operations: [
OperationOne
OperationTwo
]
}

@http(code: 200, method: "GET", uri: "/one?code")
@readonly
operation OperationOne {
input: OperationOneInput
output: OperationOneOutput
}

@http(code: 200, method: "GET", uri: "/two?code=bar")
@readonly
operation OperationTwo {
input: OperationOneInput
output: OperationOneOutput
}

structure OperationOneInput {
@httpQuery("code")
code: String

@httpQuery("state")
state: String
}

structure OperationOneOutput {
@required
@httpHeader("Content-Type")
contentType: String

@required
@httpPayload
content: Blob
}

0 comments on commit b7e600a

Please sign in to comment.