From 82cc41fb86943262db5ed2cce3349e3b98e5bc35 Mon Sep 17 00:00:00 2001 From: George Fu Date: Mon, 9 Dec 2024 21:02:41 +0000 Subject: [PATCH] preserve LazyJsonString interface --- packages/smithy-client/src/lazy-json.spec.ts | 30 +++++-- packages/smithy-client/src/lazy-json.ts | 87 ++++++++++++------- .../typescript/codegen/SymbolVisitor.java | 4 +- .../HttpProtocolGeneratorUtils.java | 4 +- .../codegen/SymbolProviderTest.java | 2 +- .../DocumentMemberDeserVisitorTest.java | 2 +- .../DocumentMemberSerVisitorTest.java | 2 +- 7 files changed, 86 insertions(+), 45 deletions(-) diff --git a/packages/smithy-client/src/lazy-json.spec.ts b/packages/smithy-client/src/lazy-json.spec.ts index b3ebd6c8b3b..07ca20fba1e 100644 --- a/packages/smithy-client/src/lazy-json.spec.ts +++ b/packages/smithy-client/src/lazy-json.spec.ts @@ -1,27 +1,39 @@ import { describe, expect, test as it } from "vitest"; import { LazyJsonString } from "./lazy-json"; + describe("LazyJsonString", () => { - it("returns identical values for toString(), valueOf(), and toJSON()", () => { - const jsonValue = LazyJsonString.from({ foo: "bar" }); - expect(jsonValue.valueOf()).toBe(JSON.stringify({ foo: "bar" })); - expect(jsonValue.toString()).toBe(JSON.stringify({ foo: "bar" })); - expect(jsonValue.toJSON()).toBe(JSON.stringify({ foo: "bar" })); + it("should have string methods", () => { + const jsonValue = new LazyJsonString('"foo"'); + expect(jsonValue.length).toBe(5); + expect(jsonValue.toString()).toBe('"foo"'); + }); + + it("should deserialize json properly", () => { + const jsonValue = new LazyJsonString('"foo"'); + expect(jsonValue.deserializeJSON()).toBe("foo"); + const wrongJsonValue = new LazyJsonString("foo"); + expect(() => wrongJsonValue.deserializeJSON()).toThrow(); + }); + + it("should get JSON string properly", () => { + const jsonValue = new LazyJsonString('{"foo", "bar"}'); + expect(jsonValue.toJSON()).toBe('{"foo", "bar"}'); }); it("can instantiate from LazyJsonString class", () => { - const original = LazyJsonString.from('"foo"'); - const newOne = LazyJsonString.from(original); + const original = new LazyJsonString('"foo"'); + const newOne = LazyJsonString.fromObject(original); expect(newOne.toString()).toBe('"foo"'); }); it("can instantiate from String class", () => { - const jsonValue = LazyJsonString.from('"foo"'); + const jsonValue = LazyJsonString.fromObject(new String('"foo"')); expect(jsonValue.toString()).toBe('"foo"'); }); it("can instantiate from object", () => { - const jsonValue = LazyJsonString.from({ foo: "bar" }); + const jsonValue = LazyJsonString.fromObject({ foo: "bar" }); expect(jsonValue.toString()).toBe('{"foo":"bar"}'); }); }); diff --git a/packages/smithy-client/src/lazy-json.ts b/packages/smithy-client/src/lazy-json.ts index 7896dd918e4..ff3c9af1837 100644 --- a/packages/smithy-client/src/lazy-json.ts +++ b/packages/smithy-client/src/lazy-json.ts @@ -1,39 +1,68 @@ /** - * @internal + * @public * - * This class allows the usage of data objects in fields that expect - * JSON strings. It serializes the data object into JSON - * if needed during the request serialization step. + * A model field with this type means that you may provide a JavaScript + * object in lieu of a JSON string, and it will be serialized to JSON + * automatically before being sent in a request. * + * For responses, you will receive a "LazyJsonString", which is a boxed String object + * with additional mixin methods. + * To get the string value, call `.toString()`, or to get the JSON object value, + * call `.deserializeJSON()` or parse it yourself. */ -export class LazyJsonString { - private constructor(private value: string) {} - - public toString(): string { - return this.value; - } +export type AutomaticJsonStringConversion = Parameters[0] | LazyJsonString; - public valueOf(): string { - return this.value; - } - - public toJSON(): string { - return this.value; - } +/** + * @internal + * + */ +export interface LazyJsonString extends String { + new (s: string): typeof LazyJsonString; - public static from(object: any): LazyJsonString { - if (object instanceof LazyJsonString) { - return object; - } else if (typeof object === "string") { - return new LazyJsonString(object); - } - return new LazyJsonString(JSON.stringify(object)); - } + /** + * @returns the JSON parsing of the string value. + */ + deserializeJSON(): any; /** - * @deprecated call from() instead. + * @returns the original string value rather than a JSON.stringified value. */ - public static fromObject(object: any): LazyJsonString { - return LazyJsonString.from(object); - } + toJSON(): string; } + +/** + * @internal + * + * Extension of the native String class in the previous implementation + * has negative global performance impact on method dispatch for strings, + * and is generally discouraged. + * + * This current implementation may look strange, but is necessary to preserve the interface and + * behavior of extending the String class. + */ +export function LazyJsonString(val: string): void { + const str = Object.assign(new String(val), { + deserializeJSON() { + return JSON.parse(val); + }, + + toString() { + return val; + }, + + toJSON() { + return val; + }, + }); + + return str as never; +} + +LazyJsonString.fromObject = (object: any): LazyJsonString => { + if (object instanceof LazyJsonString) { + return object as any; + } else if (typeof object === "string" || object?.prototype === String.prototype) { + return LazyJsonString(object as string) as any; + } + return LazyJsonString(JSON.stringify(object)) as any; +}; diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java index 4e0aef6c0b2..1e4851bc9b5 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java @@ -300,8 +300,8 @@ public Symbol stringShape(StringShape shape) { if (mediaTypeTrait.isPresent()) { String mediaType = mediaTypeTrait.get().getValue(); if (CodegenUtils.isJsonMediaType(mediaType)) { - Symbol.Builder builder = createSymbolBuilder(shape, "__LazyJsonString | string"); - return addSmithyUseImport(builder, "LazyJsonString", "__LazyJsonString").build(); + Symbol.Builder builder = createSymbolBuilder(shape, "__AutomaticJsonStringConversion | string"); + return addSmithyUseImport(builder, "AutomaticJsonStringConversion", "__AutomaticJsonStringConversion").build(); } else { LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape); } 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 index fa74c2ece7f..849cc03d80f 100644 --- 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 @@ -176,7 +176,7 @@ public static String getStringInputParam(GenerationContext context, Shape shape, if (CodegenUtils.isJsonMediaType(mediaType)) { TypeScriptWriter writer = context.getWriter(); writer.addImport("LazyJsonString", "__LazyJsonString", TypeScriptDependency.AWS_SMITHY_CLIENT); - return "__LazyJsonString.from(" + dataSource + ")"; + return "__LazyJsonString.fromObject(" + dataSource + ")"; } else { LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape); } @@ -210,7 +210,7 @@ public static String getStringOutputParam(GenerationContext context, if (CodegenUtils.isJsonMediaType(mediaType)) { TypeScriptWriter writer = context.getWriter(); writer.addImport("LazyJsonString", "__LazyJsonString", TypeScriptDependency.AWS_SMITHY_CLIENT); - return "__LazyJsonString.from(" + dataSource + ")"; + return "new __LazyJsonString(" + dataSource + ")"; } else { LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape); } diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/SymbolProviderTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/SymbolProviderTest.java index 65b659c047e..4d2cc5f5395 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/SymbolProviderTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/SymbolProviderTest.java @@ -191,7 +191,7 @@ public void usesLazyJsonStringForJsonMediaType() { SymbolProvider provider = new SymbolVisitor(model, settings); Symbol memberSymbol = provider.toSymbol(member); - assertThat(memberSymbol.getName(), equalTo("__LazyJsonString | string")); + assertThat(memberSymbol.getName(), equalTo("__AutomaticJsonStringConversion | string")); } @Test diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java index 80b762db823..9d306538565 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java @@ -98,7 +98,7 @@ public static Collection validMemberTargetTypes() { {StringShape.builder().id(id).build(), "__expectString(" + DATA_SOURCE + ")", source}, { StringShape.builder().id(id).addTrait(new MediaTypeTrait("foo+json")).build(), - "__LazyJsonString.from(" + DATA_SOURCE + ")", + "new __LazyJsonString(" + DATA_SOURCE + ")", source }, {BlobShape.builder().id(id).build(), "context.base64Decoder(" + DATA_SOURCE + ")", source}, diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberSerVisitorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberSerVisitorTest.java index f82420f3f95..fcee0c535f6 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberSerVisitorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberSerVisitorTest.java @@ -83,7 +83,7 @@ public static Collection validMemberTargetTypes() { {StringShape.builder().id(id).build(), DATA_SOURCE}, { StringShape.builder().id(id).addTrait(new MediaTypeTrait("foo+json")).build(), - "__LazyJsonString.from(" + DATA_SOURCE + ")" + "__LazyJsonString.fromObject(" + DATA_SOURCE + ")" }, {BlobShape.builder().id(id).build(), "context.base64Encoder(" + DATA_SOURCE + ")"}, {DocumentShape.builder().id(id).build(), delegate},