fix(core/protocols): rest-json default request body conditions

Author: kuhe

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddS3Config.java

@@ -61,7 +61,6 @@
 import software.amazon.smithy.typescript.codegen.auth.http.integration.AddHttpSigningPlugin;
 import software.amazon.smithy.typescript.codegen.integration.RuntimeClientPlugin;
 import software.amazon.smithy.typescript.codegen.integration.TypeScriptIntegration;
-import software.amazon.smithy.typescript.codegen.schema.SchemaGenerationAllowlist;
 import software.amazon.smithy.utils.ListUtils;
 import software.amazon.smithy.utils.MapUtils;
 import software.amazon.smithy.utils.SetUtils;

codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AwsProtocolUtils.java

@@ -96,9 +96,9 @@ static boolean includeUnsignedPayloadSigV4Header(OperationShape operation) {
      * @param visitor A ShapeVisitor that generates a serde function for shapes.
      */
     static void generateDocumentBodyShapeSerde(
-            GenerationContext context,
-            Set<Shape> shapes,
-            ShapeVisitor<Void> visitor
+        GenerationContext context,
+        Set<Shape> shapes,
+        ShapeVisitor<Void> visitor
     ) {
         // Walk all the shapes within those in the document and generate for them as well.
         Walker shapeWalker = new Walker(NeighborProviderIndex.of(context.getModel()).getProvider());
@@ -123,7 +123,7 @@ static void generateJsonParseBodyWithQueryHeader(GenerationContext context) {
         TypeScriptWriter writer = context.getWriter();
         writer.addImport("HeaderBag", "__HeaderBag", TypeScriptDependency.SMITHY_TYPES);
         writer.write(IoUtils.readUtf8Resource(
-                AwsProtocolUtils.class, "populate-body-with-query-compatibility-code-stub.ts"));
+            AwsProtocolUtils.class, "populate-body-with-query-compatibility-code-stub.ts"));
     }
 
     /**
@@ -176,9 +176,9 @@ static void generateBuildFormUrlencodedString(GenerationContext context) {
         writer.addImport("extendedEncodeURIComponent", "__extendedEncodeURIComponent",
             TypeScriptDependency.AWS_SMITHY_CLIENT);
         writer.openBlock("const buildFormUrlencodedString = (formEntries: Record<string, string>): "
-                + "string => Object.entries(formEntries).map(", ").join(\"&\");",
-                () -> writer.write("([key, value]) => __extendedEncodeURIComponent(key) + '=' + "
-                    + "__extendedEncodeURIComponent(value)"));
+                         + "string => Object.entries(formEntries).map(", ").join(\"&\");",
+            () -> writer.write("([key, value]) => __extendedEncodeURIComponent(key) + '=' + "
+                               + "__extendedEncodeURIComponent(value)"));
         writer.write("");
     }
 
@@ -251,7 +251,7 @@ static void writeIdempotencyAutofill(GenerationContext context, MemberShape memb
             TypeScriptWriter writer = context.getWriter();
             writer.addImport("v4", "generateIdempotencyToken", TypeScriptDependency.SMITHY_UUID);
             writer.openBlock("if ($L === undefined) {", "}", inputLocation, () ->
-                    writer.write("$L = generateIdempotencyToken();", inputLocation));
+                writer.write("$L = generateIdempotencyToken();", inputLocation));
         }
     }
 
@@ -266,10 +266,10 @@ static void writeIdempotencyAutofill(GenerationContext context, MemberShape memb
      * @return A string representing the proper value provider for this timestamp.
      */
     static String getInputTimestampValueProvider(
-            GenerationContext context,
-            MemberShape memberShape,
-            Format defaultFormat,
-            String inputLocation
+        GenerationContext context,
+        MemberShape memberShape,
+        Format defaultFormat,
+        String inputLocation
     ) {
         HttpBindingIndex httpIndex = HttpBindingIndex.of(context.getModel());
         TimestampFormatTrait.Format format = httpIndex.determineTimestampFormat(memberShape, DOCUMENT, defaultFormat);
@@ -278,9 +278,9 @@ static String getInputTimestampValueProvider(
 
     static void generateProtocolTests(ProtocolGenerator generator, GenerationContext context) {
         new HttpProtocolTestGenerator(context,
-                generator,
-                AwsProtocolUtils::filterProtocolTests,
-                AwsProtocolUtils::filterMalformedRequestTests).run();
+            generator,
+            AwsProtocolUtils::filterProtocolTests,
+            AwsProtocolUtils::filterMalformedRequestTests).run();
     }
 
     /**
@@ -307,28 +307,16 @@ static Map<String, TreeSet<String>> getErrorAliases(GenerationContext context,
     }
 
     private static boolean filterProtocolTests(
-            ServiceShape service,
-            OperationShape operation,
-            HttpMessageTestCase testCase,
-            TypeScriptSettings settings
+        ServiceShape service,
+        OperationShape operation,
+        HttpMessageTestCase testCase,
+        TypeScriptSettings settings
     ) {
-        // TODO: Remove when upstream tests update to serialize empty headers.
         if (testCase.getId().contains("NullAndEmptyHeaders")) {
-            return true;
-        }
-
-        // TODO This test has an arbitrary root XML name NestedXmlMapWithXmlNameRequest
-        // TODO which doesn't match the input structure. We will need to update
-        // TODO the test comparator stub to ignore this if that is indeed intended.
-        if (testCase.getId().contains("NestedXmlMapWithXmlNameSerializes")) {
-            return true;
-        }
-
-        // TODO: remove when Glacier AccountID hyphen customization is implemented: SMITHY-2614
-        if (testCase.getId().equals("GlacierAccountId")) {
-            return true;
+            return settings.generateServerSdk();
         }
 
+        // JSv3 does not populate default values on the client side
         if (testCase.getTags().contains("defaults")) {
             return true;
         }
@@ -352,10 +340,10 @@ private static boolean filterProtocolTests(
     }
 
     private static boolean filterMalformedRequestTests(
-            ServiceShape service,
-            OperationShape operation,
-            HttpMalformedRequestTestCase testCase,
-            TypeScriptSettings settings
+        ServiceShape service,
+        OperationShape operation,
+        HttpMalformedRequestTestCase testCase,
+        TypeScriptSettings settings
     ) {
         // Handling overflow/underflow of longs in JS is extraordinarily tricky.
         // Numbers are actually all 62-bit floats, and so any integral number is
@@ -374,9 +362,8 @@ private static boolean filterMalformedRequestTests(
             return true;
         }
 
-        // TODO: fix in https://github.com/aws/aws-sdk-js-v3/issues/5545
         if (testCase.getId().equals("RestJsonMalformedUnionUnknownMember")) {
-            return true;
+            return settings.generateServerSdk();
         }
 
         //TODO: reenable when the SSDK uses RE2 and not built-in regex for pattern constraints
@@ -405,15 +392,13 @@ private static boolean filterMalformedRequestTests(
             return true;
         }
 
-        // ToDo: https://github.com/aws/aws-sdk-js-v3/issues/6246
         if (testCase.getId().equals("RestJsonStringPayloadNoContentType")
             || testCase.getId().equals("RestJsonWithBodyExpectsApplicationJsonContentTypeNoHeaders")) {
-            return true;
+            return settings.generateServerSdk();
         }
 
-        // ToDo: https://github.com/aws/aws-sdk-js-v3/issues/6907
         if (testCase.getId().equals("RestJsonWithoutBodyEmptyInputExpectsEmptyContentType")) {
-            return true;
+            return settings.generateServerSdk();
         }
 
         return false;

packages/core/src/submodules/protocols/json/AwsRestJsonProtocol.spec.ts

@@ -312,5 +312,64 @@ describe(AwsRestJsonProtocol.name, () => {
         "query-default-date": "1970-01-01T00:00:00Z",
       });
     });
+
+    it("inserts a default body only when the payload is a structure shape", async () => {
+      const operation = {
+        namespace: "ns",
+        name: "Operation",
+        traits: 0,
+        input: [
+          3,
+          "ns",
+          "Input",
+          0,
+          ["payload", "name"],
+          [
+            [21 satisfies BlobSchema, { httpPayload: 1 }],
+            [0 satisfies StringSchema, { httpHeader: "name" }],
+          ],
+        ] satisfies StaticStructureSchema,
+        output: "unit" as const,
+      };
+
+      for (const body of ["", false, undefined, 0, new Uint8Array()]) {
+        const request = await protocol.serializeRequest(
+          operation,
+          {
+            name: "hi",
+            payload: body,
+          },
+          context
+        );
+        if (!body) {
+          expect(request.body).toBeFalsy();
+        }
+        expect(request.body).toEqual(body);
+      }
+
+      for (const body of [undefined, null]) {
+        const request = await protocol.serializeRequest(
+          {
+            ...operation,
+            input: [
+              3,
+              "ns",
+              "Input",
+              0,
+              ["payload", "name"],
+              [
+                [3, "ns", "PayloadStruct", { httpPayload: 1 }, ["a", "b"], [0, 0]] satisfies StaticStructureSchema,
+                [0 satisfies StringSchema, { httpHeader: "name" }],
+              ],
+            ] satisfies StaticStructureSchema,
+          },
+          {
+            payload: body,
+          },
+          context
+        );
+        expect(request.body).toEqual(`{}`);
+      }
+    });
   });
 });

packages/core/src/submodules/protocols/json/AwsRestJsonProtocol.ts

@@ -75,7 +75,8 @@ export class AwsRestJsonProtocol extends HttpBindingProtocol {
       }
     }
 
-    if (request.headers["content-type"] && !request.body) {
+    if (request.body == null && request.headers["content-type"] === this.getDefaultContentType()) {
+      // if content type is blob or string shape, we don't set a default body.
       request.body = "{}";
     }
 

packages/middleware-sdk-glacier/src/account-id-default.ts

@@ -11,7 +11,7 @@ export function accountIdDefaultMiddleware(): InitializeMiddleware<any, any> {
   return <Output extends MetadataBearer>(next: InitializeHandler<any, Output>): InitializeHandler<any, Output> =>
     async (args: InitializeHandlerArguments<any>): Promise<InitializeHandlerOutput<Output>> => {
       const { input } = args;
-      if (input.accountId === undefined) {
+      if (!input.accountId) {
         input.accountId = "-";
       }
       return next({ ...args, input });

private/aws-protocoltests-restjson-glacier/test/functional/restjson1.spec.ts

@@ -282,7 +282,7 @@ it("GlacierChecksums:Request", async () => {
  * hyphen (-) to indicate the current account. This should be default
  * behavior if the customer provides a null or empty string.
  */
-it.skip("GlacierAccountId:Request", async () => {
+it("GlacierAccountId:Request", async () => {
   const client = new GlacierClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),

private/aws-protocoltests-restjson-schema-glacier/test/functional/restjson1.spec.ts

@@ -282,7 +282,7 @@ it("GlacierChecksums:Request", async () => {
  * hyphen (-) to indicate the current account. This should be default
  * behavior if the customer provides a null or empty string.
  */
-it.skip("GlacierAccountId:Request", async () => {
+it("GlacierAccountId:Request", async () => {
   const client = new GlacierClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),

private/aws-protocoltests-restjson-schema/test/functional/restjson1.spec.ts

@@ -7030,7 +7030,7 @@ it("RestJsonNoInputAndOutputNoPayload:Response", async () => {
 /**
  * Do not send null values, but do send empty strings and empty lists over the wire in headers
  */
-it.skip("RestJsonNullAndEmptyHeaders:Request", async () => {
+it("RestJsonNullAndEmptyHeaders:Request", async () => {
   const client = new RestJsonProtocolClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),

private/aws-protocoltests-restjson/test/functional/restjson1.spec.ts

@@ -7030,7 +7030,7 @@ it("RestJsonNoInputAndOutputNoPayload:Response", async () => {
 /**
  * Do not send null values, but do send empty strings and empty lists over the wire in headers
  */
-it.skip("RestJsonNullAndEmptyHeaders:Request", async () => {
+it("RestJsonNullAndEmptyHeaders:Request", async () => {
   const client = new RestJsonProtocolClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),

private/aws-protocoltests-restxml-schema/test/functional/restxml.spec.ts

@@ -3838,7 +3838,7 @@ it("FlatNestedXmlMapResponse:Response", async () => {
 /**
  * Serializes nested XML Maps in requests that have xmlName on members
  */
-it.skip("NestedXmlMapWithXmlNameSerializes:Request", async () => {
+it("NestedXmlMapWithXmlNameSerializes:Request", async () => {
   const client = new RestXmlProtocolClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),
@@ -4090,7 +4090,7 @@ it("NoInputAndOutput:Response", async () => {
 /**
  * Do not send null values, but do send empty strings and empty lists over the wire in headers
  */
-it.skip("NullAndEmptyHeaders:Request", async () => {
+it("NullAndEmptyHeaders:Request", async () => {
   const client = new RestXmlProtocolClient({
     ...clientParams,
     requestHandler: new RequestSerializationTestHandler(),