diff --git a/.chronus/changes/http-client-python_fix-optional-body-content-type-2026-7-16-0-0-0.md b/.chronus/changes/http-client-python_fix-optional-body-content-type-2026-7-16-0-0-0.md new file mode 100644 index 00000000000..9756bdce836 --- /dev/null +++ b/.chronus/changes/http-client-python_fix-optional-body-content-type-2026-7-16-0-0-0.md @@ -0,0 +1,10 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-python" +--- + +Fix generated request builders serializing a `None` `content-type` header for an +operation with an optional body whose content-type is required/constant. The +`content-type` kwarg is now declared `Optional[str]` and the header is omitted +when it is `None`, instead of raising `ValueError: No value for given attribute`. diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py index b1da889bed6..791709407b6 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py @@ -462,6 +462,7 @@ def pop_kwargs_from_signature(self, builder: RequestBuilderType) -> list[str]: check_kwarg_dict=True, pop_headers_kwarg=(PopKwargType.CASE_INSENSITIVE if bool(builder.parameters.headers) else PopKwargType.NO), pop_params_kwarg=(PopKwargType.CASE_INSENSITIVE if bool(builder.parameters.query) else PopKwargType.NO), + is_body_optional=builder.parameters.has_body and builder.parameters.body_parameter.optional, ) @staticmethod @@ -491,6 +492,7 @@ def serialize_headers(self, builder: RequestBuilderType) -> list[str]: for h in builder.parameters.headers if not builder.has_form_data_body or h.wire_name.lower() != "content-type" ] + is_body_optional = builder.parameters.has_body and builder.parameters.body_parameter.optional retval = ["# Construct headers"] if headers else [] for header in headers: retval.extend( @@ -499,6 +501,7 @@ def serialize_headers(self, builder: RequestBuilderType) -> list[str]: "headers", self.serializer_name, self.code_model.is_legacy, + is_body_optional=is_body_optional, ) ) return retval diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/parameter_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/parameter_serializer.py index c2c66a7bb21..86d8ef2cdea 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/parameter_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/parameter_serializer.py @@ -139,6 +139,8 @@ def serialize_query_header( kwarg_name: str, serializer_name: str, is_legacy: bool, + *, + is_body_optional: bool = False, ) -> list[str]: if ( not is_legacy @@ -166,7 +168,11 @@ def serialize_query_header( param.wire_name, self.serialize_parameter(param, serializer_name), ) - if not param.optional and (param.in_method_signature or param.constant): + # When the body is optional, the content-type may legitimately be ``None`` (no body + # was sent). In that case the header must be omitted instead of serialized, otherwise + # ``_SERIALIZER.header`` raises ``ValueError: No value for given attribute``. + is_content_type_optional = getattr(param, "is_content_type", False) and is_body_optional + if not param.optional and not is_content_type_optional and (param.in_method_signature or param.constant): retval = [set_parameter] else: retval = [ @@ -184,6 +190,7 @@ def pop_kwargs_from_signature( check_client_input: bool = False, *, body_parameter: Optional[BodyParameterType] = None, + is_body_optional: Optional[bool] = None, ) -> list[str]: retval = [] @@ -195,7 +202,8 @@ def append_pop_kwarg(key: str, pop_type: PopKwargType) -> None: append_pop_kwarg("headers", pop_headers_kwarg) append_pop_kwarg("params", pop_params_kwarg) - is_body_optional = check_body_optional(body_parameter) + if is_body_optional is None: + is_body_optional = check_body_optional(body_parameter) if pop_headers_kwarg != PopKwargType.NO or pop_params_kwarg != PopKwargType.NO: retval.append("") for kwarg in parameters: @@ -206,7 +214,13 @@ def append_pop_kwarg(key: str, pop_type: PopKwargType) -> None: if is_content_type_optional and not type_annotation.startswith("Optional[") else type_annotation ) - if kwarg.client_default_value is not None or kwarg.optional or kwarg.constant or kwarg.is_api_version: + if ( + kwarg.client_default_value is not None + or kwarg.optional + or kwarg.constant + or kwarg.is_api_version + or is_content_type_optional + ): if check_client_input and kwarg.check_client_input: default_value = f"self._config.{kwarg.client_name}" elif kwarg.constant: diff --git a/packages/http-client-python/tests/unit/test_content_type_optional_body.py b/packages/http-client-python/tests/unit/test_content_type_optional_body.py new file mode 100644 index 00000000000..20456287b49 --- /dev/null +++ b/packages/http-client-python/tests/unit/test_content_type_optional_body.py @@ -0,0 +1,120 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- +"""Tests for content-type serialization when a request has an optional body. + +When an operation has an optional body whose content-type is modeled as +required/constant (e.g. a single specific media type), calling the operation +without a body leaves ``content_type = None``. The generated request builder +must therefore declare ``content_type`` as ``Optional[str]`` and guard the +header serialization with ``if content_type is not None:`` so a ``None`` value +is omitted instead of serialized (which would raise +``ValueError: No value for given attribute``). + +Regression test for https://github.com/microsoft/typespec/issues/11253. +""" +from pygen.codegen.models import CodeModel, StringType +from pygen.codegen.models.request_builder_parameter import RequestBuilderParameter +from pygen.codegen.serializers.parameter_serializer import ParameterSerializer, PopKwargType + + +def get_code_model(): + return CodeModel( + { + "clients": [ + { + "name": "client", + "namespace": "blah", + "moduleName": "blah", + "parameters": [], + "url": "", + "operationGroups": [], + } + ], + "namespace": "namespace", + }, + options={ + "show-send-request": True, + "builders-visibility": "public", + "show-operations": True, + "models-mode": "dpg", + "only-path-and-body-params-positional": True, + }, + ) + + +def get_content_type_parameter(*, required: bool): + code_model = get_code_model() + return RequestBuilderParameter( + yaml_data={ + "wireName": "content-type", + "clientName": "content_type", + "location": "header", + "optional": not required, + "implementation": "Method", + "inOverload": False, + "inOverloaded": False, + }, + code_model=code_model, + type=StringType(yaml_data={"type": "str"}, code_model=code_model), + ) + + +def test_optional_body_guards_required_content_type_header(): + """A required content-type header is guarded when the body is optional.""" + content_type = get_content_type_parameter(required=True) + lines = ParameterSerializer("").serialize_query_header( + content_type, + "headers", + "_SERIALIZER", + is_legacy=False, + is_body_optional=True, + ) + assert lines[0] == "if content_type is not None:" + assert lines[1].strip().startswith("_headers['content-type'] =") + + +def test_required_body_does_not_guard_required_content_type_header(): + """A required content-type header stays unguarded when the body is required.""" + content_type = get_content_type_parameter(required=True) + lines = ParameterSerializer("").serialize_query_header( + content_type, + "headers", + "_SERIALIZER", + is_legacy=False, + is_body_optional=False, + ) + assert len(lines) == 1 + assert lines[0].startswith("_headers['content-type'] =") + + +def test_optional_body_makes_required_content_type_kwarg_optional(): + """The popped content-type kwarg becomes ``Optional[str]`` with a ``None`` default.""" + content_type = get_content_type_parameter(required=True) + lines = ParameterSerializer.pop_kwargs_from_signature( + [content_type], + check_kwarg_dict=True, + pop_headers_kwarg=PopKwargType.CASE_INSENSITIVE, + pop_params_kwarg=PopKwargType.NO, + is_body_optional=True, + ) + declaration = next(line for line in lines if line.startswith("content_type")) + assert declaration == "content_type: Optional[str] = kwargs.pop('content_type', _headers.pop('content-type', None))" + # No body reassignment should be emitted in the request builder layer. + assert not any("content_type = content_type if" in line for line in lines) + + +def test_required_body_keeps_required_content_type_kwarg(): + """The popped content-type kwarg stays required when the body is required.""" + content_type = get_content_type_parameter(required=True) + lines = ParameterSerializer.pop_kwargs_from_signature( + [content_type], + check_kwarg_dict=True, + pop_headers_kwarg=PopKwargType.CASE_INSENSITIVE, + pop_params_kwarg=PopKwargType.NO, + is_body_optional=False, + ) + declaration = next(line for line in lines if line.startswith("content_type")) + assert declaration == "content_type: str = kwargs.pop('content_type')"