diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 0000000..67d002e --- /dev/null +++ b/PLAN.md @@ -0,0 +1,104 @@ +# Clientele: Low-Hanging Fruit Feature Plan + +## Context + +Clientele (v2.2.2) is a Python API-integration framework with two halves: a runtime decorator-based client (`clientele/api/`) and an OpenAPI→Python code generator (`clientele/generators/`). Recent releases shipped the high-demand runtime features (retries via stamina, logging, ResponseFactory testing helpers, discriminated unions, SSE streaming). The remaining gaps cluster in three areas: **generator robustness with real-world specs**, **generator coverage of common OpenAPI constructs**, and **runtime flexibility** (the one open issue, #233, is about multi-instance clients). + +This plan identifies the features with the best value-to-effort ratio, validated against the actual code (file references checked), ordered by priority. Each item is independently shippable. + +--- + +## Tier 1 — Do these first + +### 1. Fix non-string enum generation (int/float/mixed enums) — **Small** (bug-fix grade) +- **Value:** Any spec with `enum: [1, 2, 3]` (status codes, versions — very common) **crashes generation** with an opaque `AttributeError`. Verified: `schemas.py:178` builds `{v: {"type": f'"{v}"'}}` from raw enum values and `generate_enum_properties` (`schemas.py:31-36`) calls `arg.upper()` on each — fails for any non-string member. Blocks adoption outright. +- **Implementation:** In `make_schema_class` (`clientele/generators/shared/generators/schemas.py:176-178`), branch on value types: all-string keeps current path; otherwise synthesize member names (`VALUE_1`, sanitized, deduped) and emit `repr(v)` values. Add an `IntEnum` base conditional in `schema_class.jinja2`. Add a fixture spec + test alongside `tests/test_complex_schemas.py`. +- **Risk:** member-name collisions after sanitization — dedupe with suffix. + +### 2. Generate auth config from `components.securitySchemes` — **Small/Medium** +- **Value:** Nearly every real API has auth; today the generator ignores `securitySchemes` entirely (verified: `config_py.jinja2` is rendered with only `base_url` in context, `generators/api/generator.py:84-88`) and `docs/api-authentication.md` tells users to hand-edit `config.py`. This is the most visible "it just works" win available. +- **Implementation:** In `APIGenerator.generate_templates_files` (`generator.py:62`), read security schemes from the spec (via cicerone's components, falling back to the `__pydantic_extra__` pattern already used throughout `cicerone_compat.py`). Pass a normalized `auth_schemes` list to the template. Extend `config_py.jinja2` with conditionals: `http/bearer` → `bearer_token` field + `Authorization: Bearer` header injection; `apiKey` in header → field + header; `http/basic` → username/password fields. pydantic-settings gives env-var support for free. **Scope cut:** skip `apiKey`-in-query and `oauth2` flows (emit explanatory comment + MANIFEST note). +- **Risk:** cicerone may not model securitySchemes first-class — verify; the extras escape hatch covers it. `config.py` is never overwritten on regen (`generator.py:80-81`), so this only benefits fresh generations — acceptable and safe. + +### 3. `clientele validate` CLI command + graceful spec error reporting — **Small/Medium** +- **Value:** Malformed specs and missing `$refs` currently crash mid-generation with `AttributeError` (helpers like `get_schema_from_ref` silently return `{}`). A pre-flight compatibility check directly reduces the project's most common class of bug report, and gives CI users a gate. +- **Implementation:** New `validate` command in `clientele/cli.py` reusing the already-factored `_load_openapi_spec`/`_prepare_spec` (`cli.py:23-94`). Walk paths via existing `cicerone_compat.path_item_to_operations_dict` and report: unresolvable `#/components/...` refs, path-based `$refs` (degrade to `typing.Any`), cookie params (currently silently dropped), multipart bodies (unsupported), missing `responses`, non-string enums (until item 1 lands). Rich table output, exit 0/1. +- **Risk:** minimal — pure read-path code over existing modules. Label findings as warnings vs errors. + +### 4. Typed `additionalProperties` (`dict[str, Model]`) — **Small** +- **Value:** Map-like schemas (error maps, translations, metrics keyed by ID) are everywhere; currently degrade to `dict[str, Any]`, losing all validation. Root cause verified: `cicerone_compat.schema_to_dict` never copies `additionalProperties` out of the parsed model, so the type resolver never sees it. +- **Implementation:** (a) Copy `additionalProperties` through `schema_to_dict` and `normalize_openapi_31_schema` in `clientele/generators/cicerone_compat.py` (recurse for schema values, pass through booleans). (b) In `get_type` (`clientele/generators/shared/utils.py:~145`), when object has a schema-valued `additionalProperties`, return `dict[str, {inner}]`. (c) Objects with *only* `additionalProperties` should emit a `RootModel` via existing `schema_root_model.jinja2` (same pattern as array responses). +- **Risk:** forward-ref quoting inside `dict[str, "X"]` — existing quote/unquote helpers cover it. + +--- + +## Tier 2 — High value, slightly more work + +### 5. Per-call config override / multi-instance clients (open issue #233) — **Medium** +- **Value:** The only open issue, with real multi-tenant users blocked: generated `client.py` hardcodes one module-global `APIClient`, and `configure()` mutates global state (not thread-safe). Same spec + many hosts (the reporter's OCPI/EV-charging case) is impossible today. +- **Implementation:** Add a reserved `config` kwarg following the exact pattern of the existing reserved `query`/`headers` kwargs (`clientele/api/client.py:331-334`): pop in `_prepare_call`, carry on the prepared call, and in the four execute paths resolve `effective_config = override or self.config`. Lazily build the override's `http_backend` via existing `HttpxHTTPBackend.from_config` (mirrors `configure()`), so each tenant config caches its own connection pool. **No template change needed** — every generated function gains multi-tenant support for free. Document in `docs/`. +- **Risk:** param-name collision if a spec has a parameter literally named `config` — same pre-existing risk/guard as `query`/`headers`; document it. + +### 6. Request/response hooks on `BaseConfig` — **Small/Medium** +- **Value:** One feature closes three gaps: interceptors/middleware, token-refresh auth flows, and de-facto rate limiting/tracing — all currently require forking an HTTP backend. +- **Implementation:** Add `request_hooks` / `response_hooks` lists (default empty) to `BaseConfig` (`clientele/api/config.py`). Invoke at the two choke points `_send_request` / `_send_request_async` (`api/client.py:~581-657`) and the two stream paths, passing a small mutable `RequestInfo` (method, url, headers, query, json) so hooks can mutate headers. ~60 lines + tests with `FakeHTTPBackend`. +- **Risk:** API lock-in — keep the hook signature minimal; sync hooks only at first. + +### 7. Richer exception hierarchy — **Small** +- **Value:** Users want `except ClientError` vs `except ServerError` and a `status_code` attribute; today there are only two exception types (`api/exceptions.py`) and users dig into `.response.status_code`. +- **Implementation:** Add `ClientError(HTTPStatusError)` (4xx) and `ServerError(HTTPStatusError)` (5xx); pick subclass by range in `http/response.py:raise_for_status` (~lines 60-73). Add `status_code` property. Fully backwards-compatible. Update `docs/api-exceptions.md`. + +--- + +## Tier 3 — Worth doing opportunistically + +### 8. OpenAPI 3.1 type arrays → unions — **Small/Medium** +- **Value:** Correctness bug: `type: ["string", "integer"]` silently keeps only the first type (`cicerone_compat.py:51-52`), producing wrong runtime validation. +- **Implementation:** In `normalize_openapi_31_schema`, rewrite multi-type arrays to `anyOf` branches (+ `nullable` if `"null"` present); the downstream anyOf→union pipeline already works. + +### 9. Cookie parameters: warn now, support later — **Small** (warning) +- **Value:** Cookie params are *silently dropped* (`base_clients.py:142-163` handles only query/path/header). Silent data loss is the worst failure mode. +- **Implementation:** Add an `elif in_ == "cookie":` branch logging a yellow console warning; surface the same in `validate` (item 3). Full cookie support (threading through 5 backends) is deferred — Medium, low demand. + +### 10. Schema constraints → `pydantic.Field` validation — **Medium** +- **Value:** `minLength`/`pattern`/`minimum` etc. are stripped by `schema_to_dict` before generation, so generated models accept data the server will reject. +- **Implementation:** Copy constraint keys through `cicerone_compat`, then refactor `generate_class_properties` (`schemas.py:49-89`) so the four-way alias/default branching collapses into one `pydantic.Field(...)` builder that also takes `min_length`, `max_length`, `pattern`, `ge`, `le`, `multiple_of`. The refactor is the real work; the mapping is mechanical. +- **Risk:** behavior changes in the branchy alias/default code — add coverage first. + +### 11. `--check` drift mode for CI — **Medium** +- **Value:** Teams committing generated clients want CI to fail when spec and code drift; no design ambiguity. +- **Implementation:** `--check` flag on `start_api` (`cli.py`): generate into a temp dir, run the same ruff formatting, diff against the output dir ignoring user-owned `config.py`/`pyproject.toml` (per `generator.py:80,108`), exit 1 with diff summary. +- **Risk:** ruff version skew causing false positives — document. + +### 12. Usage examples in generated docstrings — **Small** +- **Value:** Cheap DX polish: copy-pasteable call example per generated function. +- **Implementation:** Pure template change in `api_get_method.jinja2` / `api_post_method.jinja2` using data already in context (func name, required args, data class). Sanitize summaries containing `"""` while there. + +--- + +## Explicitly NOT low-hanging fruit (excluded, with rationale) + +| Feature | Why excluded | +|---|---| +| **Multipart/file uploads** | Runtime hardwires `json=` payloads (`api/client.py:594, 633`); needs API design + changes to all 5 HTTP backends + generator. Real feature, multi-PR project (**Large**). | +| **Pagination helpers** | Auto-detecting cursor/page/offset conventions from arbitrary specs is unreliable; needs API design consensus. No honest small version exists. | +| **Webhooks (3.1)** | Generates *server-side* handlers — outside clientele's client-generation mission. At most list in MANIFEST. | +| **Path-based `$refs`** | Needs a general JSON-pointer resolver + naming scheme for anonymous schemas; low demand. The `typing.Any` fallback + a `validate` warning covers it. | +| **Built-in rate limiting** | Superseded by hooks (item 6) + existing stamina retries. | +| **Streaming for direct `.request()`** | Decorator path covers streaming; low demand. | + +## Suggested sequencing + +1. **Release A (generator robustness):** items 1, 3, 4 — two are bug-fix grade, plus the `validate` command that catches everything else. +2. **Release B (headline features):** items 2 (auth generation) and 5 (#233 multi-instance) — the two most visible wins. +3. **Release C (runtime ergonomics):** items 6 + 7 together. +4. Items 8–12 opportunistically / as good-first-issues (9 and 12 are ideal community contributions). + +## Verification + +- Each generator item: add a fixture to `example_openapi_specs/`, regenerate test clients under `tests/api_clients/`, assert generated code imports cleanly and round-trips via `FakeHTTPBackend` (existing pattern in the 63-file test suite). +- Item 1: spec with `enum: [1, 2, 3]` generates an importable `IntEnum`. +- Item 2: spec with bearer/apiKey/basic schemes → generated `config.py` exposes the fields and injects headers (assert via FakeHTTPBackend captured request). +- Item 5: two configs with different `base_url`s → concurrent calls hit the right host (FakeHTTPBackend records URLs). +- CLI items: run `clientele validate -f ` and `--check` against a stale dir, assert exit codes. +- Full suite: `uv run pytest`. diff --git a/clientele/api/client.py b/clientele/api/client.py index b603c0d..1601667 100644 --- a/clientele/api/client.py +++ b/clientele/api/client.py @@ -79,6 +79,21 @@ def configure( if self.config.http_backend is None: self.config.http_backend = httpx_backend.HttpxHTTPBackend.from_config(self.config) + def _resolve_config(self, config_override: api_config.BaseConfig | None) -> api_config.BaseConfig: + """ + Resolve the configuration for a single call. + + Returns the per-call override when one was passed, otherwise the + client-wide config. An override without an http_backend gets one + lazily, cached on the override itself so that each config keeps + its own connection pool (e.g. one per tenant). + """ + if config_override is None: + return self.config + if config_override.http_backend is None: + config_override.http_backend = httpx_backend.HttpxHTTPBackend.from_config(config_override) + return config_override + def close(self) -> None: """Close the synchronous HTTP client.""" if self.config.http_backend is not None: @@ -98,6 +113,7 @@ def request( data: dict[str, typing.Any] | pydantic.BaseModel | None = None, query: dict[str, typing.Any] | None = None, headers: dict[str, str] | None = None, + config: api_config.BaseConfig | None = None, **path_params: typing.Any, ) -> typing.Any: """ @@ -112,6 +128,7 @@ def request( data: Request body payload (for POST, PUT, etc.) query: Query parameters (optional) headers: Additional request headers (optional) + config: Per-call config override (optional); uses the client config when omitted **path_params: Path parameters to substitute in the URL path """ url_path = self._substitute_path(path, path_params) @@ -123,6 +140,7 @@ def request( data_payload=data_payload, headers_override=headers, response_map=response_map, + config=self._resolve_config(config), ) return self._parse_response( response=response, @@ -139,6 +157,7 @@ async def arequest( data: dict[str, typing.Any] | pydantic.BaseModel | None = None, query: dict[str, typing.Any] | None = None, headers: dict[str, str] | None = None, + config: api_config.BaseConfig | None = None, **path_params: typing.Any, ) -> typing.Any: """ @@ -153,6 +172,7 @@ async def arequest( data: Request body payload (for POST, PUT, etc.) query: Query parameters (optional) headers: Additional request headers (optional) + config: Per-call config override (optional); uses the client config when omitted **path_params: Path parameters to substitute in the URL path """ url_path = self._substitute_path(path, path_params) @@ -164,6 +184,7 @@ async def arequest( data_payload=data_payload, headers_override=headers, response_map=response_map, + config=self._resolve_config(config), ) return self._parse_response( response=response, @@ -332,6 +353,7 @@ def _prepare_call( # Extract reserved keywords that control request behavior query_override = kwargs_copy.pop("query", None) if "query" not in context.signature.parameters else None headers_override = kwargs_copy.pop("headers", None) if "headers" not in context.signature.parameters else None + config_override = kwargs_copy.pop("config", None) if "config" not in context.signature.parameters else None recognized_kwargs = {k: v for k, v in kwargs_copy.items() if k in context.signature.parameters} extra_kwargs = {k: v for k, v in kwargs_copy.items() if k not in context.signature.parameters} @@ -409,6 +431,7 @@ def _prepare_call( data_payload=data_payload, headers_override=headers_override, result_annotation=result_annotation, + config_override=config_override, ) def _execute_sync( @@ -422,6 +445,7 @@ def _execute_sync( data_payload=prepared.data_payload, headers_override=prepared.headers_override, response_map=context.response_map, + config=self._resolve_config(prepared.config_override), ) result = self._finalise_call(prepared=prepared, response=response) return result @@ -437,6 +461,7 @@ async def _execute_async( data_payload=prepared.data_payload, headers_override=prepared.headers_override, response_map=context.response_map, + config=self._resolve_config(prepared.config_override), ) result = self._finalise_call(prepared=prepared, response=response) return await result @@ -449,12 +474,13 @@ async def _execute_async_stream( """ prepared = self._prepare_call(context, args, kwargs) + config = self._resolve_config(prepared.config_override) # Extract the inner type from AsyncIterator[T] inner_type = type_utils.get_streaming_inner_type(prepared.result_annotation) # Build request kwargs - headers = {**self.config.headers, **(prepared.headers_override or {})} + headers = {**config.headers, **(prepared.headers_override or {})} request_kwargs: dict[str, typing.Any] = { "params": prepared.query_params, "headers": headers, @@ -462,8 +488,8 @@ async def _execute_async_stream( if prepared.data_payload is not None: request_kwargs["json"] = prepared.data_payload - if self.config.logger is not None: - self.config.logger.debug(f"HTTP Streaming Request: {context.method} {prepared.url_path}") + if config.logger is not None: + config.logger.debug(f"HTTP Streaming Request: {context.method} {prepared.url_path}") # For streaming, response_parser must accept str, not Response. # Cast is safe: streaming endpoints only accept Callable[[str], Any] parsers, @@ -472,9 +498,9 @@ async def _execute_async_stream( typing.Callable[[str], typing.Any] | None, context.response_parser ) - if not self.config.http_backend: + if not config.http_backend: raise RuntimeError("HTTP backend is not configured.") - stream_generator = self.config.http_backend.handle_async_stream( + stream_generator = config.http_backend.handle_async_stream( method=context.method, url=prepared.url_path, inner_type=inner_type, @@ -508,15 +534,16 @@ def _execute_sync_stream( """ prepared = self._prepare_call(context, args, kwargs) + config = self._resolve_config(prepared.config_override) # Extract the inner type from Iterator[T] inner_type = type_utils.get_streaming_inner_type(prepared.result_annotation) - if self.config.http_backend is None: + if config.http_backend is None: raise RuntimeError("HTTP backend is not configured.") # Build request kwargs - headers = {**self.config.headers, **(prepared.headers_override or {})} + headers = {**config.headers, **(prepared.headers_override or {})} request_kwargs: dict[str, typing.Any] = { "params": prepared.query_params, "headers": headers, @@ -524,8 +551,8 @@ def _execute_sync_stream( if prepared.data_payload is not None: request_kwargs["json"] = prepared.data_payload - if self.config.logger is not None: - self.config.logger.debug(f"HTTP Streaming Request: {context.method} {prepared.url_path}") + if config.logger is not None: + config.logger.debug(f"HTTP Streaming Request: {context.method} {prepared.url_path}") # For streaming, response_parser must accept str, not Response. # Cast is safe: streaming endpoints only accept Callable[[str], Any] parsers, @@ -534,7 +561,7 @@ def _execute_sync_stream( typing.Callable[[str], typing.Any] | None, context.response_parser ) - stream_generator = self.config.http_backend.handle_sync_stream( + stream_generator = config.http_backend.handle_sync_stream( method=context.method, url=prepared.url_path, inner_type=inner_type, @@ -587,30 +614,32 @@ def _send_request( data_payload: dict[str, typing.Any] | None, headers_override: dict[str, str] | None, response_map: dict[int, type[typing.Any]] | None = None, + config: api_config.BaseConfig | None = None, ) -> http_response.Response: - headers = {**self.config.headers, **(headers_override or {})} + config = config or self.config + headers = {**config.headers, **(headers_override or {})} request_kwargs: dict[str, typing.Any] = {"params": query_params, "headers": headers} if data_payload is not None: request_kwargs["json"] = data_payload - if self.config.http_backend is None: + if config.http_backend is None: raise RuntimeError("HTTP backend is not configured.") - if self.config.logger: - self.config.logger.debug(f"HTTP Request: {method} {url}") - self.config.logger.debug(f"Request Query Params: {query_params}") - self.config.logger.debug(f"Request Payload: {data_payload}") - self.config.logger.debug(f"Request Headers: {headers}") + if config.logger: + config.logger.debug(f"HTTP Request: {method} {url}") + config.logger.debug(f"Request Query Params: {query_params}") + config.logger.debug(f"Request Payload: {data_payload}") + config.logger.debug(f"Request Headers: {headers}") start_time = time.perf_counter() - response = self.config.http_backend.send_sync_request(method, url, **request_kwargs) + response = config.http_backend.send_sync_request(method, url, **request_kwargs) elapsed_time = time.perf_counter() - start_time - if self.config.logger: - self.config.logger.debug(f"HTTP Response: {method} {url} -> {response.status_code} ({elapsed_time:.3f}s)") - self.config.logger.debug(f"Response Content: {response.text}") - self.config.logger.debug(f"Response Headers: {response.headers}") + if config.logger: + config.logger.debug(f"HTTP Response: {method} {url} -> {response.status_code} ({elapsed_time:.3f}s)") + config.logger.debug(f"Response Content: {response.text}") + config.logger.debug(f"Response Headers: {response.headers}") # Only raise for status if we don't have a response_map # If we have a response_map, we want to handle error responses @@ -627,25 +656,27 @@ async def _send_request_async( data_payload: dict[str, typing.Any] | None, headers_override: dict[str, str] | None, response_map: dict[int, type[typing.Any]] | None = None, + config: api_config.BaseConfig | None = None, ) -> http_response.Response: - headers = {**self.config.headers, **(headers_override or {})} + config = config or self.config + headers = {**config.headers, **(headers_override or {})} request_kwargs: dict[str, typing.Any] = {"params": query_params, "headers": headers} if data_payload is not None: request_kwargs["json"] = data_payload - if self.config.http_backend is None: + if config.http_backend is None: raise RuntimeError("HTTP backend is not configured.") - if self.config.logger is not None: - self.config.logger.debug(f"HTTP Request: {method} {url}") + if config.logger is not None: + config.logger.debug(f"HTTP Request: {method} {url}") start_time = time.perf_counter() - response = await self.config.http_backend.send_async_request(method, url, **request_kwargs) + response = await config.http_backend.send_async_request(method, url, **request_kwargs) elapsed_time = time.perf_counter() - start_time - if self.config.logger: - self.config.logger.debug( + if config.logger: + config.logger.debug( f"HTTP Response: {method} {url} -> {response.status_code} ({elapsed_time:.3f}s)\n" f"Content: {response.text}" ) diff --git a/clientele/api/request_context.py b/clientele/api/request_context.py index 4d6b2bf..ed5007a 100644 --- a/clientele/api/request_context.py +++ b/clientele/api/request_context.py @@ -6,6 +6,7 @@ import pydantic +from clientele.api import config as api_config from clientele.api import type_utils from clientele.http import response as http_response from clientele.http import status_codes @@ -29,6 +30,7 @@ class PreparedCall(pydantic.BaseModel): data_payload: dict[str, typing.Any] | None headers_override: dict[str, str] | None result_annotation: typing.Any + config_override: api_config.BaseConfig | None = None class RequestContext(pydantic.BaseModel): diff --git a/clientele/cli.py b/clientele/cli.py index 1ca9786..c916eb4 100644 --- a/clientele/cli.py +++ b/clientele/cli.py @@ -187,8 +187,54 @@ def start_api(url, file, output, asyncio=False, regen=False): scaffold_api(url=url, file=file, output=output, asyncio=asyncio, regen=regen) +@click.command() +@click.option("-u", "--url", help="URL to openapi schema (URL)", required=False) +@click.option("-f", "--file", help="Path to openapi schema (json or yaml file)", required=False) +def validate(url, file): + """ + Check an OpenAPI schema for clientele compatibility. + + Walks the schema and reports errors (constructs that break client + generation, like unresolvable $refs) and warnings (constructs that + degrade, like cookie parameters or multipart bodies). + + Exits with status 1 if any errors are found, so it can gate CI. + """ + from rich.console import Console + + console = Console() + + if not url and not file: + raise click.UsageError("Provide a schema with -u/--url or -f/--file") + + from clientele.generators.validation import SpecValidator + + try: + spec = _prepare_spec(console=console, url=url, file=file) + except Exception as exc: + console.print(f"[red]Could not parse OpenAPI schema: {exc}") + sys.exit(1) + if not spec: + sys.exit(1) + + findings = SpecValidator(spec=spec).validate() + if not findings: + console.print("[green]⚜️ No issues found - this schema is ready to generate! ⚜️") + return + + errors = [f for f in findings if f.severity == "error"] + warnings = [f for f in findings if f.severity == "warning"] + for finding in findings: + colour = "red" if finding.severity == "error" else "yellow" + console.print(f"[{colour}]{finding.severity.upper()}[/{colour}] {finding.location}: {finding.message}") + console.print(f"\n{len(errors)} error(s), {len(warnings)} warning(s)") + if errors: + sys.exit(1) + + cli_group.add_command(version) cli_group.add_command(start_api) +cli_group.add_command(validate) if __name__ == "__main__": cli_group() diff --git a/clientele/generators/api/generator.py b/clientele/generators/api/generator.py index 1b60329..33191d9 100644 --- a/clientele/generators/api/generator.py +++ b/clientele/generators/api/generator.py @@ -9,7 +9,7 @@ from clientele import generators, settings from clientele.generators.api import clients, writer -from clientele.generators.shared import schemas, utils +from clientele.generators.shared import schemas, security, utils console = rich_console.Console() @@ -68,6 +68,14 @@ def generate_templates_files(self) -> None: base_url = self.spec.servers[0].url console.log(f"[cyan]Detected base URL from spec: {base_url}[/cyan]") + auth = security.classify_security_schemes(self.spec) + if auth: + for scheme in auth["unsupported"]: + console.log( + f"[yellow]Security scheme '{scheme['scheme_name']}' ({scheme['description']}) " + "cannot be generated automatically - configure it manually in config.py[/yellow]" + ) + writer.write_to_init(output_dir=self.output_dir) for ( client_file, @@ -83,6 +91,7 @@ def generate_templates_files(self) -> None: content = template.render( client_project_directory_path=client_project_directory_path, base_url=base_url, + auth=auth, ) write_func(content, output_dir=self.output_dir) # Manifest file diff --git a/clientele/generators/api/templates/config_py.jinja2 b/clientele/generators/api/templates/config_py.jinja2 index 00acdd3..8c71537 100644 --- a/clientele/generators/api/templates/config_py.jinja2 +++ b/clientele/generators/api/templates/config_py.jinja2 @@ -2,7 +2,9 @@ This file will never be updated on subsequent clientele runs. Use it as a space to store configuration and constants. """ - +{% if auth and auth.basic %} +import base64 +{% endif %} from pydantic import Field from clientele import api as clientele_api @@ -30,8 +32,40 @@ class Config(clientele_api.BaseConfig): """ base_url: str = "{{ base_url }}" +{% if auth %}{% if auth.bearer %} + # HTTP bearer authentication ("{{ auth.bearer.scheme_name }}" in the OpenAPI spec). + # When set, sends the "Authorization: Bearer " header. + bearer_token: str = "" +{% endif %}{% if auth.basic %} + # HTTP basic authentication ("{{ auth.basic.scheme_name }}" in the OpenAPI spec). + # When set, sends the "Authorization: Basic " header. + basic_username: str = "" + basic_password: str = "" +{% endif %}{% for key in auth.api_keys %} + # API key authentication ("{{ key.scheme_name }}" in the OpenAPI spec). + # When set, sends the "{{ key.header_name }}" header. + {{ key.field_name }}: str = "" +{% endfor %}{% for scheme in auth.unsupported %} + # Note: security scheme "{{ scheme.scheme_name }}" ({{ scheme.description }}) cannot be + # generated automatically. Configure it manually, e.g. via the headers field. +{% endfor %}{% endif %} headers: dict[str, str] = Field(default_factory=dict) timeout: float | None = 5.0 follow_redirects: bool = False verify: bool | str = True http2: bool = False +{% if auth and (auth.bearer or auth.basic or auth.api_keys) %} + def model_post_init(self, context, /) -> None: + """Inject configured credentials into headers, without overriding explicit ones.""" + super().model_post_init(context) +{% if auth.bearer %} + if self.bearer_token: + self.headers.setdefault("Authorization", f"Bearer {self.bearer_token}") +{% endif %}{% if auth.basic %} + if self.basic_username or self.basic_password: + credentials = base64.b64encode(f"{self.basic_username}:{self.basic_password}".encode()).decode() + self.headers.setdefault("Authorization", f"Basic {credentials}") +{% endif %}{% for key in auth.api_keys %} + if self.{{ key.field_name }}: + self.headers.setdefault("{{ key.header_name }}", self.{{ key.field_name }}) +{% endfor %}{% endif %} \ No newline at end of file diff --git a/clientele/generators/api/templates/schema_class.jinja2 b/clientele/generators/api/templates/schema_class.jinja2 index d429f55..f4b6435 100644 --- a/clientele/generators/api/templates/schema_class.jinja2 +++ b/clientele/generators/api/templates/schema_class.jinja2 @@ -1,4 +1,4 @@ -class {{class_name}}({{"str, enum.Enum" if enum else "pydantic.BaseModel"}}): +class {{class_name}}({{enum_base if enum else "pydantic.BaseModel"}}): {{properties if properties else " pass"}} diff --git a/clientele/generators/api/templates/schema_root_model.jinja2 b/clientele/generators/api/templates/schema_root_model.jinja2 index fce1cb5..adc090a 100644 --- a/clientele/generators/api/templates/schema_root_model.jinja2 +++ b/clientele/generators/api/templates/schema_root_model.jinja2 @@ -1,4 +1,4 @@ -class {{class_name}}(ListResponse[{{inner_type}}]): +class {{class_name}}({{base_class}}[{{inner_type}}]): pass diff --git a/clientele/generators/api/templates/schemas_py.jinja2 b/clientele/generators/api/templates/schemas_py.jinja2 index 9f25fad..5633c77 100644 --- a/clientele/generators/api/templates/schemas_py.jinja2 +++ b/clientele/generators/api/templates/schemas_py.jinja2 @@ -7,4 +7,4 @@ import enum import pydantic -from clientele.schemas import ListResponse # noqa +from clientele.schemas import DictResponse, ListResponse # noqa diff --git a/clientele/generators/cicerone_compat.py b/clientele/generators/cicerone_compat.py index 5faee3b..7d327a7 100644 --- a/clientele/generators/cicerone_compat.py +++ b/clientele/generators/cicerone_compat.py @@ -69,6 +69,9 @@ def normalize_openapi_31_schema(schema_dict: dict) -> dict: if "items" in normalized: normalized["items"] = normalize_openapi_31_schema(normalized["items"]) + if "additionalProperties" in normalized and isinstance(normalized["additionalProperties"], dict): + normalized["additionalProperties"] = normalize_openapi_31_schema(normalized["additionalProperties"]) + if "allOf" in normalized and isinstance(normalized["allOf"], list): normalized["allOf"] = [normalize_openapi_31_schema(s) for s in normalized["allOf"]] @@ -199,6 +202,11 @@ def schema_to_dict(schema) -> dict: if hasattr(schema, "__pydantic_extra__") and schema.__pydantic_extra__ and "const" in schema.__pydantic_extra__: result["const"] = schema.__pydantic_extra__["const"] + # Handle additionalProperties - it's in the extra fields as a raw dict or bool + additional_properties = get_pydantic_extra(schema, "additionalProperties") + if additional_properties is not None: + result["additionalProperties"] = additional_properties + # Handle properties if hasattr(schema, "properties") and schema.properties: result["properties"] = {k: schema_to_dict(v) for k, v in schema.properties.items()} diff --git a/clientele/generators/shared/schemas.py b/clientele/generators/shared/schemas.py index c89a312..58d2a2c 100644 --- a/clientele/generators/shared/schemas.py +++ b/clientele/generators/shared/schemas.py @@ -1,3 +1,4 @@ +import json import typing from cicerone.spec import openapi_spec as cicerone_openapi_spec @@ -63,11 +64,35 @@ def __init__(self, spec: cicerone_openapi_spec.OpenAPISpec, output_dir: str, wri self.output_dir = output_dir self.writer = writer - def generate_enum_properties(self, properties: dict) -> str: - lines = [ - f" {utils.snake_case_prop(arg.upper())} = {utils.get_type(arg_details)}\n" - for arg, arg_details in properties.items() - ] + def _enum_member_name(self, value: typing.Any) -> str: + if value is None: + return "NULL" + if isinstance(value, str): + return utils.snake_case_prop(value.upper()) + # Non-string members (ints, floats, bools) cannot be used as + # identifiers directly, so synthesize a VALUE_ name. + name = f"VALUE_{value}".replace("-", "MINUS_").replace(".", "_") + return utils.snake_case_prop(name.upper()) + + def _enum_base_class(self, values: list) -> str: + if all(isinstance(v, str) for v in values): + return "str, enum.Enum" + # bool is a subclass of int but enum.IntEnum cannot hold it + if all(isinstance(v, int) and not isinstance(v, bool) for v in values): + return "enum.IntEnum" + return "enum.Enum" + + def generate_enum_properties(self, values: list) -> str: + lines = [] + seen: dict[str, int] = {} + for value in values: + name = self._enum_member_name(value) + count = seen.get(name, 0) + 1 + seen[name] = count + if count > 1: + name = f"{name}_{count}" + py_value = json.dumps(value) if isinstance(value, str) else repr(value) + lines.append(f" {name} = {py_value}\n") return "".join(lines) def generate_headers_class(self, properties: dict, func_name: str) -> str: @@ -156,6 +181,7 @@ def _create_union_type_alias( def make_schema_class(self, schema_key: str, schema: dict) -> None: schema_key = utils.class_name_titled(schema_key) enum = False + enum_base = "str, enum.Enum" properties: str = "" if one_of := schema.get("oneOf"): @@ -178,7 +204,24 @@ def make_schema_class(self, schema_key: str, schema: dict) -> None: item_type = utils.get_type(items_schema) if items_schema else "typing.Any" item_type = utils.remove_forward_ref_quotes(item_type) template = self.writer.templates.get_template("schema_root_model.jinja2") - content = template.render(class_name=schema_key, inner_type=item_type) + content = template.render(class_name=schema_key, inner_type=item_type, base_class="ListResponse") + self.writer.write_to_schemas(content, output_dir=self.output_dir) + self.schemas[schema_key] = "" + return + + additional_properties = schema.get("additionalProperties") + if ( + schema.get("type") == "object" + and isinstance(additional_properties, dict) + and additional_properties + and not schema.get("properties") + ): + # A pure map schema (additionalProperties, no properties) gets the + # same root-model treatment as arrays so it is a real type that + # validates its values. + value_type = utils.remove_forward_ref_quotes(utils.get_type(additional_properties)) + template = self.writer.templates.get_template("schema_root_model.jinja2") + content = template.render(class_name=schema_key, inner_type=value_type, base_class="DictResponse") self.writer.write_to_schemas(content, output_dir=self.output_dir) self.schemas[schema_key] = "" return @@ -208,9 +251,10 @@ def make_schema_class(self, schema_key: str, schema: dict) -> None: ) ) properties = "".join(property_parts) - elif schema.get("enum"): + elif enum_values := schema.get("enum"): enum = True - properties = self.generate_enum_properties({v: {"type": f'"{v}"'} for v in schema["enum"]}) + enum_base = self._enum_base_class(enum_values) + properties = self.generate_enum_properties(enum_values) else: properties = self.generate_class_properties( properties=schema.get("properties", {}), @@ -218,7 +262,7 @@ def make_schema_class(self, schema_key: str, schema: dict) -> None: ) self.schemas[schema_key] = properties template = self.writer.templates.get_template("schema_class.jinja2") - content = template.render(class_name=schema_key, properties=properties, enum=enum) + content = template.render(class_name=schema_key, properties=properties, enum=enum, enum_base=enum_base) self.writer.write_to_schemas(content, output_dir=self.output_dir) def write_helpers(self) -> None: diff --git a/clientele/generators/shared/security.py b/clientele/generators/shared/security.py new file mode 100644 index 0000000..f746fc8 --- /dev/null +++ b/clientele/generators/shared/security.py @@ -0,0 +1,95 @@ +""" +Classification of OpenAPI security schemes for client generation. + +The generator can produce typed credential fields for the common schemes: +HTTP bearer, HTTP basic, and API keys sent in a header. Everything else +(oauth2, openIdConnect, mutualTLS, API keys in query/cookie) requires +manual configuration. This module classifies a spec's securitySchemes so +the generator and the validator agree on what is supported. +""" + +import typing + +from cicerone.spec import openapi_spec as cicerone_openapi_spec + +from clientele.generators.shared import utils + +# Field names already used by BaseConfig that credential fields must not shadow +RESERVED_CONFIG_FIELDS = frozenset( + { + "base_url", + "headers", + "timeout", + "follow_redirects", + "verify", + "http2", + "cache_backend", + "http_backend", + "logger", + "bearer_token", + "basic_username", + "basic_password", + } +) + + +def describe_scheme(scheme: typing.Any) -> str: + """A short human-readable description of a security scheme.""" + scheme_type = getattr(scheme, "type", None) or "unknown" + if scheme_type == "apiKey": + return f"apiKey in {getattr(scheme, 'in_', None)}" + if scheme_type == "http": + return f"http {(getattr(scheme, 'scheme', None) or '').lower()}" + return scheme_type + + +def classify_security_schemes(spec: cicerone_openapi_spec.OpenAPISpec) -> typing.Optional[dict]: + """ + Classify a spec's securitySchemes into what the generator supports. + + Returns None when the spec declares no security schemes, otherwise a dict: + bearer: {"scheme_name": str} or None (first http/bearer scheme) + basic: {"scheme_name": str} or None (first http/basic scheme) + api_keys: [{"scheme_name": str, "header_name": str, "field_name": str}] + unsupported: [{"scheme_name": str, "description": str}] + """ + components = spec.components + schemes = getattr(components, "security_schemes", None) if components else None + if not schemes: + return None + + bearer: typing.Optional[dict] = None + basic: typing.Optional[dict] = None + api_keys: list[dict] = [] + unsupported: list[dict] = [] + + for name, scheme in schemes.items(): + scheme_type = getattr(scheme, "type", None) + http_scheme = (getattr(scheme, "scheme", None) or "").lower() + in_ = getattr(scheme, "in_", None) + if scheme_type == "http" and http_scheme == "bearer" and bearer is None: + bearer = {"scheme_name": name} + elif scheme_type == "http" and http_scheme == "basic" and basic is None: + basic = {"scheme_name": name} + elif scheme_type == "apiKey" and in_ == "header": + api_keys.append({"scheme_name": name, "header_name": scheme.name}) + else: + unsupported.append({"scheme_name": name, "description": describe_scheme(scheme)}) + + if len(api_keys) == 1: + api_keys[0]["field_name"] = "api_key" + else: + seen: set[str] = set(RESERVED_CONFIG_FIELDS) + for api_key in api_keys: + field_name = utils.snake_case_prop(api_key["scheme_name"]) + while field_name in seen: + field_name = f"{field_name}_key" + seen.add(field_name) + api_key["field_name"] = field_name + + return { + "bearer": bearer, + "basic": basic, + "api_keys": api_keys, + "unsupported": unsupported, + } diff --git a/clientele/generators/shared/utils.py b/clientele/generators/shared/utils.py index a9c9fb4..eaffbf2 100644 --- a/clientele/generators/shared/utils.py +++ b/clientele/generators/shared/utils.py @@ -181,7 +181,12 @@ def get_type(t): elif t_type == DataType.BOOLEAN: base_type = "bool" elif t_type == DataType.OBJECT: - base_type = "dict[str, typing.Any]" + additional_properties = t.get("additionalProperties") + if isinstance(additional_properties, dict) and additional_properties and not t.get("properties"): + # A map schema: object whose values all match one schema + base_type = f"dict[str, {get_type(additional_properties)}]" + else: + base_type = "dict[str, typing.Any]" elif t_type == DataType.NULL: # OpenAPI 3.1.0 uses {"type": "null"} for null values # In Python, this should be None diff --git a/clientele/generators/validation.py b/clientele/generators/validation.py new file mode 100644 index 0000000..39c8d45 --- /dev/null +++ b/clientele/generators/validation.py @@ -0,0 +1,176 @@ +""" +Pre-flight validation of OpenAPI specs for clientele compatibility. + +The generator assumes a well-formed spec: unresolvable $refs either crash +generation or silently produce broken output, and unsupported constructs +(cookie parameters, multipart bodies) are silently dropped. SpecValidator +walks a parsed spec and reports these up front: + + - errors: constructs that break generation or produce broken code + - warnings: constructs that degrade to less useful code + +Used by the `clientele validate` CLI command. +""" + +import dataclasses + +from cicerone.spec import openapi_spec as cicerone_openapi_spec + +from clientele.generators import cicerone_compat +from clientele.generators.shared import security + +SCHEMA_REF_PREFIX = "#/components/schemas/" +PARAMETER_REF_PREFIX = "#/components/parameters/" + +SUPPORTED_PARAMETER_LOCATIONS = ("query", "path", "header") + + +@dataclasses.dataclass +class Finding: + severity: str # "error" or "warning" + location: str + message: str + + +class SpecValidator: + """Walks an OpenAPI spec and collects compatibility findings.""" + + def __init__(self, spec: cicerone_openapi_spec.OpenAPISpec) -> None: + self.spec = spec + self.findings: list[Finding] = [] + self.schema_names: set[str] = set() + self.parameter_names: set[str] = set() + if spec.components: + if spec.components.schemas: + self.schema_names = set(spec.components.schemas) + if getattr(spec.components, "parameters", None): + self.parameter_names = set(spec.components.parameters) + + def validate(self) -> list[Finding]: + self.findings = [] + self._check_security_schemes() + self._check_component_schemas() + self._check_paths() + return self.findings + + def _check_security_schemes(self) -> None: + auth = security.classify_security_schemes(self.spec) + if not auth: + return + for scheme in auth["unsupported"]: + self._warning( + f"security scheme '{scheme['scheme_name']}' ({scheme['description']}) cannot be " + "generated automatically; configure authentication manually in config.py", + f"components.securitySchemes.{scheme['scheme_name']}", + ) + + def _error(self, message: str, location: str) -> None: + self.findings.append(Finding(severity="error", location=location, message=message)) + + def _warning(self, message: str, location: str) -> None: + self.findings.append(Finding(severity="warning", location=location, message=message)) + + def _check_ref(self, ref: str, location: str) -> None: + if ref.startswith(SCHEMA_REF_PREFIX): + name = ref[len(SCHEMA_REF_PREFIX) :] + if name not in self.schema_names: + self._error(f"$ref references missing schema '{name}'", location) + elif ref.startswith(PARAMETER_REF_PREFIX): + name = ref[len(PARAMETER_REF_PREFIX) :] + if name not in self.parameter_names: + self._error(f"$ref references missing parameter '{name}'", location) + else: + self._warning( + f"$ref '{ref}' is not a component reference and will degrade to typing.Any", + location, + ) + + def _walk_schema(self, schema, location: str) -> None: + if not isinstance(schema, dict): + return + if ref := schema.get("$ref"): + self._check_ref(ref, location) + return + for key in ("oneOf", "anyOf", "allOf"): + for index, sub_schema in enumerate(schema.get(key) or []): + self._walk_schema(sub_schema, f"{location}.{key}[{index}]") + for prop_name, prop_schema in (schema.get("properties") or {}).items(): + self._walk_schema(prop_schema, f"{location}.{prop_name}") + if items := schema.get("items"): + self._walk_schema(items, f"{location}.items") + additional_properties = schema.get("additionalProperties") + if isinstance(additional_properties, dict): + self._walk_schema(additional_properties, f"{location}.additionalProperties") + + def _check_component_schemas(self) -> None: + if not (self.spec.components and self.spec.components.schemas): + return + for name, schema in self.spec.components.schemas.items(): + schema_dict = cicerone_compat.schema_to_dict(schema) + self._walk_schema(schema_dict, f"components.schemas.{name}") + + def _check_paths(self) -> None: + if not self.spec.paths or not self.spec.paths.items: + return + for path, path_item in self.spec.paths.items.items(): + operations = cicerone_compat.path_item_to_operations_dict(path_item) + shared_parameters = operations.get("parameters", []) + for method, operation in operations.items(): + if method == "parameters": + continue + self._check_operation(operation, shared_parameters, f"{method.upper()} {path}") + + def _check_parameter(self, param: dict, location: str) -> None: + in_ = param.get("in") + name = param.get("name", "") + if in_ == "cookie": + self._warning(f"cookie parameter '{name}' is not supported and will be skipped", location) + elif in_ not in SUPPORTED_PARAMETER_LOCATIONS: + self._warning(f"parameter '{name}' has unsupported location '{in_}' and will be skipped", location) + if schema := param.get("schema"): + self._walk_schema(schema, f"{location} parameter '{name}'") + + def _check_operation(self, operation: dict, shared_parameters: list, location: str) -> None: + for param in list(operation.get("parameters", [])) + list(shared_parameters): + if not isinstance(param, dict): + param = cicerone_compat.parameter_to_dict(param) + if ref := param.get("$ref"): + self._check_ref(ref, location) + if not ref.startswith(PARAMETER_REF_PREFIX): + continue + name = ref[len(PARAMETER_REF_PREFIX) :] + if name not in self.parameter_names: + continue + param = cicerone_compat.parameter_to_dict(self.spec.components.parameters[name]) + self._check_parameter(param, location) + + if request_body := operation.get("requestBody"): + for content_type, content in (request_body.get("content") or {}).items(): + if content_type == "multipart/form-data": + self._warning( + "multipart/form-data request bodies are generated as plain models; " + "file upload fields are not supported", + location, + ) + if isinstance(content, dict) and "schema" in content: + self._walk_schema(content["schema"], f"{location} requestBody ({content_type})") + + if "responses" not in operation: + self._warning( + "operation has no responses defined; a default 200 response will be assumed", + location, + ) + return + for status_code, response in operation["responses"].items(): + content = response.get("content") if isinstance(response, dict) else None + if not content: + # No-content responses (e.g. 204) are fully supported + continue + for content_type, media in content.items(): + if not isinstance(media, dict) or "schema" not in media: + self._warning( + f"response {status_code} ({content_type}) has no schema; typing.Any will be used", + location, + ) + continue + self._walk_schema(media["schema"], f"{location} response {status_code} ({content_type})") diff --git a/clientele/schemas.py b/clientele/schemas.py index 4ed79ee..09b2aa6 100644 --- a/clientele/schemas.py +++ b/clientele/schemas.py @@ -18,3 +18,31 @@ def __getitem__(self, index: int) -> _Item: def __iter__(self) -> typing.Iterator[_Item]: # ty: ignore[invalid-method-override] return iter(self.root) + + +class DictResponse(pydantic.RootModel[dict[str, _Item]]): + """Base class for map schemas (additionalProperties). Provides dict-like access to the root dict.""" + + def __len__(self) -> int: + return len(self.root) + + def __getitem__(self, key: str) -> _Item: + return self.root[key] + + def __iter__(self) -> typing.Iterator[str]: # ty: ignore[invalid-method-override] + return iter(self.root) + + def __contains__(self, key: str) -> bool: + return key in self.root + + def get(self, key: str, default: typing.Optional[_Item] = None) -> typing.Optional[_Item]: + return self.root.get(key, default) + + def keys(self) -> typing.KeysView[str]: + return self.root.keys() + + def values(self) -> typing.ValuesView[_Item]: + return self.root.values() + + def items(self) -> typing.ItemsView[str, _Item]: + return self.root.items() diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8057e29..46c2ed4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,11 @@ ## 2.3.0 UNRELEASED +- Support non-string enums in schema generation. Integer enums generate `enum.IntEnum` classes, number and mixed-type enums generate plain `enum.Enum` classes with `VALUE_` member names. Previously any non-string enum value crashed generation with `AttributeError: 'int' object has no attribute 'upper'`. +- Support typed `additionalProperties` in schema generation. Map-valued properties now generate `dict[str, ]` instead of `dict[str, typing.Any]`, and component schemas that are purely maps generate `clientele.schemas.DictResponse` subclasses with dict-style access (`errors["email"]`, `len()`, `.keys()`, `.items()`). Free-form objects (`additionalProperties: true` or absent) keep the existing untyped behaviour. +- New `clientele validate` command: a pre-flight compatibility check for OpenAPI schemas. Reports errors (unresolvable `$refs`) and warnings (cookie parameters, multipart bodies, missing responses or schemas) without generating any code, and exits non-zero on errors so it can gate CI. +- Generate authentication config from `components.securitySchemes`. HTTP bearer schemes generate a `bearer_token` field, HTTP basic generates `basic_username`/`basic_password`, and header API keys generate an `api_key` field on the generated `Config` — each injecting the right header when set (without overriding explicit headers), with environment variable support for free via pydantic-settings. Unsupported schemes (`oauth2`, `openIdConnect`, query/cookie API keys) get an explanatory comment in `config.py` and a warning from `clientele validate`. +- Per-call config overrides ([#233](https://github.com/phalt/clientele/issues/233)): decorated functions and the direct `request`/`arequest` methods accept a reserved `config` keyword argument that overrides the client-wide configuration for a single call — base URL, headers, logger, and HTTP backend. The client-wide config is never mutated, so concurrent calls against different hosts (multi-tenant APIs) are safe, and each override config caches its own HTTP backend/connection pool. Works in already-generated clients without regeneration. - Major refactoring of the project structure including removing of redundant code paths (mostly left over from the old generator logic). - Reorganisation of the tests and their files to mirror the codebase more. diff --git a/docs/api-authentication.md b/docs/api-authentication.md index d0b6183..46b9475 100644 --- a/docs/api-authentication.md +++ b/docs/api-authentication.md @@ -2,7 +2,52 @@ Clientele API supports multiple authentication methods through the client configuration. -## Bearer Token +## Generated from your OpenAPI schema + +When you generate a client with `clientele start-api`, the generator reads `components.securitySchemes` from the schema and adds typed credential fields to the generated `Config` class. Credentials can then be set directly, via environment variables, or via a `.env` file — and they are automatically sent as headers on every request. + +| Security scheme | Generated fields | Header sent | +| --------------- | ---------------- | ----------- | +| `http` / `bearer` | `bearer_token` | `Authorization: Bearer ` | +| `http` / `basic` | `basic_username`, `basic_password` | `Authorization: Basic ` | +| `apiKey` in `header` | `api_key` | The header named by the scheme (e.g. `X-API-Key`) | + +For example, a schema with a bearer scheme generates: + +```python +# config.py (generated) +class Config(clientele_api.BaseConfig): + base_url: str = "https://api.example.com" + + # HTTP bearer authentication ("bearerAuth" in the OpenAPI spec). + # When set, sends the "Authorization: Bearer " header. + bearer_token: str = "" + ... +``` + +Which you can configure in any of the usual ways: + +```python +# Direct instantiation +config = Config(bearer_token="my-secret-token") +``` + +```sh +# Or from the environment (or a .env file) +export BEARER_TOKEN="my-secret-token" +``` + +Credentials are only injected when they are set, and they never override a header you provide explicitly via the `headers` field. + +Some schemes cannot be generated automatically — `oauth2`, `openIdConnect`, and API keys sent in a query parameter or cookie. The generator leaves a comment in `config.py` for each one, and `clientele validate` reports them as warnings. Configure these manually using the approaches below. + +!!! note + + `config.py` is never overwritten on regeneration, so generated auth fields only appear in fresh clients. If you regenerate an existing client after the API adds auth, copy the fields in manually. + +## Manual configuration + +### Bearer Token Bearer token authentication can be configured by adding an `Authorization` header. diff --git a/docs/api-configuration.md b/docs/api-configuration.md index 63d848d..856cbd5 100644 --- a/docs/api-configuration.md +++ b/docs/api-configuration.md @@ -83,3 +83,38 @@ if __name__ == "__main__": ``` The `configure` method accepts the same parameters as the `APIClient` constructor. + +## Per-call configuration + +Every decorated function (and the direct `request`/`arequest` methods) accepts a reserved `config` keyword argument that overrides the client-wide configuration for that single call. This is the right tool when one API specification is served by many hosts — for example multi-tenant platforms or industry protocols where several parties expose the same endpoints: + +```python +from clientele import api as clientele_api + +client = clientele_api.APIClient(base_url="https://default.example.com") + +@client.get("/users/{user_id}") +def get_user(result: User, user_id: int) -> User: + return result + +# One config per party / tenant +party_a = clientele_api.BaseConfig( + base_url="https://party-a.example.com", + headers={"Authorization": "Bearer "}, +) +party_b = clientele_api.BaseConfig( + base_url="https://party-b.example.com", + headers={"Authorization": "Bearer "}, +) + +user_from_a = get_user(user_id=1, config=party_a) +user_from_b = get_user(user_id=1, config=party_b) +``` + +Unlike `configure()`, a per-call `config` never mutates the client-wide configuration, so concurrent calls with different overrides are safe — there is no shared state to race on. Each override config lazily creates and caches its own HTTP backend, so every tenant keeps its own connection pool. Reuse config objects between calls rather than building a new one per request. + +This works in generated clients too: every generated function accepts `config=` without regeneration. + +!!! note + + Like the reserved `query` and `headers` keywords, `config` is only treated as an override when the decorated function does not declare its own `config` parameter. If it does, the declared parameter wins. diff --git a/docs/openapi-cli.md b/docs/openapi-cli.md index 0908f48..68b1d3e 100644 --- a/docs/openapi-cli.md +++ b/docs/openapi-cli.md @@ -63,3 +63,115 @@ If you prefer an [asyncio](https://docs.python.org/3/library/asyncio.html) clien ```sh clientele start-api -f path/to/file.json -o my_client/ --asyncio ``` + +## Validating a schema + +The `validate` command checks an OpenAPI schema for clientele compatibility before you generate a client: + +```sh +clientele validate -f path/to/file.json +# or +clientele validate -u https://raw.githubusercontent.com/phalt/clientele/main/example_openapi_specs/best.json +``` + +It walks the schema and reports two kinds of findings: + +- **Errors** — constructs that break client generation or produce broken code, such as `$ref` references to schemas or parameters that do not exist in `components`. +- **Warnings** — constructs that degrade to less useful code: + - `$ref` references that are not component references (these become `typing.Any`) + - cookie parameters (not supported; skipped during generation) + - `multipart/form-data` request bodies (generated as plain models; file upload fields are not supported) + - operations with no `responses` (a default 200 response is assumed) + - response content with no schema (becomes `typing.Any`) + +The command exits with status `1` if any errors are found and `0` otherwise (warnings do not fail it), so you can use it to gate CI: + +```sh +clientele validate -f openapi.json && clientele start-api -f openapi.json -o my_client/ --regen +``` + +## Generated code + +### Enums + +OpenAPI `enum` schemas are generated as Python `enum` classes in `schemas.py`. The base class is chosen from the member values: + +| Enum values | Generated base class | Member naming | +| ----------- | -------------------- | ------------- | +| All strings | `str, enum.Enum` | Upper-cased value, e.g. `RED = "red"` | +| All integers | `enum.IntEnum` | `VALUE_`, e.g. `VALUE_1 = 1` | +| Numbers or mixed types | `enum.Enum` | Strings as above, others `VALUE_` | + +For example, this schema: + +```json +{ + "StatusCode": { "type": "integer", "enum": [1, 2, 3] }, + "Colour": { "type": "string", "enum": ["red", "green"] } +} +``` + +Generates: + +```python +class StatusCode(enum.IntEnum): + VALUE_1 = 1 + VALUE_2 = 2 + VALUE_3 = 3 + + +class Colour(str, enum.Enum): + RED = "red" + GREEN = "green" +``` + +Negative numbers use `MINUS_` (`VALUE_MINUS_12 = -12`) and decimal points become underscores (`VALUE_0_5 = 0.5`). If two values sanitise to the same member name, later members get a numeric suffix (`YES = "YES"`, `YES_2 = "yes"`). + +### Map types (additionalProperties) + +Object schemas with a schema-valued `additionalProperties` describe maps — objects whose values all match one schema, such as error maps or translations. These are generated as typed dictionaries. + +A map-valued property generates a typed `dict`: + +```json +{ + "Report": { + "type": "object", + "properties": { + "scores": { + "type": "object", + "additionalProperties": { "type": "integer" } + } + } + } +} +``` + +```python +class Report(pydantic.BaseModel): + scores: typing.Optional[dict[str, int]] = None +``` + +A component schema that is purely a map (no `properties` of its own) generates a `DictResponse` class — a real type that validates its values and can be used in `response_map`, with dict-style access: + +```json +{ + "ErrorMap": { + "type": "object", + "additionalProperties": { "$ref": "#/components/schemas/Error" } + } +} +``` + +```python +class ErrorMap(DictResponse[Error]): + pass +``` + +```python +errors = schemas.ErrorMap.model_validate({"email": {"message": "invalid"}}) +errors["email"].message # "invalid" +len(errors), errors.keys(), errors.items() # dict-style access +``` + +Objects with `additionalProperties: true`, `{}`, or no `additionalProperties` at all keep the untyped `dict[str, typing.Any]` behaviour, and objects that declare their own `properties` are generated as regular models. diff --git a/example_openapi_specs/additional_properties.json b/example_openapi_specs/additional_properties.json new file mode 100644 index 0000000..0cfe6b3 --- /dev/null +++ b/example_openapi_specs/additional_properties.json @@ -0,0 +1,85 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "Additional Properties Test API", + "version": "1.0.0" + }, + "servers": [{ "url": "http://localhost" }], + "paths": { + "/report": { + "get": { + "operationId": "getReport", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/Report" } + } + } + } + } + } + }, + "/scores": { + "get": { + "operationId": "getScores", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/ScoreMap" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Error": { + "type": "object", + "required": ["message"], + "properties": { + "message": { "type": "string" } + } + }, + "ScoreMap": { + "type": "object", + "additionalProperties": { "type": "integer" } + }, + "ErrorMap": { + "type": "object", + "additionalProperties": { "$ref": "#/components/schemas/Error" } + }, + "FreeForm": { + "type": "object", + "additionalProperties": true + }, + "Report": { + "type": "object", + "required": ["scores"], + "properties": { + "scores": { + "type": "object", + "additionalProperties": { "type": "integer" } + }, + "errors_by_field": { + "type": "object", + "additionalProperties": { "$ref": "#/components/schemas/Error" } + }, + "metadata": { "type": "object" }, + "tag_groups": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { "type": "string" } + } + } + } + } + } + } +} diff --git a/example_openapi_specs/enums.json b/example_openapi_specs/enums.json new file mode 100644 index 0000000..beba912 --- /dev/null +++ b/example_openapi_specs/enums.json @@ -0,0 +1,59 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "Enums Test API", + "version": "1.0.0" + }, + "servers": [{ "url": "http://localhost" }], + "paths": { + "/devices": { + "get": { + "operationId": "listDevices", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/Device" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Device": { + "type": "object", + "required": ["status", "colour"], + "properties": { + "status": { "$ref": "#/components/schemas/StatusCode" }, + "colour": { "$ref": "#/components/schemas/Colour" }, + "priority": { "$ref": "#/components/schemas/Priority" }, + "offset": { "$ref": "#/components/schemas/TimezoneOffset" }, + "sample_rate": { "$ref": "#/components/schemas/SampleRate" } + } + }, + "StatusCode": { + "type": "integer", + "enum": [1, 2, 3] + }, + "Colour": { + "type": "string", + "enum": ["red", "green", "blue"] + }, + "Priority": { + "enum": ["low", 1, 2] + }, + "TimezoneOffset": { + "type": "integer", + "enum": [-12, 0, 12] + }, + "SampleRate": { + "type": "number", + "enum": [0.5, 1.0, 2.5] + } + } + } +} diff --git a/tests/api/test_config_override.py b/tests/api/test_config_override.py new file mode 100644 index 0000000..e4b888c --- /dev/null +++ b/tests/api/test_config_override.py @@ -0,0 +1,231 @@ +""" +Tests for per-call config overrides (issue #233). + +Generated clients create one module-global APIClient, so every decorated +function is bound to a single configuration. Users integrating with +multi-tenant APIs (the same OpenAPI spec served by many hosts) could not +direct individual calls at different base URLs or credentials without +mutating global state, which is not thread-safe. + +Decorated functions and the direct request methods now accept a reserved +`config` keyword argument, following the same pattern as the reserved +`query` and `headers` kwargs: + + - the call uses the override config's base_url, headers, logger, and + HTTP backend instead of the client-wide config + - the client-wide config is not touched, so concurrent calls with + different overrides are safe + - an override config without an http_backend gets one lazily, cached + on the config itself so each tenant keeps its own connection pool + - functions that declare their own `config` parameter keep it as a + normal argument (the reserved kwarg is disabled, like `query`) +""" + +import typing + +import pydantic +import pytest + +from clientele.api import APIClient, BaseConfig +from clientele.http.fake_backend import FakeHTTPBackend +from clientele.http.response import Response + + +class Pong(pydantic.BaseModel): + ok: bool + + +def make_backend() -> FakeHTTPBackend: + return FakeHTTPBackend( + default_response=Response( + status_code=200, + content=b'{"ok": true}', + headers={"content-type": "application/json"}, + ) + ) + + +def make_config(tenant: str, backend: FakeHTTPBackend | None = None) -> BaseConfig: + return BaseConfig( + base_url=f"https://{tenant}.example.com", + headers={"X-Tenant": tenant}, + http_backend=backend if backend is not None else make_backend(), + ) + + +@pytest.fixture +def client_setup(): + default_backend = make_backend() + client = APIClient(config=make_config("default", default_backend)) + + @client.get("/ping") + def ping(result: Pong) -> Pong: + return result + + return client, ping, default_backend + + +class TestDecoratedFunctionOverride: + def test_default_config_used_without_override(self, client_setup): + _, ping, default_backend = client_setup + result = ping() + assert result.ok is True + assert len(default_backend.requests) == 1 + assert default_backend.requests[0]["kwargs"]["headers"]["X-Tenant"] == "default" + + def test_override_routes_to_its_own_backend_and_headers(self, client_setup): + _, ping, default_backend = client_setup + tenant_backend = make_backend() + tenant_config = make_config("tenant-b", tenant_backend) + + result = ping(config=tenant_config) + + assert result.ok is True + assert len(default_backend.requests) == 0 + assert len(tenant_backend.requests) == 1 + assert tenant_backend.requests[0]["kwargs"]["headers"]["X-Tenant"] == "tenant-b" + + def test_override_does_not_mutate_the_client_config(self, client_setup): + client, ping, default_backend = client_setup + tenant_config = make_config("tenant-b") + + ping(config=tenant_config) + assert client.config.base_url == "https://default.example.com" + + # The next un-overridden call still uses the client-wide config + ping() + assert len(default_backend.requests) == 1 + assert default_backend.requests[0]["kwargs"]["headers"]["X-Tenant"] == "default" + + def test_two_tenants_interleaved(self, client_setup): + _, ping, _ = client_setup + backend_a = make_backend() + backend_b = make_backend() + config_a = make_config("tenant-a", backend_a) + config_b = make_config("tenant-b", backend_b) + + ping(config=config_a) + ping(config=config_b) + ping(config=config_a) + + assert len(backend_a.requests) == 2 + assert len(backend_b.requests) == 1 + + +class TestAsyncOverride: + @pytest.mark.asyncio + async def test_async_function_accepts_config_override(self): + default_backend = make_backend() + client = APIClient(config=make_config("default", default_backend)) + + @client.get("/ping") + async def ping(result: Pong) -> Pong: + return result + + tenant_backend = make_backend() + tenant_config = make_config("tenant-b", tenant_backend) + + result = await ping(config=tenant_config) # type: ignore + + assert result.ok is True + assert len(default_backend.requests) == 0 + assert len(tenant_backend.requests) == 1 + assert tenant_backend.requests[0]["kwargs"]["headers"]["X-Tenant"] == "tenant-b" + + +class TestDirectRequestOverride: + def test_request_accepts_config_override(self, client_setup): + client, _, default_backend = client_setup + tenant_backend = make_backend() + tenant_config = make_config("tenant-b", tenant_backend) + + result = client.request("GET", "/ping", response_map={200: Pong}, config=tenant_config) + + assert result.ok is True + assert len(default_backend.requests) == 0 + assert len(tenant_backend.requests) == 1 + + @pytest.mark.asyncio + async def test_arequest_accepts_config_override(self, client_setup): + client, _, default_backend = client_setup + tenant_backend = make_backend() + tenant_config = make_config("tenant-b", tenant_backend) + + result = await client.arequest("GET", "/ping", response_map={200: Pong}, config=tenant_config) + + assert result.ok is True + assert len(default_backend.requests) == 0 + assert len(tenant_backend.requests) == 1 + + +class TestStreamingOverride: + def test_sync_stream_uses_override_backend_and_headers(self): + class RecordingStreamBackend(FakeHTTPBackend): + def __init__(self) -> None: + super().__init__() + self.stream_calls: list[dict] = [] + + def handle_sync_stream(self, method, url, inner_type=None, response_parser=None, **kwargs): + self.stream_calls.append({"method": method, "url": url, "kwargs": kwargs}) + return iter([]) + + default_backend = RecordingStreamBackend() + client = APIClient(config=make_config("default", default_backend)) + + @client.get("/events", streaming_response=True) + def stream_events(result: typing.Iterator[str]) -> typing.Iterator[str]: + return result + + override_backend = RecordingStreamBackend() + override_config = make_config("stream-tenant", override_backend) + + list(stream_events(config=override_config)) # type: ignore + + assert len(default_backend.stream_calls) == 0 + assert len(override_backend.stream_calls) == 1 + sent_headers = override_backend.stream_calls[0]["kwargs"]["headers"] + assert sent_headers["X-Tenant"] == "stream-tenant" + + +class TestLazyBackendCreation: + def test_override_without_backend_gets_one_lazily_and_caches_it(self, client_setup, monkeypatch): + _, ping, _ = client_setup + created: list[FakeHTTPBackend] = [] + + def fake_from_config(config): + backend = make_backend() + created.append(backend) + return backend + + from clientele.http import httpx_backend + + monkeypatch.setattr(httpx_backend.HttpxHTTPBackend, "from_config", staticmethod(fake_from_config)) + + tenant_config = BaseConfig(base_url="https://tenant-b.example.com") + assert tenant_config.http_backend is None + + ping(config=tenant_config) + ping(config=tenant_config) + + # The backend was created once and cached on the override config, + # so each tenant config keeps its own connection pool. + assert len(created) == 1 + assert tenant_config.http_backend is created[0] + assert len(created[0].requests) == 2 + + +class TestReservedKwargCollision: + def test_function_with_its_own_config_parameter_keeps_it(self, client_setup): + client, _, default_backend = client_setup + + @client.get("/search") + def search(result: Pong, config: str) -> Pong: + return result + + result = search(config="just-a-query-param") + + # The declared parameter wins: it is treated as a normal argument + # (here, a query parameter) and the client-wide config is used. + assert result.ok is True + assert len(default_backend.requests) == 1 + assert default_backend.requests[0]["kwargs"]["params"] == {"config": "just-a-query-param"} diff --git a/tests/api_clients/async_test_client/schemas.py b/tests/api_clients/async_test_client/schemas.py index 44afb9f..2017920 100644 --- a/tests/api_clients/async_test_client/schemas.py +++ b/tests/api_clients/async_test_client/schemas.py @@ -7,7 +7,7 @@ import pydantic -from clientele.schemas import ListResponse # noqa +from clientele.schemas import DictResponse, ListResponse # noqa class AnotherModel(pydantic.BaseModel): @@ -15,7 +15,7 @@ class AnotherModel(pydantic.BaseModel): class ComplexModelResponse(pydantic.BaseModel): - a_dict_response: dict[str, typing.Any] + a_dict_response: dict[str, str] a_enum: "ExampleEnum" a_list_of_enums: list["ExampleEnum"] a_list_of_numbers: list[int] diff --git a/tests/api_clients/test_client/schemas.py b/tests/api_clients/test_client/schemas.py index 44afb9f..2017920 100644 --- a/tests/api_clients/test_client/schemas.py +++ b/tests/api_clients/test_client/schemas.py @@ -7,7 +7,7 @@ import pydantic -from clientele.schemas import ListResponse # noqa +from clientele.schemas import DictResponse, ListResponse # noqa class AnotherModel(pydantic.BaseModel): @@ -15,7 +15,7 @@ class AnotherModel(pydantic.BaseModel): class ComplexModelResponse(pydantic.BaseModel): - a_dict_response: dict[str, typing.Any] + a_dict_response: dict[str, str] a_enum: "ExampleEnum" a_list_of_enums: list["ExampleEnum"] a_list_of_numbers: list[int] diff --git a/tests/generators/test_additional_properties.py b/tests/generators/test_additional_properties.py new file mode 100644 index 0000000..f738df0 --- /dev/null +++ b/tests/generators/test_additional_properties.py @@ -0,0 +1,230 @@ +""" +Tests for typed additionalProperties support. + +OpenAPI object schemas with a schema-valued `additionalProperties` describe +map/dictionary types (e.g. error maps, translations, metrics keyed by id). +Historically these degraded to `dict[str, typing.Any]`, losing all value +validation. + +Expected behaviour: + - a property typed `{type: object, additionalProperties: }` + generates `dict[str, ]` + - a component schema that is purely a map (additionalProperties, no + properties) generates a `DictResponse` root model class so it is a + real type (usable in response_map) with mapping-style access + - `additionalProperties: true`, `{}`, or absent keeps the existing + `dict[str, typing.Any]` / plain BaseModel behaviour +""" + +import sys +from contextlib import contextmanager + +import pydantic +import pytest + +from clientele.generators.api.generator import APIGenerator +from clientele.generators.cicerone_compat import normalize_openapi_31_schema +from clientele.generators.shared import utils +from tests.generators.integration_utils import get_spec_path, load_spec + + +@contextmanager +def _import_generated_schemas(tmp_path): + # Purge any generated modules cached by earlier tests before importing + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + sys.path.insert(0, str(tmp_path)) + try: + yield + finally: + sys.path.remove(str(tmp_path)) + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + + +@pytest.fixture(scope="module") +def generated(tmp_path_factory): + """Generate the additional_properties client once for the whole module.""" + tmp_path = tmp_path_factory.mktemp("additional_properties_client") + spec = load_spec("additional_properties.json") + generator = APIGenerator( + spec=spec, + output_dir=str(tmp_path), + asyncio=False, + regen=True, + url=None, + file=str(get_spec_path("additional_properties.json")), + ) + generator.generate() + return tmp_path + + +class TestGetTypeAdditionalProperties: + """Unit tests for type resolution of additionalProperties.""" + + @pytest.mark.parametrize( + "type_spec,expected_output", + [ + ( + {"type": "object", "additionalProperties": {"type": "integer"}}, + "dict[str, int]", + ), + ( + {"type": "object", "additionalProperties": {"type": "string"}}, + "dict[str, str]", + ), + ( + {"type": "object", "additionalProperties": {"type": "array", "items": {"type": "string"}}}, + "dict[str, list[str]]", + ), + ( + {"type": "object", "additionalProperties": {"$ref": "#/components/schemas/Error"}}, + 'dict[str, "Error"]', + ), + # Free-form objects keep the untyped behaviour + ({"type": "object", "additionalProperties": True}, "dict[str, typing.Any]"), + ({"type": "object", "additionalProperties": {}}, "dict[str, typing.Any]"), + ({"type": "object"}, "dict[str, typing.Any]"), + ], + ) + def test_get_type(self, type_spec, expected_output): + assert utils.get_type(type_spec) == expected_output + + def test_object_with_declared_properties_stays_untyped(self): + # When an object declares its own properties, the schema describes a + # structured object (generated elsewhere as a class), not a map. + type_spec = { + "type": "object", + "properties": {"name": {"type": "string"}}, + "additionalProperties": {"type": "integer"}, + } + assert utils.get_type(type_spec) == "dict[str, typing.Any]" + + +class TestOpenAPI31Normalization: + """additionalProperties schemas must be normalized recursively.""" + + def test_nullable_type_array_inside_additional_properties(self): + schema = { + "type": "object", + "additionalProperties": {"type": ["string", "null"]}, + } + normalized = normalize_openapi_31_schema(schema) + assert normalized["additionalProperties"] == {"type": "string", "nullable": True} + + def test_boolean_additional_properties_pass_through(self): + schema = {"type": "object", "additionalProperties": True} + normalized = normalize_openapi_31_schema(schema) + assert normalized["additionalProperties"] is True + + +class TestMapValuedModelProperties: + """Model properties that are maps validate their values.""" + + def test_typed_map_of_primitives(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + report = schemas.Report.model_validate({"scores": {"alice": 3, "bob": 5}}) + assert report.scores == {"alice": 3, "bob": 5} + + def test_typed_map_values_are_validated(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + with pytest.raises(pydantic.ValidationError): + schemas.Report.model_validate({"scores": {"alice": "not-an-int"}}) + + def test_typed_map_of_models_hydrates_values(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + report = schemas.Report.model_validate( + { + "scores": {}, + "errors_by_field": {"email": {"message": "invalid"}}, + } + ) + assert isinstance(report.errors_by_field["email"], schemas.Error) + assert report.errors_by_field["email"].message == "invalid" + + def test_typed_map_of_arrays(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + report = schemas.Report.model_validate({"scores": {}, "tag_groups": {"colours": ["red", "blue"]}}) + assert report.tag_groups == {"colours": ["red", "blue"]} + + def test_untyped_object_property_still_accepts_anything(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + report = schemas.Report.model_validate({"scores": {}, "metadata": {"anything": ["goes", 1, None]}}) + assert report.metadata == {"anything": ["goes", 1, None]} + + +class TestMapComponentSchemas: + """Component schemas that are purely maps become DictResponse classes.""" + + def test_map_schema_is_a_real_type(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + # Must be a class so pydantic accepts it in response_map + assert isinstance(schemas.ScoreMap, type) + + def test_map_schema_validates_and_provides_mapping_access(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + scores = schemas.ScoreMap.model_validate({"alice": 3, "bob": 5}) + assert scores["alice"] == 3 + assert len(scores) == 2 + assert set(scores) == {"alice", "bob"} + assert dict(scores.items()) == {"alice": 3, "bob": 5} + + def test_map_schema_rejects_bad_values(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + with pytest.raises(pydantic.ValidationError): + schemas.ScoreMap.model_validate({"alice": "not-an-int"}) + + def test_map_of_models_hydrates_values(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + errors = schemas.ErrorMap.model_validate({"email": {"message": "invalid"}}) + assert isinstance(errors["email"], schemas.Error) + assert errors["email"].message == "invalid" + + def test_free_form_object_schema_keeps_basemodel_shape(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + # additionalProperties: true is not a typed map; the existing + # empty BaseModel behaviour is preserved. + assert issubclass(schemas.FreeForm, pydantic.BaseModel) + + +class TestClientReturnTypes: + """Decorated client functions can use map schemas in response_map.""" + + def test_client_imports_with_map_response(self, generated): + # Importing client.py runs build_request_context which validates + # every response_map entry is a real type - this fails if ScoreMap + # were a plain type alias. The generated code is a package (it uses + # relative imports), so import it as one. + import importlib + + package_name = generated.name + sys.path.insert(0, str(generated.parent)) + try: + importlib.import_module(f"{package_name}.client") + finally: + sys.path.remove(str(generated.parent)) + for mod in list(sys.modules): + if mod == package_name or mod.startswith(f"{package_name}."): + del sys.modules[mod] diff --git a/tests/generators/test_auth_generation.py b/tests/generators/test_auth_generation.py new file mode 100644 index 0000000..e92d4d2 --- /dev/null +++ b/tests/generators/test_auth_generation.py @@ -0,0 +1,252 @@ +""" +Tests for authentication config generation from OpenAPI securitySchemes. + +Most real-world APIs declare their authentication in +components.securitySchemes, but the generator previously ignored that +section entirely - every user had to hand-edit config.py following the +docs. The generator now reads the supported schemes and produces typed +credential fields on the generated Config that inject the right headers. + +Expected behaviour: + - http/bearer -> `bearer_token` field; sets `Authorization: Bearer ` + - http/basic -> `basic_username`/`basic_password` fields; sets + `Authorization: Basic ` + - apiKey in header -> `api_key` field; sets the named header + - oauth2, openIdConnect, apiKey in query/cookie -> not auto-generated; + a comment in config.py points the user at manual configuration + - credentials are only injected when set, and never override headers + the user provided explicitly + - fields are pydantic-settings fields, so environment variables work + for free +""" + +import base64 +import importlib +import sys +from contextlib import contextmanager + +import pytest +from cicerone import parse as cicerone_parse + +from clientele.generators.api.generator import APIGenerator + + +@contextmanager +def _import_generated_config(tmp_path): + # Purge any generated modules cached by earlier tests before importing + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + sys.path.insert(0, str(tmp_path)) + try: + yield importlib.import_module("config") + finally: + sys.path.remove(str(tmp_path)) + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + + +def _generate(tmp_path, security_schemes): + spec_dict = { + "openapi": "3.0.2", + "info": {"title": "Auth Test API", "version": "1.0.0"}, + "servers": [{"url": "http://localhost"}], + "paths": { + "/ping": { + "get": { + "operationId": "ping", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "object", + "title": "PingResponse", + "properties": {"ok": {"type": "boolean"}}, + } + } + }, + } + }, + } + } + }, + "components": {"schemas": {}, "securitySchemes": security_schemes}, + } + spec = cicerone_parse.parse_spec_from_dict(spec_dict) + generator = APIGenerator( + spec=spec, + output_dir=str(tmp_path), + asyncio=False, + regen=True, + url=None, + file="openapi.json", + ) + generator.generate() + return tmp_path + + +class TestBearerScheme: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("bearer_client") + return _generate(tmp_path, {"bearerAuth": {"type": "http", "scheme": "bearer", "bearerFormat": "JWT"}}) + + def test_config_has_bearer_token_field(self, generated): + with _import_generated_config(generated) as config: + assert config.Config().bearer_token == "" + + def test_bearer_token_is_injected_into_headers(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(bearer_token="my-secret") + assert cfg.headers["Authorization"] == "Bearer my-secret" + + def test_empty_token_injects_nothing(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config() + assert "Authorization" not in cfg.headers + + def test_explicit_authorization_header_is_not_overridden(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(bearer_token="my-secret", headers={"Authorization": "custom-scheme abc"}) + assert cfg.headers["Authorization"] == "custom-scheme abc" + + def test_bearer_token_from_environment(self, generated, monkeypatch): + monkeypatch.setenv("BEARER_TOKEN", "from-the-env") + with _import_generated_config(generated) as config: + cfg = config.Config() + assert cfg.headers["Authorization"] == "Bearer from-the-env" + + +class TestBasicScheme: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("basic_client") + return _generate(tmp_path, {"basicAuth": {"type": "http", "scheme": "basic"}}) + + def test_config_has_username_and_password_fields(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config() + assert cfg.basic_username == "" + assert cfg.basic_password == "" + + def test_credentials_injected_as_basic_auth_header(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(basic_username="user", basic_password="pass") + expected = base64.b64encode(b"user:pass").decode() + assert cfg.headers["Authorization"] == f"Basic {expected}" + + def test_no_credentials_injects_nothing(self, generated): + with _import_generated_config(generated) as config: + assert "Authorization" not in config.Config().headers + + +class TestApiKeyScheme: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("apikey_client") + return _generate(tmp_path, {"apiKeyAuth": {"type": "apiKey", "in": "header", "name": "X-API-Key"}}) + + def test_config_has_api_key_field(self, generated): + with _import_generated_config(generated) as config: + assert config.Config().api_key == "" + + def test_api_key_injected_into_named_header(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(api_key="key-123") + assert cfg.headers["X-API-Key"] == "key-123" + + def test_explicit_header_is_not_overridden(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(api_key="key-123", headers={"X-API-Key": "explicit"}) + assert cfg.headers["X-API-Key"] == "explicit" + + +class TestMultipleSchemes: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("multi_client") + return _generate( + tmp_path, + { + "bearerAuth": {"type": "http", "scheme": "bearer"}, + "apiKeyAuth": {"type": "apiKey", "in": "header", "name": "X-API-Key"}, + }, + ) + + def test_both_credentials_can_be_configured_together(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config(bearer_token="tok", api_key="key") + assert cfg.headers["Authorization"] == "Bearer tok" + assert cfg.headers["X-API-Key"] == "key" + + +class TestUnsupportedSchemes: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("unsupported_client") + return _generate( + tmp_path, + { + "oauth": { + "type": "oauth2", + "flows": {"clientCredentials": {"tokenUrl": "https://example.com/token", "scopes": {}}}, + }, + "queryKey": {"type": "apiKey", "in": "query", "name": "api_key"}, + }, + ) + + def test_generation_succeeds(self, generated): + assert (generated / "config.py").exists() + + def test_config_mentions_unsupported_schemes(self, generated): + config_content = (generated / "config.py").read_text() + assert "oauth" in config_content + assert "queryKey" in config_content + + def test_config_has_no_credential_fields(self, generated): + with _import_generated_config(generated) as config: + cfg = config.Config() + assert not hasattr(cfg, "bearer_token") + assert not hasattr(cfg, "api_key") + + +class TestEndToEndRequestHeaders: + """The generated Config must put credentials on the actual request.""" + + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("e2e_client") + return _generate(tmp_path, {"bearerAuth": {"type": "http", "scheme": "bearer"}}) + + def test_bearer_token_is_sent_on_requests(self, generated): + from clientele.api import APIClient + from clientele.http.fake_backend import FakeHTTPBackend + + with _import_generated_config(generated) as config: + fake_backend = FakeHTTPBackend() + cfg = config.Config(bearer_token="my-secret", http_backend=fake_backend) + client = APIClient(config=cfg) + client.request("GET", "/ping", response_map={200: dict}) + + assert len(fake_backend.requests) == 1 + sent_headers = fake_backend.requests[0]["kwargs"]["headers"] + assert sent_headers["Authorization"] == "Bearer my-secret" + + +class TestNoSecuritySchemes: + @pytest.fixture(scope="class") + def generated(self, tmp_path_factory): + tmp_path = tmp_path_factory.mktemp("plain_client") + return _generate(tmp_path, {}) + + def test_config_has_no_auth_fields_or_hooks(self, generated): + config_content = (generated / "config.py").read_text() + assert "bearer_token" not in config_content + assert "model_post_init" not in config_content + + def test_config_instantiates(self, generated): + with _import_generated_config(generated) as config: + assert config.Config().base_url == "http://localhost" diff --git a/tests/generators/test_enum_generation.py b/tests/generators/test_enum_generation.py new file mode 100644 index 0000000..47a86a1 --- /dev/null +++ b/tests/generators/test_enum_generation.py @@ -0,0 +1,197 @@ +""" +Tests for enum schema generation. + +OpenAPI enums are not restricted to strings: integer enums (status codes, +versions), number enums, and mixed-type enums are all legal. Historically the +generator assumed string values and crashed with an AttributeError when it +called .upper() on a non-string member. + +Expected behaviour: + - string enums -> class Colour(str, enum.Enum) with UPPER_CASE members + - integer enums -> class StatusCode(enum.IntEnum) with VALUE_ members + - number enums -> class SampleRate(enum.Enum) with VALUE_ members + - mixed-type enums -> class Priority(enum.Enum), strings named as usual, + non-strings named VALUE_ + - negative numbers -> "-" becomes MINUS_ (e.g. VALUE_MINUS_12) + - generated enums must round-trip through pydantic validation +""" + +import enum +import sys +from contextlib import contextmanager + +import pydantic +import pytest + +from clientele.generators.api.generator import APIGenerator +from tests.generators.integration_utils import get_spec_path, load_spec + + +@contextmanager +def _import_generated_schemas(tmp_path): + # Purge any generated modules cached by earlier tests before importing + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + sys.path.insert(0, str(tmp_path)) + try: + yield + finally: + sys.path.remove(str(tmp_path)) + for mod in list(sys.modules): + if mod in ("schemas", "client", "config"): + del sys.modules[mod] + + +@pytest.fixture(scope="module") +def generated(tmp_path_factory): + """Generate the enums client once for the whole module.""" + tmp_path = tmp_path_factory.mktemp("enums_client") + spec = load_spec("enums.json") + generator = APIGenerator( + spec=spec, + output_dir=str(tmp_path), + asyncio=False, + regen=True, + url=None, + file=str(get_spec_path("enums.json")), + ) + generator.generate() + return tmp_path + + +class TestStringEnums: + """Regression: string enums keep their existing shape.""" + + def test_string_enum_subclasses_str_and_enum(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert issubclass(schemas.Colour, str) + assert issubclass(schemas.Colour, enum.Enum) + + def test_string_enum_members(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert schemas.Colour.RED.value == "red" + assert schemas.Colour.GREEN.value == "green" + assert schemas.Colour.BLUE.value == "blue" + + +class TestIntegerEnums: + """Integer enums must generate an enum.IntEnum, not crash.""" + + def test_generation_does_not_crash(self, generated): + # The fixture generation itself is the core regression: it used to + # raise AttributeError: 'int' object has no attribute 'upper'. + assert (generated / "schemas.py").exists() + + def test_integer_enum_is_int_enum(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert issubclass(schemas.StatusCode, enum.IntEnum) + + def test_integer_enum_members(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert schemas.StatusCode.VALUE_1.value == 1 + assert schemas.StatusCode.VALUE_2.value == 2 + assert schemas.StatusCode.VALUE_3.value == 3 + + def test_negative_integer_members(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert schemas.TimezoneOffset.VALUE_MINUS_12.value == -12 + assert schemas.TimezoneOffset.VALUE_0.value == 0 + assert schemas.TimezoneOffset.VALUE_12.value == 12 + + +class TestNumberEnums: + """Number (float) enums generate a plain enum.Enum with VALUE_ members.""" + + def test_number_enum_members(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert issubclass(schemas.SampleRate, enum.Enum) + assert schemas.SampleRate.VALUE_0_5.value == 0.5 + assert schemas.SampleRate.VALUE_1_0.value == 1.0 + assert schemas.SampleRate.VALUE_2_5.value == 2.5 + + +class TestMixedEnums: + """Mixed string/number enums generate a plain enum.Enum.""" + + def test_mixed_enum_members(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + assert issubclass(schemas.Priority, enum.Enum) + # A mixed enum cannot subclass str or int + assert not issubclass(schemas.Priority, str) + assert not issubclass(schemas.Priority, int) + assert schemas.Priority.LOW.value == "low" + assert schemas.Priority.VALUE_1.value == 1 + assert schemas.Priority.VALUE_2.value == 2 + + +class TestPydanticIntegration: + """Generated enums must validate from raw JSON values via pydantic.""" + + def test_model_validates_enum_values(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + device = schemas.Device.model_validate( + { + "status": 2, + "colour": "red", + "priority": 1, + "offset": -12, + "sample_rate": 0.5, + } + ) + assert device.status == schemas.StatusCode.VALUE_2 + assert device.colour == schemas.Colour.RED + assert device.priority == schemas.Priority.VALUE_1 + assert device.offset == schemas.TimezoneOffset.VALUE_MINUS_12 + assert device.sample_rate == schemas.SampleRate.VALUE_0_5 + + def test_model_rejects_values_outside_enum(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + with pytest.raises(pydantic.ValidationError): + schemas.Device.model_validate({"status": 99, "colour": "red"}) + + def test_model_serialises_enum_values_back_to_raw(self, generated): + with _import_generated_schemas(generated): + import schemas # type: ignore + + device = schemas.Device.model_validate({"status": 1, "colour": "blue"}) + dumped = device.model_dump(mode="json", exclude_none=True) + assert dumped["status"] == 1 + assert dumped["colour"] == "blue" + + +class TestMemberNameCollisions: + """Values that sanitise to the same member name must be deduplicated.""" + + def test_colliding_names_get_suffixes(self, tmp_path): + from clientele.generators.api import writer + from clientele.generators.shared.schemas import SchemasGenerator + + spec = load_spec("enums.json") + generator = SchemasGenerator(spec=spec, output_dir=str(tmp_path), writer=writer) + # "YES" and "yes" both upper-case to YES + generator.make_schema_class("Answer", {"type": "string", "enum": ["YES", "yes", "no"]}) + writer.flush_buffers() + content = (tmp_path / "schemas.py").read_text() + + assert 'YES = "YES"' in content + assert 'YES_2 = "yes"' in content + assert 'NO = "no"' in content diff --git a/tests/generators/test_validation.py b/tests/generators/test_validation.py new file mode 100644 index 0000000..d1e55f2 --- /dev/null +++ b/tests/generators/test_validation.py @@ -0,0 +1,295 @@ +""" +Tests for the `clientele validate` command. + +Before this command existed, incompatible specs failed half-way through +generation (missing $refs crash with AttributeError or silently produce +broken output) and unsupported constructs (cookie parameters, multipart +bodies) were silently dropped. `clientele validate` is a pre-flight check: +it walks the spec, reports errors (things that break generation) and +warnings (things that degrade), and exits non-zero on errors so it can +gate CI. +""" + +import json + +import pytest +from cicerone import parse as cicerone_parse +from click.testing import CliRunner + +from clientele import cli +from clientele.generators.validation import SpecValidator + + +@pytest.fixture +def runner(): + return CliRunner() + + +def _base_spec(**overrides): + spec = { + "openapi": "3.0.2", + "info": {"title": "Validation Test API", "version": "1.0.0"}, + "paths": { + "/things": { + "get": { + "operationId": "listThings", + "responses": { + "200": { + "description": "OK", + "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Thing"}}}, + } + }, + } + } + }, + "components": { + "schemas": { + "Thing": { + "type": "object", + "properties": {"name": {"type": "string"}}, + } + } + }, + } + spec.update(overrides) + return spec + + +def _validate(spec_dict): + spec = cicerone_parse.parse_spec_from_dict(spec_dict) + return SpecValidator(spec=spec).validate() + + +def _write_spec(tmp_path, spec_dict): + spec_path = tmp_path / "openapi.json" + spec_path.write_text(json.dumps(spec_dict)) + return spec_path + + +class TestSpecValidatorCleanSpec: + def test_clean_spec_has_no_findings(self): + assert _validate(_base_spec()) == [] + + +class TestSpecValidatorMissingRefs: + def test_missing_schema_ref_in_response_is_an_error(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["responses"]["200"]["content"]["application/json"]["schema"] = { + "$ref": "#/components/schemas/DoesNotExist" + } + findings = _validate(spec) + errors = [f for f in findings if f.severity == "error"] + assert len(errors) == 1 + assert "DoesNotExist" in errors[0].message + assert "GET /things" in errors[0].location + + def test_missing_schema_ref_in_component_property_is_an_error(self): + spec = _base_spec() + spec["components"]["schemas"]["Thing"]["properties"]["owner"] = {"$ref": "#/components/schemas/Missing"} + findings = _validate(spec) + errors = [f for f in findings if f.severity == "error"] + assert len(errors) == 1 + assert "Missing" in errors[0].message + assert "Thing" in errors[0].location + + def test_missing_schema_ref_in_array_items_is_an_error(self): + spec = _base_spec() + spec["components"]["schemas"]["Thing"]["properties"]["tags"] = { + "type": "array", + "items": {"$ref": "#/components/schemas/Tag"}, + } + findings = _validate(spec) + errors = [f for f in findings if f.severity == "error"] + assert len(errors) == 1 + assert "Tag" in errors[0].message + + def test_missing_parameter_ref_is_an_error(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["parameters"] = [{"$ref": "#/components/parameters/NoSuchParam"}] + findings = _validate(spec) + errors = [f for f in findings if f.severity == "error"] + assert len(errors) == 1 + assert "NoSuchParam" in errors[0].message + + def test_resolvable_refs_are_not_reported(self): + spec = _base_spec() + spec["components"]["schemas"]["Owner"] = { + "type": "object", + "properties": {"name": {"type": "string"}}, + } + spec["components"]["schemas"]["Thing"]["properties"]["owner"] = {"$ref": "#/components/schemas/Owner"} + assert _validate(spec) == [] + + +class TestSpecValidatorUnsupportedRefs: + def test_non_component_ref_is_a_warning(self): + spec = _base_spec() + spec["components"]["schemas"]["Thing"]["properties"]["odd"] = {"$ref": "#/paths/~1things/get/responses/200"} + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "typing.Any" in warnings[0].message + + +class TestSpecValidatorParameters: + def test_cookie_parameter_is_a_warning(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["parameters"] = [ + { + "name": "session_id", + "in": "cookie", + "required": False, + "schema": {"type": "string"}, + } + ] + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "session_id" in warnings[0].message + assert "cookie" in warnings[0].message.lower() + assert "GET /things" in warnings[0].location + + def test_query_path_and_header_parameters_are_supported(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["parameters"] = [ + {"name": "page", "in": "query", "schema": {"type": "integer"}}, + {"name": "X-Trace", "in": "header", "schema": {"type": "string"}}, + ] + assert _validate(spec) == [] + + +class TestSpecValidatorRequestBodies: + def test_multipart_body_is_a_warning(self): + spec = _base_spec() + spec["paths"]["/things"]["post"] = { + "operationId": "uploadThing", + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "type": "object", + "properties": {"file": {"type": "string", "format": "binary"}}, + } + } + } + }, + "responses": {"204": {"description": "No Content"}}, + } + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "multipart/form-data" in warnings[0].message + assert "POST /things" in warnings[0].location + + +class TestSpecValidatorResponses: + def test_operation_without_responses_is_a_warning(self): + spec = _base_spec() + del spec["paths"]["/things"]["get"]["responses"] + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "responses" in warnings[0].message.lower() + + def test_response_content_without_schema_is_a_warning(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["responses"]["200"]["content"]["application/json"] = {} + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "schema" in warnings[0].message.lower() + + def test_no_content_responses_are_fine(self): + spec = _base_spec() + spec["paths"]["/things"]["get"]["responses"] = {"204": {"description": "No Content"}} + assert _validate(spec) == [] + + +class TestSpecValidatorSecuritySchemes: + def test_supported_schemes_have_no_findings(self): + spec = _base_spec() + spec["components"]["securitySchemes"] = { + "bearerAuth": {"type": "http", "scheme": "bearer"}, + "basicAuth": {"type": "http", "scheme": "basic"}, + "apiKeyAuth": {"type": "apiKey", "in": "header", "name": "X-API-Key"}, + } + assert _validate(spec) == [] + + def test_oauth2_scheme_is_a_warning(self): + spec = _base_spec() + spec["components"]["securitySchemes"] = { + "oauth": { + "type": "oauth2", + "flows": {"clientCredentials": {"tokenUrl": "https://example.com/token", "scopes": {}}}, + } + } + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "oauth" in warnings[0].message + assert "components.securitySchemes" in warnings[0].location + + def test_api_key_in_query_is_a_warning(self): + spec = _base_spec() + spec["components"]["securitySchemes"] = { + "queryKey": {"type": "apiKey", "in": "query", "name": "api_key"}, + } + findings = _validate(spec) + warnings = [f for f in findings if f.severity == "warning"] + assert len(warnings) == 1 + assert "queryKey" in warnings[0].message + + +class TestValidateCommand: + def test_requires_url_or_file(self, runner): + result = runner.invoke(cli.cli_group, ["validate"]) + assert result.exit_code != 0 + + def test_clean_spec_exits_zero(self, runner, tmp_path): + spec_path = _write_spec(tmp_path, _base_spec()) + result = runner.invoke(cli.cli_group, ["validate", "--file", str(spec_path)]) + assert result.exit_code == 0 + assert "no issues" in result.output.lower() + + def test_spec_with_errors_exits_one(self, runner, tmp_path): + spec = _base_spec() + spec["paths"]["/things"]["get"]["responses"]["200"]["content"]["application/json"]["schema"] = { + "$ref": "#/components/schemas/DoesNotExist" + } + spec_path = _write_spec(tmp_path, spec) + result = runner.invoke(cli.cli_group, ["validate", "--file", str(spec_path)]) + assert result.exit_code == 1 + assert "DoesNotExist" in result.output + + def test_spec_with_only_warnings_exits_zero(self, runner, tmp_path): + spec = _base_spec() + spec["paths"]["/things"]["get"]["parameters"] = [{"name": "sid", "in": "cookie", "schema": {"type": "string"}}] + spec_path = _write_spec(tmp_path, spec) + result = runner.invoke(cli.cli_group, ["validate", "--file", str(spec_path)]) + assert result.exit_code == 0 + assert "warning" in result.output.lower() + + def test_unparseable_file_exits_one_with_message(self, runner, tmp_path): + bad = tmp_path / "openapi.json" + bad.write_text("{ this is not json") + result = runner.invoke(cli.cli_group, ["validate", "--file", str(bad)]) + assert result.exit_code == 1 + assert "could not" in result.output.lower() or "error" in result.output.lower() + + def test_swagger_2_spec_is_auto_converted_and_validates(self, runner, tmp_path): + # cicerone auto-converts Swagger 2.0 specs to OpenAPI 3.0 at parse + # time (the same path start-api uses), so validate accepts them. + spec = { + "swagger": "2.0", + "info": {"title": "Old API", "version": "1.0.0"}, + "paths": {}, + } + spec_path = _write_spec(tmp_path, spec) + result = runner.invoke(cli.cli_group, ["validate", "--file", str(spec_path)]) + assert result.exit_code == 0 + + def test_validates_real_example_spec(self, runner): + from tests.generators.integration_utils import get_spec_path + + result = runner.invoke(cli.cli_group, ["validate", "--file", str(get_spec_path("best.json"))]) + assert result.exit_code == 0