Skip to content

Commit 60d06cd

Browse files
authored
Merge branch 'main' into landonxjames/eventheader-enum
2 parents 532f87e + 0438c1b commit 60d06cd

File tree

7 files changed

+123
-18
lines changed

7 files changed

+123
-18
lines changed

.changelog/1760126889.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
applies_to:
3+
- client
4+
authors:
5+
- rcoh
6+
references: [ "smithy-rs#4346" ]
7+
breaking: false
8+
new_feature: false
9+
bug_fix: true
10+
---
11+
12+
Fix bug where httpQueryParams were silently dropped when no other query parameters were modeled.

AGENTS.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,55 @@
1010

1111
Protocol files: `codegen-{core,server}/.../protocols/`
1212

13+
## Protocol Tests
14+
15+
Protocol tests validate that generated code correctly implements Smithy protocols (like restJson1, awsJson1_1, etc.).
16+
17+
### Adding Protocol Tests
18+
19+
Protocol tests are defined in Smithy model files using `@httpRequestTests` and `@httpResponseTests` traits:
20+
21+
```smithy
22+
@http(uri: "/my-operation", method: "GET")
23+
@httpRequestTests([
24+
{
25+
id: "MyOperationRequest",
26+
documentation: "Test description",
27+
protocol: "aws.protocols#restJson1",
28+
method: "GET",
29+
uri: "/my-operation",
30+
queryParams: ["param1=value1", "param2=value2"],
31+
params: {
32+
queryMap: {
33+
"param1": "value1",
34+
"param2": "value2"
35+
}
36+
},
37+
appliesTo: "client",
38+
}
39+
])
40+
operation MyOperation {
41+
input: MyOperationInput,
42+
}
43+
```
44+
45+
### Key Protocol Test Locations
46+
47+
- **`codegen-core/common-test-models/rest-json-extras.smithy`** - restJson1 protocol tests
48+
- **`codegen-core/common-test-models/constraints.smithy`** - Constraint validation tests with restJson1
49+
- **`codegen-client-test/model/main.smithy`** - awsJson1_1 protocol tests
50+
51+
### httpQueryParams Bug Investigation
52+
53+
When investigating the `@httpQueryParams` bug (where query parameters weren't appearing in requests), the issue was in `RequestBindingGenerator.kt` line 173. The bug occurred when:
54+
55+
1. An operation had ONLY `@httpQueryParams` (no regular `@httpQuery` parameters)
56+
2. The condition `if (dynamicParams.isEmpty() && literalParams.isEmpty() && mapParams.isEmpty())` would skip generating the `uri_query` function
57+
58+
The fix was to ensure `mapParams.isEmpty()` was included in the condition check. The current implementation correctly generates query parameters for `@httpQueryParams` even when no other query parameters exist.
59+
60+
**Testing httpQueryParams**: Create operations with only `@httpQueryParams` to ensure they generate proper query strings in requests.
61+
1362
## preludeScope: Rust Prelude Types
1463

1564
**Always use `preludeScope` for Rust prelude types:**

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/http/RequestBindingGenerator.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class RequestBindingGenerator(
170170
val dynamicParams = index.getRequestBindings(operationShape, HttpBinding.Location.QUERY)
171171
val mapParams = index.getRequestBindings(operationShape, HttpBinding.Location.QUERY_PARAMS)
172172
val literalParams = httpTrait.uri.queryLiterals
173-
if (dynamicParams.isEmpty() && literalParams.isEmpty()) {
173+
if (dynamicParams.isEmpty() && literalParams.isEmpty() && mapParams.isEmpty()) {
174174
return false
175175
}
176176
val preloadedParams = literalParams.keys + dynamicParams.map { it.locationName }

codegen-core/common-test-models/rest-json-extras.smithy

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ service RestJsonExtras {
6666
CaseInsensitiveErrorOperation,
6767
EmptyStructWithContentOnWireOp,
6868
QueryPrecedence,
69+
HttpQueryParamsOnlyOperation,
6970
],
7071
errors: [ExtraError]
7172
}
@@ -350,3 +351,46 @@ structure EmptyStructWithContentOnWireOpOutput {
350351
operation EmptyStructWithContentOnWireOp {
351352
output: EmptyStructWithContentOnWireOpOutput,
352353
}
354+
355+
@http(uri: "/http-query-params-only", method: "GET")
356+
@httpRequestTests([
357+
{
358+
id: "HttpQueryParamsOnlyRequest",
359+
documentation: "Test that httpQueryParams are included in request when no other query parameters exist",
360+
protocol: "aws.protocols#restJson1",
361+
method: "GET",
362+
uri: "/http-query-params-only",
363+
queryParams: ["shouldExpandRoles=true", "shouldShowOnlyAuthForThisDocument=false"],
364+
params: {
365+
queryMap: {
366+
"shouldExpandRoles": "true",
367+
"shouldShowOnlyAuthForThisDocument": "false"
368+
}
369+
},
370+
appliesTo: "client",
371+
},
372+
{
373+
id: "HttpQueryParamsOnlyEmptyRequest",
374+
documentation: "Test that empty httpQueryParams map results in no query parameters",
375+
protocol: "aws.protocols#restJson1",
376+
method: "GET",
377+
uri: "/http-query-params-only",
378+
params: {
379+
queryMap: {}
380+
},
381+
appliesTo: "client",
382+
}
383+
])
384+
operation HttpQueryParamsOnlyOperation {
385+
input: HttpQueryParamsOnlyInput,
386+
}
387+
388+
structure HttpQueryParamsOnlyInput {
389+
@httpQueryParams
390+
queryMap: QueryMap,
391+
}
392+
393+
map QueryMap {
394+
key: String,
395+
value: String,
396+
}

codegen-server-test/integration-tests/Cargo.lock

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust-runtime/aws-smithy-checksums/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "aws-smithy-checksums"
3-
version = "0.63.9"
3+
version = "0.63.10"
44
authors = [
55
"AWS Rust SDK Team <aws-sdk-rust@amazon.com>",
66
"Zelda Hessler <zhessler@amazon.com>",

rust-runtime/aws-smithy-checksums/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ impl Sha1 {
266266

267267
fn finalize(self) -> Bytes {
268268
use sha1::Digest;
269-
Bytes::copy_from_slice(self.hasher.finalize().as_slice())
269+
Bytes::copy_from_slice(self.hasher.finalize().as_ref())
270270
}
271271

272272
// Size of the checksum in bytes
@@ -302,7 +302,7 @@ impl Sha256 {
302302

303303
fn finalize(self) -> Bytes {
304304
use sha2::Digest;
305-
Bytes::copy_from_slice(self.hasher.finalize().as_slice())
305+
Bytes::copy_from_slice(self.hasher.finalize().as_ref())
306306
}
307307

308308
// Size of the checksum in bytes
@@ -340,7 +340,7 @@ impl Md5 {
340340
#[warn(dead_code)]
341341
fn finalize(self) -> Bytes {
342342
use md5::Digest;
343-
Bytes::copy_from_slice(self.hasher.finalize().as_slice())
343+
Bytes::copy_from_slice(self.hasher.finalize().as_ref())
344344
}
345345

346346
// Size of the checksum in bytes

0 commit comments

Comments
 (0)