Fail to build for services with unsupported payloads

Author: spfink

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

@@ -19,11 +19,13 @@
 import static software.amazon.awssdk.codegen.RemoveUnusedShapes.removeUnusedShapes;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.codegen.customization.CodegenCustomizationProcessor;
@@ -53,6 +55,8 @@
  */
 public class IntermediateModelBuilder {
 
+    private static final List<String> NO_BODY_METHODS = Arrays.asList("GET", "HEAD", "DELETE");
+
     private static final Logger log = LoggerFactory.getLogger(IntermediateModelBuilder.class);
     private final CustomizationConfig customConfig;
     private final ServiceModel service;
@@ -157,9 +161,28 @@ public IntermediateModel build() {
 
         setSimpleMethods(trimmedModel);
 
+        validateOperations(trimmedModel);
+
         return trimmedModel;
     }
 
+    /**
+     * Validates that operations that use GET, DELETE or HEAD do not have an HTTP body.
+     */
+    private void validateOperations(IntermediateModel model) {
+        List<String> methods = model.getOperations()
+                                    .entrySet()
+                                    .stream()
+                                    .filter(e -> e.getValue().getInputShape().hasPayloadMembers())
+                                    .filter(e -> NO_BODY_METHODS.contains(e.getValue().getInputShape().getMarshaller().getVerb()))
+                                    .map(e -> e.getValue().getInputShape().getC2jName())
+                                    .collect(Collectors.toList());
+
+        if (methods.size() > 0) {
+            throw new RuntimeException("An HTTP body is not allowed on GET/DELETE/HEAD requests. Invalid operations: " + methods);
+        }
+    }
+
     /**
      * Link the member to it's corresponding shape (if it exists).
      *

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

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+package software.amazon.awssdk.codegen;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import org.junit.Test;
+import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
+import software.amazon.awssdk.codegen.model.service.ServiceModel;
+import software.amazon.awssdk.codegen.utils.ModelLoaderUtils;
+
+public class IntermediateModelBuilderTest {
+
+    @Test(expected = RuntimeException.class)
+    public void modelWithUnsupportedPayloadMethods_ThrowsException() {
+        File serviceModel = new File(getClass().getResource("poet/client/c2j/unsupportedpayloads/service-2.json").getFile());
+        File customizationModel = new File(getClass().getResource("poet/client/c2j/unsupportedpayloads/customization.config").getFile());
+
+        C2jModels models = C2jModels.builder()
+                                    .serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, serviceModel))
+                                    .customizationConfig(ModelLoaderUtils.loadModel(CustomizationConfig.class, customizationModel))
+                                    .build();
+
+        try {
+            new IntermediateModelBuilder(models).build();
+        } catch (RuntimeException e) {
+            assertTrue(e.getMessage().contains("GetBlacklistReportsRequest"));
+            assertTrue(e.getMessage().contains("GetDedicatedIpsRequest"));
+            throw e;
+        }
+
+        fail("Expected RuntimeException");
+    }
+
+}

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/unsupportedpayloads/customization.config

@@ -0,0 +1,5 @@
+{
+    "authPolicyActions" : {
+        "skip" : true
+    }
+}

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/unsupportedpayloads/service-2.json

@@ -32,8 +32,6 @@ public interface StructuredJsonGenerator {
      */
     StructuredJsonGenerator NO_OP = new StructuredJsonGenerator() {
 
-        private final byte[] emptyBytes = new byte[0];
-
         @Override
         public StructuredJsonGenerator writeStartArray() {
             return this;
@@ -126,7 +124,7 @@ public StructuredJsonGenerator writeNumber(String number) {
 
         @Override
         public byte[] getBytes() {
-            return emptyBytes;
+            return null;
         }
 
         @Override

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/StructuredJsonGenerator.java

@@ -196,9 +196,12 @@ private SdkHttpFullRequest finishMarshalling() {
             }
 
             byte[] content = jsonGenerator.getBytes();
-            request.contentStreamProvider(() -> new ByteArrayInputStream(content));
-            if (content.length > 0) {
-                request.putHeader(CONTENT_LENGTH, Integer.toString(content.length));
+
+            if (content != null) {
+                request.contentStreamProvider(() -> new ByteArrayInputStream(content));
+                if (content.length > 0) {
+                    request.putHeader(CONTENT_LENGTH, Integer.toString(content.length));
+                }
             }
         }
 

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/marshall/JsonProtocolMarshaller.java

@@ -15,10 +15,17 @@
 
 package software.amazon.awssdk.core.client.handler;
 
+import static software.amazon.awssdk.http.SdkHttpMethod.DELETE;
+import static software.amazon.awssdk.http.SdkHttpMethod.GET;
+import static software.amazon.awssdk.http.SdkHttpMethod.HEAD;
+
 import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.core.SdkRequest;
 import software.amazon.awssdk.core.SdkResponse;
+import software.amazon.awssdk.core.SdkStandardLogger;
 import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
 import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 import software.amazon.awssdk.core.client.config.SdkClientOption;
@@ -29,10 +36,14 @@
 import software.amazon.awssdk.core.interceptor.InterceptorContext;
 import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
 import software.amazon.awssdk.http.SdkHttpFullRequest;
+import software.amazon.awssdk.http.SdkHttpMethod;
 import software.amazon.awssdk.utils.StringUtils;
 
 @SdkProtectedApi
 public abstract class BaseClientHandler {
+
+    private static final List<SdkHttpMethod> NO_BODY_METHODS = Arrays.asList(GET, DELETE, HEAD);
+
     private SdkClientConfiguration clientConfiguration;
 
     protected BaseClientHandler(SdkClientConfiguration clientConfiguration) {
@@ -64,6 +75,7 @@ static <InputT extends SdkRequest, OutputT> InterceptorContext finalizeSdkHttpFu
         request = modifyEndpointHostIfNeeded(request, clientConfiguration, executionParams);
 
         addHttpRequest(executionContext, request);
+        warnOnUnsupportedRequests(request);
         runAfterMarshallingInterceptors(executionContext);
         return runModifyHttpRequestAndHttpContentInterceptors(executionContext);
     }
@@ -108,6 +120,13 @@ private static SdkHttpFullRequest modifyEndpointHostIfNeeded(SdkHttpFullRequest
                               .build();
     }
 
+    private static void warnOnUnsupportedRequests(SdkHttpFullRequest request) {
+        if (NO_BODY_METHODS.contains(request.method()) && request.contentStreamProvider().isPresent()) {
+            SdkStandardLogger.REQUEST_LOGGER.warn(() -> NO_BODY_METHODS +
+                                                        " requests with bodies are not supported by the default SDK clients.");
+        }
+    }
+
     private static void addHttpRequest(ExecutionContext executionContext, SdkHttpFullRequest request) {
         InterceptorContext interceptorContext = executionContext.interceptorContext().copy(b -> b.httpRequest(request));
         executionContext.interceptorContext(interceptorContext);

core/sdk-core/src/main/java/software/amazon/awssdk/core/client/handler/BaseClientHandler.java

@@ -43,14 +43,6 @@
       "input":{"shape":"FurtherNestedContainersStructure"},
       "output":{"shape":"FurtherNestedContainersStructure"}
     },
-    "GetOperationWithBody":{
-      "name":"GetOperationWithBody",
-      "http":{
-        "method":"GET",
-        "requestUri":"/2016-03-11/getOperationWithBody"
-      },
-      "input":{"shape":"GetOperationWithBodyInput"}
-    },
     "HeadOperation":{
       "name":"HeadOperation",
       "http":{
@@ -303,12 +295,6 @@
         "ListOfNested":{"shape":"ListOfNested"}
       }
     },
-    "GetOperationWithBodyInput":{
-      "type":"structure",
-      "members":{
-        "StringMember":{"shape":"String"}
-      }
-    },
     "IdempotentOperationStructure":{
       "type":"structure",
       "required":["PathIdempotentToken"],