From 228127e4f327ad9a7d71c781080143228a4272e7 Mon Sep 17 00:00:00 2001 From: Volv G Date: Fri, 28 Aug 2026 06:26:35 -0700 Subject: [PATCH] Fix generated client wire names for renamed parameters The static generator emitted the collision-avoided Python local name as the JSON/query/path key. A request-body field literally named `body` is renamed to the local `body_2`, so `admin_post_notices("text", ...)` sent `{"body_2": ...}` and omitted the required `body` field; `admin_patch_notices` had the same defect. The dynamic transport was already correct because it keys the payload off `CliParameter.original_name`. Thread `original_name` through `_param_signature` as a `_ParamName` (local_name, wire_name) pair so the emitted `json_data`, `params`, and `path_params` literals all use the schema wire name while the method signature keeps the collision-free local. Only a missing name falls back to the local identifier, so schema-valid empty property names keep their exact spelling. Also emit `# type: ignore[misc, assignment]` for the pydantic v1 `ConfigDict` fallback in the runtime template; `[assignment]` alone left downstream regenerations reporting `runtime.py: Cannot assign to a type [misc]`. Regenerated the checked-in runtime module from the snapshot. Assisted-By: devx/9c659b46-4731-49d5-8fa1-b00487ef84e8 --- .../src/tangle_api/generated/runtime.py | 2 +- .../src/tangle_cli/openapi/codegen.py | 52 ++++-- tests/test_codegen.py | 161 ++++++++++++++++++ 3 files changed, 199 insertions(+), 16 deletions(-) diff --git a/packages/tangle-api/src/tangle_api/generated/runtime.py b/packages/tangle-api/src/tangle_api/generated/runtime.py index aced4f7..ab464e8 100644 --- a/packages/tangle-api/src/tangle_api/generated/runtime.py +++ b/packages/tangle-api/src/tangle_api/generated/runtime.py @@ -9,7 +9,7 @@ try: from pydantic import ConfigDict except ImportError: # pragma: no cover - pydantic v1 fallback - ConfigDict = None # type: ignore[assignment] + ConfigDict = None # type: ignore[misc, assignment] class TangleGeneratedModel(BaseModel): diff --git a/packages/tangle-cli/src/tangle_cli/openapi/codegen.py b/packages/tangle-cli/src/tangle_cli/openapi/codegen.py index 24441e8..0dd7a38 100644 --- a/packages/tangle-cli/src/tangle_cli/openapi/codegen.py +++ b/packages/tangle-cli/src/tangle_cli/openapi/codegen.py @@ -24,7 +24,7 @@ import urllib.request from collections.abc import Sequence from pathlib import Path -from typing import Any +from typing import Any, NamedTuple from .parser import ( DEFAULT_OPENAPI_PATH, @@ -384,7 +384,7 @@ def generate_runtime() -> str: try: from pydantic import ConfigDict except ImportError: # pragma: no cover - pydantic v1 fallback - ConfigDict = None # type: ignore[assignment] + ConfigDict = None # type: ignore[misc, assignment] class TangleGeneratedModel(BaseModel): @@ -464,6 +464,21 @@ def generate_models( lines.append("") return "\n".join(lines) + +class _ParamName(NamedTuple): + """Pair a generated Python parameter name with its OpenAPI wire name. + + ``local_name`` is the collision-avoided Python identifier used in the + generated method signature; ``wire_name`` is the schema/OpenAPI name that + must be used as the JSON/query/path key sent to the backend. They differ + whenever the parser had to rename a field (for example a request-body field + literally named ``body`` becomes the local ``body_2``). + """ + + local_name: str + wire_name: str + + def _method_name(group_name: str, command_name: str) -> str: return f"{_safe_identifier(group_name)}_{_safe_identifier(command_name)}" @@ -481,7 +496,7 @@ def _param_signature( has_request_body: bool, *, raw_body_override: bool = False, -) -> tuple[str, list[str], list[str], list[str], set[str], bool]: +) -> tuple[str, list[_ParamName], list[_ParamName], list[_ParamName], set[str], bool]: required: list[Any] = [] optional: list[Any] = [] for parameter in parameters: @@ -489,25 +504,32 @@ def _param_signature( ordered = required + optional seen: set[str] = set() signature_parts: list[str] = [] - path_names: list[str] = [] - query_names: list[str] = [] - body_names: list[str] = [] + path_names: list[_ParamName] = [] + query_names: list[_ParamName] = [] + body_names: list[_ParamName] = [] required_body_names: set[str] = set() for parameter in ordered: name = _safe_identifier(parameter.local_name) if name in seen: continue seen.add(name) + # Collision-avoiding Python locals (e.g. ``body_2``) must never leak into + # the emitted wire payload; the schema field name is the wire key. Only + # a missing name falls back to the local identifier, so schema-valid + # empty property names keep their exact wire spelling. + original_name = getattr(parameter, "original_name", None) + wire_name = name if original_name is None else str(original_name) + entry = _ParamName(local_name=name, wire_name=wire_name) if parameter.required: signature_parts.append(f"{name}: Any") else: signature_parts.append(f"{name}: Any = None") if parameter.location == "path": - path_names.append(name) + path_names.append(entry) elif parameter.location == "query": - query_names.append(name) + query_names.append(entry) elif parameter.location == "body": - body_names.append(name) + body_names.append(entry) if parameter.required: required_body_names.add(name) include_body = has_request_body @@ -517,27 +539,27 @@ def _param_signature( return ", ".join(signature_parts), path_names, query_names, body_names, required_body_names, include_body -def _dict_literal(names: list[str]) -> str: +def _dict_literal(names: list[_ParamName]) -> str: if not names: return "None" - return "{" + ", ".join(f"{name!r}: {name}" for name in names) + "}" + return "{" + ", ".join(f"{name.wire_name!r}: {name.local_name}" for name in names) + "}" -def _body_dict_literal(names: list[str], required_names: set[str]) -> str: +def _body_dict_literal(names: list[_ParamName], required_names: set[str]) -> str: if not names: return "None" - optional_names = [name for name in names if name not in required_names] + optional_names = [name for name in names if name.local_name not in required_names] if not optional_names: return _dict_literal(names) optional_literal = _dict_literal(optional_names) optional_expr = f"key: value for key, value in {optional_literal}.items() if value is not None" if not required_names: return "{" + optional_expr + "}" - required_literal = _dict_literal([name for name in names if name in required_names]) + required_literal = _dict_literal([name for name in names if name.local_name in required_names]) return "{" + f"**{required_literal}, **{{{optional_expr}}}" + "}" -def _merged_body_dict_literal(names: list[str], required_names: set[str]) -> str: +def _merged_body_dict_literal(names: list[_ParamName], required_names: set[str]) -> str: """Return request JSON with generic body fields overridden by named fields.""" return f"{{**(body or {{}}), **{_body_dict_literal(names, required_names)}}}" diff --git a/tests/test_codegen.py b/tests/test_codegen.py index fa9a074..baf1c6d 100644 --- a/tests/test_codegen.py +++ b/tests/test_codegen.py @@ -1023,3 +1023,164 @@ def test_generate_operations_uses_concrete_return_annotations() -> None: assert "response_model=self._response_model('FooResponse', FooResponse)" in operations assert "def things_delete(self, id: Any) -> None:" in operations assert "def unknown_list(self) -> Any:" in operations + + +def _renamed_parameter_schema() -> dict: + """Schema whose body/query/path names collide with generated Python locals.""" + + return { + "openapi": "3.1.0", + "paths": { + "/api/admin/notices": { + "post": { + "operationId": "create_notice", + "parameters": [ + { + "name": "order.by", + "in": "query", + "required": False, + "schema": {"type": "string"}, + }, + { + "name": "token", + "in": "query", + "required": False, + "schema": {"type": "string"}, + }, + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["body"], + "properties": { + "body": {"type": "string"}, + "title": {"type": "string"}, + "variant": {"type": "string"}, + }, + } + } + } + }, + } + }, + "/api/admin/notices/{notice-id}": { + "patch": { + "operationId": "update_notice", + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {"body": {"type": "string"}}, + } + } + } + }, + } + }, + }, + "components": {"schemas": {}}, + } + + +def test_generate_operations_uses_wire_names_for_renamed_parameters(monkeypatch, tmp_path) -> None: + openapi = tmp_path / "openapi.json" + out = tmp_path / "renamed_param_api" + openapi.write_text(json.dumps(_renamed_parameter_schema()), encoding="utf-8") + + codegen.generate(openapi, out) + + operations = (out / "operations.py").read_text(encoding="utf-8") + post = operations.split("def admin_notices(", 1)[1].split(" def ", 1)[0] + patch = operations.split("def admin_patch_notices(", 1)[1].split("__all__", 1)[0] + + # Python locals stay collision-free, wire keys stay schema-faithful. + assert "def admin_notices(self, body_2: Any, " in operations + assert "'body_2'" not in operations + assert "'body': body_2" in post + assert "'order.by': order_by" in post + assert "'token': token_2" in post + assert "'notice-id': notice_id" in patch + assert "'body': body_2" in patch + + monkeypatch.syspath_prepend(str(tmp_path)) + generated_operations = importlib.import_module("renamed_param_api.operations") + + class Client(generated_operations.GeneratedTangleApiOperations): + def __init__(self) -> None: + self.calls = [] + + def _request_json(self, *args, **kwargs): + self.calls.append((args, kwargs)) + return {"ok": True} + + client = Client() + client.admin_notices("notice text", title="Heads up", variant="info") + client.admin_notices("notice text", order_by="created_at", token_2="t0ken") + client.admin_patch_notices("abc123", body_2="updated text") + + create = client.calls[0][1] + assert create["json_data"] == { + "body": "notice text", + "title": "Heads up", + "variant": "info", + } + assert create["params"] == {"order.by": None, "token": None} + assert client.calls[1][1]["params"] == {"order.by": "created_at", "token": "t0ken"} + assert client.calls[2][1]["path_params"] == {"notice-id": "abc123"} + assert client.calls[2][1]["json_data"] == {"body": "updated text"} + + +def test_generate_runtime_type_ignore_covers_required_mypy_error_codes(tmp_path) -> None: + runtime = codegen.generate_runtime() + + assert "ConfigDict = None # type: ignore[misc, assignment]" in runtime + assert "# type: ignore[assignment]" not in runtime + + openapi = tmp_path / "openapi.json" + out = tmp_path / "runtime_api" + openapi.write_text(json.dumps(_schema()), encoding="utf-8") + + codegen.generate(openapi, out) + + assert (out / "runtime.py").read_text(encoding="utf-8") == runtime + + +def test_generate_operations_preserves_empty_wire_names(tmp_path) -> None: + openapi = tmp_path / "openapi.json" + out = tmp_path / "empty_name_api" + openapi.write_text( + json.dumps({ + "openapi": "3.1.0", + "paths": { + "/api/oddities": { + "post": { + "operationId": "create_oddity", + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [""], + "properties": {"": {"type": "string"}}, + } + } + } + }, + } + } + }, + "components": {"schemas": {}}, + }), + encoding="utf-8", + ) + + codegen.generate(openapi, out) + + operations = (out / "operations.py").read_text(encoding="utf-8") + + # An empty JSON property name is schema-valid and must not be replaced by + # the generated Python local name. + assert "**{'': " in operations