feat: xml serde reduction (smithy-lang#1108)

* feat: xml serde reduction * use regular maps in StringStore * address review comments * unused import * update openBlock to java text block * update StringStore::assignKey * set StringStore final, improve readability of writeRequestQueryParam
trivikr · Dec 18, 2023 · 3ad9987 · 3ad9987
1 parent 913ab87
commit 3ad9987
Show file tree

Hide file tree

Showing 7 changed files with 210 additions and 44 deletions.
diff --git a/.changeset/selfish-pens-scream.md b/.changeset/selfish-pens-scream.md
@@ -0,0 +1,2 @@
+---
+---
diff --git a/.gitignore b/.gitignore
@@ -37,6 +37,7 @@ smithy-typescript-integ-tests/yarn.lock
 
 # Issue https://github.com/awslabs/smithy-typescript/issues/425
 smithy-typescript-codegen/bin/
+smithy-typescript-codegen-test/bin/
 smithy-typescript-ssdk-codegen-test-utils/bin/
 smithy-typescript-codegen-test/example-weather-customizations/bin/
 

diff --git a/...main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java b/...main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java
@@ -490,12 +490,19 @@ private void generateErrorEventUnmarshaller(
     private void readEventHeaders(GenerationContext context, StructureShape event) {
         TypeScriptWriter writer = context.getWriter();
         List<MemberShape> headerMembers = event.getAllMembers().values().stream()
-                .filter(member -> member.hasTrait(EventHeaderTrait.class)).collect(Collectors.toList());
+                .filter(member -> member.hasTrait(EventHeaderTrait.class)).toList();
         for (MemberShape headerMember : headerMembers) {
             String memberName = headerMember.getMemberName();
-            writer.openBlock("if (output.headers[$S] !== undefined) {", "}", memberName, () -> {
-                writer.write("contents.$1L = output.headers[$1S].value;", memberName);
-            });
+            String varName = context.getStringStore().var(memberName);
+
+            writer.write(
+                """
+                if (output.headers[$1L] !== undefined) {
+                    contents[$1L] = output.headers[$1L].value;
+                }
+                """,
+                varName
+            );
         }
     }
 

diff --git a/...a/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java b/...a/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java
@@ -202,6 +202,10 @@ public void generateSharedComponents(GenerationContext context) {
         HttpProtocolGeneratorUtils.generateMetadataDeserializer(context, getApplicationProtocol().getResponseType());
         HttpProtocolGeneratorUtils.generateCollectBodyString(context);
         HttpProtocolGeneratorUtils.generateHttpBindingUtils(context);
+
+        writer.write(
+            context.getStringStore().flushVariableDeclarationCode()
+        );
     }
 
     @Override
@@ -673,7 +677,9 @@ private void generateOperationRequestSerializer(
             // Get the hostname, path, port, and scheme from client's resolved endpoint.
             // Then construct the request from them. The client's resolved endpoint can
             // be default one or supplied by users.
-            writer.write("const {hostname, protocol = $S, port, path: basePath} = await context.endpoint();", "https");
+
+            writer.addImport("requestBuilder", "rb", TypeScriptDependency.SMITHY_CORE);
+            writer.write("const b = rb(input, context);");
 
             writeRequestHeaders(context, operation, bindingIndex);
             writeResolvedPath(context, operation, bindingIndex, trait);
@@ -692,24 +698,17 @@ private void generateOperationRequestSerializer(
             boolean hasHostPrefix = operation.hasTrait(EndpointTrait.class);
             if (hasHostPrefix) {
                 HttpProtocolGeneratorUtils.writeHostPrefix(context, operation);
+                writer.write("b.hn(resolvedHostname);");
             }
-            writer.openBlock("return new $T({", "});", requestType, () -> {
-                writer.write("protocol,");
-                if (hasHostPrefix) {
-                    writer.write("hostname: resolvedHostname,");
-                } else {
-                    writer.write("hostname,");
-                }
-                writer.write("port,");
-                writer.write("method: $S,", trait.getMethod());
-                writer.write("headers,");
-                writer.write("path: resolvedPath,");
-                if (hasQueryComponents) {
-                    writer.write("query,");
-                }
-                // Always set the body,
-                writer.write("body,");
-            });
+            writer.write("b.m($S)", trait.getMethod());
+            writer.write(".h(headers)");
+            if (hasQueryComponents) {
+                writer.write(".q(query)");
+            }
+            // Always set the body,
+            writer.write(".b(body);");
+
+            writer.write("return b.build();");
         });
 
         writer.write("");
@@ -762,8 +761,7 @@ private void writeResolvedPath(
             : Collections.emptyMap();
 
         // Always write the bound path, but only the actual segments.
-        writer.write("let resolvedPath = `$L` + $S;",
-                "${basePath?.endsWith('/') ? basePath.slice(0, -1) : (basePath || '')}",
+        writer.write("b.bp(\"$L\");",
                 "/" + trait.getUri().getSegments().stream()
                     .filter(segment -> {
                         if (!useEndpointsV2) {
@@ -804,7 +802,7 @@ private void writeResolvedPath(
 
                 // Get the correct label to use.
                 Segment uriLabel = uriLabels.stream().filter(s -> s.getContent().equals(memberName)).findFirst().get();
-                writer.write("resolvedPath = __resolvedPath(resolvedPath, input, '$L', $L, '$L', $L)",
+                writer.write("b.p('$L', $L, '$L', $L)",
                     memberName,
                     labelValueProvider,
                     uriLabel.toString(),
@@ -830,7 +828,7 @@ private boolean writeRequestQueryString(
             writer.openBlock("const query: any = map({", "});", () -> {
                 if (!queryLiterals.isEmpty()) {
                     // Write any query literals present in the uri.
-                    queryLiterals.forEach((k, v) -> writer.write("$S: [, $S],", k, v));
+                    queryLiterals.forEach((k, v) -> writer.write("[$L]: [, $S],", context.getStringStore().var(k), v));
                 }
                 // Handle any additional query params bindings.
                 // If query string parameter is also present in httpQuery, it would be overwritten.
@@ -879,36 +877,45 @@ private void writeRequestQueryParam(
         String queryValue = getInputValue(
             context,
             binding.getLocation(),
-            "input." + memberName + memberAssertionComponent,
+            "input[" + context.getStringStore().var(memberName) + "]" + memberAssertionComponent,
             binding.getMember(),
             target
         );
 
+        String simpleAccessExpression = "input["
+            + context.getStringStore().var(memberName)
+            + "]" + memberAssertionComponent;
+
+        boolean isSimpleAccessExpression = Objects.equals(
+            simpleAccessExpression,
+            queryValue
+        );
+
         writer.addImport("expectNonNull", "__expectNonNull", TypeScriptDependency.AWS_SMITHY_CLIENT);
 
-        if (Objects.equals("input." + memberName + memberAssertionComponent, queryValue)) {
+        if (isSimpleAccessExpression) {
             String value = isRequired ? "__expectNonNull($L, `" + memberName + "`)" : "$L";
             // simple undefined check
             writer.write(
-                "$S: [," + value + idempotencyComponent + "],",
-                binding.getLocationName(),
+                "[$L]: [," + value + idempotencyComponent + "],",
+                context.getStringStore().var(binding.getLocationName()),
                 queryValue
             );
         } else {
             if (isRequired) {
                 // __expectNonNull is immediately invoked and not inside a function.
                 writer.write(
-                    "$S: [__expectNonNull(input.$L, `$L`) != null, () => $L],",
-                    binding.getLocationName(),
+                    "[$L]: [__expectNonNull(input.$L, `$L`) != null, () => $L],",
+                    context.getStringStore().var(binding.getLocationName()),
                     memberName,
                     memberName,
                     queryValue // no idempotency token default for required members
                 );
             } else {
                 // undefined check with lazy eval
                 writer.write(
-                    "$S: [() => input.$L !== void 0, () => ($L)$L],",
-                    binding.getLocationName(),
+                    "[$L]: [() => input.$L !== void 0, () => ($L)$L],",
+                    context.getStringStore().var(binding.getLocationName()),
                     memberName,
                     queryValue,
                     idempotencyComponent
@@ -976,7 +983,9 @@ private void writeRequestHeaders(
     }
 
     private void writeNormalHeader(GenerationContext context, HttpBinding binding) {
-        String memberLocation = "input." + context.getSymbolProvider().toMemberName(binding.getMember());
+        String memberLocation = "input["
+            + context.getStringStore().var(context.getSymbolProvider().toMemberName(binding.getMember()))
+            + "]";
         Shape target = context.getModel().expectShape(binding.getMember().getTarget());
 
         String headerKey = binding.getLocationName().toLowerCase(Locale.US);
@@ -996,8 +1005,8 @@ private void writeNormalHeader(GenerationContext context, HttpBinding binding) {
             }
             // evaluated value has a function or method call attached
             headerBuffer.put(headerKey, String.format(
-                "'%s': [() => isSerializableHeaderValue(%s), () => %s],",
-                headerKey,
+                "[%s]: [() => isSerializableHeaderValue(%s), () => %s],",
+                context.getStringStore().var(headerKey),
                 memberLocation + defaultValue,
                 headerValue + defaultValue
             ));
@@ -1008,8 +1017,8 @@ private void writeNormalHeader(GenerationContext context, HttpBinding binding) {
                 value = headerValue + " || " + s.substring(s.indexOf(": ") + 2, s.length() - 1);
             }
             headerBuffer.put(headerKey, String.format(
-                "'%s': %s,",
-                headerKey,
+                "[%s]: %s,",
+                context.getStringStore().var(headerKey),
                 value
             ));
         }
@@ -2231,14 +2240,20 @@ private void readNormalHeaders(
             String memberName = context.getSymbolProvider().toMemberName(binding.getMember());
             String headerName = binding.getLocationName().toLowerCase(Locale.US);
             Shape target = context.getModel().expectShape(binding.getMember().getTarget());
-            String headerValue = getOutputValue(context, binding.getLocation(),
-                    outputName + ".headers['" + headerName + "']", binding.getMember(), target);
-            String checkedValue = outputName + ".headers['" + headerName + "']";
+            String headerValue = getOutputValue(
+                context, binding.getLocation(),
+                outputName + ".headers[" + context.getStringStore().var(headerName) + "]",
+                binding.getMember(), target
+            );
+            String checkedValue = outputName + ".headers[" + context.getStringStore().var(headerName) + "]";
 
             if (checkedValue.equals(headerValue)) {
-                writer.write("$L: [, $L],", memberName, headerValue);
+                writer.write("[$L]: [, $L],", context.getStringStore().var(memberName), headerValue);
             } else {
-                writer.write("$L: [() => void 0 !== $L, () => $L],", memberName, checkedValue, headerValue);
+                writer.write(
+                    "[$L]: [() => void 0 !== $L, () => $L],",
+                    context.getStringStore().var(memberName), checkedValue, headerValue
+                );
             }
         }
     }

diff --git a/.../java/software/amazon/smithy/typescript/codegen/integration/HttpRpcProtocolGenerator.java b/.../java/software/amazon/smithy/typescript/codegen/integration/HttpRpcProtocolGenerator.java
@@ -173,6 +173,10 @@ public void generateSharedComponents(GenerationContext context) {
         // Write common request header to be shared by all requests
         writeSharedRequestHeaders(context);
         writer.write("");
+
+        writer.write(
+            context.getStringStore().flushVariableDeclarationCode()
+        );
     }
 
     @Override

diff --git a/...rc/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java b/...rc/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java
@@ -30,6 +30,7 @@
 import software.amazon.smithy.typescript.codegen.TypeScriptDelegator;
 import software.amazon.smithy.typescript.codegen.TypeScriptSettings;
 import software.amazon.smithy.typescript.codegen.TypeScriptWriter;
+import software.amazon.smithy.typescript.codegen.util.StringStore;
 import software.amazon.smithy.utils.CaseUtils;
 import software.amazon.smithy.utils.SmithyUnstableApi;
 
@@ -313,6 +314,7 @@ class GenerationContext {
         private TypeScriptDelegator writerDelegator;
         private TypeScriptWriter writer;
         private String protocolName;
+        private StringStore stringStore = new StringStore();
 
         public TypeScriptSettings getSettings() {
             return settings;
@@ -400,5 +402,9 @@ public GenerationContext withWriter(TypeScriptWriter newWriter) {
             copyContext.setWriter(newWriter);
             return copyContext;
         }
+
+        public StringStore getStringStore() {
+            return stringStore;
+        }
     }
 }