From e57b0b758047990e20b394d92fb8d8b12dabfab8 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen Date: Mon, 25 May 2026 23:37:07 -0400 Subject: [PATCH 1/2] Implement client error correction --- .../MemberErrorCorrectionGenerator.java | 212 ++++++++++++++++++ .../generators/StructureGenerator.java | 76 ++++++- 2 files changed, 285 insertions(+), 3 deletions(-) create mode 100644 codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java new file mode 100644 index 000000000..e094a82d7 --- /dev/null +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java @@ -0,0 +1,212 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package software.amazon.smithy.python.codegen.generators; + +import software.amazon.smithy.model.shapes.BigDecimalShape; +import software.amazon.smithy.model.shapes.BigIntegerShape; +import software.amazon.smithy.model.shapes.BlobShape; +import software.amazon.smithy.model.shapes.BooleanShape; +import software.amazon.smithy.model.shapes.ByteShape; +import software.amazon.smithy.model.shapes.DocumentShape; +import software.amazon.smithy.model.shapes.DoubleShape; +import software.amazon.smithy.model.shapes.EnumShape; +import software.amazon.smithy.model.shapes.FloatShape; +import software.amazon.smithy.model.shapes.IntEnumShape; +import software.amazon.smithy.model.shapes.IntegerShape; +import software.amazon.smithy.model.shapes.ListShape; +import software.amazon.smithy.model.shapes.LongShape; +import software.amazon.smithy.model.shapes.MapShape; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeVisitor; +import software.amazon.smithy.model.shapes.ShortShape; +import software.amazon.smithy.model.shapes.StringShape; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.shapes.TimestampShape; +import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.StreamingTrait; +import software.amazon.smithy.python.codegen.GenerationContext; +import software.amazon.smithy.python.codegen.SymbolProperties; +import software.amazon.smithy.python.codegen.writer.PythonWriter; +import software.amazon.smithy.utils.SmithyInternalApi; + +/** + * Emits the Python expression used to fill a missing required member during client error + * correction. + * + * @see Smithy + * spec: Client error correction + */ +@SmithyInternalApi +public final class MemberErrorCorrectionGenerator extends ShapeVisitor.DataShapeVisitor { + + private final GenerationContext context; + private final PythonWriter writer; + + public MemberErrorCorrectionGenerator(GenerationContext context, PythonWriter writer) { + this.context = context; + this.writer = writer; + } + + /** + * @return {@code true} if the visitor will emit a default expression for this shape. + */ + public static boolean hasDefault(Shape target) { + return switch (target.getType()) { + // Note on streaming shapes: + // - Streaming unions (event streams) are filtered out earlier by + // StructureGenerator#filterEventStreamMember and never reach this visitor, + // so UNION can unconditionally return true here. + // - Streaming blobs are NOT filtered earlier, so we explicitly exclude them + // below. Per Smithy spec § 13.3.1, a missing streaming blob is already + // handled by the deserializer (an empty HTTP body becomes a zero-length + // AsyncBytesReader), so client error correction is unnecessary. + case BOOLEAN, BYTE, SHORT, INTEGER, LONG, BIG_INTEGER, FLOAT, DOUBLE, BIG_DECIMAL, + STRING, TIMESTAMP, DOCUMENT, LIST, MAP, ENUM, INT_ENUM, STRUCTURE, UNION -> + true; + case BLOB -> !target.hasTrait(StreamingTrait.class); + default -> false; + }; + } + + @Override + public Boolean booleanShape(BooleanShape shape) { + writer.writeInline("False"); + return true; + } + + @Override + public Boolean byteShape(ByteShape shape) { + writer.writeInline("0"); + return true; + } + + @Override + public Boolean shortShape(ShortShape shape) { + writer.writeInline("0"); + return true; + } + + @Override + public Boolean integerShape(IntegerShape shape) { + writer.writeInline("0"); + return true; + } + + @Override + public Boolean longShape(LongShape shape) { + writer.writeInline("0"); + return true; + } + + @Override + public Boolean bigIntegerShape(BigIntegerShape shape) { + writer.writeInline("0"); + return true; + } + + @Override + public Boolean floatShape(FloatShape shape) { + writer.writeInline("0.0"); + return true; + } + + @Override + public Boolean doubleShape(DoubleShape shape) { + writer.writeInline("0.0"); + return true; + } + + @Override + public Boolean bigDecimalShape(BigDecimalShape shape) { + writer.addStdlibImport("decimal", "Decimal"); + writer.writeInline("Decimal(0)"); + return true; + } + + @Override + public Boolean stringShape(StringShape shape) { + writer.writeInline("\"\""); + return true; + } + + @Override + public Boolean blobShape(BlobShape shape) { + writer.writeInline("b\"\""); + return true; + } + + @Override + public Boolean timestampShape(TimestampShape shape) { + writer.addStdlibImport("datetime", "datetime"); + writer.addStdlibImport("datetime", "timezone"); + writer.writeInline("datetime.fromtimestamp(0, tz=timezone.utc)"); + return true; + } + + @Override + public Boolean documentShape(DocumentShape shape) { + writer.addImport("smithy_core.documents", "Document"); + writer.writeInline("Document(None)"); + return true; + } + + @Override + public Boolean listShape(ListShape shape) { + writer.writeInline("[]"); + return true; + } + + @Override + public Boolean mapShape(MapShape shape) { + writer.writeInline("{}"); + return true; + } + + @Override + public Boolean enumShape(EnumShape shape) { + // TODO: the Smithy spec recommends enum types define an unknown variant. If a + // future change adds an unknown variant to the generated enum class (e.g. + // MyEnum.unknown(value)), revisit this to emit it instead of the bare "". + writer.writeInline("\"\""); + return true; + } + + @Override + public Boolean intEnumShape(IntEnumShape shape) { + // TODO: the Smithy spec recommends intEnum types define an unknown variant. If a + // future change adds an unknown variant to the generated intEnum class (e.g. + // MyIntEnum.unknown(value)), revisit this to emit it instead of the bare 0. + writer.writeInline("0"); + return true; + } + + @Override + public Boolean unionShape(UnionShape shape) { + var unknownSymbol = context.symbolProvider() + .toSymbol(shape) + .expectProperty(SymbolProperties.UNION_UNKNOWN); + writer.addImport(unknownSymbol, unknownSymbol.getName()); + writer.writeInline("$L(tag=\"\")", unknownSymbol.getName()); + return true; + } + + @Override + public Boolean structureShape(StructureShape shape) { + // Delegate to the target struct's _smithy_default() so nested required fields are + // also filled in. Recursion terminates because Smithy forbids recursive paths whose + // members are all @required: + // https://smithy.io/2.0/spec/aggregate-types.html#recursive-shape-definitions + var symbol = context.symbolProvider().toSymbol(shape); + writer.addImport(symbol, symbol.getName()); + writer.writeInline("$L._smithy_default()", symbol.getName()); + return true; + } + + @Override + public Boolean memberShape(MemberShape shape) { + return context.model().expectShape(shape.getTarget()).accept(this); + } +} diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java index 55d4daf67..62377697c 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java @@ -104,12 +104,15 @@ class $L: ${C|} + ${C|} + """, symbol.getName(), writer.consumer(w -> writeClassDocs()), writer.consumer(w -> writeProperties()), writer.consumer(w -> generateSerializeMethod()), - writer.consumer(w -> generateDeserializeMethod())); + writer.consumer(w -> generateDeserializeMethod()), + writer.consumer(w -> generateSmithyDefaultMethod())); } private void renderError() { @@ -147,6 +150,8 @@ class $1L($2T): ${7C|} + ${8C|} + """, symbol.getName(), baseError, @@ -154,7 +159,8 @@ class $1L($2T): writer.consumer(w -> writeClassDocs()), writer.consumer(w -> writeProperties()), writer.consumer(w -> generateSerializeMethod()), - writer.consumer(w -> generateDeserializeMethod())); + writer.consumer(w -> generateDeserializeMethod()), + writer.consumer(w -> generateSmithyDefaultMethod())); } private void writeClassDocs() { @@ -375,14 +381,78 @@ def _consumer(schema: Schema, de: ShapeDeserializer) -> None: logger.debug("Unexpected member schema: %s", schema) deserializer.read_struct($T, consumer=_consumer) + ${C|} return kwargs """, writer.consumer(w -> deserializeMembers(shape.members())), - schemaSymbol); + schemaSymbol, + writer.consumer(w -> writeErrorCorrection())); writer.popState(); } + /** + * Emits client error correction for required members the server failed to serialize. + * + * @see Smithy + * spec: Client error correction + */ + private void writeErrorCorrection() { + var visitor = new MemberErrorCorrectionGenerator(context, writer); + for (MemberShape member : requiredMembers) { + var target = model.expectShape(member.getTarget()); + if (!MemberErrorCorrectionGenerator.hasDefault(target)) { + // Streaming shapes have no synthesizable default; let the dataclass raise. + continue; + } + writer.pushState(); + writer.putContext("memberName", symbolProvider.toMemberName(member)); + writer.write(""" + if ${memberName:S} not in kwargs: + kwargs[${memberName:S}] = ${C|}""", + writer.consumer(w -> target.accept(visitor))); + writer.popState(); + } + } + + /** + * Emits a {@code _smithy_default()} classmethod that constructs an instance with all + * required members filled in via client error correction. Used to fill nested structure + * members per the Smithy spec. If the structure has any required member whose target has + * no synthesizable default (i.e. a streaming blob), {@code _smithy_default()} is omitted: + * a generated {@code cls()} call would be missing a required argument. But such structures + * can only appear as a top-level operation input or output (per spec § 13.3), never as a + * nested-struct member, so {@code _smithy_default()} would never be invoked on them anyway. + */ + private void generateSmithyDefaultMethod() { + for (MemberShape member : requiredMembers) { + var target = model.expectShape(member.getTarget()); + if (!MemberErrorCorrectionGenerator.hasDefault(target)) { + return; + } + } + writer.write(""" + @classmethod + def _smithy_default(cls) -> Self: + return cls(${C|}) + """, + writer.consumer(w -> writeSmithyDefaultArguments())); + } + + private void writeSmithyDefaultArguments() { + var visitor = new MemberErrorCorrectionGenerator(context, writer); + var first = true; + for (MemberShape member : requiredMembers) { + var target = model.expectShape(member.getTarget()); + if (!first) { + writer.writeInline(", "); + } + first = false; + writer.writeInline("$L=", symbolProvider.toMemberName(member)); + target.accept(visitor); + } + } + private void deserializeMembers(Collection members) { int index = -1; for (MemberShape member : members) { From 0055ee6ed112e4ddaa2ce9e6a2ec0a06c16b4f41 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen Date: Thu, 28 May 2026 21:58:43 -0400 Subject: [PATCH 2/2] address review feedback and add enum/intEnum unknown variant --- codegen/build.gradle.kts | 2 +- .../codegen/HttpProtocolTestGenerator.java | 9 +- .../python/codegen/PythonSymbolProvider.java | 12 +- .../codegen/generators/EnumGenerator.java | 56 +++++++- .../codegen/generators/IntEnumGenerator.java | 59 +++++++- .../MemberDeserializerGenerator.java | 12 +- .../MemberErrorCorrectionGenerator.java | 127 ++++++++++-------- .../generators/StructureGenerator.java | 44 +++++- .../src/smithy_json/_private/serializers.py | 3 +- 9 files changed, 247 insertions(+), 77 deletions(-) diff --git a/codegen/build.gradle.kts b/codegen/build.gradle.kts index a0a733616..df6e5391c 100644 --- a/codegen/build.gradle.kts +++ b/codegen/build.gradle.kts @@ -15,5 +15,5 @@ allprojects { group = "software.amazon.smithy.python" - version = "0.3.0" + version = "0.3.1" } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java index c432847bb..a997235a1 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java @@ -763,12 +763,15 @@ public Void nullNode(NullNode node) { @Override public Void numberNode(NumberNode node) { - // TODO: Add support for timestamp, int-enum, and others if (inputShape.isTimestampShape()) { var parsed = CodegenUtils.parseTimestampNode(model, inputShape, node); writer.writeInline(CodegenUtils.getDatetimeConstructor(writer, parsed)); } else if (inputShape.isFloatShape() || inputShape.isDoubleShape()) { writer.writeInline("float($L)", node.getValue()); + } else if (inputShape.isIntEnumShape()) { + var enumSymbol = + context.symbolProvider().toSymbol(inputShape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.writeInline("$T($L)", enumSymbol, node.getValue()); } else { writer.writeInline("$L", node.getValue()); } @@ -800,6 +803,10 @@ public Void stringNode(StringNode node) { }; writer.writeInline("float($S)", value); + } else if (inputShape.isEnumShape()) { + var enumSymbol = + context.symbolProvider().toSymbol(inputShape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.writeInline("$T($S)", enumSymbol, node.getValue()); } else { writer.writeInline("$S", node.getValue()); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java index a44f7cb91..b6dca5436 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java @@ -329,15 +329,9 @@ public Symbol intEnumShape(IntEnumShape shape) { } private Symbol genericEnum(Shape shape) { - var enumSymbol = createGeneratedSymbolBuilder(shape, getDefaultShapeName(shape), SHAPES_FILE).build(); - - // We add this enum symbol as a property on a generic string/int symbol - // rather than returning the enum symbol directly because we only - // generate the enum constants for convenience. We actually want - // to pass around plain types rather than what is effectively - // a namespace class. - return createSymbolBuilder(shape, shape.isEnumShape() ? "str" : "int") - .putProperty(SymbolProperties.ENUM_SYMBOL, escaper.escapeSymbol(shape, enumSymbol)) + Symbol symbol = createGeneratedSymbolBuilder(shape, getDefaultShapeName(shape), SHAPES_FILE).build(); + return symbol.toBuilder() + .putProperty(SymbolProperties.ENUM_SYMBOL, escaper.escapeSymbol(shape, symbol)) .build(); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/EnumGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/EnumGenerator.java index 354054503..20eafc44b 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/EnumGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/EnumGenerator.java @@ -13,7 +13,23 @@ import software.amazon.smithy.utils.SmithyInternalApi; /** - * Renders enums. + * Renders enums as a {@code StrEnum} subclass. + * + *

Beyond the named members, the generated class has: + *

    + *
  • {@code is_unknown} — public. {@code True} when the value didn't come + * from a known member: either the service returned a value newer than + * this SDK or client error correction filled in a placeholder.
    • + *
  • {@code _missing_} — invoked when deserializing a value the SDK + * doesn't recognize.
    • + *
  • {@code _unknown} — invoked from {@link MemberErrorCorrectionGenerator} + * to fill a missing required member.
    • + *
  • {@code __eq__} / {@code __hash__} — overridden so an unknown value + * is not equal to any known member, even if its underlying string + * happens to match one.
    • + *
+ * + * @see Smithy spec: enum */ @SmithyInternalApi public final class EnumGenerator implements Runnable { @@ -30,6 +46,7 @@ public void run() { var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); context.writerDelegator().useShapeWriter(shape, writer -> { writer.addStdlibImport("enum", "StrEnum"); + writer.addStdlibImport("typing", "Self"); writer.openBlock("class $L(StrEnum):", "", enumSymbol.getName(), () -> { shape.getTrait(DocumentationTrait.class).ifPresent(trait -> { writer.writeDocs(trait.getValue(), context); @@ -43,6 +60,43 @@ public void run() { writer.writeDocs(trait.getValue(), context); }); } + + writer.write(""" + + @classmethod + def _unknown(cls, value: str) -> "Self": + pseudo = str.__new__(cls, value) + pseudo._name_ = f"" + pseudo._value_ = value + return pseudo + + @classmethod + def _missing_(cls, value: object) -> "Self | None": + if isinstance(value, str): + return cls._unknown(value) + return None + + @property + def is_unknown(self) -> bool: + \"""True if this value was not known at SDK generation time.\""" + return self._name_ not in type(self).__members__ + + def __eq__(self, other: object) -> bool: + if self.is_unknown: + return ( + isinstance(other, type(self)) + and other.is_unknown + and self._value_ == other._value_ + ) + if isinstance(other, type(self)) and other.is_unknown: + return False + return super().__eq__(other) + + def __hash__(self) -> int: + if self.is_unknown: + return hash(("", type(self).__name__, self._value_)) + return super().__hash__() + """); }); }); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/IntEnumGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/IntEnumGenerator.java index 8816f9d38..bbc07b61c 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/IntEnumGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/IntEnumGenerator.java @@ -13,6 +13,25 @@ import software.amazon.smithy.python.codegen.SymbolProperties; import software.amazon.smithy.utils.SmithyInternalApi; +/** + * Renders intEnums as an {@code IntEnum} subclass. + * + *

Beyond the named members, the generated class has: + *

    + *
  • {@code is_unknown} — public. {@code True} when the value didn't come + * from a known member: either the service returned a value newer than + * this SDK or client error correction filled in a placeholder.
    • + *
  • {@code _missing_} — invoked when deserializing a value the SDK + * doesn't recognize.
    • + *
  • {@code _unknown} — invoked from {@link MemberErrorCorrectionGenerator} + * to fill a missing required member.
    • + *
  • {@code __eq__} / {@code __hash__} — overridden so an unknown value + * is not equal to any known member, even if its underlying integer + * happens to match one.
    • + *
+ * + * @see Smithy spec: intEnum + */ @SmithyInternalApi public final class IntEnumGenerator implements Runnable { @@ -27,6 +46,7 @@ public void run() { var enumSymbol = directive.symbol().expectProperty(SymbolProperties.ENUM_SYMBOL); directive.context().writerDelegator().useShapeWriter(directive.shape(), writer -> { writer.addStdlibImport("enum", "IntEnum"); + writer.addStdlibImport("typing", "Self"); writer.openBlock("class $L(IntEnum):", "", enumSymbol.getName(), () -> { directive.shape().getTrait(DocumentationTrait.class).ifPresent(trait -> { writer.writeDocs(trait.getValue(), directive.context()); @@ -35,11 +55,48 @@ public void run() { for (MemberShape member : directive.shape().members()) { var name = directive.symbolProvider().toMemberName(member); var value = member.expectTrait(EnumValueTrait.class).expectIntValue(); - writer.write("$L = $L\n", name, value); + writer.write("$L = $L", name, value); member.getTrait(DocumentationTrait.class).ifPresent(trait -> { writer.writeDocs(trait.getValue(), directive.context()); }); } + + writer.write(""" + + @classmethod + def _unknown(cls, value: int) -> "Self": + pseudo = int.__new__(cls, value) + pseudo._name_ = f"" + pseudo._value_ = value + return pseudo + + @classmethod + def _missing_(cls, value: object) -> "Self | None": + if isinstance(value, int): + return cls._unknown(value) + return None + + @property + def is_unknown(self) -> bool: + \"""True if this value was not known at SDK generation time.\""" + return self._name_ not in type(self).__members__ + + def __eq__(self, other: object) -> bool: + if self.is_unknown: + return ( + isinstance(other, type(self)) + and other.is_unknown + and self._value_ == other._value_ + ) + if isinstance(other, type(self)) and other.is_unknown: + return False + return super().__eq__(other) + + def __hash__(self) -> int: + if self.is_unknown: + return hash(("", type(self).__name__, self._value_)) + return super().__hash__() + """); }); }); } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberDeserializerGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberDeserializerGenerator.java index 53873b818..b816d87ee 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberDeserializerGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberDeserializerGenerator.java @@ -135,7 +135,11 @@ public Void integerShape(IntegerShape shape) { @Override public Void intEnumShape(IntEnumShape shape) { - writeDeserializer("integer"); + pushMemberState(); + var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.write("$T(${deserializer:L}.read_integer(${C|}))", + enumSymbol, + writer.consumer(w -> writeSchema())); return null; } @@ -183,7 +187,11 @@ public Void stringShape(StringShape shape) { @Override public Void enumShape(EnumShape shape) { - writeDeserializer("string"); + pushMemberState(); + var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.write("$T(${deserializer:L}.read_string(${C|}))", + enumSymbol, + writer.consumer(w -> writeSchema())); return null; } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java index e094a82d7..459957a2b 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/MemberErrorCorrectionGenerator.java @@ -4,6 +4,8 @@ */ package software.amazon.smithy.python.codegen.generators; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.NullableIndex; import software.amazon.smithy.model.shapes.BigDecimalShape; import software.amazon.smithy.model.shapes.BigIntegerShape; import software.amazon.smithy.model.shapes.BlobShape; @@ -26,6 +28,7 @@ import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.shapes.TimestampShape; import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.DefaultTrait; import software.amazon.smithy.model.traits.StreamingTrait; import software.amazon.smithy.python.codegen.GenerationContext; import software.amazon.smithy.python.codegen.SymbolProperties; @@ -40,7 +43,7 @@ * spec: Client error correction */ @SmithyInternalApi -public final class MemberErrorCorrectionGenerator extends ShapeVisitor.DataShapeVisitor { +public final class MemberErrorCorrectionGenerator extends ShapeVisitor.DataShapeVisitor { private final GenerationContext context; private final PythonWriter writer; @@ -53,7 +56,7 @@ public MemberErrorCorrectionGenerator(GenerationContext context, PythonWriter wr /** * @return {@code true} if the visitor will emit a default expression for this shape. */ - public static boolean hasDefault(Shape target) { + public static boolean hasDefault(Shape target, Model model) { return switch (target.getType()) { // Note on streaming shapes: // - Streaming unions (event streams) are filtered out earlier by @@ -64,149 +67,165 @@ public static boolean hasDefault(Shape target) { // handled by the deserializer (an empty HTTP body becomes a zero-length // AsyncBytesReader), so client error correction is unnecessary. case BOOLEAN, BYTE, SHORT, INTEGER, LONG, BIG_INTEGER, FLOAT, DOUBLE, BIG_DECIMAL, - STRING, TIMESTAMP, DOCUMENT, LIST, MAP, ENUM, INT_ENUM, STRUCTURE, UNION -> + STRING, TIMESTAMP, DOCUMENT, LIST, MAP, ENUM, INT_ENUM, UNION -> true; case BLOB -> !target.hasTrait(StreamingTrait.class); + case STRUCTURE -> structHasDefault((StructureShape) target, model); default -> false; }; } + /** + * We can build a default for a struct only when we can build a default for each of its + * required members, so we have to recurse into nested structs. The recursion is safe + * because Smithy doesn't allow cycles where every member along the path is @required; + * we'll always reach a base case (a primitive, list, map, etc.) before looping back. + * + * See https://smithy.io/2.0/spec/aggregate-types.html#recursive-shape-definitions + */ + private static boolean structHasDefault(StructureShape struct, Model model) { + var index = NullableIndex.of(model); + for (MemberShape member : struct.members()) { + if (index.isMemberNullable(member) || member.hasTrait(DefaultTrait.class)) { + continue; + } + if (!hasDefault(model.expectShape(member.getTarget()), model)) { + return false; + } + } + return true; + } + @Override - public Boolean booleanShape(BooleanShape shape) { + public Void booleanShape(BooleanShape shape) { writer.writeInline("False"); - return true; + return null; } @Override - public Boolean byteShape(ByteShape shape) { + public Void byteShape(ByteShape shape) { writer.writeInline("0"); - return true; + return null; } @Override - public Boolean shortShape(ShortShape shape) { + public Void shortShape(ShortShape shape) { writer.writeInline("0"); - return true; + return null; } @Override - public Boolean integerShape(IntegerShape shape) { + public Void integerShape(IntegerShape shape) { writer.writeInline("0"); - return true; + return null; } @Override - public Boolean longShape(LongShape shape) { + public Void longShape(LongShape shape) { writer.writeInline("0"); - return true; + return null; } @Override - public Boolean bigIntegerShape(BigIntegerShape shape) { + public Void bigIntegerShape(BigIntegerShape shape) { writer.writeInline("0"); - return true; + return null; } @Override - public Boolean floatShape(FloatShape shape) { + public Void floatShape(FloatShape shape) { writer.writeInline("0.0"); - return true; + return null; } @Override - public Boolean doubleShape(DoubleShape shape) { + public Void doubleShape(DoubleShape shape) { writer.writeInline("0.0"); - return true; + return null; } @Override - public Boolean bigDecimalShape(BigDecimalShape shape) { + public Void bigDecimalShape(BigDecimalShape shape) { writer.addStdlibImport("decimal", "Decimal"); writer.writeInline("Decimal(0)"); - return true; + return null; } @Override - public Boolean stringShape(StringShape shape) { + public Void stringShape(StringShape shape) { writer.writeInline("\"\""); - return true; + return null; } @Override - public Boolean blobShape(BlobShape shape) { + public Void blobShape(BlobShape shape) { writer.writeInline("b\"\""); - return true; + return null; } @Override - public Boolean timestampShape(TimestampShape shape) { + public Void timestampShape(TimestampShape shape) { writer.addStdlibImport("datetime", "datetime"); writer.addStdlibImport("datetime", "timezone"); writer.writeInline("datetime.fromtimestamp(0, tz=timezone.utc)"); - return true; + return null; } @Override - public Boolean documentShape(DocumentShape shape) { + public Void documentShape(DocumentShape shape) { writer.addImport("smithy_core.documents", "Document"); writer.writeInline("Document(None)"); - return true; + return null; } @Override - public Boolean listShape(ListShape shape) { + public Void listShape(ListShape shape) { writer.writeInline("[]"); - return true; + return null; } @Override - public Boolean mapShape(MapShape shape) { + public Void mapShape(MapShape shape) { writer.writeInline("{}"); - return true; + return null; } @Override - public Boolean enumShape(EnumShape shape) { - // TODO: the Smithy spec recommends enum types define an unknown variant. If a - // future change adds an unknown variant to the generated enum class (e.g. - // MyEnum.unknown(value)), revisit this to emit it instead of the bare "". - writer.writeInline("\"\""); - return true; + public Void enumShape(EnumShape shape) { + var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.addImport(enumSymbol, enumSymbol.getName()); + writer.writeInline("$L._unknown(\"\")", enumSymbol.getName()); + return null; } @Override - public Boolean intEnumShape(IntEnumShape shape) { - // TODO: the Smithy spec recommends intEnum types define an unknown variant. If a - // future change adds an unknown variant to the generated intEnum class (e.g. - // MyIntEnum.unknown(value)), revisit this to emit it instead of the bare 0. - writer.writeInline("0"); - return true; + public Void intEnumShape(IntEnumShape shape) { + var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.addImport(enumSymbol, enumSymbol.getName()); + writer.writeInline("$L._unknown(0)", enumSymbol.getName()); + return null; } @Override - public Boolean unionShape(UnionShape shape) { + public Void unionShape(UnionShape shape) { var unknownSymbol = context.symbolProvider() .toSymbol(shape) .expectProperty(SymbolProperties.UNION_UNKNOWN); writer.addImport(unknownSymbol, unknownSymbol.getName()); writer.writeInline("$L(tag=\"\")", unknownSymbol.getName()); - return true; + return null; } @Override - public Boolean structureShape(StructureShape shape) { - // Delegate to the target struct's _smithy_default() so nested required fields are - // also filled in. Recursion terminates because Smithy forbids recursive paths whose - // members are all @required: - // https://smithy.io/2.0/spec/aggregate-types.html#recursive-shape-definitions + public Void structureShape(StructureShape shape) { var symbol = context.symbolProvider().toSymbol(shape); writer.addImport(symbol, symbol.getName()); writer.writeInline("$L._smithy_default()", symbol.getName()); - return true; + return null; } @Override - public Boolean memberShape(MemberShape shape) { + public Void memberShape(MemberShape shape) { return context.model().expectShape(shape.getTarget()).accept(this); } } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java index 62377697c..0ad0338eb 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java @@ -278,6 +278,15 @@ private String getDefaultValue(PythonWriter writer, MemberShape member) { return CodegenUtils.getDatetimeConstructor(writer, value); } else if (target.isBlobShape()) { return String.format("b'%s'", defaultNode.expectStringNode().getValue()); + } else if (target.isEnumShape()) { + // Wrap rather than emit a bare string so the value matches the field type. + var enumSymbol = symbolProvider.toSymbol(target).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.addImport(enumSymbol, enumSymbol.getName()); + return String.format("%s(\"%s\")", enumSymbol.getName(), defaultNode.expectStringNode().getValue()); + } else if (target.isIntEnumShape()) { + var enumSymbol = symbolProvider.toSymbol(target).expectProperty(SymbolProperties.ENUM_SYMBOL); + writer.addImport(enumSymbol, enumSymbol.getName()); + return String.format("%s(%s)", enumSymbol.getName(), defaultNode.expectNumberNode().getValue()); } if (target.isDocumentShape()) { @@ -401,7 +410,7 @@ private void writeErrorCorrection() { var visitor = new MemberErrorCorrectionGenerator(context, writer); for (MemberShape member : requiredMembers) { var target = model.expectShape(member.getTarget()); - if (!MemberErrorCorrectionGenerator.hasDefault(target)) { + if (!MemberErrorCorrectionGenerator.hasDefault(target, model)) { // Streaming shapes have no synthesizable default; let the dataclass raise. continue; } @@ -418,16 +427,19 @@ private void writeErrorCorrection() { /** * Emits a {@code _smithy_default()} classmethod that constructs an instance with all * required members filled in via client error correction. Used to fill nested structure - * members per the Smithy spec. If the structure has any required member whose target has - * no synthesizable default (i.e. a streaming blob), {@code _smithy_default()} is omitted: - * a generated {@code cls()} call would be missing a required argument. But such structures - * can only appear as a top-level operation input or output (per spec § 13.3), never as a - * nested-struct member, so {@code _smithy_default()} would never be invoked on them anyway. + * members per the Smithy spec. Only emitted when this structure is actually referenced + * as the target of a required structure member elsewhere in the model. If the structure + * has any required member whose target has no synthesizable default (a streaming blob, + * or another structure whose own required members transitively have no default), + * {@code _smithy_default()} is also omitted. */ private void generateSmithyDefaultMethod() { + if (!isRequiredStructMemberTarget()) { + return; + } for (MemberShape member : requiredMembers) { var target = model.expectShape(member.getTarget()); - if (!MemberErrorCorrectionGenerator.hasDefault(target)) { + if (!MemberErrorCorrectionGenerator.hasDefault(target, model)) { return; } } @@ -439,6 +451,24 @@ def _smithy_default(cls) -> Self: writer.consumer(w -> writeSmithyDefaultArguments())); } + /** + * Returns true if any structure in the model has a python-required member whose target + * is this shape. + */ + private boolean isRequiredStructMemberTarget() { + var index = NullableIndex.of(model); + for (var struct : model.getStructureShapes()) { + for (var member : struct.members()) { + if (!index.isMemberNullable(member) + && !member.hasTrait(DefaultTrait.class) + && member.getTarget().equals(shape.getId())) { + return true; + } + } + } + return false; + } + private void writeSmithyDefaultArguments() { var visitor = new MemberErrorCorrectionGenerator(context, writer); var first = true; diff --git a/packages/smithy-json/src/smithy_json/_private/serializers.py b/packages/smithy-json/src/smithy_json/_private/serializers.py index 7146923e4..42f4ea476 100644 --- a/packages/smithy-json/src/smithy_json/_private/serializers.py +++ b/packages/smithy-json/src/smithy_json/_private/serializers.py @@ -300,7 +300,8 @@ def write_string(self, value: str) -> None: self._sink.write(b'"') def write_int(self, value: int) -> None: - self._sink.write(repr(value).encode("utf-8")) + # int() unwraps IntEnum members; otherwise repr would emit "". + self._sink.write(str(int(value)).encode("utf-8")) def write_float(self, value: float | Decimal) -> None: if not self._write_non_numeric_float(value=value):