Skip to content

Commit 97d5898

Browse files
joewyzjoewyz
authored
Fix httpQuery related protocol tests (#945)
* Fix httpQuery related protocol tests * Remove blob from httpQuerySerializer
1 parent e22bb5c commit 97d5898

8 files changed

Lines changed: 148 additions & 22 deletions

File tree

‎client/client-core/src/main/java/software/amazon/smithy/java/client/core/plugins/InjectIdempotencyTokenPlugin.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import software.amazon.smithy.java.client.core.ClientPlugin;
1111
import software.amazon.smithy.java.client.core.interceptors.ClientInterceptor;
1212
import software.amazon.smithy.java.client.core.interceptors.InputHook;
13+
import software.amazon.smithy.java.context.Context;
1314
import software.amazon.smithy.java.core.schema.SchemaUtils;
1415
import software.amazon.smithy.java.core.schema.SerializableStruct;
1516
import software.amazon.smithy.java.logging.InternalLogger;
@@ -24,6 +25,7 @@
2425
public final class InjectIdempotencyTokenPlugin implements ClientPlugin {
2526

2627
public static final InjectIdempotencyTokenPlugin INSTANCE = new InjectIdempotencyTokenPlugin();
28+
public static final Context.Key<String> IDEMPOTENCY_TOKEN_PROVIDER = Context.key("IdempotencyTokenProvider");
2729

2830
private static final InternalLogger LOGGER = InternalLogger.getLogger(InjectIdempotencyTokenPlugin.class);
2931
private static final ClientInterceptor INTERCEPTOR = new Injector();
@@ -52,7 +54,9 @@ public <I extends SerializableStruct> I modifyBeforeSerialization(InputHook<I, ?
5254
if (value == null) {
5355
var builder = operation.inputBuilder();
5456
SchemaUtils.copyShape(hook.input(), builder);
55-
builder.setMemberValue(tokenMember, UUID.randomUUID().toString());
57+
var tokenProvider = hook.context().get(IDEMPOTENCY_TOKEN_PROVIDER);
58+
var tokenValue = tokenProvider != null ? tokenProvider : UUID.randomUUID().toString();
59+
builder.setMemberValue(tokenMember, tokenValue);
5660
LOGGER.debug("Injecting idempotency token into {} input", operation.schema().id());
5761
return builder.build();
5862
}

‎client/client-rpcv2-cbor/src/it/java/software/amazon/smithy/java/client/rpcv2/RpcV2CborProtocolTests.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ public void requestTest(DataStream expected, DataStream actual) {
3636
@ProtocolTestFilter(
3737
skipTests = {
3838
"RpcV2CborClientPopulatesDefaultsValuesWhenMissingInResponse",
39-
"RpcV2CborInvalidGreetingError",
40-
"RpcV2CborEmptyComplexError",
41-
"RpcV2CborComplexError"
4239
})
4340
public void responseTest(Runnable test) {
4441
test.run();

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/OperationGenerator.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ public final class ${shape:T} implements ${operationType:C} {
202202
shape,
203203
ServiceIndex.AuthSchemeMode.NO_AUTH_AWARE));
204204

205+
var inputShape = directive.model().expectShape(shape.getInputShape());
205206
eventStreamIndex.getInputInfo(shape).ifPresentOrElse(info -> {
206207
writer.putContext("supplier", Supplier.class);
207208
writer.putContext("hasInputEventStream", true);
@@ -210,7 +211,7 @@ public final class ${shape:T} implements ${operationType:C} {
210211
"inputEventType",
211212
directive.symbolProvider().toSymbol(info.getEventStreamTarget()));
212213
}, () -> {
213-
for (var member : shape.members()) {
214+
for (var member : inputShape.members()) {
214215
if (directive.model().expectShape(member.getTarget()).hasTrait(StreamingTrait.class)) {
215216
writer.putContext("inputStreamMember", member.getMemberName());
216217
break;
@@ -226,7 +227,7 @@ public final class ${shape:T} implements ${operationType:C} {
226227
"outputEventType",
227228
directive.symbolProvider().toSymbol(info.getEventStreamTarget()));
228229
}, () -> {
229-
for (var member : shape.members()) {
230+
for (var member : directive.model().expectShape(shape.getOutputShape()).members()) {
230231
if (directive.model().expectShape(member.getTarget()).hasTrait(StreamingTrait.class)) {
231232
writer.putContext("outputStreamMember", member.getMemberName());
232233
break;
@@ -235,7 +236,7 @@ public final class ${shape:T} implements ${operationType:C} {
235236
});
236237

237238
// Add the idempotency token member.
238-
for (var member : shape.members()) {
239+
for (var member : inputShape.members()) {
239240
if (member.hasTrait(IdempotencyTokenTrait.class)) {
240241
writer.putContext("idempotencyTokenMember", member.getMemberName());
241242
break;

‎http/http-binding/src/main/java/software/amazon/smithy/java/http/binding/HttpBindingSerializer.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ public void writeStruct(Schema schema, SerializableStruct struct) {
101101
responseStatus = bindingMatcher.responseStatus();
102102
}
103103

104+
// Add fixed query string parameters from @http trait's uri field
105+
if (!uriPattern.getQueryLiterals().isEmpty()) {
106+
for (var entry : uriPattern.getQueryLiterals().entrySet()) {
107+
queryStringParams.add(entry.getKey(), entry.getValue());
108+
}
109+
}
110+
104111
// Prescanning names from @httpHeader for @httpPrefixHeaders
105112
for (var member : schema.members()) {
106113
if (member.hasTrait(TraitKey.HTTP_HEADER_TRAIT)) {
@@ -248,7 +255,7 @@ protected ShapeSerializer before(Schema schema) {
248255
schema.expectTrait(TraitKey.HTTP_PREFIX_HEADERS_TRAIT).getValue(),
249256
serializer.headerConsumer,
250257
serializer.namesFromHttpHeader);
251-
case QUERY_PARAMS -> new HttpQueryParamsSerializer(serializer.queryStringParams::add);
258+
case QUERY_PARAMS -> new HttpQueryParamsSerializer(serializer.queryStringParams::addForQueryParams);
252259
case BODY -> ShapeSerializer.nullSerializer(); // handled in HttpBindingSerializer#writeStruct.
253260
case PAYLOAD -> {
254261
payloadSerializer = new PayloadSerializer(serializer, serializer.payloadCodec);

‎http/http-binding/src/main/java/software/amazon/smithy/java/http/binding/HttpQuerySerializer.java‎

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,13 @@
77

88
import java.math.BigDecimal;
99
import java.math.BigInteger;
10-
import java.nio.ByteBuffer;
1110
import java.time.Instant;
1211
import java.util.function.BiConsumer;
1312
import software.amazon.smithy.java.core.schema.Schema;
1413
import software.amazon.smithy.java.core.schema.TraitKey;
1514
import software.amazon.smithy.java.core.serde.ShapeSerializer;
1615
import software.amazon.smithy.java.core.serde.SpecificShapeSerializer;
1716
import software.amazon.smithy.java.core.serde.TimestampFormatter;
18-
import software.amazon.smithy.java.io.ByteBufferUtils;
1917
import software.amazon.smithy.model.traits.HttpQueryTrait;
2018

2119
final class HttpQuerySerializer extends SpecificShapeSerializer {
@@ -28,7 +26,7 @@ public HttpQuerySerializer(BiConsumer<String, String> queryWriter) {
2826

2927
@Override
3028
public <T> void writeList(Schema schema, T listState, int size, BiConsumer<T, ShapeSerializer> consumer) {
31-
consumer.accept(listState, this);
29+
consumer.accept(listState, new ListElementSerializer(schema.getTrait(TraitKey.HTTP_QUERY_TRAIT)));
3230
}
3331

3432
private void writeQuery(HttpQueryTrait trait, String value) {
@@ -115,14 +113,6 @@ public void writeString(Schema schema, String value) {
115113
}
116114
}
117115

118-
@Override
119-
public void writeBlob(Schema schema, ByteBuffer value) {
120-
var queryTrait = schema.getTrait(TraitKey.HTTP_QUERY_TRAIT);
121-
if (queryTrait != null) {
122-
writeQuery(queryTrait, ByteBufferUtils.base64Encode(value));
123-
}
124-
}
125-
126116
@Override
127117
public void writeTimestamp(Schema schema, Instant value) {
128118
var queryTrait = schema.getTrait(TraitKey.HTTP_QUERY_TRAIT);
@@ -134,4 +124,71 @@ public void writeTimestamp(Schema schema, Instant value) {
134124
writeQuery(queryTrait, formatter.writeString(value));
135125
}
136126
}
127+
128+
private class ListElementSerializer extends SpecificShapeSerializer {
129+
private final HttpQueryTrait parentTrait;
130+
131+
ListElementSerializer(HttpQueryTrait trait) {
132+
this.parentTrait = trait;
133+
}
134+
135+
@Override
136+
public void writeBoolean(Schema schema, boolean value) {
137+
writeQuery(parentTrait, Boolean.toString(value));
138+
}
139+
140+
@Override
141+
public void writeShort(Schema schema, short value) {
142+
writeQuery(parentTrait, Short.toString(value));
143+
}
144+
145+
@Override
146+
public void writeByte(Schema schema, byte value) {
147+
writeQuery(parentTrait, Byte.toString(value));
148+
}
149+
150+
@Override
151+
public void writeInteger(Schema schema, int value) {
152+
writeQuery(parentTrait, Integer.toString(value));
153+
}
154+
155+
@Override
156+
public void writeLong(Schema schema, long value) {
157+
writeQuery(parentTrait, Long.toString(value));
158+
}
159+
160+
@Override
161+
public void writeFloat(Schema schema, float value) {
162+
writeQuery(parentTrait, Float.toString(value));
163+
}
164+
165+
@Override
166+
public void writeDouble(Schema schema, double value) {
167+
writeQuery(parentTrait, Double.toString(value));
168+
}
169+
170+
@Override
171+
public void writeBigInteger(Schema schema, BigInteger value) {
172+
writeQuery(parentTrait, value.toString());
173+
}
174+
175+
@Override
176+
public void writeBigDecimal(Schema schema, BigDecimal value) {
177+
writeQuery(parentTrait, value.toString());
178+
}
179+
180+
@Override
181+
public void writeString(Schema schema, String value) {
182+
writeQuery(parentTrait, value);
183+
}
184+
185+
@Override
186+
public void writeTimestamp(Schema schema, Instant value) {
187+
var timestampFormatTrait = schema.getTrait(TraitKey.TIMESTAMP_FORMAT_TRAIT);
188+
TimestampFormatter formatter = timestampFormatTrait != null
189+
? TimestampFormatter.of(timestampFormatTrait)
190+
: TimestampFormatter.Prelude.DATE_TIME;
191+
writeQuery(parentTrait, formatter.writeString(value));
192+
}
193+
}
137194
}

‎io/src/main/java/software/amazon/smithy/java/io/uri/QueryStringBuilder.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@
66
package software.amazon.smithy.java.io.uri;
77

88
import java.util.ArrayList;
9+
import java.util.HashSet;
910
import java.util.List;
1011
import java.util.Map;
12+
import java.util.Set;
1113

1214
/**
1315
* Used to build a query string from key value pair parameters.
1416
*/
1517
public final class QueryStringBuilder {
1618

1719
private final List<String> values = new ArrayList<>();
20+
private final Set<String> keysFromHttpQuery = new HashSet<>();
1821

1922
/**
2023
* Clears the contents of the query string builder.
@@ -34,9 +37,27 @@ public void clear() {
3437
*/
3538
public void add(String key, String value) {
3639
values.add(key);
40+
keysFromHttpQuery.add(key);
3741
values.add(value);
3842
}
3943

44+
/**
45+
* Add a query string parameter and value to the query string comes from httpQueryParams trait.
46+
* <p>
47+
* The given key and value will be percent-encoded. If the value is already percent-encoded, it will be
48+
* double percent-encoded. Query string parameters from httpQuery should take precedence if there are
49+
* duplicate keys from @httpQuery.
50+
*
51+
* @param key Key of the parameter.
52+
* @param value Value of the parameter (or null).
53+
*/
54+
public void addForQueryParams(String key, String value) {
55+
if (!keysFromHttpQuery.contains(key)) {
56+
values.add(key);
57+
values.add(value);
58+
}
59+
}
60+
4061
/**
4162
* Add a query string parameter and values to the query string.
4263
* <p>

‎protocol-test-harness/src/main/java/software/amazon/smithy/java/protocoltests/harness/Assertions.java‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@
99
import static org.junit.jupiter.api.Assertions.assertNotNull;
1010

1111
import java.net.URI;
12+
import java.util.HashSet;
1213
import java.util.List;
1314
import java.util.Map;
1415
import java.util.Set;
1516
import java.util.stream.Collectors;
1617
import software.amazon.smithy.java.http.api.HttpMessage;
1718
import software.amazon.smithy.java.http.api.HttpRequest;
19+
import software.amazon.smithy.protocoltests.traits.HttpRequestTestCase;
1820

1921
/**
2022
* Provides a number of testing utilities for validating protocol test results.
@@ -41,8 +43,37 @@ private Assertions() {}
4143
'{',
4244
'}');
4345

44-
static void assertUriEquals(URI uri, String expected) {
45-
assertEquals(expected, uri.getRawPath());
46+
static void assertUriEquals(HttpRequestTestCase testCase, URI uri) {
47+
assertEquals(testCase.getUri(), uri.getRawPath());
48+
// Only evaluate query params when params are expected in test case.
49+
if (!testCase.getQueryParams().isEmpty()) {
50+
assertQueryParamsEquals(testCase.getQueryParams(), uri.getRawQuery());
51+
}
52+
}
53+
54+
private static void assertQueryParamsEquals(List<String> expectedParams, String actualQuery) {
55+
var expectedSet = paserQueryParamsList(expectedParams);
56+
var actualSet = parseQueryParamsString(actualQuery);
57+
assertEquals(expectedSet, actualSet, "Query parameters mismatch");
58+
}
59+
60+
private static Set<String> parseQueryParamsString(String query) {
61+
Set<String> result = new HashSet<>();
62+
// Raw query string is in format "param1=value1&param2=value2&param3=value3"
63+
for (String paramPair : query.split("&")) {
64+
var pair = paramPair.split("=", 2);
65+
result.add(pair[0] + "=" + (pair.length == 2 ? pair[1] : ""));
66+
}
67+
return result;
68+
}
69+
70+
private static Set<String> paserQueryParamsList(List<String> params) {
71+
Set<String> result = new HashSet<>();
72+
for (String paramPair : params) {
73+
var pair = paramPair.split("=", 2);
74+
result.add(pair[0] + "=" + (pair.length == 2 ? pair[1] : ""));
75+
}
76+
return result;
4677
}
4778

4879
static void assertHostEquals(HttpRequest request, String expected) {

‎protocol-test-harness/src/main/java/software/amazon/smithy/java/protocoltests/harness/HttpClientRequestProtocolTestProvider.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import software.amazon.smithy.java.client.core.auth.scheme.AuthSchemeOption;
2222
import software.amazon.smithy.java.client.core.auth.scheme.AuthSchemeResolver;
2323
import software.amazon.smithy.java.client.core.endpoint.EndpointResolver;
24+
import software.amazon.smithy.java.client.core.plugins.InjectIdempotencyTokenPlugin;
2425
import software.amazon.smithy.java.client.http.HttpMessageExchange;
2526
import software.amazon.smithy.java.context.Context;
2627
import software.amazon.smithy.java.core.schema.ApiOperation;
@@ -90,6 +91,13 @@ protected Stream<TestTemplateInvocationContext> generateProtocolTests(
9091
testCase.getBodyMediaType().orElse(null))
9192
.deserializeInto(inputBuilder);
9293

94+
// Add fixed idempotency token provider for protocol tests.
95+
if (operation.operationModel().idempotencyTokenMember() != null) {
96+
overrideBuilder.putConfig(
97+
InjectIdempotencyTokenPlugin.IDEMPOTENCY_TOKEN_PROVIDER,
98+
"00000000-0000-4000-8000-000000000000");
99+
}
100+
93101
return new RequestTestInvocationContext(
94102
testCase,
95103
store.mockClient(),
@@ -159,7 +167,7 @@ public Object resolveParameter(
159167
) throws ParameterResolutionException {
160168
mockClient.clientRequest(input, apiOperation, overrideConfig);
161169
var request = requestSupplier.get();
162-
Assertions.assertUriEquals(request.uri(), testCase.getUri());
170+
Assertions.assertUriEquals(testCase, request.uri());
163171
testCase.getResolvedHost()
164172
.ifPresent(resolvedHost -> Assertions.assertHostEquals(request, resolvedHost));
165173
Assertions.assertHeadersEqual(request, testCase.getHeaders());

0 commit comments

Comments
 (0)