From e1bf4f94e7019ac46dcf5e04f10954128b89e497 Mon Sep 17 00:00:00 2001 From: Rohan Gavankar Date: Mon, 9 Nov 2026 10:10:26 -0500 Subject: [PATCH 1/3] feat(smithy-core): auto-fill idempotency tokens before serialization Operation inputs with an @idempotencyToken member now get a UUID v4 when the caller leaves the member unset. The fill runs once per call in the shared request pipeline, right after the whole modify_before_serialization interceptor chain. So read_before_execution and modify_before_serialization see the caller's own input, and an interceptor that returns a new input still gets a token. Serialization runs once per call, so every retry attempt sends the same token. A caller-supplied token is sent unchanged, and the caller's input object 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. Sync and async clients both get this, since they share the pipeline. The token generator is private. Callers who need deduplication across calls should pass their own token per call. A client-wide generator could not help there, since it still gives each new call a new token. botocore and the JS v3 SDK hard-code a UUID v4, and the PHP SDK only lets callers swap the random-byte source, so none of them exposes an option that returns the token itself. The Python field name of the token member is cached on APIOperation, so operations without a token pay one attribute read per call. The protocol test generator now pins the token for operations with an @idempotencyToken member by patching the private generator. That lets the QueryIdempotencyTokenAutoFill tests run for restJson1, restXml, awsQuery and ec2Query, so they come off the skip lists. --- .../aws-query/tests/test_awsquery_protocol.py | 10 +- .../ec2-query/tests/test_ec2query_protocol.py | 10 +- .../tests/test_restjson_protocol.py | 10 +- .../rest-xml/tests/test_restxml_protocol.py | 10 +- .../codegen/AwsQueryProtocolGenerator.java | 3 - .../codegen/Ec2QueryProtocolGenerator.java | 3 - .../aws/codegen/RestXmlProtocolGenerator.java | 3 - .../codegen/HttpProtocolTestGenerator.java | 31 ++ .../RestJsonProtocolGenerator.java | 3 - ...ture-bf9b6e3c5d50439b86cedc949e32bf07.json | 4 + .../smithy-core/src/smithy_core/_pipeline.py | 46 +- .../smithy-core/src/smithy_core/schemas.py | 22 + .../tests/functional/test_retries.py | 2 + .../tests/unit/aio/_pipeline_harness.py | 40 +- .../smithy-core/tests/unit/aio/test_client.py | 397 +++++++++++++++++- .../smithy-core/tests/unit/test_client.py | 42 +- 16 files changed, 613 insertions(+), 23 deletions(-) create mode 100644 packages/smithy-core/.changes/next-release/smithy-core-feature-bf9b6e3c5d50439b86cedc949e32bf07.json 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/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/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/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/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/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/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..726404ed3 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,391 @@ 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( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + + await harness.pipeline(_token_call(TokenInput(message="hi"))) + + assert token_generator.calls == 1 + [serialized] = harness.protocol.serialized_inputs + assert serialized.client_token == "first-token" + assert serialized.message == "hi" + + +async def test_set_idempotency_token_is_sent_unchanged( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = TokenInput(client_token="caller-token") + + await harness.pipeline(_token_call(input)) + + assert token_generator.calls == 0 + assert harness.protocol.serialized_inputs[0] is input + assert input.client_token == "caller-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 + + +async def test_empty_string_idempotency_token_is_sent_unchanged( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = TokenInput(client_token="") + + await harness.pipeline(_token_call(input)) + + # Only an unset (None) token is filled; "" is an explicit caller value. + assert token_generator.calls == 0 + [serialized] = harness.protocol.serialized_inputs + assert serialized is input + assert serialized.client_token == "" + + +_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_caller_input_is_not_mutated_by_token_fill( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = TokenInput(message="hi") + + await harness.pipeline(_token_call(input)) + + assert input.client_token is None + [serialized] = harness.protocol.serialized_inputs + assert serialized is not input + 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 + + +async def test_non_dataclass_input_is_passed_through( + token_generator: _CountingGenerator, +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = _NonDataclassTokenInput() + + await harness.pipeline(_token_call(input)) + + assert token_generator.calls == 0 + assert harness.protocol.serialized_inputs[0] is input + + +@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 + + +async def test_input_without_the_derived_field_is_passed_through( + token_generator: _CountingGenerator, caplog: pytest.LogCaptureFixture +) -> None: + harness = pipeline_harness(NonDuplexTransport()) + input = _MisnamedTokenInput() + + 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 From c8211c3958153af88845a197cfaddeb3e15305c7 Mon Sep 17 00:00:00 2001 From: Rohan Gavankar Date: Tue, 10 Nov 2026 15:32:03 -0500 Subject: [PATCH 2/3] feat(codegen): note autopopulated idempotency tokens in member docs Generated docs for an @idempotencyToken member of an operation input now end with "This field is autopopulated if not provided.", matching botocore. Members without docs get that sentence alone. Tokens outside operation inputs are not filled by the client, so they get no note. The Java generators share CodegenUtils.getMemberDocs, used for both the .pyi stubs and eager models, and the Python generator applies the same rule. The regenerated protocol-test stubs pick up the note. --- .../aws-query/src/awsquery/models.pyi | 1 + .../ec2-query/src/ec2query/models.pyi | 1 + .../src/restjson/_private/models_0.pyi | 1 + .../rest-xml/src/restxml/models.pyi | 1 + .../smithy/python/codegen/CodegenUtils.java | 30 +++++++++ .../generators/ModelStubGenerator.java | 3 +- .../generators/StructureGenerator.java | 4 +- .../python/codegen/CodegenUtilsTest.java | 67 +++++++++++++++++++ ...ment-a291243bd41246c8a0e325d0b3a865e2.json | 4 ++ .../smithy-python/src/smithy_python/models.py | 14 +++- .../smithy-python/tests/unit/test_models.py | 47 +++++++++++++ 11 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 packages/smithy-python/.changes/next-release/smithy-python-enhancement-a291243bd41246c8a0e325d0b3a865e2.json 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/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/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-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/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/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/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-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, From 0c6f659a5b8cc591cb0d2ee1b6fbba380cb8d183 Mon Sep 17 00:00:00 2001 From: Rohan Gavankar Date: Wed, 11 Nov 2026 14:24:07 -0500 Subject: [PATCH 3/3] test(smithy-core): merge overlapping idempotency token tests - Fold the caller-input mutation check into the unset-token test, since both used the same input and token. - Parametrize the caller-supplied token test on "caller-token" and "". - Parametrize the can't-fill test on a non-dataclass input and a misnamed dataclass field, so the warning is checked for both. --- .../smithy-core/tests/unit/aio/test_client.py | 73 ++++++------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/packages/smithy-core/tests/unit/aio/test_client.py b/packages/smithy-core/tests/unit/aio/test_client.py index 726404ed3..f582a5b6d 100644 --- a/packages/smithy-core/tests/unit/aio/test_client.py +++ b/packages/smithy-core/tests/unit/aio/test_client.py @@ -675,30 +675,37 @@ def _token_call( ) -async def test_unset_idempotency_token_is_generated_once( +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(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_generator: _CountingGenerator, token: str ) -> None: harness = pipeline_harness(NonDuplexTransport()) - input = TokenInput(client_token="caller-token") + input = TokenInput(client_token=token) await harness.pipeline(_token_call(input)) assert token_generator.calls == 0 - assert harness.protocol.serialized_inputs[0] is input - assert input.client_token == "caller-token" + [serialized] = harness.protocol.serialized_inputs + assert serialized is input + assert serialized.client_token == token class _FailingTwiceTransport(BodyRecordingTransport): @@ -738,21 +745,6 @@ def serialize_request(**kwargs: Any) -> StubRequest: assert token_generator.calls == 1 -async def test_empty_string_idempotency_token_is_sent_unchanged( - token_generator: _CountingGenerator, -) -> None: - harness = pipeline_harness(NonDuplexTransport()) - input = TokenInput(client_token="") - - await harness.pipeline(_token_call(input)) - - # Only an unset (None) token is filled; "" is an explicit caller value. - assert token_generator.calls == 0 - [serialized] = harness.protocol.serialized_inputs - assert serialized is input - assert serialized.client_token == "" - - _STRING_SCHEMA = Schema(id=ShapeID("smithy.api#String"), shape_type=ShapeType.STRING) _PASCAL_TOKEN_OPERATION: APIOperation[Any, Any] = APIOperation( @@ -790,20 +782,6 @@ async def test_pascal_case_token_member_is_filled_on_its_python_field( assert serialized.client_token == "first-token" -async def test_caller_input_is_not_mutated_by_token_fill( - token_generator: _CountingGenerator, -) -> None: - harness = pipeline_harness(NonDuplexTransport()) - input = TokenInput(message="hi") - - await harness.pipeline(_token_call(input)) - - assert input.client_token is None - [serialized] = harness.protocol.serialized_inputs - assert serialized is not input - assert serialized.client_token == "first-token" - - async def test_input_without_idempotency_member_is_passed_through( token_generator: _CountingGenerator, ) -> None: @@ -890,18 +868,6 @@ def serialize(self, serializer: Any) -> None: pass -async def test_non_dataclass_input_is_passed_through( - token_generator: _CountingGenerator, -) -> None: - harness = pipeline_harness(NonDuplexTransport()) - input = _NonDataclassTokenInput() - - await harness.pipeline(_token_call(input)) - - assert token_generator.calls == 0 - assert harness.protocol.serialized_inputs[0] is input - - @dataclass(kw_only=True) class _PostInitTokenInput: client_token: str | None = None @@ -960,11 +926,18 @@ def serialize(self, serializer: Any) -> None: pass -async def test_input_without_the_derived_field_is_passed_through( - token_generator: _CountingGenerator, caplog: pytest.LogCaptureFixture +@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 = _MisnamedTokenInput() + input = input_type() with caplog.at_level(logging.WARNING): await harness.pipeline(_token_call(input))