diff --git a/codegen-output/aws-query/src/awsquery/models.pyi b/codegen-output/aws-query/src/awsquery/models.pyi index 8586a6ca2..c621469e0 100644 --- a/codegen-output/aws-query/src/awsquery/models.pyi +++ b/codegen-output/aws-query/src/awsquery/models.pyi @@ -341,6 +341,7 @@ class QueryIdempotencyTokenAutoFillInput(_SerdeShape): """Dataclass for QueryIdempotencyTokenAutoFillInput structure.""" token: str | None = ... + """This field is autopopulated if not provided.""" @dataclass(kw_only=True) class QueryIdempotencyTokenAutoFillOutput(_SerdeShape): diff --git a/codegen-output/aws-query/tests/test_awsquery_protocol.py b/codegen-output/aws-query/tests/test_awsquery_protocol.py index efbec6365..3bf0996c2 100644 --- a/codegen-output/aws-query/tests/test_awsquery_protocol.py +++ b/codegen-output/aws-query/tests/test_awsquery_protocol.py @@ -1,6 +1,7 @@ # Code generated by smithy-python-codegen DO NOT EDIT. from datetime import datetime, timezone +from unittest.mock import patch from urllib.parse import parse_qsl from pytest import fail, mark, raises @@ -1254,7 +1255,10 @@ async def test_sdk_appends_gzip_and_ignores_http_provided_encoding_aws_query_req ) -@mark.xfail() +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_query_protocol_idempotency_token_auto_fill_request_query_idempotency_token_auto_fill() -> ( None ): @@ -1335,6 +1339,10 @@ async def test_query_protocol_idempotency_token_auto_fill_request_query_idempote ) +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_query_protocol_idempotency_token_auto_fill_is_set_request_query_idempotency_token_auto_fill() -> ( None ): diff --git a/codegen-output/ec2-query/src/ec2query/models.pyi b/codegen-output/ec2-query/src/ec2query/models.pyi index a4950d0a0..861cda1f8 100644 --- a/codegen-output/ec2-query/src/ec2query/models.pyi +++ b/codegen-output/ec2-query/src/ec2query/models.pyi @@ -256,6 +256,7 @@ class QueryIdempotencyTokenAutoFillInput(_SerdeShape): """Dataclass for QueryIdempotencyTokenAutoFillInput structure.""" token: str | None = ... + """This field is autopopulated if not provided.""" @dataclass(kw_only=True) class QueryIdempotencyTokenAutoFillOutput(_SerdeShape): diff --git a/codegen-output/ec2-query/tests/test_ec2query_protocol.py b/codegen-output/ec2-query/tests/test_ec2query_protocol.py index aa4711e08..d6fc8dc31 100644 --- a/codegen-output/ec2-query/tests/test_ec2query_protocol.py +++ b/codegen-output/ec2-query/tests/test_ec2query_protocol.py @@ -1,6 +1,7 @@ # Code generated by smithy-python-codegen DO NOT EDIT. from datetime import datetime, timezone +from unittest.mock import patch from urllib.parse import parse_qsl from pytest import fail, mark, raises @@ -989,7 +990,10 @@ async def test_sdk_appends_gzip_and_ignores_http_provided_encoding_ec2_query_req ) -@mark.xfail() +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_ec2_protocol_idempotency_token_auto_fill_request_query_idempotency_token_auto_fill() -> ( None ): @@ -1070,6 +1074,10 @@ async def test_ec2_protocol_idempotency_token_auto_fill_request_query_idempotenc ) +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_ec2_protocol_idempotency_token_auto_fill_is_set_request_query_idempotency_token_auto_fill() -> ( None ): diff --git a/codegen-output/rest-json-1/src/restjson/_private/models_0.pyi b/codegen-output/rest-json-1/src/restjson/_private/models_0.pyi index 4b59caccd..baff64be5 100644 --- a/codegen-output/rest-json-1/src/restjson/_private/models_0.pyi +++ b/codegen-output/rest-json-1/src/restjson/_private/models_0.pyi @@ -2255,6 +2255,7 @@ class QueryIdempotencyTokenAutoFillInput(_SerdeShape): """Dataclass for QueryIdempotencyTokenAutoFillInput structure.""" token: str | None = ... + """This field is autopopulated if not provided.""" @dataclass(kw_only=True) class QueryIdempotencyTokenAutoFillOutput(_SerdeShape): diff --git a/codegen-output/rest-json-1/tests/test_restjson_protocol.py b/codegen-output/rest-json-1/tests/test_restjson_protocol.py index 6de4c170e..b972806b9 100644 --- a/codegen-output/rest-json-1/tests/test_restjson_protocol.py +++ b/codegen-output/rest-json-1/tests/test_restjson_protocol.py @@ -2,6 +2,7 @@ from datetime import datetime, timezone import json +from unittest.mock import patch from urllib.parse import parse_qsl from pytest import fail, mark, raises @@ -10936,7 +10937,10 @@ async def test_sdk_appended_gzip_after_provided_encoding_rest_json1_request_put_ ) -@mark.xfail() +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_rest_json_query_idempotency_token_auto_fill_request_query_idempotency_token_auto_fill() -> ( None ): @@ -11011,6 +11015,10 @@ async def test_rest_json_query_idempotency_token_auto_fill_request_query_idempot ) +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_rest_json_query_idempotency_token_auto_fill_is_set_request_query_idempotency_token_auto_fill() -> ( None ): diff --git a/codegen-output/rest-xml/src/restxml/models.pyi b/codegen-output/rest-xml/src/restxml/models.pyi index 9bb42d629..49a8eead1 100644 --- a/codegen-output/rest-xml/src/restxml/models.pyi +++ b/codegen-output/rest-xml/src/restxml/models.pyi @@ -942,6 +942,7 @@ class QueryIdempotencyTokenAutoFillInput(_SerdeShape): """Dataclass for QueryIdempotencyTokenAutoFillInput structure.""" token: str | None = ... + """This field is autopopulated if not provided.""" @dataclass(kw_only=True) class QueryIdempotencyTokenAutoFillOutput(_SerdeShape): diff --git a/codegen-output/rest-xml/tests/test_restxml_protocol.py b/codegen-output/rest-xml/tests/test_restxml_protocol.py index 6c1d7719e..287a5e3a0 100644 --- a/codegen-output/rest-xml/tests/test_restxml_protocol.py +++ b/codegen-output/rest-xml/tests/test_restxml_protocol.py @@ -1,6 +1,7 @@ # Code generated by smithy-python-codegen DO NOT EDIT. from datetime import datetime, timezone +from unittest.mock import patch from urllib.parse import parse_qsl from pytest import fail, mark, raises @@ -5864,7 +5865,10 @@ async def test_sdk_appended_gzip_after_provided_encoding_rest_xml_request_put_wi ) -@mark.xfail() +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_query_idempotency_token_auto_fill_request_query_idempotency_token_auto_fill() -> ( None ): @@ -5939,6 +5943,10 @@ async def test_query_idempotency_token_auto_fill_request_query_idempotency_token ) +@patch( + "smithy_core._pipeline._default_idempotency_token", + new=lambda: "00000000-0000-4000-8000-000000000000", +) async def test_query_idempotency_token_auto_fill_is_set_request_query_idempotency_token_auto_fill() -> ( None ): diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsQueryProtocolGenerator.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsQueryProtocolGenerator.java index 862deb912..8075f6431 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsQueryProtocolGenerator.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsQueryProtocolGenerator.java @@ -23,9 +23,6 @@ public final class AwsQueryProtocolGenerator implements ProtocolGenerator { "SDKAppliedContentEncoding_awsQuery", "SDKAppendsGzipAndIgnoresHttpProvidedEncoding_awsQuery", - // TODO: support idempotency token autofill - "QueryProtocolIdempotencyTokenAutoFill", - // TODO: support of the endpoint trait "AwsQueryEndpointTraitWithHostLabel", "AwsQueryEndpointTrait"); diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/Ec2QueryProtocolGenerator.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/Ec2QueryProtocolGenerator.java index c10aaadff..30dd0b54c 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/Ec2QueryProtocolGenerator.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/Ec2QueryProtocolGenerator.java @@ -23,9 +23,6 @@ public final class Ec2QueryProtocolGenerator implements ProtocolGenerator { "SDKAppliedContentEncoding_ec2Query", "SDKAppendsGzipAndIgnoresHttpProvidedEncoding_ec2Query", - // TODO: support idempotency token autofill - "Ec2ProtocolIdempotencyTokenAutoFill", - // TODO: support of the endpoint trait "Ec2QueryEndpointTraitWithHostLabel", "Ec2QueryEndpointTrait"); diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/RestXmlProtocolGenerator.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/RestXmlProtocolGenerator.java index fc714904c..02340265e 100644 --- a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/RestXmlProtocolGenerator.java +++ b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/RestXmlProtocolGenerator.java @@ -25,9 +25,6 @@ public final class RestXmlProtocolGenerator implements ProtocolGenerator { "SDKAppliedContentEncoding_restXml", "SDKAppendedGzipAfterProvidedEncoding_restXml", - // TODO: support idempotency token autofill - "QueryIdempotencyTokenAutoFill", - // TODO: support of the endpoint trait "RestXmlEndpointTraitWithHostLabelAndHttpBinding", "RestXmlEndpointTraitWithHostLabel", 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 cdd21fa7c..ad5beaf38 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 @@ -29,11 +29,14 @@ import software.amazon.smithy.codegen.core.Symbol; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.NullableIndex; +import software.amazon.smithy.model.knowledge.OperationIndex; 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.DocumentationTrait; import software.amazon.smithy.model.traits.ErrorTrait; +import software.amazon.smithy.model.traits.IdempotencyTokenTrait; import software.amazon.smithy.model.traits.StreamingTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait.Format; @@ -179,6 +182,33 @@ public static boolean isEventStreamMember(Model model, MemberShape member) { return target.isUnionShape() && target.hasTrait(StreamingTrait.class); } + /** + * The note appended to the docs of an idempotency token member the client fills. + * Matches botocore's wording. + */ + public static final String IDEMPOTENCY_TOKEN_DOCS = "This field is autopopulated if not provided."; + + /** + * Gets the documentation to generate for a member. + * + *

This is the member's documentation trait (or its target's). Members the client + * auto-fills, {@code @idempotencyToken} members of an operation input, also get + * {@link #IDEMPOTENCY_TOKEN_DOCS} as a final paragraph, or as their only docs. + * + * @param model The model being generated. + * @param member The member to document. + * @return Returns the member's documentation, if it has any. + */ + public static Optional getMemberDocs(Model model, MemberShape member) { + var docs = member.getMemberTrait(model, DocumentationTrait.class).map(DocumentationTrait::getValue); + if (!member.hasTrait(IdempotencyTokenTrait.class) + || !OperationIndex.of(model).isInputStructure(member.getContainer())) { + return docs; + } + return Optional.of(docs.map(value -> value + "\n\n" + IDEMPOTENCY_TOKEN_DOCS) + .orElse(IDEMPOTENCY_TOKEN_DOCS)); + } + /** * Determines whether a member is required in the generated Python constructor — that is, * neither nullable nor carrying a default value. 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 1d1d1f868..a5fa13736 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 @@ -34,6 +34,7 @@ import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.traits.IdempotencyTokenTrait; import software.amazon.smithy.model.traits.StreamingTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait.Format; import software.amazon.smithy.protocoltests.traits.AppliesTo; @@ -70,6 +71,10 @@ public final class HttpProtocolTestGenerator implements Runnable { private static final Symbol TEST_HTTP_SERVICE_ERR_SYMBOL = Symbol.builder() .name("TestHttpServiceError") .build(); + /** The idempotency token value protocol tests expect when the caller leaves the member unset. */ + private static final String TEST_IDEMPOTENCY_TOKEN = "00000000-0000-4000-8000-000000000000"; + /** The runtime function that generates auto-filled idempotency tokens. */ + private static final String IDEMPOTENCY_TOKEN_GENERATOR = "smithy_core._pipeline._default_idempotency_token"; private final PythonSettings settings; private final Model model; @@ -160,6 +165,7 @@ private void generateRequestTest(OperationShape operation, HttpRequestTestCase t testCase, String.format("%s_request_%s", testCase.getId(), operation.getId().getName()), testFilter.test(operation, testCase), + () -> writeIdempotencyTokenPatch(operation), () -> { var hostSplit = testCase.getHost().orElse("example.com").split("/", 2); var host = hostSplit[0]; @@ -634,6 +640,16 @@ private void writeTestBlock( String testName, boolean shouldSkip, Runnable f + ) { + writeTestBlock(testCase, testName, shouldSkip, () -> {}, f); + } + + private void writeTestBlock( + HttpMessageTestCase testCase, + String testName, + boolean shouldSkip, + Runnable decorators, + Runnable f ) { LOGGER.fine(String.format("Writing test block for %s", testName)); writer.addDependency(SmithyPythonDependency.PYTEST); @@ -644,6 +660,7 @@ private void writeTestBlock( writer.addImport(SmithyPythonDependency.PYTEST.packageName(), "mark", "mark"); writer.write("@mark.xfail()"); } + decorators.run(); writer.openBlock("async def test_$L() -> None:", "", CaseUtils.toSnakeCase(testName), () -> { testCase.getDocumentation().ifPresent(docs -> writer.writeDocs(docs, context)); f.run(); @@ -690,6 +707,20 @@ private void writeSigV4TestConfig() { """, RuntimeTypes.STATIC_CREDENTIALS_RESOLVER); } + /** + * Pins the auto-filled idempotency token so request tests can assert it. + * The generator is private to the runtime, so the test patches it. Only + * written for operations whose input has an {@code @idempotencyToken} member. + */ + private void writeIdempotencyTokenPatch(OperationShape operation) { + var inputShape = model.expectShape(operation.getInputShape(), StructureShape.class); + if (inputShape.members().stream().noneMatch(member -> member.hasTrait(IdempotencyTokenTrait.class))) { + return; + } + writer.addStdlibImport("unittest.mock", "patch"); + writer.write("@patch($S, new=lambda: $S)", IDEMPOTENCY_TOKEN_GENERATOR, TEST_IDEMPOTENCY_TOKEN); + } + private void writeUtilStubs(Symbol serviceSymbol) { LOGGER.fine(String.format("Writing utility stubs for %s : %s", serviceSymbol.getName(), protocol.getName())); writer.addLocallyDefinedSymbol(TEST_HTTP_SERVICE_ERR_SYMBOL); diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ModelStubGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ModelStubGenerator.java index f7ca1d86a..b87c110c0 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ModelStubGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ModelStubGenerator.java @@ -267,8 +267,7 @@ private void writeShapeDocs(PythonWriter writer, Shape shape, String fallback) { } private void writeMemberDocs(PythonWriter writer, MemberShape member) { - member.getMemberTrait(model, DocumentationTrait.class) - .ifPresent(trait -> writer.writeDocs(trait.getValue(), context)); + CodegenUtils.getMemberDocs(model, member).ifPresent(docs -> writer.writeDocs(docs, context)); } private boolean carriesResponseMetadata(Shape shape) { 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 76eabd7a2..fa4070ef0 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 @@ -294,9 +294,7 @@ private void writeProperties() { } private void writeMemberDocs(MemberShape member) { - member.getMemberTrait(model, DocumentationTrait.class).ifPresent(trait -> { - writer.writeDocs(trait.getValue(), context); - }); + CodegenUtils.getMemberDocs(model, member).ifPresent(docs -> writer.writeDocs(docs, context)); } private List filterPropertyMembers(List members) { diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/RestJsonProtocolGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/RestJsonProtocolGenerator.java index 2701b6a6d..449bb181a 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/RestJsonProtocolGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/integrations/RestJsonProtocolGenerator.java @@ -31,9 +31,6 @@ public class RestJsonProtocolGenerator implements ProtocolGenerator { private static final Set TESTS_TO_SKIP = Set.of( - // This requires support of idempotency autofill - "RestJsonQueryIdempotencyTokenAutoFill", - // This requires support of the httpChecksumRequired trait "RestJsonHttpChecksumRequired", diff --git a/codegen/core/src/test/java/software/amazon/smithy/python/codegen/CodegenUtilsTest.java b/codegen/core/src/test/java/software/amazon/smithy/python/codegen/CodegenUtilsTest.java index df3848d02..56a8cb0b6 100644 --- a/codegen/core/src/test/java/software/amazon/smithy/python/codegen/CodegenUtilsTest.java +++ b/codegen/core/src/test/java/software/amazon/smithy/python/codegen/CodegenUtilsTest.java @@ -4,13 +4,16 @@ */ package software.amazon.smithy.python.codegen; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.Optional; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.ShapeId; @@ -34,6 +37,70 @@ public void testIsAwsServiceWithoutAwsServiceTrait() { assertFalse(CodegenUtils.isAwsService(context)); } + private static final Model TOKEN_MODEL = Model.assembler() + .addUnparsedModel("token.smithy", """ + $version: "2.0" + namespace com.example + + operation CreateThing { + input := { + /// Unique identifier for the request. + @idempotencyToken + documentedToken: String + + /// The name. + name: String + + nested: Nested + } + } + + operation UpdateThing { + input := { + @idempotencyToken + undocumentedToken: String + } + } + + structure Nested { + @idempotencyToken + token: String + } + """) + .assemble() + .unwrap(); + + private static Optional memberDocs(String member) { + return CodegenUtils.getMemberDocs(TOKEN_MODEL, + TOKEN_MODEL.expectShape( + ShapeId.from("com.example#" + member), + MemberShape.class)); + } + + @Test + public void testIdempotencyTokenNoteIsAppendedToMemberDocs() { + assertEquals( + Optional.of("Unique identifier for the request.\n\n" + CodegenUtils.IDEMPOTENCY_TOKEN_DOCS), + memberDocs("CreateThingInput$documentedToken")); + } + + @Test + public void testIdempotencyTokenNoteIsTheOnlyDocsOfAnUndocumentedMember() { + assertEquals( + Optional.of(CodegenUtils.IDEMPOTENCY_TOKEN_DOCS), + memberDocs("UpdateThingInput$undocumentedToken")); + } + + @Test + public void testOtherMembersKeepTheirDocs() { + assertEquals(Optional.of("The name."), memberDocs("CreateThingInput$name")); + } + + @Test + public void testIdempotencyTokenOutsideAnOperationInputGetsNoNote() { + assertEquals(Optional.empty(), memberDocs("Nested$token")); + } + /** * Helper method to create a mock GenerationContext with a service shape. * diff --git a/packages/smithy-core/.changes/next-release/smithy-core-feature-bf9b6e3c5d50439b86cedc949e32bf07.json b/packages/smithy-core/.changes/next-release/smithy-core-feature-bf9b6e3c5d50439b86cedc949e32bf07.json new file mode 100644 index 000000000..628235f1d --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-feature-bf9b6e3c5d50439b86cedc949e32bf07.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "Unset `@idempotencyToken` input members are now auto-filled with a UUID v4 before serialization. The same token is sent on every retry." +} \ No newline at end of file diff --git a/packages/smithy-core/src/smithy_core/_pipeline.py b/packages/smithy-core/src/smithy_core/_pipeline.py index 7fb7f6966..ad1fda210 100644 --- a/packages/smithy-core/src/smithy_core/_pipeline.py +++ b/packages/smithy-core/src/smithy_core/_pipeline.py @@ -8,6 +8,7 @@ """ import logging +import uuid from collections.abc import Mapping, Sequence from copy import copy from dataclasses import replace @@ -180,9 +181,17 @@ def _handle_execution[I: SerializeableShape, O: DeserializeableShape]( try: interceptor.read_before_execution(input_context) + # Filled after the whole modify_before_serialization chain, so an + # interceptor that returns a new input still gets a token, and + # read_before_execution sees the caller's own input. Serialization runs + # once per call, so every retry attempt sends the same token. input_context = replace( input_context, - request=interceptor.modify_before_serialization(input_context), + request=_fill_idempotency_token( + interceptor.modify_before_serialization(input_context), + call.operation, + self._logger, + ), ) interceptor.read_before_serialization(input_context) @@ -660,3 +669,38 @@ def _rewind_position(body: Any) -> Program[int | None]: return (yield lambda: _seek(body, 0, 1)) except (OSError, ValueError, NotImplementedError): return None + + +def _default_idempotency_token() -> str: + # Protocol tests patch this to pin the token. It is not a public hook. + return str(uuid.uuid4()) + + +def _fill_idempotency_token[I: SerializeableShape]( + input: I, operation: APIOperation[I, Any], logger: logging.Logger +) -> I: + """Return the input with its idempotency token set, if the caller left it unset. + + A token set by the caller or by an interceptor is sent as-is. The caller's input + is never mutated: a generated token goes on a copy. Inputs that aren't + dataclasses with the derived field, such as hand-written shapes, are passed + through unchanged and a warning is logged. + """ + field_name = operation._idempotency_token_field # pyright: ignore[reportPrivateUsage] + if field_name is None or getattr(input, field_name, None) is not None: + return input + if field_name not in getattr(type(input), "__dataclass_fields__", {}): + logger.warning( + "Not filling idempotency token: input %s has no dataclass field %r, so " + "the request is sent without one", + type(input).__name__, + field_name, + ) + return input + + # A shallow copy plus a direct set, rather than dataclasses.replace, so the + # input's __init__ and __post_init__ don't run again. object.__setattr__ also + # works on frozen dataclasses. + filled = copy(input) + object.__setattr__(filled, field_name, _default_idempotency_token()) + return filled diff --git a/packages/smithy-core/src/smithy_core/schemas.py b/packages/smithy-core/src/smithy_core/schemas.py index c6b020dc1..7a7f1fe35 100644 --- a/packages/smithy-core/src/smithy_core/schemas.py +++ b/packages/smithy-core/src/smithy_core/schemas.py @@ -495,3 +495,25 @@ def long_polling(self) -> bool: """Whether the service may hold the request open until information is available.""" return self.schema.get_trait(LongPollTrait) is not None + + @cached_property + def _idempotency_token_field(self) -> str | None: + """Python attribute name of the idempotency token member, or None. + + Cached because the client reads it on every call. + """ + member = self.idempotency_token_member + if member is None: + return None + + # Imported here so that importing schemas doesn't load the naming module. + from ._model_naming import member_field_name + + # Same derivation the generated input dataclass used for its field names. + # Operation inputs are never errors, enums or outputs. + return member_field_name( + member.expect_member_name(), + is_error=False, + carries_response_metadata=False, + is_enum=False, + ) diff --git a/packages/smithy-core/tests/functional/test_retries.py b/packages/smithy-core/tests/functional/test_retries.py index 098bc110f..979b3d014 100644 --- a/packages/smithy-core/tests/functional/test_retries.py +++ b/packages/smithy-core/tests/functional/test_retries.py @@ -221,6 +221,8 @@ def _call(self, strategy: AdaptiveRetryStrategy) -> Any: call.retryable.return_value = True call.retry_strategy = strategy call.retry_scope = None + # A MagicMock attribute would look like a token field name. + call.operation._idempotency_token_field = None return call def _request_context(self) -> RequestContext[Any, Any]: diff --git a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py index 33a5eff91..2b335dbb3 100644 --- a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py +++ b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py @@ -18,7 +18,7 @@ from smithy_core.schemas import APIOperation, Schema from smithy_core.serializers import ShapeSerializer from smithy_core.shapes import ShapeID, ShapeType -from smithy_core.traits import StreamingTrait +from smithy_core.traits import IdempotencyTokenTrait, StreamingTrait from smithy_core.types import TypedProperties _STRING = Schema(id=ShapeID("smithy.api#String"), shape_type=ShapeType.STRING) @@ -319,3 +319,41 @@ def client_call() -> ClientCall[Any, Any]: endpoint_resolver=StubEndpointResolver(), retry_strategy=None, # type: ignore[arg-type] # unused for streaming input ) + + +_TOKEN_INPUT_SCHEMA = Schema.collection( + id=ShapeID("com.example#TokenInput"), + members={ + "clientToken": {"target": _STRING, "traits": [IdempotencyTokenTrait()]}, + "message": {"target": _STRING}, + }, +) + +_TOKEN_OUTPUT_SCHEMA = Schema.collection( + id=ShapeID("com.example#TokenOutput"), + members={"message": {"target": _STRING}}, +) + +TOKEN_OPERATION: APIOperation[Any, StubOutput] = APIOperation( + static_schema=( + ShapeType.OPERATION.value, + "com.example", + "TokenOperation", + 0, + _TOKEN_INPUT_SCHEMA, + _TOKEN_OUTPUT_SCHEMA, + (), + ), + effective_auth_schemes=[], +) + + +@dataclass(kw_only=True) +class TokenInput: + """Shaped like a generated operation input with an idempotency token member.""" + + client_token: str | None = None + message: str | None = None + + def serialize(self, serializer: ShapeSerializer) -> None: + pass diff --git a/packages/smithy-core/tests/unit/aio/test_client.py b/packages/smithy-core/tests/unit/aio/test_client.py index ffaee0f86..f582a5b6d 100644 --- a/packages/smithy-core/tests/unit/aio/test_client.py +++ b/packages/smithy-core/tests/unit/aio/test_client.py @@ -1,8 +1,10 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from collections.abc import AsyncIterator, Callable -from dataclasses import replace +import logging +import uuid +from collections.abc import AsyncIterator, Callable, Iterator +from dataclasses import dataclass, field, replace from io import BytesIO, UnsupportedOperation from typing import Any, cast from unittest.mock import AsyncMock, Mock, patch @@ -39,12 +41,15 @@ SimpleRetryStrategy, StandardRetryStrategy, ) -from smithy_core.shapes import ShapeID +from smithy_core.schemas import APIOperation, Schema +from smithy_core.shapes import ShapeID, ShapeType +from smithy_core.traits import IdempotencyTokenTrait from smithy_core.types import TypedProperties from ._pipeline_harness import ( DATA_STREAM_OPERATION, OPERATION, + TOKEN_OPERATION, DuplexTransport, NonDuplexTransport, StubEndpoint, @@ -54,6 +59,7 @@ StubInput, StubOutput, StubRequest, + TokenInput, UndeclaredTransport, client_call, pipeline_harness, @@ -637,6 +643,364 @@ async def test_data_stream_that_fails_to_rewind_returns_the_attempts_error() -> ) +class _CountingGenerator: + def __init__(self, *tokens: str) -> None: + self.tokens = list(tokens) or ["generated-token"] + self.calls = 0 + + def __call__(self) -> str: + token = self.tokens[self.calls % len(self.tokens)] + self.calls += 1 + return token + + +@pytest.fixture +def token_generator() -> Iterator[_CountingGenerator]: + generator = _CountingGenerator("first-token", "second-token", "third-token") + with patch("smithy_core._pipeline._default_idempotency_token", new=generator): + yield generator + + +def _token_call( + input: Any, + *, + interceptor: Interceptor[Any, Any, Any, Any] | None = None, + retry_strategy: Any = None, +) -> Any: + return replace( + retryable_client_call(retry_strategy), + input=input, + operation=TOKEN_OPERATION, + interceptor=InterceptorChain([interceptor] if interceptor else []), + ) + + +async def test_unset_idempotency_token_is_generated_once_on_a_copy( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = TokenInput(message="hi") + + await harness.pipeline(_token_call(input)) + + assert token_generator.calls == 1 + [serialized] = harness.protocol.serialized_inputs + assert serialized is not input + assert serialized.client_token == "first-token" + assert serialized.message == "hi" + # The caller's own input is never mutated. + assert input.client_token is None + + +# Only an unset (None) token is filled; "" is an explicit caller value. +@pytest.mark.parametrize("token", ["caller-token", ""], ids=["caller-token", "empty"]) +async def test_set_idempotency_token_is_sent_unchanged( + token_generator: _CountingGenerator, token: str +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = TokenInput(client_token=token) + + await harness.pipeline(_token_call(input)) + + assert token_generator.calls == 0 + [serialized] = harness.protocol.serialized_inputs + assert serialized is input + assert serialized.client_token == token + + +class _FailingTwiceTransport(BodyRecordingTransport): + async def send(self, request: Any) -> Any: + response = await super().send(request) + if self.send_calls <= 2: + raise CallError("Service unavailable", is_retry_safe=True) + return response + + +async def test_idempotency_token_is_the_same_on_every_retry( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(_FailingTwiceTransport()) + + def serialize_request(**kwargs: Any) -> StubRequest: + harness.protocol.serialize_request_calls += 1 + request = StubRequest() + request.body = kwargs["input"].client_token.encode() + return request + + harness.protocol.serialize_request = serialize_request # type: ignore[method-assign] + strategy = SimpleRetryStrategy( + backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=0), + max_attempts=3, + ) + + await harness.pipeline(_token_call(TokenInput(), retry_strategy=strategy)) + + assert harness.protocol.serialize_request_calls == 1 + assert harness.transport.send_calls == 3 + assert cast(BodyRecordingTransport, harness.transport).bodies == [ + b"first-token", + b"first-token", + b"first-token", + ] + assert token_generator.calls == 1 + + +_STRING_SCHEMA = Schema(id=ShapeID("smithy.api#String"), shape_type=ShapeType.STRING) + +_PASCAL_TOKEN_OPERATION: APIOperation[Any, Any] = APIOperation( + static_schema=( + ShapeType.OPERATION.value, + "com.example", + "PascalTokenOperation", + 0, + Schema.collection( + id=ShapeID("com.example#PascalTokenInput"), + members={ + "ClientToken": { + "target": _STRING_SCHEMA, + "traits": [IdempotencyTokenTrait()], + }, + }, + ), + Schema.collection(id=ShapeID("com.example#PascalTokenOutput"), members={}), + (), + ), + effective_auth_schemes=[], +) + + +async def test_pascal_case_token_member_is_filled_on_its_python_field( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + call = replace(_token_call(TokenInput()), operation=_PASCAL_TOKEN_OPERATION) + + await harness.pipeline(call) + + # The model member "ClientToken" maps to the Python field "client_token". + [serialized] = harness.protocol.serialized_inputs + assert serialized.client_token == "first-token" + + +async def test_input_without_idempotency_member_is_passed_through( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + call = retryable_client_call() + + await harness.pipeline(call) + + assert token_generator.calls == 0 + assert harness.protocol.serialized_inputs[0] is call.input + + +async def test_default_idempotency_token_is_a_uuid4() -> None: + harness = pipeline_harness(NonDuplexTransport()) + + await harness.pipeline(_token_call(TokenInput())) + await harness.pipeline(_token_call(TokenInput())) + + first, second = (i.client_token for i in harness.protocol.serialized_inputs) + assert uuid.UUID(first).version == 4 + assert first != second + + +def test_idempotency_token_field_name_is_cached() -> None: + operation = replace(TOKEN_OPERATION) + first = operation._idempotency_token_field # pyright: ignore[reportPrivateUsage] + + assert first == "client_token" + assert operation.__dict__["_idempotency_token_field"] == "client_token" + + +class _RecordTokens(Interceptor[Any, Any, Any, Any]): + def __init__(self, *, rebuild_input: bool = False) -> None: + self.rebuild_input = rebuild_input + self.read_before_execution_input: Any = None + self.modify_before_serialization_token: Any = "unset" + self.read_before_serialization_token: Any = None + + def read_before_execution(self, context: InputContext[Any]) -> None: + self.read_before_execution_input = context.request + + def modify_before_serialization(self, context: InputContext[Any]) -> Any: + self.modify_before_serialization_token = context.request.client_token + if self.rebuild_input: + return TokenInput(message="rebuilt") + return context.request + + def read_before_serialization(self, context: InputContext[Any]) -> None: + self.read_before_serialization_token = context.request.client_token + + +async def test_token_is_filled_after_modify_before_serialization( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + interceptor = _RecordTokens() + input = TokenInput() + + await harness.pipeline(_token_call(input, interceptor=interceptor)) + + assert interceptor.read_before_execution_input is input + assert interceptor.modify_before_serialization_token is None + assert interceptor.read_before_serialization_token == "first-token" + + +async def test_input_rebuilt_by_an_interceptor_still_gets_a_token( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + interceptor = _RecordTokens(rebuild_input=True) + + await harness.pipeline(_token_call(TokenInput(), interceptor=interceptor)) + + [serialized] = harness.protocol.serialized_inputs + assert serialized.message == "rebuilt" + assert serialized.client_token == "first-token" + + +class _NonDataclassTokenInput: + def __init__(self) -> None: + self.client_token: str | None = None + + def serialize(self, serializer: Any) -> None: + pass + + +@dataclass(kw_only=True) +class _PostInitTokenInput: + client_token: str | None = None + message: str | None = None + post_init_calls: int = field(default=0, compare=False) + + def __post_init__(self) -> None: + self.post_init_calls += 1 + + def serialize(self, serializer: Any) -> None: + pass + + +@dataclass(kw_only=True, frozen=True) +class _FrozenTokenInput: + client_token: str | None = None + + def serialize(self, serializer: Any) -> None: + pass + + +async def test_token_fill_does_not_rerun_post_init( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = _PostInitTokenInput(message="hi") + + await harness.pipeline(_token_call(input)) + + [serialized] = harness.protocol.serialized_inputs + assert serialized is not input + assert serialized.client_token == "first-token" + assert serialized.message == "hi" + assert serialized.post_init_calls == 1 + assert input.client_token is None + + +async def test_frozen_dataclass_input_gets_a_token( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = _FrozenTokenInput() + + await harness.pipeline(_token_call(input)) + + [serialized] = harness.protocol.serialized_inputs + assert serialized.client_token == "first-token" + assert input.client_token is None + + +@dataclass(kw_only=True) +class _MisnamedTokenInput: + token: str | None = None + + def serialize(self, serializer: Any) -> None: + pass + + +@pytest.mark.parametrize( + "input_type", + [_NonDataclassTokenInput, _MisnamedTokenInput], + ids=["non-dataclass", "misnamed-field"], +) +async def test_input_that_cannot_be_filled_is_passed_through_with_a_warning( + token_generator: _CountingGenerator, + caplog: pytest.LogCaptureFixture, + input_type: type[Any], +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = input_type() + + with caplog.at_level(logging.WARNING): + await harness.pipeline(_token_call(input)) + + assert token_generator.calls == 0 + assert harness.protocol.serialized_inputs[0] is input + [record] = [r for r in caplog.records if "idempotency token" in r.message] + assert record.levelno == logging.WARNING + # Logged through the client's logger, alongside the rest of the pipeline's logs + assert record.name == harness.pipeline._logger.name # pyright: ignore[reportPrivateUsage] + + +class _SetToken(Interceptor[Any, Any, Any, Any]): + def modify_before_serialization(self, context: InputContext[Any]) -> Any: + return replace(context.request, client_token="interceptor-token") + + +async def test_token_set_by_an_interceptor_is_sent_unchanged( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + + await harness.pipeline(_token_call(TokenInput(), interceptor=_SetToken())) + + assert token_generator.calls == 0 + [serialized] = harness.protocol.serialized_inputs + assert serialized.client_token == "interceptor-token" + + +class _RecordCompletion(Interceptor[Any, Any, Any, Any]): + def __init__(self) -> None: + self.completion_response: Any = None + self.after_execution_response: Any = None + + def modify_before_completion( + self, context: OutputContext[Any, Any, Any, Any] + ) -> Any: + self.completion_response = context.response + return context.response + + def read_after_execution(self, context: OutputContext[Any, Any, Any, Any]) -> None: + self.after_execution_response = context.response + + +async def test_failing_token_generation_fails_the_call() -> None: + harness = pipeline_harness(NonDuplexTransport()) + interceptor = _RecordCompletion() + error = RuntimeError("no entropy") + + def generator() -> str: + raise error + + with patch("smithy_core._pipeline._default_idempotency_token", new=generator): + with pytest.raises(SmithyError) as exc_info: + await harness.pipeline(_token_call(TokenInput(), interceptor=interceptor)) + + assert exc_info.value.__cause__ is error + assert interceptor.completion_response is error + assert interceptor.after_execution_response is error + assert harness.protocol.serialize_request_calls == 0 + assert harness.transport.send_calls == 0 + + async def test_input_event_stream_is_not_retried() -> None: harness = pipeline_harness(NonDuplexTransport()) call = _streaming_call_failing_once(harness, BytesIO(b""), operation=OPERATION) diff --git a/packages/smithy-core/tests/unit/test_client.py b/packages/smithy-core/tests/unit/test_client.py index 1d2286d9c..9b4d7ab0a 100644 --- a/packages/smithy-core/tests/unit/test_client.py +++ b/packages/smithy-core/tests/unit/test_client.py @@ -3,7 +3,7 @@ from collections.abc import Sequence from dataclasses import replace from typing import Any, cast -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest from smithy_core.client import Client, ClientCall, RequestPipeline @@ -16,7 +16,14 @@ StandardRetryStrategy, ) -from .aio._pipeline_harness import StubOutput, StubProtocol, retryable_client_call +from .aio._pipeline_harness import ( + TOKEN_OPERATION, + StubOutput, + StubProtocol, + StubResponse, + TokenInput, + retryable_client_call, +) def test_build_call_adds_retry_strategy_to_request_properties() -> None: @@ -175,3 +182,34 @@ def send(request: Any) -> _TrackedResponse: with pytest.raises(CallError, match="original"): _pipeline(transport, protocol)(_call(max_attempts=1)) + + +class _SyncProtocol(StubProtocol): + def deserialize_response_program(self, **kwargs: Any) -> Program[StubOutput]: + self.deserialize_response_calls += 1 + return (yield lambda: StubOutput()) + + +class _SyncTransport: + TIMEOUT_EXCEPTIONS: tuple[type[Exception], ...] = () + + def send(self, request: Any) -> StubResponse: + return StubResponse() + + +def test_sync_pipeline_fills_unset_idempotency_token() -> None: + protocol = _SyncProtocol() + pipeline: RequestPipeline[Any, Any] = RequestPipeline( + protocol=cast(Any, protocol), transport=cast(Any, _SyncTransport()) + ) + input = TokenInput() + call = replace(retryable_client_call(), input=input, operation=TOKEN_OPERATION) + + with patch( + "smithy_core._pipeline._default_idempotency_token", new=lambda: "sync-token" + ): + pipeline(call) + + [serialized] = protocol.serialized_inputs + assert serialized.client_token == "sync-token" + assert input.client_token is None diff --git a/packages/smithy-python/.changes/next-release/smithy-python-enhancement-a291243bd41246c8a0e325d0b3a865e2.json b/packages/smithy-python/.changes/next-release/smithy-python-enhancement-a291243bd41246c8a0e325d0b3a865e2.json new file mode 100644 index 000000000..2d8fe8664 --- /dev/null +++ b/packages/smithy-python/.changes/next-release/smithy-python-enhancement-a291243bd41246c8a0e325d0b3a865e2.json @@ -0,0 +1,4 @@ +{ + "type": "enhancement", + "description": "Generated docs for `@idempotencyToken` members of operation inputs now note that the field is autopopulated if not provided." +} \ No newline at end of file diff --git a/packages/smithy-python/src/smithy_python/models.py b/packages/smithy-python/src/smithy_python/models.py index 29be7e6ea..d6ae2cd48 100644 --- a/packages/smithy-python/src/smithy_python/models.py +++ b/packages/smithy-python/src/smithy_python/models.py @@ -272,7 +272,7 @@ def _write_structure(self, shape: _model.Shape) -> None: reference, "" if mandatory else " = ...", ) - if docs := _documentation(member): + if docs := _member_documentation(member, shape.id in self.inputs): _write_docstring(writer, docs) if carries_metadata: writer.line("response_metadata: ", _RESPONSE_METADATA, " = ...") @@ -425,6 +425,18 @@ def _documentation(shape: _model.Shape) -> str | None: return value if isinstance(value, str) else None +# Matches CodegenUtils.IDEMPOTENCY_TOKEN_DOCS in the Java generator, and botocore. +_AUTOPOPULATED_NOTE = "This field is autopopulated if not provided." + + +def _member_documentation(member: _model.MemberShape, in_input: bool) -> str | None: + """A member's docs, noting that the client fills an unset idempotency token.""" + docs = _documentation(member) + if not (in_input and member.has_trait("idempotencyToken")): + return docs + return f"{docs}\n\n{_AUTOPOPULATED_NOTE}" if docs else _AUTOPOPULATED_NOTE + + def _is_event_stream_member(model: _model.Model, member: _model.MemberShape) -> bool: target = model.get_target(member) return target.type is _model.ShapeType.UNION and target.has_trait("streaming") diff --git a/packages/smithy-python/tests/unit/test_models.py b/packages/smithy-python/tests/unit/test_models.py index 2d93ae8d6..18d545c84 100644 --- a/packages/smithy-python/tests/unit/test_models.py +++ b/packages/smithy-python/tests/unit/test_models.py @@ -505,6 +505,53 @@ def test_explicit_input_and_output_traits_match_runtime_and_stub( assert output.response_metadata is not None +def test_idempotency_token_members_of_inputs_are_documented_as_autopopulated( + tmp_path: Path, +) -> None: + package = "generated_idempotency_docs" + token: dict[str, object] = { + "target": "smithy.api#String", + "traits": {"smithy.api#idempotencyToken": {}}, + } + source_root = _generate_custom( + tmp_path, + { + "smithy": "2.0", + "shapes": { + "example#CreateInput": { + "type": "structure", + "traits": {"smithy.api#input": {}}, + "members": { + "documentedToken": { + "target": "smithy.api#String", + "traits": { + "smithy.api#idempotencyToken": {}, + "smithy.api#documentation": "Unique request id.", + }, + } + }, + }, + "example#UpdateInput": { + "type": "structure", + "traits": {"smithy.api#input": {}}, + "members": {"undocumentedToken": token}, + }, + "example#Nested": { + "type": "structure", + "members": {"nestedToken": token}, + }, + }, + }, + package, + ) + stub = (source_root / package / "models.pyi").read_text() + note = "This field is autopopulated if not provided." + assert f"Unique request id.\n\n {note}" in stub + assert f'undocumented_token: str | None = ...\n "{note}"' in stub + # Only operation inputs are filled by the client, so only they get the note. + assert stub.count(note) == 2 + + def test_stub_and_runtime_use_the_same_member_names( tmp_path: Path, monkeypatch: pytest.MonkeyPatch,