Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changelog/1760126889.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
applies_to:
- client
authors:
- rcoh
references: [ "smithy-rs#4346" ]
breaking: false
new_feature: false
bug_fix: true
---

Fix bug where httpQueryParams were silently dropped when no other query parameters were modeled.
49 changes: 49 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,55 @@

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

## Protocol Tests

Protocol tests validate that generated code correctly implements Smithy protocols (like restJson1, awsJson1_1, etc.).

### Adding Protocol Tests

Protocol tests are defined in Smithy model files using `@httpRequestTests` and `@httpResponseTests` traits:

```smithy
@http(uri: "/my-operation", method: "GET")
@httpRequestTests([
{
id: "MyOperationRequest",
documentation: "Test description",
protocol: "aws.protocols#restJson1",
method: "GET",
uri: "/my-operation",
queryParams: ["param1=value1", "param2=value2"],
params: {
queryMap: {
"param1": "value1",
"param2": "value2"
}
},
appliesTo: "client",
}
])
operation MyOperation {
input: MyOperationInput,
}
```

### Key Protocol Test Locations

- **`codegen-core/common-test-models/rest-json-extras.smithy`** - restJson1 protocol tests
- **`codegen-core/common-test-models/constraints.smithy`** - Constraint validation tests with restJson1
- **`codegen-client-test/model/main.smithy`** - awsJson1_1 protocol tests

### httpQueryParams Bug Investigation

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:

1. An operation had ONLY `@httpQueryParams` (no regular `@httpQuery` parameters)
2. The condition `if (dynamicParams.isEmpty() && literalParams.isEmpty() && mapParams.isEmpty())` would skip generating the `uri_query` function

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.

**Testing httpQueryParams**: Create operations with only `@httpQueryParams` to ensure they generate proper query strings in requests.

## preludeScope: Rust Prelude Types

**Always use `preludeScope` for Rust prelude types:**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class RequestBindingGenerator(
val dynamicParams = index.getRequestBindings(operationShape, HttpBinding.Location.QUERY)
val mapParams = index.getRequestBindings(operationShape, HttpBinding.Location.QUERY_PARAMS)
val literalParams = httpTrait.uri.queryLiterals
if (dynamicParams.isEmpty() && literalParams.isEmpty()) {
if (dynamicParams.isEmpty() && literalParams.isEmpty() && mapParams.isEmpty()) {
return false
}
val preloadedParams = literalParams.keys + dynamicParams.map { it.locationName }
Expand Down
44 changes: 44 additions & 0 deletions codegen-core/common-test-models/rest-json-extras.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ service RestJsonExtras {
CaseInsensitiveErrorOperation,
EmptyStructWithContentOnWireOp,
QueryPrecedence,
HttpQueryParamsOnlyOperation,
],
errors: [ExtraError]
}
Expand Down Expand Up @@ -350,3 +351,46 @@ structure EmptyStructWithContentOnWireOpOutput {
operation EmptyStructWithContentOnWireOp {
output: EmptyStructWithContentOnWireOpOutput,
}

@http(uri: "/http-query-params-only", method: "GET")
@httpRequestTests([
{
id: "HttpQueryParamsOnlyRequest",
documentation: "Test that httpQueryParams are included in request when no other query parameters exist",
protocol: "aws.protocols#restJson1",
method: "GET",
uri: "/http-query-params-only",
queryParams: ["shouldExpandRoles=true", "shouldShowOnlyAuthForThisDocument=false"],
params: {
queryMap: {
"shouldExpandRoles": "true",
"shouldShowOnlyAuthForThisDocument": "false"
}
},
appliesTo: "client",
},
{
id: "HttpQueryParamsOnlyEmptyRequest",
documentation: "Test that empty httpQueryParams map results in no query parameters",
protocol: "aws.protocols#restJson1",
method: "GET",
uri: "/http-query-params-only",
params: {
queryMap: {}
},
appliesTo: "client",
}
])
operation HttpQueryParamsOnlyOperation {
input: HttpQueryParamsOnlyInput,
}

structure HttpQueryParamsOnlyInput {
@httpQueryParams
queryMap: QueryMap,
}

map QueryMap {
key: String,
value: String,
}
26 changes: 13 additions & 13 deletions codegen-server-test/integration-tests/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading