Use list of string for queryParams

queryParams in protocol compliance tests previously used a map of query string parameter names to values. However, this approach makes it impossible to define test cases for query string parameter lists where the serialized query string keys are repeated (which is the default serialization format used in Smithy HTTP protocol bindings). Now queryParams is a list of the query string parameter key value pairs (e.g., `queryParams: ["foo=bar", "foo=baz%20bam"]`. This allows for testing any kind of query string serialization format, including repeated key parameters.
smithy-lang · Jan 3, 2020 · 7de413d · 7de413d
1 parent fe6dc6a
commit 7de413d
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 41 deletions.
diff --git a/docs/source/spec/http-protocol-compliance-tests.rst b/docs/source/spec/http-protocol-compliance-tests.rst
@@ -108,20 +108,31 @@ that support the following properties:
         It's possible that specific authentication schemes might influence
         the serialization logic of an HTTP request.
     * - queryParams
-      - ``Map<String, String>``
-      - A map of expected query string parameters.
+      - ``[string]``
+      - A list of the expected serialized query string parameters.
+
+        Each element in the list is a query string key value pair
+        that starts with the query string parameter name optionally
+        followed by "=", optionally followed by the query string
+        parameter value. For example, "foo=bar", "foo=", and "foo"
+        are all valid values.
+
+        .. note::
+
+            This kind of list is used instead of a map so that query string
+            parameter values for lists can be represented using repeated
+            key-value pairs.
+
+        The query string parameter name and the value MUST appear in the
+        format in which it is expected to be sent over the wire; if a key or
+        value needs to be percent-encoded, then it MUST appear
+        percent-encoded in this list.
 
         A serialized HTTP request is not in compliance with the protocol
         if any query string parameter defined in ``queryParams`` is not
         defined in the request or if the value of a query string parameter
         in the request differs from the expected value.
 
-        Each key represents the query string parameter name, and each
-        value represents the query string parameter value. Both keys and
-        values MUST appear in the format in which it is expected to be
-        sent over the wire; if a key or value needs to be percent-encoded,
-        then it MUST appear percent-encoded in this map.
-
         ``queryParams`` applies no constraints on additional query parameters.
     * - forbidQueryParams
       - [``string``]
@@ -204,10 +215,14 @@ that uses :ref:`HTTP binding traits <http-traits>`.
                 protocol: "example",
                 params: {
                     "greeting": "Hi",
-                    "name": "Teddy"
+                    "name": "Teddy",
+                    "query": "Hello there"
                 },
                 method: "POST",
                 uri: "/",
+                queryParams: [
+                    "Hi=Hello%20there"
+                ],
                 headers: {
                     "X-Greeting": "Hi",
                 },
@@ -221,6 +236,9 @@ that uses :ref:`HTTP binding traits <http-traits>`.
             @httpHeader("X-Greeting")
             greeting: String,
 
+            @httpQuery("Hi")
+            query: String,
+
             name: String
         }
 
@@ -244,17 +262,21 @@ that uses :ref:`HTTP binding traits <http-traits>`.
                             {
                                 "id": "say_hello",
                                 "protocol": "example",
+                                "method": "POST",
+                                "uri": "/",
                                 "headers": {
                                     "X-Greeting": "Hi"
                                 },
+                                "queryParams": [
+                                    "Hi=Hello%20there"
+                                ],
                                 "body": "{\"name\": \"Teddy\"}",
                                 "bodyMediaType": "application/json"
                                 "params": {
                                     "greeting": "Hi",
-                                    "name": "Teddy"
-                                },
-                                "method": "POST",
-                                "uri": "/"
+                                    "name": "Teddy",
+                                    "query": "Hello there"
+                                }
                             }
                         ]
                     }

diff --git a/...traits/src/main/java/software/amazon/smithy/protocoltests/traits/HttpRequestTestCase.java b/...traits/src/main/java/software/amazon/smithy/protocoltests/traits/HttpRequestTestCase.java
@@ -16,10 +16,7 @@
 package software.amazon.smithy.protocoltests.traits;
 
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import software.amazon.smithy.model.node.ArrayNode;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
@@ -41,15 +38,15 @@ public final class HttpRequestTestCase extends HttpMessageTestCase implements To
 
     private final String method;
     private final String uri;
-    private final Map<String, String> queryParams;
+    private final List<String> queryParams;
     private final List<String> forbidQueryParams;
     private final List<String> requireQueryParams;
 
     private HttpRequestTestCase(Builder builder) {
         super(builder);
         method = SmithyBuilder.requiredState(METHOD, builder.method);
         uri = SmithyBuilder.requiredState(URI, builder.uri);
-        queryParams = Collections.unmodifiableMap(new LinkedHashMap<>(builder.queryParams));
+        queryParams = ListUtils.copyOf(builder.queryParams);
         forbidQueryParams = ListUtils.copyOf(builder.forbidQueryParams);
         requireQueryParams = ListUtils.copyOf(builder.requireQueryParams);
     }
@@ -62,7 +59,7 @@ public String getUri() {
         return uri;
     }
 
-    public Map<String, String> getQueryParams() {
+    public List<String> getQueryParams() {
         return queryParams;
     }
 
@@ -80,10 +77,8 @@ public static HttpRequestTestCase fromNode(Node node) {
         ObjectNode o = node.expectObjectNode();
         builder.method(o.expectStringMember(METHOD).getValue());
         builder.uri(o.expectStringMember(URI).getValue());
-        o.getObjectMember(QUERY_PARAMS).ifPresent(headers -> {
-            headers.getStringMap().forEach((k, v) -> {
-                builder.putQueryParam(k, v.expectStringNode().getValue());
-            });
+        o.getArrayMember(QUERY_PARAMS).ifPresent(queryParams -> {
+            builder.queryParams(queryParams.getElementsAs(StringNode::getValue));
         });
         o.getArrayMember(FORBID_QUERY_PARAMS).ifPresent(params -> {
             builder.forbidQueryParams(params.getElementsAs(StringNode::getValue));
@@ -100,7 +95,7 @@ public Node toNode() {
         node.withMember(METHOD, getMethod());
         node.withMember(URI, getUri());
         if (!queryParams.isEmpty()) {
-            node.withMember(QUERY_PARAMS, ObjectNode.fromStringMap(getQueryParams()));
+            node.withMember(QUERY_PARAMS, ArrayNode.fromStrings(getQueryParams()));
         }
         if (!forbidQueryParams.isEmpty()) {
             node.withMember(FORBID_QUERY_PARAMS, ArrayNode.fromStrings(getForbidQueryParams()));
@@ -134,7 +129,7 @@ public static final class Builder extends HttpMessageTestCase.Builder<Builder, H
 
         private String method;
         private String uri;
-        private final Map<String, String> queryParams = new LinkedHashMap<>();
+        private final List<String> queryParams = new ArrayList<>();
         private final List<String> forbidQueryParams = new ArrayList<>();
         private final List<String> requireQueryParams = new ArrayList<>();
 
@@ -150,14 +145,9 @@ public Builder uri(String uri) {
             return this;
         }
 
-        public Builder queryParams(Map<String, String> queryParams) {
+        public Builder queryParams(List<String> queryParams) {
             this.queryParams.clear();
-            this.queryParams.putAll(queryParams);
-            return this;
-        }
-
-        public Builder putQueryParam(String key, String value) {
-            queryParams.put(key, value);
+            this.queryParams.addAll(queryParams);
             return this;
         }
 

diff --git a/smithy-protocol-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy b/smithy-protocol-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy
@@ -41,21 +41,24 @@ structure HttpRequestTestCase {
     /// logic of an HTTP request.
     authScheme: String,
 
-    /// A map of expected query string parameters.
+    /// A list of the expected serialized query string parameters.
+    ///
+    /// Each element in the list is a query string key value pair
+    /// that starts with the query string parameter name optionally
+    /// followed by "=", optionally followed by the query string
+    /// parameter value. For example, "foo=bar", "foo=", and "foo"
+    /// are all valid values. The query string parameter name and
+    /// the value MUST appear in the format in which it is expected
+    /// to be sent over the wire; if a key or value needs to be
+    /// percent-encoded, then it MUST appear percent-encoded in this list.
     ///
     /// A serialized HTTP request is not in compliance with the protocol
     /// if any query string parameter defined in `queryParams` is not
     /// defined in the request or if the value of a query string parameter
     /// in the request differs from the expected value.
     ///
-    /// Each key represents the query string parameter name, and each
-    /// value represents the query string parameter value. Both keys and
-    /// values MUST appear in the format in which it is expected to be
-    /// sent over the wire; if a key or value needs to be percent-encoded,
-    /// then it MUST appear percent-encoded in this map.
-    ///
     /// `queryParams` applies no constraints on additional query parameters.
-    queryParams: StringMap,
+    queryParams: StringList,
 
     /// A list of query string parameter names that must not appear in the
     /// serialized HTTP request.

diff --git a/...ources/software/amazon/smithy/protocoltests/traits/errorfiles/all-request-features.smithy b/...ources/software/amazon/smithy/protocoltests/traits/errorfiles/all-request-features.smithy
@@ -11,7 +11,7 @@ use smithy.test#httpRequestTests
         authScheme: "test",
         method: "POST",
         uri: "/",
-        queryParams: {"foo": "baz"},
+        queryParams: ["foo=baz"],
         forbidQueryParams: ["Nope"],
         requireQueryParams: ["Yap"],
         headers: {"X-Foo": "baz"},