From e57b0b758047990e20b394d92fb8d8b12dabfab8 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen Date: Mon, 25 May 2026 23:37:07 -0400 Subject: [PATCH 1/5] 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/5] 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): From 22fca6c7344ce83df7ec26c498e3c63671698dd8 Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Wed, 10 Jun 2026 23:19:55 -0400 Subject: [PATCH 3/5] testing fixes --- .../smithy/python/codegen/CodegenUtils.java | 13 ++ .../codegen/HttpProtocolTestGenerator.java | 4 +- .../python/codegen/PythonSymbolProvider.java | 4 +- .../codegen/RequiredMemberTargetIndex.java | 48 ++++ .../python/codegen/SymbolProperties.java | 7 - .../codegen/generators/EnumGenerator.java | 64 +---- .../codegen/generators/IntEnumGenerator.java | 64 +---- .../MemberDeserializerGenerator.java | 4 +- .../MemberErrorCorrectionGenerator.java | 220 ++++++++---------- .../generators/StructureGenerator.java | 57 ++--- .../codegen/writer/PythonDelegator.java | 29 +-- packages/smithy-core/src/smithy_core/types.py | 46 +++- packages/smithy-core/tests/unit/test_types.py | 62 +++++ 13 files changed, 313 insertions(+), 309 deletions(-) create mode 100644 codegen/core/src/main/java/software/amazon/smithy/python/codegen/RequiredMemberTargetIndex.java diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java index b039dd582..a6def8968 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java @@ -31,6 +31,7 @@ import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.traits.DefaultTrait; import software.amazon.smithy.model.traits.ErrorTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait.Format; @@ -128,6 +129,18 @@ public static boolean isErrorMessage(Model model, MemberShape shape) { && model.expectShape(shape.getContainer()).hasTrait(ErrorTrait.class); } + /** + * Determines whether a member is required in the generated Python constructor — that is, + * neither nullable nor carrying a default value. + * + * @param index A nullable index for the model. + * @param member The member to check. + * @return Returns whether the member is required in generated code. + */ + public static boolean isRequiredMember(NullableIndex index, MemberShape member) { + return !index.isMemberNullable(member) && !member.hasTrait(DefaultTrait.class); + } + /** * Executes a given shell command in a given directory. * 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 a997235a1..4d3c9333d 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 @@ -770,7 +770,7 @@ public Void numberNode(NumberNode node) { writer.writeInline("float($L)", node.getValue()); } else if (inputShape.isIntEnumShape()) { var enumSymbol = - context.symbolProvider().toSymbol(inputShape).expectProperty(SymbolProperties.ENUM_SYMBOL); + context.symbolProvider().toSymbol(inputShape); writer.writeInline("$T($L)", enumSymbol, node.getValue()); } else { writer.writeInline("$L", node.getValue()); @@ -805,7 +805,7 @@ public Void stringNode(StringNode node) { writer.writeInline("float($S)", value); } else if (inputShape.isEnumShape()) { var enumSymbol = - context.symbolProvider().toSymbol(inputShape).expectProperty(SymbolProperties.ENUM_SYMBOL); + context.symbolProvider().toSymbol(inputShape); 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 b6dca5436..944d404da 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 @@ -330,9 +330,7 @@ public Symbol intEnumShape(IntEnumShape shape) { private Symbol genericEnum(Shape shape) { Symbol symbol = createGeneratedSymbolBuilder(shape, getDefaultShapeName(shape), SHAPES_FILE).build(); - return symbol.toBuilder() - .putProperty(SymbolProperties.ENUM_SYMBOL, escaper.escapeSymbol(shape, symbol)) - .build(); + return escaper.escapeSymbol(shape, symbol); } @Override diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RequiredMemberTargetIndex.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RequiredMemberTargetIndex.java new file mode 100644 index 000000000..619056be4 --- /dev/null +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RequiredMemberTargetIndex.java @@ -0,0 +1,48 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package software.amazon.smithy.python.codegen; + +import java.util.HashSet; +import java.util.Set; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.KnowledgeIndex; +import software.amazon.smithy.model.knowledge.NullableIndex; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.utils.SmithyInternalApi; + +/** + * Knowledge index of all shapes that are the target of a python-required structure member, + * i.e. the shapes that may need a synthesized default during client error correction. + * + *

Computed once per model and cached via {@link Model#getKnowledge}. + */ +@SmithyInternalApi +public final class RequiredMemberTargetIndex implements KnowledgeIndex { + + private final Set targets = new HashSet<>(); + + private RequiredMemberTargetIndex(Model model) { + var index = NullableIndex.of(model); + for (var struct : model.getStructureShapes()) { + for (var member : struct.members()) { + if (CodegenUtils.isRequiredMember(index, member)) { + targets.add(member.getTarget()); + } + } + } + } + + public static RequiredMemberTargetIndex of(Model model) { + return model.getKnowledge(RequiredMemberTargetIndex.class, RequiredMemberTargetIndex::new); + } + + /** + * @param shape The shape to check. + * @return Returns whether the shape is the target of any python-required structure member. + */ + public boolean isRequiredMemberTarget(ShapeId shape) { + return targets.contains(shape); + } +} diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/SymbolProperties.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/SymbolProperties.java index aa35a1da0..992b8af21 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/SymbolProperties.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/SymbolProperties.java @@ -35,13 +35,6 @@ public final class SymbolProperties { */ public static final Property STDLIB = Property.named("stdlib"); - /** - * Contains a symbol representing the class containing known enum values for the symbol. - * - *

In type signatures, the base int or str is used instead for forwards compatibility. - */ - public static final Property ENUM_SYMBOL = Property.named("enumSymbol"); - /** * Contains a symbol representing the unknown variant of a union. */ 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 20eafc44b..b3cc99b4e 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 @@ -9,25 +9,17 @@ import software.amazon.smithy.model.traits.DocumentationTrait; import software.amazon.smithy.model.traits.EnumValueTrait; import software.amazon.smithy.python.codegen.GenerationContext; -import software.amazon.smithy.python.codegen.SymbolProperties; +import software.amazon.smithy.python.codegen.SmithyPythonDependency; import software.amazon.smithy.utils.SmithyInternalApi; /** * 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.
    • - *
+ *

The {@code UnknownEnumMixin} base from smithy-core handles values that + * weren't known at generation time: deserializing an unrecognized value (or + * filling a missing required member during client error correction, see + * {@link MemberErrorCorrectionGenerator}) produces a pseudo-member with + * {@code is_unknown} set rather than raising. * * @see Smithy spec: enum */ @@ -43,11 +35,12 @@ public EnumGenerator(GenerationContext context, EnumShape enumShape) { @Override public void run() { - var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + var enumSymbol = context.symbolProvider().toSymbol(shape); context.writerDelegator().useShapeWriter(shape, writer -> { writer.addStdlibImport("enum", "StrEnum"); - writer.addStdlibImport("typing", "Self"); - writer.openBlock("class $L(StrEnum):", "", enumSymbol.getName(), () -> { + writer.addDependency(SmithyPythonDependency.SMITHY_CORE); + writer.addImport("smithy_core.types", "UnknownEnumMixin"); + writer.openBlock("class $L(UnknownEnumMixin, StrEnum):", "", enumSymbol.getName(), () -> { shape.getTrait(DocumentationTrait.class).ifPresent(trait -> { writer.writeDocs(trait.getValue(), context); }); @@ -60,43 +53,6 @@ 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 bbc07b61c..33f8e4ee5 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 @@ -10,25 +10,17 @@ import software.amazon.smithy.model.traits.EnumValueTrait; import software.amazon.smithy.python.codegen.GenerationContext; import software.amazon.smithy.python.codegen.PythonSettings; -import software.amazon.smithy.python.codegen.SymbolProperties; +import software.amazon.smithy.python.codegen.SmithyPythonDependency; 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.
    • - *
+ *

The {@code UnknownEnumMixin} base from smithy-core handles values that + * weren't known at generation time: deserializing an unrecognized value (or + * filling a missing required member during client error correction, see + * {@link MemberErrorCorrectionGenerator}) produces a pseudo-member with + * {@code is_unknown} set rather than raising. * * @see Smithy spec: intEnum */ @@ -43,11 +35,12 @@ public IntEnumGenerator(GenerateIntEnumDirective { writer.addStdlibImport("enum", "IntEnum"); - writer.addStdlibImport("typing", "Self"); - writer.openBlock("class $L(IntEnum):", "", enumSymbol.getName(), () -> { + writer.addDependency(SmithyPythonDependency.SMITHY_CORE); + writer.addImport("smithy_core.types", "UnknownEnumMixin"); + writer.openBlock("class $L(UnknownEnumMixin, IntEnum):", "", enumSymbol.getName(), () -> { directive.shape().getTrait(DocumentationTrait.class).ifPresent(trait -> { writer.writeDocs(trait.getValue(), directive.context()); }); @@ -60,43 +53,6 @@ public void run() { 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 b816d87ee..c14a93159 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 @@ -136,7 +136,7 @@ public Void integerShape(IntegerShape shape) { @Override public Void intEnumShape(IntEnumShape shape) { pushMemberState(); - var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + var enumSymbol = context.symbolProvider().toSymbol(shape); writer.write("$T(${deserializer:L}.read_integer(${C|}))", enumSymbol, writer.consumer(w -> writeSchema())); @@ -188,7 +188,7 @@ public Void stringShape(StringShape shape) { @Override public Void enumShape(EnumShape shape) { pushMemberState(); - var enumSymbol = context.symbolProvider().toSymbol(shape).expectProperty(SymbolProperties.ENUM_SYMBOL); + var enumSymbol = context.symbolProvider().toSymbol(shape); writer.write("$T(${deserializer:L}.read_string(${C|}))", enumSymbol, writer.consumer(w -> writeSchema())); 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 459957a2b..f4c4e435a 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,7 +4,7 @@ */ package software.amazon.smithy.python.codegen.generators; -import software.amazon.smithy.model.Model; +import java.util.function.Consumer; import software.amazon.smithy.model.knowledge.NullableIndex; import software.amazon.smithy.model.shapes.BigDecimalShape; import software.amazon.smithy.model.shapes.BigIntegerShape; @@ -21,211 +21,193 @@ 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.DefaultTrait; import software.amazon.smithy.model.traits.StreamingTrait; +import software.amazon.smithy.python.codegen.CodegenUtils; 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 + * Produces the Python expression used to fill a missing required member during client error * correction. * + *

Visiting a shape returns a consumer that writes the default expression, or {@code null} + * if no default can be synthesized for the shape (a streaming shape, or a structure with a + * required member that itself has no synthesizable default). Callers decide what to do with + * unsynthesizable members; the visitor is the single source of truth for what is synthesizable. + * * @see Smithy * 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; - public MemberErrorCorrectionGenerator(GenerationContext context, PythonWriter writer) { + public MemberErrorCorrectionGenerator(GenerationContext context) { 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, Model model) { - 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, 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 Void booleanShape(BooleanShape shape) { - writer.writeInline("False"); - return null; + public Consumer booleanShape(BooleanShape shape) { + return writer -> writer.writeInline("False"); } @Override - public Void byteShape(ByteShape shape) { - writer.writeInline("0"); - return null; + public Consumer byteShape(ByteShape shape) { + return writer -> writer.writeInline("0"); } @Override - public Void shortShape(ShortShape shape) { - writer.writeInline("0"); - return null; + public Consumer shortShape(ShortShape shape) { + return writer -> writer.writeInline("0"); } @Override - public Void integerShape(IntegerShape shape) { - writer.writeInline("0"); - return null; + public Consumer integerShape(IntegerShape shape) { + return writer -> writer.writeInline("0"); } @Override - public Void longShape(LongShape shape) { - writer.writeInline("0"); - return null; + public Consumer longShape(LongShape shape) { + return writer -> writer.writeInline("0"); } @Override - public Void bigIntegerShape(BigIntegerShape shape) { - writer.writeInline("0"); - return null; + public Consumer bigIntegerShape(BigIntegerShape shape) { + return writer -> writer.writeInline("0"); } @Override - public Void floatShape(FloatShape shape) { - writer.writeInline("0.0"); - return null; + public Consumer floatShape(FloatShape shape) { + return writer -> writer.writeInline("0.0"); } @Override - public Void doubleShape(DoubleShape shape) { - writer.writeInline("0.0"); - return null; + public Consumer doubleShape(DoubleShape shape) { + return writer -> writer.writeInline("0.0"); } @Override - public Void bigDecimalShape(BigDecimalShape shape) { - writer.addStdlibImport("decimal", "Decimal"); - writer.writeInline("Decimal(0)"); - return null; + public Consumer bigDecimalShape(BigDecimalShape shape) { + return writer -> { + writer.addStdlibImport("decimal", "Decimal"); + writer.writeInline("Decimal(0)"); + }; } @Override - public Void stringShape(StringShape shape) { - writer.writeInline("\"\""); - return null; + public Consumer stringShape(StringShape shape) { + return writer -> writer.writeInline("\"\""); } @Override - public Void blobShape(BlobShape shape) { - writer.writeInline("b\"\""); - return null; + public Consumer blobShape(BlobShape shape) { + // 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. + if (shape.hasTrait(StreamingTrait.class)) { + return null; + } + return writer -> writer.writeInline("b\"\""); } @Override - public Void timestampShape(TimestampShape shape) { - writer.addStdlibImport("datetime", "datetime"); - writer.addStdlibImport("datetime", "timezone"); - writer.writeInline("datetime.fromtimestamp(0, tz=timezone.utc)"); - return null; + public Consumer timestampShape(TimestampShape shape) { + return writer -> { + writer.addStdlibImport("datetime", "datetime"); + writer.addStdlibImport("datetime", "timezone"); + writer.writeInline("datetime.fromtimestamp(0, tz=timezone.utc)"); + }; } @Override - public Void documentShape(DocumentShape shape) { - writer.addImport("smithy_core.documents", "Document"); - writer.writeInline("Document(None)"); - return null; + public Consumer documentShape(DocumentShape shape) { + return writer -> { + writer.addImport("smithy_core.documents", "Document"); + writer.writeInline("Document(None)"); + }; } @Override - public Void listShape(ListShape shape) { - writer.writeInline("[]"); - return null; + public Consumer listShape(ListShape shape) { + return writer -> writer.writeInline("[]"); } @Override - public Void mapShape(MapShape shape) { - writer.writeInline("{}"); - return null; + public Consumer mapShape(MapShape shape) { + return writer -> writer.writeInline("{}"); } @Override - 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; + public Consumer enumShape(EnumShape shape) { + var enumSymbol = context.symbolProvider().toSymbol(shape); + return writer -> { + writer.addImport(enumSymbol, enumSymbol.getName()); + writer.writeInline("$L._unknown(\"\")", enumSymbol.getName()); + }; } @Override - 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; + public Consumer intEnumShape(IntEnumShape shape) { + var enumSymbol = context.symbolProvider().toSymbol(shape); + return writer -> { + writer.addImport(enumSymbol, enumSymbol.getName()); + writer.writeInline("$L._unknown(0)", enumSymbol.getName()); + }; } @Override - public Void unionShape(UnionShape shape) { + public Consumer unionShape(UnionShape shape) { + // Streaming unions (event streams) have no synthesizable default; they also never + // appear as dataclass properties, so missing ones don't need correction. + if (shape.hasTrait(StreamingTrait.class)) { + return null; + } var unknownSymbol = context.symbolProvider() .toSymbol(shape) .expectProperty(SymbolProperties.UNION_UNKNOWN); - writer.addImport(unknownSymbol, unknownSymbol.getName()); - writer.writeInline("$L(tag=\"\")", unknownSymbol.getName()); - return null; + return writer -> { + writer.addImport(unknownSymbol, unknownSymbol.getName()); + writer.writeInline("$L(tag=\"\")", unknownSymbol.getName()); + }; } + /** + * 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 + */ @Override - public Void structureShape(StructureShape shape) { + public Consumer structureShape(StructureShape shape) { + var index = NullableIndex.of(context.model()); + for (MemberShape member : shape.members()) { + if (!CodegenUtils.isRequiredMember(index, member)) { + continue; + } + if (context.model().expectShape(member.getTarget()).accept(this) == null) { + return null; + } + } var symbol = context.symbolProvider().toSymbol(shape); - writer.addImport(symbol, symbol.getName()); - writer.writeInline("$L._smithy_default()", symbol.getName()); - return null; + return writer -> { + writer.addImport(symbol, symbol.getName()); + writer.writeInline("$L._smithy_default()", symbol.getName()); + }; } @Override - public Void memberShape(MemberShape shape) { + public Consumer 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 0ad0338eb..f006b5f0b 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 @@ -29,6 +29,7 @@ import software.amazon.smithy.python.codegen.CodegenUtils; import software.amazon.smithy.python.codegen.GenerationContext; import software.amazon.smithy.python.codegen.PythonSettings; +import software.amazon.smithy.python.codegen.RequiredMemberTargetIndex; import software.amazon.smithy.python.codegen.SymbolProperties; import software.amazon.smithy.python.codegen.writer.PythonWriter; import software.amazon.smithy.utils.SmithyInternalApi; @@ -67,10 +68,10 @@ public StructureGenerator( var optional = new ArrayList(); var index = NullableIndex.of(context.model()); for (MemberShape member : shape.members()) { - if (index.isMemberNullable(member) || member.hasTrait(DefaultTrait.class)) { - optional.add(member); - } else { + if (CodegenUtils.isRequiredMember(index, member)) { required.add(member); + } else { + optional.add(member); } } this.requiredMembers = filterPropertyMembers(required); @@ -280,11 +281,11 @@ private String getDefaultValue(PythonWriter writer, MemberShape member) { 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); + var enumSymbol = symbolProvider.toSymbol(target); 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); + var enumSymbol = symbolProvider.toSymbol(target); writer.addImport(enumSymbol, enumSymbol.getName()); return String.format("%s(%s)", enumSymbol.getName(), defaultNode.expectNumberNode().getValue()); } @@ -407,11 +408,11 @@ def _consumer(schema: Schema, de: ShapeDeserializer) -> None: * spec: Client error correction */ private void writeErrorCorrection() { - var visitor = new MemberErrorCorrectionGenerator(context, writer); + var visitor = new MemberErrorCorrectionGenerator(context); for (MemberShape member : requiredMembers) { - var target = model.expectShape(member.getTarget()); - if (!MemberErrorCorrectionGenerator.hasDefault(target, model)) { - // Streaming shapes have no synthesizable default; let the dataclass raise. + var defaultExpression = model.expectShape(member.getTarget()).accept(visitor); + if (defaultExpression == null) { + // No synthesizable default (e.g. a streaming blob); let the dataclass raise. continue; } writer.pushState(); @@ -419,7 +420,7 @@ private void writeErrorCorrection() { writer.write(""" if ${memberName:S} not in kwargs: kwargs[${memberName:S}] = ${C|}""", - writer.consumer(w -> target.accept(visitor))); + writer.consumer(defaultExpression)); writer.popState(); } } @@ -434,52 +435,30 @@ private void writeErrorCorrection() { * {@code _smithy_default()} is also omitted. */ private void generateSmithyDefaultMethod() { - if (!isRequiredStructMemberTarget()) { + if (!RequiredMemberTargetIndex.of(model).isRequiredMemberTarget(shape.getId())) { return; } - for (MemberShape member : requiredMembers) { - var target = model.expectShape(member.getTarget()); - if (!MemberErrorCorrectionGenerator.hasDefault(target, model)) { - return; - } + var visitor = new MemberErrorCorrectionGenerator(context); + if (shape.accept(visitor) == null) { + return; } writer.write(""" @classmethod def _smithy_default(cls) -> Self: return cls(${C|}) """, - writer.consumer(w -> writeSmithyDefaultArguments())); + writer.consumer(w -> writeSmithyDefaultArguments(visitor))); } - /** - * 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); + private void writeSmithyDefaultArguments(MemberErrorCorrectionGenerator visitor) { 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); + model.expectShape(member.getTarget()).accept(visitor).accept(writer); } } diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonDelegator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonDelegator.java index 0e6f5c7ca..cc10ce5c1 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonDelegator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonDelegator.java @@ -5,13 +5,9 @@ package software.amazon.smithy.python.codegen.writer; import software.amazon.smithy.build.FileManifest; -import software.amazon.smithy.codegen.core.Symbol; import software.amazon.smithy.codegen.core.SymbolProvider; import software.amazon.smithy.codegen.core.WriterDelegator; -import software.amazon.smithy.model.shapes.MemberShape; -import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.python.codegen.PythonSettings; -import software.amazon.smithy.python.codegen.SymbolProperties; import software.amazon.smithy.utils.SmithyInternalApi; /** @@ -27,30 +23,7 @@ public PythonDelegator( ) { super( fileManifest, - new EnumSymbolProviderWrapper(symbolProvider), + symbolProvider, new PythonWriter.PythonWriterFactory(settings)); } - - private static final class EnumSymbolProviderWrapper implements SymbolProvider { - - private final SymbolProvider wrapped; - - EnumSymbolProviderWrapper(SymbolProvider wrapped) { - this.wrapped = wrapped; - } - - @Override - public Symbol toSymbol(Shape shape) { - Symbol symbol = wrapped.toSymbol(shape); - if (shape.isEnumShape() || shape.isIntEnumShape()) { - symbol = symbol.expectProperty(SymbolProperties.ENUM_SYMBOL); - } - return symbol; - } - - @Override - public String toMemberName(MemberShape shape) { - return wrapped.toMemberName(shape); - } - } } diff --git a/packages/smithy-core/src/smithy_core/types.py b/packages/smithy-core/src/smithy_core/types.py index b902e4e6a..f55f192b1 100644 --- a/packages/smithy-core/src/smithy_core/types.py +++ b/packages/smithy-core/src/smithy_core/types.py @@ -9,7 +9,7 @@ from datetime import datetime from email.utils import format_datetime, parsedate_to_datetime from enum import Enum -from typing import TYPE_CHECKING, Any, overload +from typing import TYPE_CHECKING, Any, ClassVar, Self, overload from .exceptions import ExpectationNotMetError from .interfaces import PropertyKey as _PropertyKey @@ -66,6 +66,50 @@ def from_json(j: Any) -> "JsonBlob": return json_string +class UnknownEnumMixin: + """Adds unknown-value support to a generated enum. + + Must be mixed into an :class:`enum.Enum` with a data type, e.g. + ``class Color(UnknownEnumMixin, StrEnum)``. A value that doesn't match any + known member — because the service is newer than the SDK, or because client + error correction filled in a placeholder — is represented as a pseudo-member + with :attr:`is_unknown` set instead of raising ``ValueError``. Pseudo-members + keep the native ``str``/``int`` semantics, so they still compare equal to + their raw value. + + See `Smithy's docs `_ + for more details on client error correction. + """ + + if TYPE_CHECKING: + # Set by EnumType when this is mixed into an enum with a data type. + _member_type_: ClassVar[type[Any]] + _name_: str + _value_: Any + + _smithy_unknown: bool = False + + @classmethod + def _unknown(cls, value: Any) -> Self: + member_type: Any = cls._member_type_ + pseudo: Self = member_type.__new__(cls, value) + pseudo._name_ = f"" + pseudo._value_ = value + pseudo._smithy_unknown = True + return pseudo + + @classmethod + def _missing_(cls, value: object) -> Self | None: + if isinstance(value, cls._member_type_): + 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._smithy_unknown + + class TimestampFormat(Enum): """Smithy-defined timestamp formats with serialization and deserialization helpers. diff --git a/packages/smithy-core/tests/unit/test_types.py b/packages/smithy-core/tests/unit/test_types.py index 3dac29464..1d4a85753 100644 --- a/packages/smithy-core/tests/unit/test_types.py +++ b/packages/smithy-core/tests/unit/test_types.py @@ -3,6 +3,7 @@ # pyright: reportPrivateUsage=false from datetime import UTC, datetime +from enum import IntEnum, StrEnum from typing import Any, assert_type import pytest @@ -14,6 +15,7 @@ PropertyKey, TimestampFormat, TypedProperties, + UnknownEnumMixin, ) @@ -351,3 +353,63 @@ def test_parametric_property() -> None: properties[parametric] = {"foo": "bar"} assert assert_type(properties[parametric], dict[str, str]) == {"foo": "bar"} + + +class _Color(UnknownEnumMixin, StrEnum): + RED = "RED" + GREEN = "GREEN" + + +class _Code(UnknownEnumMixin, IntEnum): + OK = 200 + NOT_FOUND = 404 + + +def test_unknown_enum_known_values_resolve_to_members() -> None: + assert _Color("RED") is _Color.RED + assert _Code(200) is _Code.OK + assert not _Color.RED.is_unknown + assert not _Code.OK.is_unknown + + +def test_unknown_enum_unrecognized_value_does_not_raise() -> None: + color = _Color("CHARTREUSE") + assert color.is_unknown + assert color.value == "CHARTREUSE" + + code = _Code(418) + assert code.is_unknown + assert code.value == 418 + + +def test_unknown_enum_keeps_native_equality() -> None: + assert _Color("CHARTREUSE") == "CHARTREUSE" + assert _Code(418) == 418 + assert _Color("CHARTREUSE") == _Color("CHARTREUSE") + assert _Color("CHARTREUSE") != _Color.RED + + # Pseudo-members hash like their raw value, so dict lookups by raw value work. + codes: dict[Any, str] = {_Code(418): "teapot"} + assert codes[418] == "teapot" + + +def test_unknown_enum_placeholder_for_error_correction() -> None: + color = _Color._unknown("") + assert color.is_unknown + assert color == "" + + code = _Code._unknown(0) + assert code.is_unknown + assert code == 0 + + +def test_unknown_enum_rejects_mismatched_type() -> None: + with pytest.raises(ValueError): + _Color(0) # type: ignore[arg-type] + with pytest.raises(ValueError): + _Code("RED") # type: ignore[arg-type] + + +def test_unknown_enum_str_and_serialization_behavior() -> None: + assert str(_Color("CHARTREUSE")) == "CHARTREUSE" + assert int(_Code(418)) == 418 From b87515b0fd5fe6261419d84ff1d0232cbc7ce099 Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Thu, 11 Jun 2026 02:02:19 -0400 Subject: [PATCH 4/5] Omit empty error-correction line in deserialize_kwargs Structures with no correctable required members previously emitted a stray blank line between read_struct() and return kwargs, causing whitespace-only churn in regenerated clients. --- .../generators/StructureGenerator.java | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) 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 f006b5f0b..4f54d5c14 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 @@ -9,8 +9,11 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.function.Consumer; import java.util.logging.Logger; import java.util.stream.Collectors; import software.amazon.smithy.codegen.core.SymbolProvider; @@ -374,6 +377,9 @@ private void generateDeserializeMethod() { var schemaSymbol = symbolProvider.toSymbol(shape).expectProperty(SymbolProperties.SCHEMA); writer.putContext("schema", schemaSymbol); + var corrections = errorCorrections(); + writer.putContext("errorCorrection", !corrections.isEmpty()); + // TODO: either formalize deserialize_kwargs or remove it when http serde is converted writer.write(""" @classmethod @@ -391,38 +397,49 @@ def _consumer(schema: Schema, de: ShapeDeserializer) -> None: logger.debug("Unexpected member schema: %s", schema) deserializer.read_struct($T, consumer=_consumer) + ${?errorCorrection} ${C|} + ${/errorCorrection} return kwargs """, writer.consumer(w -> deserializeMembers(shape.members())), schemaSymbol, - writer.consumer(w -> writeErrorCorrection())); + writer.consumer(w -> writeErrorCorrection(corrections))); writer.popState(); } /** - * Emits client error correction for required members the server failed to serialize. + * Collects client error correction defaults for required members the server failed + * to serialize, keyed by member name. Members whose targets have no synthesizable + * default (e.g. a streaming blob) are omitted; the dataclass will raise for those. * * @see Smithy * spec: Client error correction */ - private void writeErrorCorrection() { + private Map> errorCorrections() { var visitor = new MemberErrorCorrectionGenerator(context); + var corrections = new LinkedHashMap>(); for (MemberShape member : requiredMembers) { var defaultExpression = model.expectShape(member.getTarget()).accept(visitor); if (defaultExpression == null) { - // No synthesizable default (e.g. a streaming blob); let the dataclass raise. continue; } + corrections.put(symbolProvider.toMemberName(member), defaultExpression); + } + return corrections; + } + + private void writeErrorCorrection(Map> corrections) { + corrections.forEach((memberName, defaultExpression) -> { writer.pushState(); - writer.putContext("memberName", symbolProvider.toMemberName(member)); + writer.putContext("memberName", memberName); writer.write(""" if ${memberName:S} not in kwargs: kwargs[${memberName:S}] = ${C|}""", writer.consumer(defaultExpression)); writer.popState(); - } + }); } /** From 61e0151b6007bd3040e2a211f7e4bcdddf23932e Mon Sep 17 00:00:00 2001 From: Yuxuan Chen Date: Thu, 11 Jun 2026 16:41:43 -0400 Subject: [PATCH 5/5] Distinguish error-corrected enum placeholders from wire-unknown values --- .../MemberErrorCorrectionGenerator.java | 6 ++- ...ture-a6b53cf89aa64daa8f9c25cc52c20875.json | 4 ++ packages/smithy-core/src/smithy_core/types.py | 34 ++++++++++----- packages/smithy-core/tests/unit/test_types.py | 42 ++++++++++++++++--- ...gfix-2eaf1f49875f4690b7c58f7735f1de7d.json | 4 ++ 5 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 packages/smithy-core/.changes/next-release/smithy-core-feature-a6b53cf89aa64daa8f9c25cc52c20875.json create mode 100644 packages/smithy-json/.changes/next-release/smithy-json-bugfix-2eaf1f49875f4690b7c58f7735f1de7d.json 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 f4c4e435a..372af8327 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 @@ -151,7 +151,7 @@ public Consumer enumShape(EnumShape shape) { var enumSymbol = context.symbolProvider().toSymbol(shape); return writer -> { writer.addImport(enumSymbol, enumSymbol.getName()); - writer.writeInline("$L._unknown(\"\")", enumSymbol.getName()); + writer.writeInline("$L._corrected(\"\")", enumSymbol.getName()); }; } @@ -160,7 +160,9 @@ public Consumer intEnumShape(IntEnumShape shape) { var enumSymbol = context.symbolProvider().toSymbol(shape); return writer -> { writer.addImport(enumSymbol, enumSymbol.getName()); - writer.writeInline("$L._unknown(0)", enumSymbol.getName()); + // -1 is used (rather than 0) as the placeholder because it is least likely to + // collide with a real member value. + writer.writeInline("$L._corrected(-1)", enumSymbol.getName()); }; } diff --git a/packages/smithy-core/.changes/next-release/smithy-core-feature-a6b53cf89aa64daa8f9c25cc52c20875.json b/packages/smithy-core/.changes/next-release/smithy-core-feature-a6b53cf89aa64daa8f9c25cc52c20875.json new file mode 100644 index 000000000..4652c7839 --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-feature-a6b53cf89aa64daa8f9c25cc52c20875.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "Added `UnknownEnumMixin` for representing unknown and error-corrected enum/intEnum variants." +} \ No newline at end of file diff --git a/packages/smithy-core/src/smithy_core/types.py b/packages/smithy-core/src/smithy_core/types.py index f55f192b1..e927b41fb 100644 --- a/packages/smithy-core/src/smithy_core/types.py +++ b/packages/smithy-core/src/smithy_core/types.py @@ -69,16 +69,11 @@ def from_json(j: Any) -> "JsonBlob": class UnknownEnumMixin: """Adds unknown-value support to a generated enum. - Must be mixed into an :class:`enum.Enum` with a data type, e.g. - ``class Color(UnknownEnumMixin, StrEnum)``. A value that doesn't match any - known member — because the service is newer than the SDK, or because client - error correction filled in a placeholder — is represented as a pseudo-member - with :attr:`is_unknown` set instead of raising ``ValueError``. Pseudo-members - keep the native ``str``/``int`` semantics, so they still compare equal to - their raw value. - - See `Smithy's docs `_ - for more details on client error correction. + Mixed into an :class:`enum.Enum` with a data type, e.g. + ``class Color(UnknownEnumMixin, StrEnum)``. Unrecognized values become + pseudo-members (flagged by :attr:`is_unknown`) instead of raising. A value + from the wire keeps native equality; an error-correction placeholder + (:meth:`_corrected`) equals only itself. """ if TYPE_CHECKING: @@ -88,6 +83,7 @@ class UnknownEnumMixin: _value_: Any _smithy_unknown: bool = False + _smithy_corrected: bool = False @classmethod def _unknown(cls, value: Any) -> Self: @@ -98,12 +94,30 @@ def _unknown(cls, value: Any) -> Self: pseudo._smithy_unknown = True return pseudo + @classmethod + def _corrected(cls, value: Any) -> Self: + pseudo = cls._unknown(value) + pseudo._smithy_corrected = True + return pseudo + @classmethod def _missing_(cls, value: object) -> Self | None: if isinstance(value, cls._member_type_): return cls._unknown(value) return None + def __eq__(self, other: object) -> bool: + if self._smithy_corrected or getattr(other, "_smithy_corrected", False): + return self is other + return super().__eq__(other) + + def __ne__(self, other: object) -> bool: + result = self.__eq__(other) + return result if result is NotImplemented else not result + + def __hash__(self) -> int: + return super().__hash__() + @property def is_unknown(self) -> bool: """True if this value was not known at SDK generation time.""" diff --git a/packages/smithy-core/tests/unit/test_types.py b/packages/smithy-core/tests/unit/test_types.py index 1d4a85753..8debfbf4a 100644 --- a/packages/smithy-core/tests/unit/test_types.py +++ b/packages/smithy-core/tests/unit/test_types.py @@ -361,6 +361,7 @@ class _Color(UnknownEnumMixin, StrEnum): class _Code(UnknownEnumMixin, IntEnum): + ZERO = 0 OK = 200 NOT_FOUND = 404 @@ -393,14 +394,45 @@ def test_unknown_enum_keeps_native_equality() -> None: assert codes[418] == "teapot" -def test_unknown_enum_placeholder_for_error_correction() -> None: - color = _Color._unknown("") +def test_wire_unknown_keeps_native_equality() -> None: + # A value the SDK doesn't recognize keeps native equality (forwards-compatible). + wire = _Code(418) + assert wire.is_unknown + assert not wire._smithy_corrected + assert wire == 418 + + +def test_error_corrected_placeholder_is_equal_to_nothing() -> None: + # An invented placeholder for a missing required member equals only itself — + # not its raw value, not a known member sharing it, not another placeholder. + color = _Color._corrected("") assert color.is_unknown - assert color == "" + assert color._smithy_corrected + assert color != "" + assert color != _Color.RED + assert color != _Color._corrected("") + assert color == color - code = _Code._unknown(0) + code = _Code._corrected(0) assert code.is_unknown - assert code == 0 + assert code._smithy_corrected + assert code != 0 + assert code != _Code.ZERO + assert code != _Code._corrected(0) + assert code == code + + assert {_Code.ZERO: "zero"}.get(code) is None + + +def test_wire_unknown_and_corrected_differ_on_equality() -> None: + # Both report is_unknown, but the wire-unknown value keeps native equality + # while the invented placeholder matches nothing. + wire = _Code(418) + corrected = _Code._corrected(418) + assert wire.is_unknown and corrected.is_unknown + assert wire == 418 + assert corrected != 418 + assert wire != corrected def test_unknown_enum_rejects_mismatched_type() -> None: diff --git a/packages/smithy-json/.changes/next-release/smithy-json-bugfix-2eaf1f49875f4690b7c58f7735f1de7d.json b/packages/smithy-json/.changes/next-release/smithy-json-bugfix-2eaf1f49875f4690b7c58f7735f1de7d.json new file mode 100644 index 000000000..ebbb1b2e3 --- /dev/null +++ b/packages/smithy-json/.changes/next-release/smithy-json-bugfix-2eaf1f49875f4690b7c58f7735f1de7d.json @@ -0,0 +1,4 @@ +{ + "type": "bugfix", + "description": "Serialize `IntEnum` members as their underlying integer instead of their `repr`." +} \ No newline at end of file