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

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

@@ -22,9 +22,11 @@
 import static software.amazon.awssdk.codegen.internal.Utils.isScalar;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import software.amazon.awssdk.codegen.internal.TypeUtils;
 import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
 import software.amazon.awssdk.codegen.model.intermediate.EnumModel;
@@ -36,6 +38,7 @@
 import software.amazon.awssdk.codegen.model.intermediate.ReturnTypeModel;
 import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
 import software.amazon.awssdk.codegen.model.intermediate.VariableModel;
+import software.amazon.awssdk.codegen.model.service.ErrorMap;
 import software.amazon.awssdk.codegen.model.service.Location;
 import software.amazon.awssdk.codegen.model.service.Member;
 import software.amazon.awssdk.codegen.model.service.Operation;
@@ -54,6 +57,7 @@
 
     private final IntermediateModelBuilder builder;
     private final NamingStrategy namingStrategy;
+    private Set<Shape> directOperationShapes;
 
     AddShapes(IntermediateModelBuilder builder) {
         this.builder = builder;
@@ -311,8 +315,13 @@
 
         ParameterHttpMapping mapping = new ParameterHttpMapping();
 
+        // https://smithy.io/2.0/spec/http-bindings.html
+        Location location = isDirectOperationShape(parentShape, allC2jShapes)
+                            ? Location.forValue(member.getLocation())
+                            : null;
+
         Shape memberShape = allC2jShapes.get(member.getShape());
-        mapping.withLocation(Location.forValue(member.getLocation()))
+        mapping.withLocation(location)
                .withPayload(member.isPayload()).withStreaming(member.isStreaming())
                .withFlattened(isFlattened(member, memberShape))
                .withUnmarshallLocationName(deriveUnmarshallerLocationName(memberShape, memberName, member))
@@ -323,6 +332,26 @@
         return mapping;
     }
 
+    private boolean isDirectOperationShape(Shape parentShape, Map<String, Shape> allC2jShapes) {
+        if (directOperationShapes == null) {
+            directOperationShapes = new HashSet<>();
+            for (Operation operation : builder.getService().getOperations().values()) {
+                if (operation.getInput() != null) {
+                    directOperationShapes.add(allC2jShapes.get(operation.getInput().getShape()));
+                }
+                if (operation.getOutput() != null) {
+                    directOperationShapes.add(allC2jShapes.get(operation.getOutput().getShape()));
+                }
+                if (operation.getErrors() != null) {
+                    for (ErrorMap error : operation.getErrors()) {
+                        directOperationShapes.add(allC2jShapes.get(error.getShape()));
+                    }
+                }
+            }
+        }
+        return directOperationShapes.contains(parentShape);
+    }
+
     private boolean isFlattened(Member member, Shape memberShape) {
         return member.isFlattened()
                || memberShape.isFlattened();
@@ -342,38 +371,52 @@
      */
     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 = "Could not find request URI for input shape '" + shapeName
+                + "'. No operation was found that references this shape as its input.";
+            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,23 @@ 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);
+    }
+
 }

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,11 +44,15 @@
 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.MemberModel;
+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;
 import software.amazon.awssdk.codegen.validation.ModelValidator;
+import software.amazon.awssdk.codegen.validation.ValidationEntry;
 import software.amazon.awssdk.codegen.validation.ValidationErrorId;
 
 public class CodeGeneratorTest {
@@ -176,6 +181,41 @@
             });
     }
 
+    @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()
@@ -244,6 +284,11 @@
         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)) {