Fix codegen to ignore httpLabel on non-input shapes per Smithy spec by RanVaknin · Pull Request #6789 · aws/aws-sdk-java-v2

RanVaknin · 2026-03-12T21:36:25Z

Context

Per the Smithy spec, httpLabel on non-input shapes "has no meaning and is simply ignored." However, AddShapes.findRequestUri() was throwing a ModelInvalidException when it found a shape with a URI bound member that wasn't a direct operation input.

When the shape isn't a direct operation input, it now returns empty instead of throwing (results in codegen ignoring this member). The existing validation for operations with a missing requestUri is unchanged.

Test coverage matrix

The following table shows whether a combination of location / shape has meaning in codegen or is ignored (member gets serialized in PAYLOAD):

Location	On Input	On Output	On Error	On Nested
URI	pre-existing, honored	added, ignored	added, ignored	added, ignored
QUERY_STRING	pre-existing, honored	added, ignored	added, ignore	added, ignored
HEADER	pre-existing, honored	added, honored	added, honored	added, ignored
HEADERS	same path as HEADER	same path as HEADER	same path as HEADER	added, ignored
STATUS_CODE	added, ignored	added, honored	added, ignored	added, ignored

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

codegen/src/test/java/software/amazon/awssdk/codegen/AddShapesTest.java

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

codegen/src/test/resources/software/amazon/awssdk/codegen/uri-on-non-input-shape-service.json

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

joviegas · 2026-03-18T18:22:45Z

codegen/src/test/java/software/amazon/awssdk/codegen/AddShapesTest.java

        MemberModel requiredMemberModel = requestShapeModel.findMemberModelByC2jName(queryParamName);

        assertThat(requestShapeModel.getRequired()).contains(queryParamName);
-        assertThat(requiredMemberModel.getHttp().getLocation()).isEqualTo(Location.QUERY_STRING);


why are we removing this assertion ?

Because it was asserting the buggy behavior. NestedQueryParameterOperation is not a direct operation input/output/errors.
I should have added an assert null instead (null would result in the default behavior of it being serialized to the payload instead)

joviegas · 2026-03-18T18:26:14Z

.../test/resources/software/amazon/awssdk/codegen/poet/model/nestedqueryparameteroperation.java

        .getter(getter(NestedQueryParameterOperation::queryParamOne))
        .setter(setter(Builder::queryParamOne))
-        .traits(LocationTrait.builder().location(MarshallLocation.QUERY_PARAM).locationName("QueryParamOne").build(),
+        .traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("QueryParamOne").build(),


I see here the MarshallLocation of existing test case is getting changed , any reason on why the existing shape is getting changed ?

Same reason as above, "location":"querystring" on a nested member that is not an input/output/errors should go in the payload.

joviegas · 2026-03-26T08:19:04Z

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

+                      .anyMatch(o -> allC2jShapes.get(o.getOutput().getShape()).equals(shape));
+    }
+
+    private boolean isDirectOperationShape(Shape shape, Map<String, Shape> allC2jShapes) {


Nit: Can we rename this to isDirectInputOutputOrErrorShape

Or maybe just isTopLevelShape?

joviegas

For the changes you discussed offline

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

alextwoods · 2026-03-26T20:48:24Z

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

+                      .anyMatch(o -> allC2jShapes.get(o.getOutput().getShape()).equals(shape));
+    }
+
+    private boolean isDirectOperationShape(Shape shape, Map<String, Shape> allC2jShapes) {


Or maybe just isTopLevelShape?

alextwoods · 2026-03-26T20:50:20Z

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

+    private boolean isDirectInputShape(Shape shape, Map<String, Shape> allC2jShapes) {
+        return builder.getService().getOperations().values().stream()
+                      .filter(o -> o.getInput() != null)
+                      .anyMatch(o -> allC2jShapes.get(o.getInput().getShape()).equals(shape));


This probably isnt a big deal (but I ran into it in the Smithy POC...) - the equals method in shape is just a reference check. That is probably actually what we want here so its fine. But would be an issue if the shape itself was duplicated or something.

codegen/src/test/java/software/amazon/awssdk/codegen/AddShapesTest.java

sonarqubecloud · 2026-03-30T15:29:24Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

RanVaknin requested a review from a team as a code owner March 12, 2026 21:36

RanVaknin added changelog-not-required Indicate changelog entry is not required for a specific PR no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required labels Mar 13, 2026

RanVaknin force-pushed the rvaknin/fix-codegen-locationUri branch from dd6a8b4 to cbb38f4 Compare March 13, 2026 19:49

RanVaknin changed the title ~~Include shape name in REQUEST_URI_NOT_FOUND validation error message~~ Fix codegen to ignore httpLabel on non-input shapes per Smithy spec Mar 13, 2026

joviegas reviewed Mar 13, 2026

View reviewed changes

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java Show resolved Hide resolved

joviegas reviewed Mar 17, 2026

View reviewed changes

joviegas reviewed Mar 18, 2026

View reviewed changes

joviegas approved these changes Mar 19, 2026

View reviewed changes

joviegas reviewed Mar 26, 2026

View reviewed changes

joviegas approved these changes Mar 26, 2026

View reviewed changes

RanVaknin added this pull request to the merge queue Mar 26, 2026

RanVaknin removed this pull request from the merge queue due to a manual request Mar 26, 2026

joviegas requested changes Mar 26, 2026

View reviewed changes

alextwoods reviewed Mar 26, 2026

View reviewed changes

RanVaknin added 13 commits March 26, 2026 15:01

Include shape name in REQUEST_URI_NOT_FOUND validation error message

e5416e9

Fix codegen logic to allow non input shape uri bound member

0c9d3ed

Fix logic to ignore instead of throw

ca5ab90

Fix javadoc

00c382e

Ignore HTTP binding locations on non-input shapes per Smithy spec

d3f7a4d

Account for error shapes as well

47ebbad

Fix comments

982c247

Fix comments 2

675d1d5

Add assertion

fd9411d

Add protocol test

f2a1388

Refadctor to remove cache complexity, add coverage

114c432

Corrected logic and added test for locationName

795acc6

Fix comments

67931ac

RanVaknin force-pushed the rvaknin/fix-codegen-locationUri branch from d7b62c3 to 67931ac Compare March 26, 2026 22:01

Update fixture file

459008d

Conversation

RanVaknin commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Test coverage matrix

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joviegas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 30, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RanVaknin commented Mar 12, 2026 •

edited

Loading