Fix codegen to ignore httpLabel on non-input shapes per Smithy spec

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

@@ -311,18 +311,67 @@ private ParameterHttpMapping generateParameterHttpMapping(Shape parentShape,
 
         ParameterHttpMapping mapping = new ParameterHttpMapping();
 
+        // Per the Smithy spec, HTTP binding traits are only honored on top-level shapes (direct operation
+        // input/output/error). When a location trait is ignored, its locationName is also ignored so the member
+        // name is used as the wire name. https://smithy.io/2.0/spec/http-bindings.html
+        Location resolvedLocation = resolveLocation(parentShape, member, allC2jShapes);
+        boolean locationIgnored = member.getLocation() != null && resolvedLocation == null;
+
         Shape memberShape = allC2jShapes.get(member.getShape());
-        mapping.withLocation(Location.forValue(member.getLocation()))
+        String marshallLocationName = locationIgnored
+            ? memberName : deriveMarshallerLocationName(memberShape, memberName, member, protocol);
+        String unmarshallLocationName = locationIgnored
+            ? memberName : deriveUnmarshallerLocationName(memberShape, memberName, member);
+
+        mapping.withLocation(resolvedLocation)
                .withPayload(member.isPayload()).withStreaming(member.isStreaming())
                .withFlattened(isFlattened(member, memberShape))
-               .withUnmarshallLocationName(deriveUnmarshallerLocationName(memberShape, memberName, member))
-               .withMarshallLocationName(
-                        deriveMarshallerLocationName(memberShape, memberName, member, protocol))
+               .withUnmarshallLocationName(unmarshallLocationName)
+               .withMarshallLocationName(marshallLocationName)
                .withIsGreedy(isGreedy(parentShape, allC2jShapes, mapping));
 
         return mapping;
     }
 
+    private Location resolveLocation(Shape parentShape, Member member, Map<String, Shape> allC2jShapes) {
+        Location location = Location.forValue(member.getLocation());
+        if (location == null) {
+            return null;
+        }
+        switch (location) {
+            case URI:
+            case QUERY_STRING:
+                return isDirectInputShape(parentShape, allC2jShapes) ? location : null;
+            case HEADER:
+            case HEADERS:
+                return isTopLevelShape(parentShape, allC2jShapes) ? location : null;
+            case STATUS_CODE:
+                return isDirectOutputShape(parentShape, allC2jShapes) ? location : null;
+            default:
+                return location;
+        }
+    }
+
+    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));
+    }
+
+    private boolean isDirectOutputShape(Shape shape, Map<String, Shape> allC2jShapes) {
+        return builder.getService().getOperations().values().stream()
+                      .filter(o -> o.getOutput() != null)
+                      .anyMatch(o -> allC2jShapes.get(o.getOutput().getShape()).equals(shape));
+    }
+
+    private boolean isTopLevelShape(Shape shape, Map<String, Shape> allC2jShapes) {
+        return builder.getService().getOperations().values().stream()
+                      .anyMatch(o -> (o.getInput() != null && allC2jShapes.get(o.getInput().getShape()).equals(shape))
+                                     || (o.getOutput() != null && allC2jShapes.get(o.getOutput().getShape()).equals(shape))
+                                     || (o.getErrors() != null && o.getErrors().stream()
+                                             .anyMatch(e -> allC2jShapes.get(e.getShape()).equals(shape))));
+    }
+
     private boolean isFlattened(Member member, Shape memberShape) {
         return member.isFlattened()
                || memberShape.isFlattened();
@@ -342,38 +391,52 @@ private boolean isRequiredMember(String memberName, Shape memberShape) {
      */
     private boolean isGreedy(Shape parentShape, Map<String, Shape> allC2jShapes, ParameterHttpMapping mapping) {
         if (mapping.getLocation() == Location.URI) {
-            // If the location is URI we can assume the parent shape is an input shape.
-            String requestUri = findRequestUri(parentShape, allC2jShapes);
-            if (requestUri.contains(String.format("{%s+}", mapping.getMarshallLocationName()))) {
+            Optional<String> requestUri = findRequestUri(parentShape, allC2jShapes);
+            if (requestUri.isPresent()
+                && requestUri.get().contains(String.format("{%s+}", mapping.getMarshallLocationName()))) {
                 return true;
             }
         }
         return false;
     }
 
     /**
-     * Given an input shape, finds the Request URI for the operation that input is referenced from.
+     * Given a shape, finds the Request URI for the operation that references it as input.
+     * Returns empty if the shape is not a direct operation input.
      *
-     * @param parentShape  Input shape to find operation's request URI for.
+     * @param parentShape  Shape to find operation's request URI for.
      * @param allC2jShapes All shapes in the service model.
-     * @return Request URI for operation.
-     * @throws RuntimeException If operation can't be found.
+     * @return Request URI for operation, or empty if the shape is not a direct operation input.
      */
-    private String findRequestUri(Shape parentShape, Map<String, Shape> allC2jShapes) {
+    private Optional<String> findRequestUri(Shape parentShape, Map<String, Shape> allC2jShapes) {
         Optional<Operation> operation = builder.getService().getOperations().values().stream()
                                                .filter(o -> o.getInput() != null)
                                                .filter(o -> allC2jShapes.get(o.getInput().getShape()).equals(parentShape))
                                                .findFirst();
 
-        return operation.map(o -> o.getHttp().getRequestUri())
-                        .orElseThrow(() -> {
-                            String detailMsg = "Could not find request URI for input shape for operation: " + operation;
-                            ValidationEntry entry =
-                                new ValidationEntry().withErrorId(ValidationErrorId.REQUEST_URI_NOT_FOUND)
-                                                     .withDetailMessage(detailMsg)
-                                                     .withSeverity(ValidationErrorSeverity.DANGER);
-                            return ModelInvalidException.builder().validationEntries(Collections.singletonList(entry)).build();
-                        });
+        if (!operation.isPresent()) {
+            // Not a direct operation input shape, should be ignored.
+            // https://smithy.io/2.0/spec/http-bindings.html#httplabel-is-only-used-on-top-level-input
+            return Optional.empty();
+        }
+
+        String requestUri = operation.get().getHttp().getRequestUri();
+        if (requestUri == null) {
+            String shapeName = allC2jShapes.entrySet().stream()
+                .filter(e -> e.getValue().equals(parentShape))
+                .map(Map.Entry::getKey)
+                .findFirst()
+                .orElseThrow(() -> new IllegalStateException("Shape not found in model: " + parentShape));
+            String detailMsg = "Operation referencing input shape '" + shapeName
+                + "' has no requestUri configured in its HTTP binding.";
+            ValidationEntry entry =
+                new ValidationEntry().withErrorId(ValidationErrorId.REQUEST_URI_NOT_FOUND)
+                                     .withDetailMessage(detailMsg)
+                                     .withSeverity(ValidationErrorSeverity.DANGER);
+            throw ModelInvalidException.builder().validationEntries(Collections.singletonList(entry)).build();
+        }
+
+        return Optional.of(requestUri);
     }
 
     private String deriveUnmarshallerLocationName(Shape memberShape, String memberName, Member member) {

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

@@ -84,8 +84,37 @@ void generateShapeModel_memberRequiredByNestedShape_setsMemberModelAsRequired()
         MemberModel requiredMemberModel = requestShapeModel.findMemberModelByC2jName(queryParamName);
 
         assertThat(requestShapeModel.getRequired()).contains(queryParamName);
-        assertThat(requiredMemberModel.getHttp().getLocation()).isEqualTo(Location.QUERY_STRING);
+        assertThat(requiredMemberModel.getHttp().getLocation()).isNull();
         assertThat(requiredMemberModel.isRequired()).isTrue();
     }
 
+    @Test
+    void generateShapeModel_locationOnNestedShape_isIgnored() {
+        ShapeModel nestedShape = intermediateModel.getShapes().get("NestedQueryParameterOperation");
+        MemberModel queryParam = nestedShape.findMemberModelByC2jName("QueryParamOne");
+        assertThat(queryParam.getHttp().getLocation()).isNull();
+    }
+
+    @Test
+    void generateShapeModel_locationOnDirectInputShape_isPreserved() {
+        ShapeModel inputShape = intermediateModel.getShapes().get("QueryParameterOperationRequest");
+        assertThat(inputShape.findMemberModelByC2jName("PathParam").getHttp().getLocation()).isEqualTo(Location.URI);
+        assertThat(inputShape.findMemberModelByC2jName("QueryParamOne").getHttp().getLocation()).isEqualTo(Location.QUERY_STRING);
+        assertThat(inputShape.findMemberModelByC2jName("StringHeaderMember").getHttp().getLocation()).isEqualTo(Location.HEADER);
+    }
+
+    @Test
+    void generateShapeModel_locationNameOnNestedShape_usesMemberNameForMarshalling() {
+        ShapeModel inputShape = intermediateModel.getShapes().get("NestedQueryParameterOperation");
+        assertThat(inputShape.findMemberModelByC2jName("NestedHeaderMember").getHttp().getMarshallLocationName()).isEqualTo("NestedHeaderMember");
+    }
+
+    @Test
+    void generateShapeModel_locationNameOnTopLevelShape_honorsLocationName() {
+        ShapeModel inputShape = intermediateModel.getShapes().get("QueryParameterOperationRequest");
+        MemberModel member = inputShape.findMemberModelByC2jName("PayloadMemberWithCustomName");
+        assertThat(member.getHttp().getLocation()).isNull();
+        assertThat(member.getHttp().getMarshallLocationName()).isEqualTo("CustomWireName");
+    }
+
 }

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

@@ -16,6 +16,7 @@
 package software.amazon.awssdk.codegen;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
@@ -43,7 +44,9 @@
 import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
 import software.amazon.awssdk.codegen.model.config.customization.UnderscoresInNameBehavior;
 import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
+import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
 import software.amazon.awssdk.codegen.model.rules.endpoints.EndpointTestSuiteModel;
+import software.amazon.awssdk.codegen.model.service.Location;
 import software.amazon.awssdk.codegen.model.service.ServiceModel;
 import software.amazon.awssdk.codegen.poet.ClientTestModels;
 import software.amazon.awssdk.codegen.validation.ModelInvalidException;
@@ -176,6 +179,41 @@ void execute_endpointsTestReferencesUnknownOperationMember_throwsValidationError
             });
     }
 
+    @Test
+    void execute_uriLocationOnNonInputShape_isIgnored() throws IOException {
+        C2jModels models = C2jModels.builder()
+                                    .customizationConfig(CustomizationConfig.create())
+                                    .serviceModel(getUriOnNonInputShapeServiceModel())
+                                    .build();
+
+        // Per the Smithy spec, httpLabel on non-input shapes has no meaning and is simply ignored.
+        assertThatNoException().isThrownBy(
+            () -> generateCodeFromC2jModels(models, outputDir, true, Collections.emptyList()));
+
+        IntermediateModel intermediateModel = new IntermediateModelBuilder(models).build();
+
+        ShapeModel inputShape = intermediateModel.getShapes().get("SomeOperationRequest");
+        assertThat(inputShape.findMemberModelByC2jName("thingId").getHttp().getLocation()).isEqualTo(Location.URI);
+
+        ShapeModel nestedShape = intermediateModel.getShapes().get("NestedOptions");
+        assertThat(nestedShape.findMemberModelByC2jName("pageSize").getHttp().getLocation()).isNull();
+        assertThat(nestedShape.findMemberModelByC2jName("pageSize").getHttp().isGreedy()).isFalse();
+        assertThat(nestedShape.findMemberModelByC2jName("headerParam").getHttp().getLocation()).isNull();
+        assertThat(nestedShape.findMemberModelByC2jName("queryParam").getHttp().getLocation()).isNull();
+        assertThat(nestedShape.findMemberModelByC2jName("prefixHeaders").getHttp().getLocation()).isNull();
+
+        ShapeModel sharedShape = intermediateModel.getShapes().get("SharedShapeOperationRequest");
+        assertThat(sharedShape.findMemberModelByC2jName("sharedId").getHttp().getLocation()).isEqualTo(Location.URI);
+
+        Path generatedNestedOptions = Files.walk(outputDir)
+            .filter(p -> p.getFileName().toString().equals("NestedOptions.java"))
+            .findFirst()
+            .orElseThrow(() -> new AssertionError("NestedOptions.java not found in generated output"));
+        String actual = new String(Files.readAllBytes(generatedNestedOptions), StandardCharsets.UTF_8);
+        String expected = resourceAsString("expected-nested-options.java");
+        assertThat(actual).isEqualTo(expected);
+    }
+
     @Test
     void execute_operationHasNoRequestUri_throwsValidationError() throws IOException {
         C2jModels models = C2jModels.builder()
@@ -186,7 +224,10 @@ void execute_operationHasNoRequestUri_throwsValidationError() throws IOException
         assertThatThrownBy(() -> generateCodeFromC2jModels(models, outputDir, true, Collections.emptyList()))
             .isInstanceOf(ModelInvalidException.class)
             .matches(e -> ((ModelInvalidException) e).validationEntries().get(0).getErrorId()
-                          == ValidationErrorId.REQUEST_URI_NOT_FOUND);
+                          == ValidationErrorId.REQUEST_URI_NOT_FOUND)
+            .matches(e -> ((ModelInvalidException) e).validationEntries().get(0).getDetailMessage()
+                          .equals("Operation referencing input shape 'OperationWithUriMappedParamRequest'"
+                                  + " has no requestUri configured in its HTTP binding."));
     }
 
     private void generateCodeFromC2jModels(C2jModels c2jModels, Path outputDir) {
@@ -244,6 +285,11 @@ private ServiceModel getMissingRequestUriServiceModel() throws IOException {
         return Jackson.load(ServiceModel.class, json);
     }
 
+    private ServiceModel getUriOnNonInputShapeServiceModel() throws IOException {
+        String json = resourceAsString("uri-on-non-input-shape-service.json");
+        return Jackson.load(ServiceModel.class, json);
+    }
+
     private String resourceAsString(String name) throws IOException {
         ByteArrayOutputStream baos;
         try (InputStream resourceAsStream = getClass().getResourceAsStream(name)) {