From 3ad998771ead1d26f38bf37aa424b0e829243ba7 Mon Sep 17 00:00:00 2001 From: George Fu Date: Mon, 18 Dec 2023 15:33:43 -0500 Subject: [PATCH] feat: xml serde reduction (#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 --- .changeset/selfish-pens-scream.md | 2 + .gitignore | 1 + .../integration/EventStreamGenerator.java | 15 +- .../HttpBindingProtocolGenerator.java | 95 +++++++------ .../integration/HttpRpcProtocolGenerator.java | 4 + .../integration/ProtocolGenerator.java | 6 + .../typescript/codegen/util/StringStore.java | 131 ++++++++++++++++++ 7 files changed, 210 insertions(+), 44 deletions(-) create mode 100644 .changeset/selfish-pens-scream.md create mode 100644 smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/util/StringStore.java diff --git a/.changeset/selfish-pens-scream.md b/.changeset/selfish-pens-scream.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/selfish-pens-scream.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.gitignore b/.gitignore index 318dc60a888..2e9f855bd0d 100644 --- 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/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java index 9d80dd5f327..7f668c05c79 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/EventStreamGenerator.java +++ b/smithy-typescript-codegen/src/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 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/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 index 2542212d67b..d63447348a9 100644 --- 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 @@ -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,27 +877,36 @@ 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 @@ -907,8 +914,8 @@ private void writeRequestQueryParam( } 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/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 index bb63d58f91f..366bcd658c5 100644 --- 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 @@ -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/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java index 21adc02411f..892064207c2 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/ProtocolGenerator.java +++ b/smithy-typescript-codegen/src/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; + } } } diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/util/StringStore.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/util/StringStore.java new file mode 100644 index 00000000000..45ef1da911a --- /dev/null +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/util/StringStore.java @@ -0,0 +1,131 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.typescript.codegen.util; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.Map; +import java.util.Objects; +import java.util.Queue; +import java.util.Set; +import java.util.TreeMap; +import java.util.function.Function; +import software.amazon.smithy.utils.SmithyInternalApi; + +/** + * Intended for use at the + * {@link software.amazon.smithy.typescript.codegen.integration.ProtocolGenerator.GenerationContext} + * level, this class allocates and tracks variables assigned to string literals, allowing a + * form of compression on long protocol serde files. + */ +@SmithyInternalApi +public final class StringStore { + // order doesn't matter for this map. + private final Map literalToVariable = new HashMap<>(); + + // this map should be ordered for consistent codegen output. + private final TreeMap variableToLiteral = new TreeMap<>(); + + // controls incremental output. + private final Set writelog = new HashSet<>(); + + /** + * @param literal - a literal string value. + * @return the variable name assigned for that string, which may have been encountered before. + */ + public String var(String literal) { + Objects.requireNonNull(literal); + return literalToVariable.computeIfAbsent(literal, this::assignKey); + } + + /** + * Outputs the generated code for any constants that have been + * allocated but not yet retrieved. + */ + public String flushVariableDeclarationCode() { + StringBuilder sourceCode = new StringBuilder(); + + for (Map.Entry entry : variableToLiteral.entrySet()) { + String v = entry.getKey(); + String l = entry.getValue(); + if (writelog.add(v)) { + sourceCode.append(String.format("const %s = \"%s\";%n", v, l)); + } + } + + return sourceCode.toString(); + } + + /** + * Assigns a new variable for a given string literal. + * Avoid calling assignKey more than once for a given literal, for example with + * {@link HashMap#computeIfAbsent(Object, Function)}, since it would + * allocate two different variables. + */ + private String assignKey(String literal) { + String variable = allocateVariable(literal); + variableToLiteral.put(variable, literal); + return variable; + } + + /** + * Assigns a unique variable using the letters from the literal. + * Prefers the uppercase or word-starting letters. + */ + private String allocateVariable(String literal) { + String[] sections = literal.split("[-_\\s]"); + StringBuilder v = new StringBuilder("_"); + Queue deconfliction = new LinkedList<>(); + if (sections.length > 1) { + for (String s : sections) { + char c = s.charAt(0); + if (isAllowedChar(c)) { + v.append(c); + } + } + } else { + for (int i = 0; i < literal.length(); i++) { + char c = literal.charAt(i); + if ((c >= 'A' && c <= 'Z') || (isNeutral(v.toString()) && isAllowedChar(c))) { + v.append(c); + } else if (isAllowedChar(c)) { + deconfliction.add(c); + } + } + } + if (v.isEmpty()) { + v.append("v"); + } + while (variableToLiteral.containsKey(v.toString())) { + if (!deconfliction.isEmpty()) { + v.append(deconfliction.poll()); + } else { + v.append('_'); + } + } + return v.toString(); + } + + /** + * @return true if char is in A-Za-z. + */ + private boolean isAllowedChar(char c) { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); + } + + /** + * @return true if the variable has only underscores. + */ + private boolean isNeutral(String variable) { + for (int i = 0; i < variable.length(); i++) { + if (variable.charAt(i) != '_') { + return false; + } + } + return true; + } +}