From ea512cc0ce064f1f51d3bb3c1a3211a06c13d751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Tue, 23 May 2023 20:29:38 -0700 Subject: [PATCH 1/2] Validate that query literal do not conflict with query params --- .../validators/HttpQueryTraitValidator.java | 43 +++++++++++++++++- ...ery-literals-and-bindings-conflict1.errors | 1 + ...ery-literals-and-bindings-conflict1.smithy | 36 +++++++++++++++ ...ery-literals-and-bindings-conflict2.errors | 1 + ...ery-literals-and-bindings-conflict2.smithy | 36 +++++++++++++++ ...ery-literals-and-bindings-conflict3.errors | 2 + ...ery-literals-and-bindings-conflict3.smithy | 45 +++++++++++++++++++ 7 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.smithy diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java index 8b25211edcc..85011e10bf3 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java @@ -23,10 +23,15 @@ import java.util.Map; import java.util.Set; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +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.ServiceShape; 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; @@ -41,7 +46,7 @@ public List validate(Model model) { if (!model.isTraitApplied(HttpQueryTrait.class)) { return Collections.emptyList(); } else { - return validateBindings(getQueryBindings(model)); + return validateBindings(getQueryBindings(model), getStructureToOperations(model)); } } @@ -64,7 +69,10 @@ private Map>> getQueryBindings(Model mod return queryBindings; } - private List validateBindings(Map>> queryBindings) { + private List validateBindings( + Map>> queryBindings, + Map> structureToOperations + ) { List events = new ArrayList<>(); for (Map.Entry>> entry : queryBindings.entrySet()) { @@ -77,8 +85,39 @@ private List validateBindings(Map operations = structureToOperations.getOrDefault(entry.getKey(), + Collections.emptyList()); + for (OperationShape operation : operations) { + UriPattern pattern = operation.expectTrait(HttpTrait.class).getUri(); + for (Map.Entry literalEntry : pattern.getQueryLiterals().entrySet()) { + String literalKey = literalEntry.getKey(); + if (entry.getValue().containsKey(literalKey)) { + events.add(error(entry.getKey(), String.format( + "`httpQuery` parameter name binding conflicts found for the `%s` parameter, the " + + "http trait for the `%s` operation defines a query literal with the same name", + literalKey, operation.getId()))); + } + } + } } return events; } + + private Map> getStructureToOperations(Model model) { + Map> structureToOperations = new HashMap<>(); + for (ServiceShape service : model.getServiceShapes()) { + for (OperationShape operation : TopDownIndex.of(model).getContainedOperations(service)) { + if (operation.hasTrait(HttpTrait.class)) { + model.expectShape(operation.getInputShape()) + .asStructureShape() + .ifPresent(structure -> structureToOperations + .computeIfAbsent(structure, key -> new ArrayList<>()) + .add(operation)); + } + } + } + return structureToOperations; + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors new file mode 100644 index 00000000000..565b2682c61 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors @@ -0,0 +1 @@ +[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.smithy new file mode 100644 index 00000000000..34f1bca3372 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.smithy @@ -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 +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors new file mode 100644 index 00000000000..565b2682c61 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors @@ -0,0 +1 @@ +[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.smithy new file mode 100644 index 00000000000..03761e492f4 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.smithy @@ -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 +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors new file mode 100644 index 00000000000..4a5f112b381 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors @@ -0,0 +1,2 @@ +[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait +[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationTwo` operation defines a query literal with the same name | HttpQueryTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.smithy new file mode 100644 index 00000000000..175fdf34456 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.smithy @@ -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 +} From a279fbdaa3e7e6020aabd8d948a6f10de2e75995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Wed, 31 May 2023 11:08:27 -0700 Subject: [PATCH 2/2] Address PR comments --- .../smithy/model/pattern/UriPattern.java | 1 + .../validators/HttpQueryTraitValidator.java | 24 +++++++------------ ...ery-literals-and-bindings-conflict1.errors | 2 +- ...ery-literals-and-bindings-conflict2.errors | 2 +- ...ery-literals-and-bindings-conflict3.errors | 5 ++-- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java b/smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java index d923ec81686..26be0d4faa0 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java @@ -178,4 +178,5 @@ public boolean equals(Object other) { public int hashCode() { return super.hashCode() + queryLiterals.hashCode(); } + } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java index 85011e10bf3..45212dc355d 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpQueryTraitValidator.java @@ -23,11 +23,10 @@ import java.util.Map; import java.util.Set; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.knowledge.TopDownIndex; +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.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.traits.HttpQueryTrait; @@ -94,9 +93,8 @@ private List validateBindings( String literalKey = literalEntry.getKey(); if (entry.getValue().containsKey(literalKey)) { events.add(error(entry.getKey(), String.format( - "`httpQuery` parameter name binding conflicts found for the `%s` parameter, the " - + "http trait for the `%s` operation defines a query literal with the same name", - literalKey, operation.getId()))); + "`httpQuery` name `%s` conflicts with the `http` trait of the `%s` operation: `%s`", + literalKey, operation.getId(), pattern))); } } } @@ -106,17 +104,13 @@ private List validateBindings( } private Map> getStructureToOperations(Model model) { + OperationIndex index = OperationIndex.of(model); Map> structureToOperations = new HashMap<>(); - for (ServiceShape service : model.getServiceShapes()) { - for (OperationShape operation : TopDownIndex.of(model).getContainedOperations(service)) { - if (operation.hasTrait(HttpTrait.class)) { - model.expectShape(operation.getInputShape()) - .asStructureShape() - .ifPresent(structure -> structureToOperations - .computeIfAbsent(structure, key -> new ArrayList<>()) - .add(operation)); - } - } + for (OperationShape operation : model.getOperationShapesWithTrait(HttpTrait.class)) { + index.getInput(operation) + .ifPresent(structure -> structureToOperations + .computeIfAbsent(structure, key -> new ArrayList<>()) + .add(operation)); } return structureToOperations; } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors index 565b2682c61..4e30b31cbbf 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict1.errors @@ -1 +1 @@ -[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait +[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationOne` operation: `/example?code` | HttpQueryTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors index 565b2682c61..c9c0bb15786 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict2.errors @@ -1 +1 @@ -[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait +[ERROR] smithy.example#OperationOneInput: `httpQuery` name `code` conflicts with the `http` trait of the `smithy.example#OperationOne` operation: `/example?code=bar` | HttpQueryTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors index 4a5f112b381..f38f447f920 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-query-literals-and-bindings-conflict3.errors @@ -1,2 +1,3 @@ -[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationOne` operation defines a query literal with the same name | HttpQueryTrait -[ERROR] smithy.example#OperationOneInput: `httpQuery` parameter name binding conflicts found for the `code` parameter, the http trait for the `smithy.example#OperationTwo` operation defines a query literal with the same name | HttpQueryTrait +[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 +