diff --git a/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-enhancement-4299501c9795442da9b75d8e826246c0.json b/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-enhancement-4299501c9795442da9b75d8e826246c0.json new file mode 100644 index 000000000..428c249cd --- /dev/null +++ b/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-enhancement-4299501c9795442da9b75d8e826246c0.json @@ -0,0 +1,4 @@ +{ + "type": "enhancement", + "description": "Reused parsed XML response trees across AWS Query and Rest XML metadata, error identification, and shape deserialization." +} diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py index eec6a77a1..be91d1d91 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py @@ -2,7 +2,15 @@ # SPDX-License-Identifier: Apache-2.0 """Helpers for reading awsQuery response bodies that are not shape-modeled.""" -from xml.etree.ElementTree import Element +from xml.etree.ElementTree import Element, ParseError, fromstring + + +def parse_xml_root(body: bytes) -> Element | None: + """Parse an XML body, returning None when it is empty or malformed.""" + try: + return fromstring(body) # noqa: S314 + except ParseError: + return None def local_name(tag: str) -> str: diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py index f3726af15..72be77448 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py @@ -1,7 +1,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from typing import TYPE_CHECKING, Any -from xml.etree.ElementTree import ParseError, fromstring +from xml.etree.ElementTree import Element from smithy_core.documents import TypeRegistry from smithy_core.exceptions import ( @@ -15,7 +15,7 @@ from smithy_core.shapes import ShapeID from ...traits import AwsQueryErrorTrait -from ._xml import find_child, local_name +from ._xml import find_child, local_name, parse_xml_root try: from smithy_xml import XMLCodec @@ -36,12 +36,10 @@ def _assert_xml() -> None: def _parse_aws_query_error_code( - body: bytes, wrapper_elements: tuple[str, ...] + element: Element | None, wrapper_elements: tuple[str, ...] ) -> str | None: """Parse the ``Code`` field from a wrapped awsQuery error response.""" - try: - element = fromstring(body) # noqa: S314 - except ParseError: + if element is None: return None if wrapper_elements: @@ -78,7 +76,7 @@ def _resolve_aws_query_error_shape_id( def create_aws_query_error( *, - body: bytes, + body: bytes | Element | None, operation: APIOperation[Any, Any], error_registry: TypeRegistry, default_namespace: str, @@ -88,7 +86,8 @@ def create_aws_query_error( retry_after: float | None = None, ) -> CallError: """Create a modeled or generic CallError from an awsQuery error response.""" - code = _parse_aws_query_error_code(body, wrapper_elements) + root = parse_xml_root(body) if isinstance(body, bytes) else body + code = _parse_aws_query_error_code(root, wrapper_elements) if code is not None: shape_id = _resolve_aws_query_error_shape_id( code=code, @@ -96,7 +95,7 @@ def create_aws_query_error( error_registry=error_registry, default_namespace=default_namespace, ) - if shape_id is not None: + if shape_id is not None and root is not None: error_shape = error_registry.get(shape_id) if not issubclass(error_shape, ModeledError): raise ExpectationNotMetError( @@ -105,8 +104,8 @@ def create_aws_query_error( ) _assert_xml() - deserializer = XMLCodec().create_deserializer( - body, wrapper_elements=wrapper_elements + deserializer = XMLCodec().create_deserializer_from_element( + root, wrapper_elements=wrapper_elements ) modeled_error = error_shape.deserialize(deserializer) if retry_after is not None: diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py index c95b36d2a..e910c53ef 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py @@ -1,19 +1,18 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from xml.etree.ElementTree import ParseError, fromstring +from xml.etree.ElementTree import Element -from ._xml import find_child +from ._xml import find_child, parse_xml_root -def parse_aws_query_request_id(body: bytes) -> str | None: +def parse_aws_query_request_id(source: bytes | Element | None) -> str | None: """Parse the request ID from an awsQuery response body. There is no request ID header. Successes nest it under ``ResponseMetadata``, errors put it directly under the root. """ - try: - root = fromstring(body) # noqa: S314 - except ParseError: + root = parse_xml_root(source) if isinstance(source, bytes) else source + if root is None: return None metadata = find_child(root, "ResponseMetadata") diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/restxml.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/restxml.py index bece04092..60bf16aaf 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/restxml.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/restxml.py @@ -1,9 +1,9 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from typing import NamedTuple -from xml.etree.ElementTree import ParseError, fromstring +from xml.etree.ElementTree import Element -from .query._xml import find_child, local_name +from .query._xml import find_child, local_name, parse_xml_root class RestXmlErrorInfo(NamedTuple): @@ -21,16 +21,15 @@ class RestXmlErrorInfo(NamedTuple): """The elements enclosing the error's members.""" -def parse_rest_xml_error(body: bytes) -> RestXmlErrorInfo: +def parse_rest_xml_error(source: bytes | Element | None) -> RestXmlErrorInfo: """Parse the code, message, and wrapper elements of a restXml error response. Errors are normally wrapped as ``...``, but services with ``noErrorWrapping`` (such as S3) send a bare ````. Both are accepted so the protocol needn't be configured for either. """ - try: - element = fromstring(body) # noqa: S314 - except ParseError: + element = parse_xml_root(source) if isinstance(source, bytes) else source + if element is None: return _EMPTY wrapper_elements: tuple[str, ...] = ("Error",) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py b/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py index 0c20ec8b3..77338706a 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py @@ -5,6 +5,7 @@ from inspect import iscoroutinefunction from io import BytesIO from typing import TYPE_CHECKING, Any, ClassVar, Final +from xml.etree.ElementTree import Element from smithy_core import URI as _URI from smithy_core.aio.interfaces import ( @@ -43,6 +44,7 @@ ) from smithy_http.deserializers import HTTPResponseDeserializer +from .._private.query._xml import parse_xml_root from .._private.query.errors import create_aws_query_error from .._private.query.metadata import parse_aws_query_request_id from .._private.query.serializers import Ec2QueryShapeSerializer, QueryShapeSerializer @@ -334,7 +336,8 @@ async def _create_error( body = response_body.read() retry_after = parse_retry_after(response) - error_info = parse_rest_xml_error(body) + root = parse_xml_root(body) + error_info = parse_rest_xml_error(root) code = error_info.code error_id = None if code is not None: @@ -344,7 +347,7 @@ async def _create_error( error_id = error_schema.id break - if error_id is not None and error_id in error_registry: + if error_id is not None and error_id in error_registry and root is not None: error_shape = error_registry.get(error_id) if not issubclass(error_shape, ModeledError): raise ExpectationNotMetError( @@ -353,7 +356,11 @@ async def _create_error( ) deserializer = HTTPResponseDeserializer( - payload_codec=_XMLErrorCodec(self._codec, error_info.wrapper_elements), + payload_codec=_XMLErrorCodec( + self._codec, + error_info.wrapper_elements, + root, + ), http_trait=operation.schema.expect_trait(HTTPTrait), response=response, body=body, @@ -397,6 +404,7 @@ class _XMLErrorCodec(Codec): codec: "XMLCodec" wrapper_elements: tuple[str, ...] + root: Element @property def media_type(self) -> str: @@ -406,8 +414,8 @@ def create_serializer(self, sink: BytesWriter) -> ShapeSerializer: return self.codec.create_serializer(sink) def create_deserializer(self, source: bytes | BytesReader) -> ShapeDeserializer: - return self.codec.create_deserializer( - source, wrapper_elements=self.wrapper_elements + return self.codec.create_deserializer_from_element( + self.root, wrapper_elements=self.wrapper_elements ) @@ -737,11 +745,12 @@ async def deserialize_response[ context: TypedProperties, ) -> OperationOutput: body = await response.consume_body_async() + root = parse_xml_root(body) # Recorded before any branch below returns or raises, so successes, empty # outputs and errors alike can report the identifier. Bound to this # response so extraction never attributes it to a different attempt. - request_id = parse_aws_query_request_id(body) + request_id = parse_aws_query_request_id(root) if request_id is not None: context[_QUERY_REQUEST_ID] = _RecordedQueryRequestId(response, request_id) @@ -749,9 +758,9 @@ async def deserialize_response[ raise await self._create_error( operation=operation, response=response, - response_body=body, error_registry=error_registry, context=context, + root=root, ) if len(body) == 0: @@ -764,8 +773,14 @@ async def deserialize_response[ ) wrapper_elements = self._response_wrapper_elements(operation) - deserializer = self.payload_codec.create_deserializer( - body, wrapper_elements=wrapper_elements + deserializer = ( + self.payload_codec.create_deserializer( + body, wrapper_elements=wrapper_elements + ) + if root is None + else self.payload_codec.create_deserializer_from_element( + root, wrapper_elements=wrapper_elements + ) ) return operation.output.deserialize(deserializer) @@ -782,12 +797,12 @@ async def _create_error( *, operation: APIOperation[Any, Any], response: HTTPResponse, - response_body: bytes, error_registry: TypeRegistry, context: TypedProperties, + root: Element | None, ) -> CallError: return create_aws_query_error( - body=response_body, + body=root, operation=operation, error_registry=error_registry, default_namespace=self._default_namespace, diff --git a/packages/smithy-aws-core/tests/unit/aio/test_protocols.py b/packages/smithy-aws-core/tests/unit/aio/test_protocols.py index cff8349b3..e71d52bd5 100644 --- a/packages/smithy-aws-core/tests/unit/aio/test_protocols.py +++ b/packages/smithy-aws-core/tests/unit/aio/test_protocols.py @@ -3,7 +3,8 @@ from dataclasses import dataclass from typing import Any, cast -from unittest.mock import Mock +from unittest.mock import Mock, patch +from xml.etree import ElementTree import pytest from ijson.common import IncompleteJSONError # type: ignore[reportMissingTypeStubs] @@ -511,28 +512,63 @@ async def test_aws_query_resolves_modeled_error_from_query_error_trait() -> None namespace="com.test", service_target="QueryService", version="2020-01-08" ) ) - with pytest.raises(_ModeledQueryError) as exc_info: - await protocol.deserialize_response( - operation=_mock_operation( - _operation_schema("FailingOperation"), - error_schemas=[_INVALID_ACTION_ERROR_SCHEMA], - ), + with patch( + "smithy_aws_core._private.query._xml.fromstring", + wraps=ElementTree.fromstring, + ) as parse: + with pytest.raises(_ModeledQueryError) as exc_info: + await protocol.deserialize_response( + operation=_mock_operation( + _operation_schema("FailingOperation"), + error_schemas=[_INVALID_ACTION_ERROR_SCHEMA], + ), + request=cast(HTTPRequest, Mock()), + response=HTTPResponse( + status=400, + fields=tuples_to_fields([]), + body=( + b"InvalidAction" + b"bad request" + ), + ), + error_registry=TypeRegistry( + {ShapeID("com.test#InvalidActionError"): _ModeledQueryError} + ), + context=TypedProperties(), + ) + + assert exc_info.value.message == "bad request" + parse.assert_called_once() + + +async def test_aws_query_parses_success_body_once() -> None: + protocol = AwsQueryClientProtocol( + ProtocolSettings( + namespace="com.test", service_target="QueryService", version="2020-01-08" + ) + ) + operation = _mock_operation(_operation_schema("EmptyOperation")) + cast(Any, operation).output = _EmptyOutput + cast(Any, operation).output_schema = _EMPTY_OUTPUT_SCHEMA + + with patch( + "smithy_aws_core._private.query._xml.fromstring", + wraps=ElementTree.fromstring, + ) as parse: + output = await protocol.deserialize_response( + operation=operation, request=cast(HTTPRequest, Mock()), response=HTTPResponse( - status=400, + status=200, fields=tuples_to_fields([]), - body=( - b"InvalidAction" - b"bad request" - ), - ), - error_registry=TypeRegistry( - {ShapeID("com.test#InvalidActionError"): _ModeledQueryError} + body=b"", ), + error_registry=TypeRegistry({}), context=TypedProperties(), ) - assert exc_info.value.message == "bad request" + assert isinstance(output, _EmptyOutput) + parse.assert_called_once() async def test_aws_query_resolves_modeled_error_from_default_namespace_fallback() -> ( @@ -593,6 +629,33 @@ async def test_aws_query_returns_generic_error_for_unknown_code() -> None: ) +async def test_aws_query_parses_invalid_error_body_once() -> None: + protocol = AwsQueryClientProtocol( + ProtocolSettings( + namespace="com.test", service_target="QueryService", version="2020-01-08" + ) + ) + + with patch( + "smithy_aws_core._private.query._xml.fromstring", + wraps=ElementTree.fromstring, + ) as parse: + with pytest.raises(CallError): + await protocol.deserialize_response( + operation=_mock_operation(_operation_schema("FailingOperation")), + request=cast(HTTPRequest, Mock()), + response=HTTPResponse( + status=500, + fields=tuples_to_fields([]), + body=b"not xml", + ), + error_registry=TypeRegistry({}), + context=TypedProperties(), + ) + + parse.assert_called_once() + + async def test_aws_query_reports_request_id_from_the_response_body() -> None: protocol = AwsQueryClientProtocol( ProtocolSettings( @@ -805,26 +868,31 @@ def _rest_xml_operation() -> APIOperation[Any, Any]: async def test_rest_xml_modeled_error_falls_back_to_message_element() -> None: - with pytest.raises(_NoSuchBucket) as exc_info: - await _rest_xml_protocol().deserialize_response( - operation=_rest_xml_operation(), - request=cast(HTTPRequest, Mock()), - response=HTTPResponse( - status=404, - fields=tuples_to_fields([]), - body=( - b"NoSuchBucket" - b"The specified bucket does not exist" + with patch( + "smithy_aws_core._private.query._xml.fromstring", + wraps=ElementTree.fromstring, + ) as parse: + with pytest.raises(_NoSuchBucket) as exc_info: + await _rest_xml_protocol().deserialize_response( + operation=_rest_xml_operation(), + request=cast(HTTPRequest, Mock()), + response=HTTPResponse( + status=404, + fields=tuples_to_fields([]), + body=( + b"NoSuchBucket" + b"The specified bucket does not exist" + ), ), - ), - error_registry=TypeRegistry( - {ShapeID("com.test#NoSuchBucket"): _NoSuchBucket} - ), - context=TypedProperties(), - ) + error_registry=TypeRegistry( + {ShapeID("com.test#NoSuchBucket"): _NoSuchBucket} + ), + context=TypedProperties(), + ) assert exc_info.value.message == "The specified bucket does not exist" assert str(exc_info.value) == "The specified bucket does not exist" + parse.assert_called_once() _STATS_SCHEMA = Schema.collection( diff --git a/packages/smithy-xml/.changes/next-release/smithy-xml-feature-f40d86af0a1d4f88a11786d45575ea09.json b/packages/smithy-xml/.changes/next-release/smithy-xml-feature-f40d86af0a1d4f88a11786d45575ea09.json new file mode 100644 index 000000000..ff39b2e85 --- /dev/null +++ b/packages/smithy-xml/.changes/next-release/smithy-xml-feature-f40d86af0a1d4f88a11786d45575ea09.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "XML payloads are now eagerly parsed before deserialization, improving performance for both `bytes` and `BytesReader` inputs." +} diff --git a/packages/smithy-xml/README.md b/packages/smithy-xml/README.md index bcd7da956..ba1307f3b 100644 --- a/packages/smithy-xml/README.md +++ b/packages/smithy-xml/README.md @@ -2,3 +2,9 @@ This package provides generic XML serialization and deserialization support for Smithy clients and servers. + +## Deserialization + +`XMLCodec` eagerly parses `bytes` and `BytesReader` payloads before +deserializing Smithy shapes. This improves deserialization performance while +temporarily materializing the complete XML tree. diff --git a/packages/smithy-xml/src/smithy_xml/__init__.py b/packages/smithy-xml/src/smithy_xml/__init__.py index 533717dca..f0a079e94 100644 --- a/packages/smithy-xml/src/smithy_xml/__init__.py +++ b/packages/smithy-xml/src/smithy_xml/__init__.py @@ -1,9 +1,8 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from io import BytesIO from typing import Any -from xml.etree.ElementTree import iterparse +from xml.etree.ElementTree import Element, fromstring, parse from smithy_core.codecs import Codec from smithy_core.deserializers import ShapeDeserializer @@ -13,7 +12,6 @@ from smithy_core.types import TimestampFormat from ._private.deserializers import XMLShapeDeserializer as _XMLShapeDeserializer -from ._private.readers import XMLEventReader as _XMLEventReader from ._private.serializers import XMLShapeSerializer as _XMLShapeSerializer from .settings import XMLSettings @@ -60,11 +58,25 @@ def create_deserializer( *, wrapper_elements: tuple[str, ...] = (), ) -> ShapeDeserializer: - if isinstance(source, bytes): - source = BytesIO(source) - reader = _XMLEventReader( - iterparse(source, events=("start", "end")) # noqa: S314 + root = ( + fromstring(source) # noqa: S314 + if isinstance(source, bytes) + else parse(source).getroot() # noqa: S314 ) + return self.create_deserializer_from_element( + root, + wrapper_elements=wrapper_elements, + ) + + def create_deserializer_from_element( + self, + root: Element, + *, + wrapper_elements: tuple[str, ...] = (), + ) -> ShapeDeserializer: + """Create a deserializer from an already parsed XML root element.""" return _XMLShapeDeserializer( - settings=self._settings, reader=reader, wrapper_elements=wrapper_elements + settings=self._settings, + root=root, + wrapper_elements=wrapper_elements, ) diff --git a/packages/smithy-xml/src/smithy_xml/_private/deserializers.py b/packages/smithy-xml/src/smithy_xml/_private/deserializers.py index 74474baaf..b458a784c 100644 --- a/packages/smithy-xml/src/smithy_xml/_private/deserializers.py +++ b/packages/smithy-xml/src/smithy_xml/_private/deserializers.py @@ -3,7 +3,7 @@ import datetime from base64 import b64decode -from collections.abc import Callable +from collections.abc import Callable, Iterable from decimal import Decimal from xml.etree.ElementTree import Element @@ -19,7 +19,6 @@ ) from ..settings import XMLSettings -from .readers import XMLEvent, XMLEventReader from .traits import member_xml_name @@ -61,28 +60,33 @@ def __init__(self, message: str) -> None: class XMLShapeDeserializer(ShapeDeserializer): - """Deserializer that reads XML from a streaming pull parser.""" + """Deserializer backed by a fully materialized ElementTree value.""" def __init__( self, settings: XMLSettings, - reader: XMLEventReader, + root: Element, wrapper_elements: tuple[str, ...] = (), + *, + elements: tuple[Element, ...] | None = None, + xml_names: dict[ShapeID, dict[str, Schema]] | None = None, ) -> None: self._settings = settings - self._reader = reader - self._xml_names: dict[ShapeID, dict[str, Schema]] = {} - self._preconsumed_start: Element | None = None - - # Wrapper elements are protocol transport containers (e.g. awsQuery's - # ). The last wrapper's start element is kept - # so that the next read can reuse it. - for wrapper in wrapper_elements: - event = next(self._reader) - if event.type != "start": - raise XMLParseError(f"Expected start element, got '{event.type}'") - _validate_element_name(wrapper, event.elem) - self._preconsumed_start = event.elem + self._xml_names = xml_names if xml_names is not None else {} + self._elements = elements + self._from_wrapper = bool(wrapper_elements) + + current = root + for index, wrapper in enumerate(wrapper_elements): + _validate_element_name(wrapper, current) + if index < len(wrapper_elements) - 1: + child = next(iter(current), None) + if child is None: + raise XMLParseError( + f"Expected wrapper element '{wrapper_elements[index + 1]}'" + ) + current = child + self._element = current def is_null(self) -> bool: return False @@ -123,194 +127,123 @@ def read_timestamp(self, schema: Schema) -> datetime.datetime: if self._settings.use_timestamp_format: if format_trait := schema.get_trait(TimestampFormatTrait): format = format_trait.format - - text = self._read_text() - return format.deserialize(text) + return format.deserialize(self._read_text()) def read_struct( self, schema: Schema, - consumer: Callable[[Schema, "ShapeDeserializer"], None], + consumer: Callable[[Schema, ShapeDeserializer], None], ) -> None: + element = self._element xml_names = self._get_xml_names(schema) - start_from_wrapper = self._preconsumed_start is not None - start_elem = self._consume_start_event() - # Wrapper elements are protocol transport containers, not modeled structs, - # so their attributes cannot be deserialized as struct members. - if not start_from_wrapper: + if not self._from_wrapper: for member_schema in schema.members.values(): if member_schema.get_trait(XMLAttributeTrait) is None: continue - # Prefixed names (e.g. ``xsi:name``) are parsed into a - # namespace-qualified key, so only the local parts are compared. - expected_attr_name = _local_attr_name(member_xml_name(member_schema)) - for attr_name, attr_value in start_elem.attrib.items(): - attr_local_name = _local_name(attr_name) - if attr_local_name == expected_attr_name: + expected_name = _local_attr_name(member_xml_name(member_schema)) + for attribute_name, value in element.attrib.items(): + if _local_name(attribute_name) == expected_name: consumer( member_schema, - _AttributeDeserializer(attr_value, self._settings), + _AttributeDeserializer(value, self._settings), ) break - # Flattened members lack an enclosing element, so there is no way to - # know when all items have been parsed. Their events are collected - # during iteration and replayed through a bounded reader afterwards. - flattened_buffers: dict[str, list[XMLEvent]] = {} - flattened_names = { - xml_name: member_schema - for xml_name, member_schema in xml_names.items() - if member_schema.get_trait(XMLFlattenedTrait) is not None - } - - while self._reader.peek().type != "end": - tag = _local_name(self._reader.peek().elem.tag) - - if tag in flattened_names: - flattened_buffers.setdefault(tag, []).extend(self._buffer_element()) - elif tag in xml_names: - consumer(xml_names[tag], self) + flattened: dict[str, list[Element]] = {} + for child in element: + name = _local_name(child.tag) + member_schema = xml_names.get(name) + if member_schema is None: + continue + if member_schema.get_trait(XMLFlattenedTrait) is not None: + flattened.setdefault(name, []).append(child) else: - # Skip unknown tag - self._consume_start_event() - self._skip_to_end() - - next(self._reader) + consumer(member_schema, self._for_element(child)) - for tag, events in flattened_buffers.items(): - member_schema = flattened_names[tag] - buffered_de = XMLShapeDeserializer( - self._settings, - XMLEventReader(iter(events)), + for name, elements in flattened.items(): + consumer( + xml_names[name], + self._for_elements(elements), ) - consumer(member_schema, buffered_de) def read_list( self, schema: Schema, - consumer: Callable[["ShapeDeserializer"], None], + consumer: Callable[[ShapeDeserializer], None], ) -> None: - is_flattened = schema.get_trait(XMLFlattenedTrait) is not None - if not is_flattened: - self._consume_start_event() - while self._reader.peek().type != "end": - consumer(self) + elements: Iterable[Element] + if schema.get_trait(XMLFlattenedTrait) is not None: + elements = self._elements or (self._element,) else: - while self._reader.has_next(): - consumer(self) + elements = self._element - if not is_flattened: - next(self._reader) + for element in elements: + consumer(self._for_element(element)) def read_map( self, schema: Schema, - consumer: Callable[[str, "ShapeDeserializer"], None], + consumer: Callable[[str, ShapeDeserializer], None], ) -> None: - is_flattened = schema.get_trait(XMLFlattenedTrait) is not None - key_schema = schema.members["key"] - value_schema = schema.members["value"] - key_tag = member_xml_name(key_schema) - value_tag = member_xml_name(value_schema) - - if not is_flattened: - self._consume_start_event() - while self._reader.peek().type != "end": - self._read_map_entry(key_tag, value_tag, consumer) + key_tag = member_xml_name(schema.members["key"]) + value_tag = member_xml_name(schema.members["value"]) + entries: Iterable[Element] + if schema.get_trait(XMLFlattenedTrait) is not None: + entries = self._elements or (self._element,) else: - while self._reader.has_next(): - self._read_map_entry(key_tag, value_tag, consumer) + entries = self._element - if not is_flattened: - next(self._reader) - - def _read_text(self) -> str: - """Consume a complete element (start through end) and return its text.""" - elem = self._consume_start_event() - self._skip_to_end() - # elem.text is populated only after consuming the "end" event - return elem.text or "" - - def _consume_start_event(self) -> Element: - """Consume and return the next start element. - - If a start element was pre-consumed (e.g. from consuming wrapper elements), - it is returned first and cleared. - """ - if self._preconsumed_start is not None: - elem = self._preconsumed_start - self._preconsumed_start = None - return elem - event = next(self._reader) - if event.type != "start": - raise XMLParseError(f"Expected start element, got '{event.type}'") - return event.elem - - def _skip_to_end(self) -> None: - """Skip to the matching end event. Assumes start was already consumed.""" - depth = 1 - while depth > 0: - event = next(self._reader) - if event.type == "start": - depth += 1 - elif event.type == "end": - depth -= 1 - - def _buffer_element(self) -> list[XMLEvent]: - """Buffer a complete element's events (start through matching end).""" - events: list[XMLEvent] = [] - event = next(self._reader) - events.append(event) - depth = 1 - while depth > 0: - event = next(self._reader) - events.append(event) - if event.type == "start": - depth += 1 - elif event.type == "end": - depth -= 1 - return events - - def _get_xml_names(self, schema: Schema) -> dict[str, Schema]: - """Get or build the XML element name -> member schema mapping for a shape.""" - if schema.id in self._xml_names: - return self._xml_names[schema.id] - result: dict[str, Schema] = {} - for member_schema in schema.members.values(): - if member_schema.get_trait(XMLAttributeTrait) is not None: - continue - xml_name = member_xml_name(member_schema) - result[xml_name] = member_schema - self._xml_names[schema.id] = result - return result + for entry in entries: + self._read_map_entry(entry, key_tag, value_tag, consumer) def _read_map_entry( self, + entry: Element, key_tag: str, value_tag: str, - consumer: Callable[[str, "ShapeDeserializer"], None], + consumer: Callable[[str, ShapeDeserializer], None], ) -> None: - """Read one map entry element and emit key/value pairs via consumer.""" - self._consume_start_event() - key: str | None = None - while self._reader.peek().type != "end": - child_tag = _local_name(self._reader.peek().elem.tag) - if child_tag == key_tag: - key = self._read_text() - elif child_tag == value_tag: + for child in entry: + name = _local_name(child.tag) + if name == key_tag: + key = child.text or "" + elif name == value_tag: if key is None: raise XMLParseError( "Map key element must appear before value element" ) - consumer(key, self) - else: - # Skip unknown child tag - self._consume_start_event() - self._skip_to_end() + consumer(key, self._for_element(child)) + + def _read_text(self) -> str: + return self._element.text or "" + + def _for_element(self, element: Element) -> "XMLShapeDeserializer": + return XMLShapeDeserializer( + settings=self._settings, + root=element, + xml_names=self._xml_names, + ) + + def _for_elements(self, elements: list[Element]) -> "XMLShapeDeserializer": + return XMLShapeDeserializer( + settings=self._settings, + root=elements[0], + elements=tuple(elements), + xml_names=self._xml_names, + ) - next(self._reader) + def _get_xml_names(self, schema: Schema) -> dict[str, Schema]: + if schema.id in self._xml_names: + return self._xml_names[schema.id] + result = { + member_xml_name(member_schema): member_schema + for member_schema in schema.members.values() + if member_schema.get_trait(XMLAttributeTrait) is None + } + self._xml_names[schema.id] = result + return result class _AttributeDeserializer(SpecificShapeDeserializer): diff --git a/packages/smithy-xml/src/smithy_xml/_private/readers.py b/packages/smithy-xml/src/smithy_xml/_private/readers.py deleted file mode 100644 index 98164f1e2..000000000 --- a/packages/smithy-xml/src/smithy_xml/_private/readers.py +++ /dev/null @@ -1,55 +0,0 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -from collections.abc import Iterator -from typing import NamedTuple -from xml.etree.ElementTree import Element - - -class XMLEvent(NamedTuple): - type: str - elem: Element - - -class XMLEventReader: - """Buffered iterator over XML pull parser events with peek support. - - Wraps an iterator of ``(event, element)`` tuples — either from - ``iterparse`` (streaming from a byte source) or from an in-memory list - (for flattened member replay). - """ - - def __init__(self, events: Iterator[tuple[str, Element] | XMLEvent]) -> None: - self._iter = events - self._pending: XMLEvent | None = None - - def __iter__(self): - return self - - def __next__(self) -> XMLEvent: - if self._pending is not None: - result = self._pending - self._pending = None - return result - return self._next() - - def _next(self) -> XMLEvent: - event = next(self._iter) - if isinstance(event, XMLEvent): - return event - event_type, elem = event - return XMLEvent(event_type, elem) - - def has_next(self) -> bool: - if self._pending is not None: - return True - try: - self._pending = self._next() - return True - except StopIteration: - return False - - def peek(self) -> XMLEvent: - if self._pending is None: - self._pending = self._next() - return self._pending diff --git a/packages/smithy-xml/tests/unit/test_deserializers.py b/packages/smithy-xml/tests/unit/test_deserializers.py index f3ee9e703..3bdf76b16 100644 --- a/packages/smithy-xml/tests/unit/test_deserializers.py +++ b/packages/smithy-xml/tests/unit/test_deserializers.py @@ -3,7 +3,9 @@ import math from datetime import datetime from decimal import Decimal +from io import BytesIO from typing import Any +from xml.etree.ElementTree import ParseError, fromstring import pytest from smithy_core.prelude import ( @@ -17,6 +19,7 @@ TIMESTAMP, ) from smithy_xml import XMLCodec +from smithy_xml._private.deserializers import XMLShapeDeserializer from . import ( STRING_LIST_SCHEMA, @@ -67,6 +70,31 @@ def test_xml_deserializer(expected: Any, given: bytes) -> None: assert actual == expected +@pytest.mark.parametrize( + "source", + [ + b"", + BytesIO(b""), + ], +) +def test_deserializer_is_eager(source: bytes | BytesIO) -> None: + deserializer = XMLCodec().create_deserializer(source) + assert isinstance(deserializer, XMLShapeDeserializer) + + +def test_creates_deserializer_from_parsed_element() -> None: + root = fromstring(b"value") + deserializer = XMLCodec().create_deserializer_from_element(root) + + assert deserializer.read_string(STRING) == "value" + + +def test_invalid_xml_uses_existing_error_type() -> None: + with pytest.raises(ParseError): + deserializer = XMLCodec().create_deserializer(b"") + deserializer.read_string(STRING) + + def test_read_document_raises() -> None: """XML does not support document types.""" deserializer = XMLCodec().create_deserializer(b"foo")