Skip to content

Commit ad803e2

Browse files
committed
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.
1 parent fe6dc6a commit ad803e2

File tree

4 files changed

+56
-41
lines changed

4 files changed

+56
-41
lines changed

docs/source/spec/http-protocol-compliance-tests.rst

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -108,20 +108,31 @@ that support the following properties:
108108
It's possible that specific authentication schemes might influence
109109
the serialization logic of an HTTP request.
110110
* - queryParams
111-
- ``Map<String, String>``
112-
- A map of expected query string parameters.
111+
- ``[string]``
112+
- A list of the expected serialized query string parameters.
113+
114+
Each element in the list is a query string key value pair
115+
that starts with the query string parameter name optionally
116+
followed by "=", optionally followed by the query string
117+
parameter value. For example, "foo=bar", "foo=", and "foo"
118+
are all valid values.
119+
120+
.. note::
121+
122+
This kind of list is used instead of a map so that query string
123+
parameter values for lists can be represented using repeated
124+
key-value pairs.
125+
126+
The query string parameter name and the value MUST appear in the
127+
format in which it is expected to be sent over the wire; if a key or
128+
value needs to be percent-encoded, then it MUST appear
129+
percent-encoded in this list.
113130
114131
A serialized HTTP request is not in compliance with the protocol
115132
if any query string parameter defined in ``queryParams`` is not
116133
defined in the request or if the value of a query string parameter
117134
in the request differs from the expected value.
118135
119-
Each key represents the query string parameter name, and each
120-
value represents the query string parameter value. Both keys and
121-
values MUST appear in the format in which it is expected to be
122-
sent over the wire; if a key or value needs to be percent-encoded,
123-
then it MUST appear percent-encoded in this map.
124-
125136
``queryParams`` applies no constraints on additional query parameters.
126137
* - forbidQueryParams
127138
- [``string``]
@@ -204,10 +215,14 @@ that uses :ref:`HTTP binding traits <http-traits>`.
204215
protocol: "example",
205216
params: {
206217
"greeting": "Hi",
207-
"name": "Teddy"
218+
"name": "Teddy",
219+
"query": "Hello there"
208220
},
209221
method: "POST",
210222
uri: "/",
223+
queryParams: [
224+
"Hi=Hello%20there"
225+
],
211226
headers: {
212227
"X-Greeting": "Hi",
213228
},
@@ -221,6 +236,9 @@ that uses :ref:`HTTP binding traits <http-traits>`.
221236
@httpHeader("X-Greeting")
222237
greeting: String,
223238
239+
@httpQuery("Hi")
240+
query: String,
241+
224242
name: String
225243
}
226244
@@ -244,17 +262,21 @@ that uses :ref:`HTTP binding traits <http-traits>`.
244262
{
245263
"id": "say_hello",
246264
"protocol": "example",
265+
"method": "POST",
266+
"uri": "/",
247267
"headers": {
248268
"X-Greeting": "Hi"
249269
},
270+
"queryParams": [
271+
"Hi=Hello%20there"
272+
],
250273
"body": "{\"name\": \"Teddy\"}",
251274
"bodyMediaType": "application/json"
252275
"params": {
253276
"greeting": "Hi",
254-
"name": "Teddy"
255-
},
256-
"method": "POST",
257-
"uri": "/"
277+
"name": "Teddy",
278+
"query": "Hello there"
279+
}
258280
}
259281
]
260282
}

smithy-protocol-test-traits/src/main/java/software/amazon/smithy/protocoltests/traits/HttpRequestTestCase.java

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
package software.amazon.smithy.protocoltests.traits;
1717

1818
import java.util.ArrayList;
19-
import java.util.Collections;
20-
import java.util.LinkedHashMap;
2119
import java.util.List;
22-
import java.util.Map;
2320
import software.amazon.smithy.model.node.ArrayNode;
2421
import software.amazon.smithy.model.node.Node;
2522
import software.amazon.smithy.model.node.ObjectNode;
@@ -41,15 +38,15 @@ public final class HttpRequestTestCase extends HttpMessageTestCase implements To
4138

4239
private final String method;
4340
private final String uri;
44-
private final Map<String, String> queryParams;
41+
private final List<String> queryParams;
4542
private final List<String> forbidQueryParams;
4643
private final List<String> requireQueryParams;
4744

4845
private HttpRequestTestCase(Builder builder) {
4946
super(builder);
5047
method = SmithyBuilder.requiredState(METHOD, builder.method);
5148
uri = SmithyBuilder.requiredState(URI, builder.uri);
52-
queryParams = Collections.unmodifiableMap(new LinkedHashMap<>(builder.queryParams));
49+
queryParams = ListUtils.copyOf(builder.queryParams);
5350
forbidQueryParams = ListUtils.copyOf(builder.forbidQueryParams);
5451
requireQueryParams = ListUtils.copyOf(builder.requireQueryParams);
5552
}
@@ -62,7 +59,7 @@ public String getUri() {
6259
return uri;
6360
}
6461

65-
public Map<String, String> getQueryParams() {
62+
public List<String> getQueryParams() {
6663
return queryParams;
6764
}
6865

@@ -80,10 +77,8 @@ public static HttpRequestTestCase fromNode(Node node) {
8077
ObjectNode o = node.expectObjectNode();
8178
builder.method(o.expectStringMember(METHOD).getValue());
8279
builder.uri(o.expectStringMember(URI).getValue());
83-
o.getObjectMember(QUERY_PARAMS).ifPresent(headers -> {
84-
headers.getStringMap().forEach((k, v) -> {
85-
builder.putQueryParam(k, v.expectStringNode().getValue());
86-
});
80+
o.getArrayMember(QUERY_PARAMS).ifPresent(queryParams -> {
81+
builder.queryParams(queryParams.getElementsAs(StringNode::getValue));
8782
});
8883
o.getArrayMember(FORBID_QUERY_PARAMS).ifPresent(params -> {
8984
builder.forbidQueryParams(params.getElementsAs(StringNode::getValue));
@@ -100,7 +95,7 @@ public Node toNode() {
10095
node.withMember(METHOD, getMethod());
10196
node.withMember(URI, getUri());
10297
if (!queryParams.isEmpty()) {
103-
node.withMember(QUERY_PARAMS, ObjectNode.fromStringMap(getQueryParams()));
98+
node.withMember(QUERY_PARAMS, ArrayNode.fromStrings(getQueryParams()));
10499
}
105100
if (!forbidQueryParams.isEmpty()) {
106101
node.withMember(FORBID_QUERY_PARAMS, ArrayNode.fromStrings(getForbidQueryParams()));
@@ -134,7 +129,7 @@ public static final class Builder extends HttpMessageTestCase.Builder<Builder, H
134129

135130
private String method;
136131
private String uri;
137-
private final Map<String, String> queryParams = new LinkedHashMap<>();
132+
private final List<String> queryParams = new ArrayList<>();
138133
private final List<String> forbidQueryParams = new ArrayList<>();
139134
private final List<String> requireQueryParams = new ArrayList<>();
140135

@@ -150,14 +145,9 @@ public Builder uri(String uri) {
150145
return this;
151146
}
152147

153-
public Builder queryParams(Map<String, String> queryParams) {
148+
public Builder queryParams(List<String> queryParams) {
154149
this.queryParams.clear();
155-
this.queryParams.putAll(queryParams);
156-
return this;
157-
}
158-
159-
public Builder putQueryParam(String key, String value) {
160-
queryParams.put(key, value);
150+
this.queryParams.addAll(queryParams);
161151
return this;
162152
}
163153

smithy-protocol-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,24 @@ structure HttpRequestTestCase {
4141
/// logic of an HTTP request.
4242
authScheme: String,
4343

44-
/// A map of expected query string parameters.
44+
/// A list of the expected serialized query string parameters.
45+
///
46+
/// Each element in the list is a query string key value pair
47+
/// that starts with the query string parameter name optionally
48+
/// followed by "=", optionally followed by the query string
49+
/// parameter value. For example, "foo=bar", "foo=", and "foo"
50+
/// are all valid values. The query string parameter name and
51+
/// the value MUST appear in the format in which it is expected
52+
/// to be sent over the wire; if a key or value needs to be
53+
/// percent-encoded, then it MUST appear percent-encoded in this list.
4554
///
4655
/// A serialized HTTP request is not in compliance with the protocol
4756
/// if any query string parameter defined in `queryParams` is not
4857
/// defined in the request or if the value of a query string parameter
4958
/// in the request differs from the expected value.
5059
///
51-
/// Each key represents the query string parameter name, and each
52-
/// value represents the query string parameter value. Both keys and
53-
/// values MUST appear in the format in which it is expected to be
54-
/// sent over the wire; if a key or value needs to be percent-encoded,
55-
/// then it MUST appear percent-encoded in this map.
56-
///
5760
/// `queryParams` applies no constraints on additional query parameters.
58-
queryParams: StringMap,
61+
queryParams: StringList,
5962

6063
/// A list of query string parameter names that must not appear in the
6164
/// serialized HTTP request.

smithy-protocol-test-traits/src/test/resources/software/amazon/smithy/protocoltests/traits/errorfiles/all-request-features.smithy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use smithy.test#httpRequestTests
1111
authScheme: "test",
1212
method: "POST",
1313
uri: "/",
14-
queryParams: {"foo": "baz"},
14+
queryParams: ["foo=baz"],
1515
forbidQueryParams: ["Nope"],
1616
requireQueryParams: ["Yap"],
1717
headers: {"X-Foo": "baz"},

0 commit comments

Comments
 (0)