add support for parsing body in either dispatcher or deser

AllanZhengYP · AllanZhengYP · commit c9454c4c3884 · 2019-12-23T23:33:12.000-08:00
For some protocols, error type flag exists in error response body,
then we need to collect response stream to JS object and parse the
error type; For other protocols, error type flag doesn't exist in
error response body, then we don't need to collect the response
stream in error dispatcher. Instead, we can treat the error like
normal response. So that error shape supports the same traits as
normal responses like streaming, payload etc.

This is done by add a new flag in Protocol generator--
isErrorCodeInBody. When it return true, it means error type flag
exists in error response body, then body is parsed in errors
dispatcher, and each error deser only need to deal with parsed
response body in JS object format. When it returns false, it means
error type can be inferred without touching response body, then
error deser can access the error response intact.
diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java
@@ -595,7 +595,7 @@ private void generateOperationDeserializer(
 
         // Write out the error deserialization dispatcher.
         Set<StructureShape> errorShapes = HttpProtocolGeneratorUtils.generateErrorDispatcher(
-                context, operation, responseType, this::writeErrorCodeParser);
+                context, operation, responseType, this::writeErrorCodeParser, this.isErrorCodeInBody());
         deserializingErrorShapes.addAll(errorShapes);
     }
 
@@ -607,11 +607,13 @@ private void generateErrorDeserializer(GenerationContext context, StructureShape
         Symbol errorSymbol = symbolProvider.toSymbol(error);
         String errorDeserMethodName = ProtocolGenerator.getDeserFunctionName(errorSymbol,
                 context.getProtocolName()) + "Response";
+        boolean isBodyParsed = this.isErrorCodeInBody();
 
         writer.openBlock("const $L = async (\n"
-                       + "  output: any,\n"
+                       + "  $L: any,\n"
                        + "  context: __SerdeContext\n"
-                       + "): Promise<$T> => {", "};", errorDeserMethodName, errorSymbol, () -> {
+                       + "): Promise<$T> => {", "};",
+                errorDeserMethodName, isBodyParsed ? "parsedOutput" : "output", errorSymbol, () -> {
             writer.openBlock("const contents: $T = {", "};", errorSymbol, () -> {
                 writer.write("__type: $S,", error.getId().getName());
                 writer.write("$$fault: $S,", error.getTrait(ErrorTrait.class).get().getValue());
@@ -622,7 +624,7 @@ private void generateErrorDeserializer(GenerationContext context, StructureShape
             });
 
             readHeaders(context, error, bindingIndex);
-            List<HttpBinding> documentBindings = readResponseBody(context, error, bindingIndex);
+            List<HttpBinding> documentBindings = readErrorResponseBody(context, error, bindingIndex, isBodyParsed);
             // Track all shapes bound to the document so their deserializers may be generated.
             documentBindings.forEach(binding -> {
                 Shape target = model.expectShape(binding.getMember().getTarget());
@@ -634,6 +636,23 @@ private void generateErrorDeserializer(GenerationContext context, StructureShape
         writer.write("");
     }
 
+    private List<HttpBinding> readErrorResponseBody(
+            GenerationContext context,
+            Shape error,
+            HttpBindingIndex bindingIndex,
+            boolean isBodyParsed
+    ) {
+        TypeScriptWriter writer = context.getWriter();
+        if (isBodyParsed) {
+            // Body is already parsed in error dispatcher, simply assign body to data.
+            writer.write("const data: any = output.body;");
+            return ListUtils.of();
+        } else {
+            // Deserialize response body just like in normal response.
+            return readResponseBody(context, error, bindingIndex);
+        }
+    }
+
     private void readHeaders(
             GenerationContext context,
             Shape operationOrError,
@@ -691,6 +710,7 @@ private List<HttpBinding> readResponseBody(
         List<HttpBinding> documentBindings = bindingIndex.getResponseBindings(operationOrError, Location.DOCUMENT);
         documentBindings.sort(Comparator.comparing(HttpBinding::getMemberName));
         List<HttpBinding> payloadBindings = bindingIndex.getResponseBindings(operationOrError, Location.PAYLOAD);
+
         OperationIndex operationIndex = context.getModel().getKnowledge(OperationIndex.class);
         StructureShape operationOutputOrError = operationOrError.asStructureShape()
                 .orElseGet(() -> operationIndex.getOutput(operationOrError).orElse(null));
@@ -909,6 +929,14 @@ private String getNumberOutputParam(Location bindingType, String dataSource, Sha
      */
     protected abstract void writeErrorCodeParser(GenerationContext context);
 
+    /**
+     * A boolean indicates whether body is collected and parsed in error code parser.
+     * If so, each error shape deserializer should not parse body again.
+     *
+     * @return returns whether the error code exists in response body
+     */
+    protected abstract boolean isErrorCodeInBody();
+
     /**
      * Writes the code needed to deserialize the output document of a response.
      *
diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java
@@ -142,13 +142,15 @@ static void generateCollectBodyString(GenerationContext context) {
      * @param operation The operation to generate for.
      * @param responseType The response type for the HTTP protocol.
      * @param errorCodeGenerator A consumer
+     * @param shouldParseErrorBody Flag indicating whether need to parse response body
      * @return A set of all error structure shapes for the operation that were dispatched to.
      */
     static Set<StructureShape> generateErrorDispatcher(
             GenerationContext context,
             OperationShape operation,
             SymbolReference responseType,
-            Consumer<GenerationContext> errorCodeGenerator
+            Consumer<GenerationContext> errorCodeGenerator,
+            boolean shouldParseErrorBody
     ) {
         TypeScriptWriter writer = context.getWriter();
         SymbolProvider symbolProvider = context.getSymbolProvider();
@@ -162,15 +164,14 @@ static Set<StructureShape> generateErrorDispatcher(
                        + "  output: $T,\n"
                        + "  context: __SerdeContext,\n"
                        + "): Promise<$T> {", "}", errorMethodName, responseType, outputType, () -> {
-            writer.write("const data: any = await parseBody(output.body, context);");
-            // We only consume the parsedOutput if we're dispatching, so only generate if we will.
-            if (!operation.getErrors().isEmpty()) {
-                // Create a holding object since we have already parsed the body, but retain the rest of the output.
-                writer.openBlock("const parsedOutput: any = {", "};", () -> {
-                    writer.write("...output,");
-                    writer.write("body: data,");
-                });
-            }
+            // Prepare error response for parsing error code. If error code needs to be parsed from response body
+            // then we collect body and parse it to JS object, otherwise leave the response body as is.
+            writer.openBlock(
+                    "const $L: any = {", "};", shouldParseErrorBody ? "parsedOutput" : "errorOutput", () -> {
+                writer.write("...output,");
+                writer.write("body: $L,",
+                        shouldParseErrorBody ? "await parseBody(output.body, context)" : "output.body");
+            });
 
             // Error responses must be at least SmithyException and MetadataBearer implementations.
             writer.addImport("SmithyException", "__SmithyException",
@@ -191,7 +192,8 @@ static Set<StructureShape> generateErrorDispatcher(
                             context.getProtocolName()) + "Response";
                     writer.openBlock("case $S:\ncase $S:", "  break;", errorId.getName(), errorId.toString(), () -> {
                         // Dispatch to the error deserialization function.
-                        writer.write("response = await $L(parsedOutput, context);", errorDeserMethodName);
+                        writer.write("response = await $L($L, context);",
+                                errorDeserMethodName, shouldParseErrorBody ? "parsedOutput" : "errorOutput");
                     });
                 });
 
diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpRpcProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpRpcProtocolGenerator.java
@@ -254,7 +254,7 @@ private void generateOperationDeserializer(GenerationContext context, OperationS
 
         // Write out the error deserialization dispatcher.
         Set<StructureShape> errorShapes = HttpProtocolGeneratorUtils.generateErrorDispatcher(
-                context, operation, responseType, this::writeErrorCodeParser);
+                context, operation, responseType, this::writeErrorCodeParser, this.isErrorCodeInBody());
         deserializingErrorShapes.addAll(errorShapes);
     }
 
@@ -311,10 +311,13 @@ private void readResponseBody(GenerationContext context, OperationShape operatio
      * Writes the code that loads an {@code errorCode} String with the content used
      * to dispatch errors to specific serializers.
      *
-     * <p>Three variables will be in scope:
+     * <p>Two variables will be in scope:
      *   <ul>
-     *       <li>{@code output}: a value of the HttpResponse type.</li>
-     *       <li>{@code data}: the contents of the response body.</li>
+     *       <li>{@code errorOutput} or {@code parsedOutput}: a value of the HttpResponse type.
+     *          {@code errorOutput} is a raw HttpResponse whereas {@code parsedOutput} is a HttpResponse type with
+     *          body parsed to JavaScript object.
+     *          The actual value available is determined by {@link #isErrorCodeInBody}
+     *       </li>
      *       <li>{@code context}: the SerdeContext.</li>
      *   </ul>
      *
@@ -328,6 +331,17 @@ private void readResponseBody(GenerationContext context, OperationShape operatio
      */
     protected abstract void writeErrorCodeParser(GenerationContext context);
 
+    /**
+     * Indicates whether body is collected and parsed in error dispatcher.
+     *
+     * <p>If returns true, {@link #writeErrorCodeParser} will have {@code parsedOutput} in scope
+     *
+     * <P>If returns false, {@link #writeErrorCodeParser} will have {@code errorOutput} in scope
+     *
+     * @return returns whether the error code exists in response body
+     */
+    protected abstract boolean isErrorCodeInBody();
+
     /**
      * Writes the code needed to deserialize the output document of a response.
      *
