diff --git a/.chronus/changes/fix-api-version-validation-attr-2026-7-24-11-30-0.md b/.chronus/changes/fix-api-version-validation-attr-2026-7-24-11-30-0.md new file mode 100644 index 00000000000..449eb014807 --- /dev/null +++ b/.chronus/changes/fix-api-version-validation-attr-2026-7-24-11-30-0.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-python" +--- + +Fix the generated `_validation.py` `@api_version_validation` decorator so it reads the correct client config attribute for the API version. It previously hardcoded `client._config.api_version`, but the attribute name is derived from the API-version parameter's `client_name`. For specs that name the versioning parameter something other than `apiVersion` (e.g. `self.version`), the lookup raised `AttributeError` that the decorator silently swallowed, disabling all API-version validation for those clients. The emitter now bakes the real attribute name into the generated decorator so it reads `config.` directly. diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/general_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/general_serializer.py index c55dc359c65..d44dbc8bc02 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/general_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/general_serializer.py @@ -320,7 +320,27 @@ def serialize_model_base_file(self) -> str: def serialize_validation_file(self) -> str: template = self.env.get_template("validation.py.jinja2") - return template.render(code_model=self.code_model) + return template.render( + code_model=self.code_model, + client_api_version_name=self._api_version_config_attr_name(), + ) + + def _api_version_config_attr_name(self) -> str: + """Name of the config attribute that stores the API version. + + The attribute name is derived from the API-version parameter's ``client_name`` + (e.g. ``apiVersion`` -> ``api_version``, or Storage's ``version`` -> ``version``). + ``_validation.py`` holds a single shared decorator for the whole package, so when + every client agrees on the name we bake it in directly; otherwise we fall back to + the conventional ``api_version``. + """ + names = { + parameter.client_name + for client in self.code_model.clients + for parameter in client.config.parameters.parameters + if parameter.is_api_version + } + return next(iter(names)) if len(names) == 1 else "api_version" def serialize_cross_language_definition_file(self) -> str: cross_langauge_def_dict = { diff --git a/packages/http-client-python/generator/pygen/codegen/templates/validation.py.jinja2 b/packages/http-client-python/generator/pygen/codegen/templates/validation.py.jinja2 index fe534e7fe44..b7f70f0a6f2 100644 --- a/packages/http-client-python/generator/pygen/codegen/templates/validation.py.jinja2 +++ b/packages/http-client-python/generator/pygen/codegen/templates/validation.py.jinja2 @@ -29,7 +29,7 @@ def api_version_validation(**kwargs): try: # this assumes the client has an _api_version attribute client = args[0] - client_api_version = client._config.api_version # pylint: disable=protected-access + client_api_version = client._config.{{ client_api_version_name }} # pylint: disable=protected-access except AttributeError: return func(*args, **kwargs) diff --git a/packages/http-client-python/tests/unit/test_api_version_validation.py b/packages/http-client-python/tests/unit/test_api_version_validation.py new file mode 100644 index 00000000000..39ad9a0e7c4 --- /dev/null +++ b/packages/http-client-python/tests/unit/test_api_version_validation.py @@ -0,0 +1,140 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- +"""Tests for the generated ``_validation.py`` ``@api_version_validation`` decorator. + +Regression coverage for the bug where the generated ``_validation.py`` decorator +hardcoded ``client._config.api_version``. When the versioning parameter is named +something other than ``apiVersion`` (e.g. Azure Storage names it ``version`` via +``@apiVersion @header("x-ms-version") version: string``), the config attribute is +``self.version`` and NOT ``self.api_version``. The hardcoded lookup then raised +``AttributeError`` which the decorator silently swallowed, disabling ALL +api-version validation for those clients. + +The fix bakes the real config attribute name (the api-version parameter's +``client_name``) into the rendered ``_validation.py`` so the decorator reads +``config.`` directly. +""" + +import pytest +from jinja2 import Environment, PackageLoader + +from pygen.codegen.models import Client, CodeModel +from pygen.codegen.models.parameter import ConfigParameter +from pygen.codegen.models.parameter_list import ClientGlobalParameterList +from pygen.codegen.models.primitive_types import StringType +from pygen.codegen.serializers.general_serializer import GeneralSerializer + + +@pytest.fixture +def 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", + "version-tolerant": True, + "flavor": "azure", + }, + ) + + +def _env() -> Environment: + return Environment( + loader=PackageLoader("pygen.codegen", "templates"), + keep_trailing_newline=True, + line_statement_prefix="##", + line_comment_prefix="###", + trim_blocks=True, + lstrip_blocks=True, + ) + + +def _api_version_config_parameter(code_model, client_name: str) -> ConfigParameter: + return ConfigParameter( + yaml_data={ + "clientName": client_name, + "optional": True, + "location": "query", + "wireName": "api-version", + "implementation": "Client", + "isApiVersion": True, + "description": "The API version.", + }, + code_model=code_model, + type=StringType({"type": "string"}, code_model), + ) + + +def _client_with_api_version(code_model, client_name): + client = Client( + { + "name": "client", + "namespace": "blah", + "moduleName": "blah", + "parameters": [], + "url": "", + "operationGroups": [], + }, + code_model, + parameters=ClientGlobalParameterList({}, code_model, parameters=[]), + ) + if client_name is not None: + client.config.parameters.parameters.append(_api_version_config_parameter(code_model, client_name)) + code_model.clients = [client] + return client + + +def _render(code_model) -> str: + return GeneralSerializer(code_model=code_model, env=_env()).serialize_validation_file() + + +def test_reads_version_attribute_for_non_conventional_param(code_model): + # Storage-like: the versioning parameter is named ``version`` -> config attr + # is ``self.version``, so the decorator must read ``config.version`` directly. + _client_with_api_version(code_model, "version") + + rendered = _render(code_model) + + assert "client_api_version = client._config.version" in rendered + assert "config.api_version" not in rendered + assert "getattr(" not in rendered + + +def test_reads_api_version_attribute_for_conventional_param(code_model): + _client_with_api_version(code_model, "api_version") + + rendered = _render(code_model) + + assert "client_api_version = client._config.api_version" in rendered + + +def test_defaults_to_api_version_when_no_api_version_param(code_model): + _client_with_api_version(code_model, None) + + rendered = _render(code_model) + + assert "client_api_version = client._config.api_version" in rendered + + +def test_api_version_config_attr_name_helper(code_model): + _client_with_api_version(code_model, "version") + serializer = GeneralSerializer(code_model=code_model, env=_env()) + + assert serializer._api_version_config_attr_name() == "version"