diff --git a/DESIGN.md b/DESIGN.md index 91218f91..62430f7d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -78,18 +78,27 @@ The final call-flow is: 1. Snapshot metadata and scan-lifetime warnings; assemble operations and hoist definitions into the complete component map. Compilation events accumulate locally rather than updating the registry during the walk. -2. Convert inline request, response (including `itemSchema`), and parameter schemas: - 3.1 and 3.2 use `_convert_operation_schemas_to_3_1` for legacy nullable/example - syntax; 3.0 uses `_convert_operation_schemas_to_3_0` for nullable unions, refs, - and other downgrade forms, recording losses in encounter order. Inline nullable - refs resolve against the **original complete component map**, before replacement. +2. Convert inline request, response (including `itemSchema` and headers), and + parameter schemas: 3.1 and 3.2 use `_convert_operation_schemas_to_3_1` for legacy + nullable/example syntax; 3.0 uses `_convert_operation_schemas_to_3_0` for nullable + unions, refs, and other downgrade forms, recording losses in encounter order. + Inline nullable refs resolve against the **original complete component map**, + before replacement. 3. Convert named components separately with `_convert_schemas_to_3_1` or `_convert_schemas_to_3_0`: inline schemas and components are distinct locations, not duplicate passes. The 3.0 component conversion also resolves refs against the original map, replacing it only after all converted components are returned. -4. For 3.0, retain `_check_schemas_3_0_compatible` after conversion. It rejects or - warns about residual constructs such as non-null type arrays that conversion - does not normalize. Faithfully converted nullable schemas must not trigger it. +4. For 3.0, retain the residual compatibility audit after conversion: + `_check_operation_schemas_3_0_compatible` for inline operation schemas and + `_check_schemas_3_0_compatible` for components. Each rejects (strict) or reports + a `type` array or null-`anyOf` that conversion could not normalize, naming the + construct and where it was found. Conversion rewrites non-null `type` arrays to + `anyOf` except a multi-type array beside a `discriminator`, which a combinator + would activate, and faithfully converted schemas must not trigger the audit. + Inline schemas include response header schemas. A null-only schema converts to + `enum: [null]` with `nullable`, which keeps it exact beneath `not`. Non-strict + findings join the schema-loss findings below rather than being logged on their own. + Keywords OpenAPI 3.0 does not define are not audited. 5. Normalize and validate the document before custom-method relocation and QUERY removal. Relocate custom methods for 3.2; record unsupported method/QUERY drops before 3.2. Append schema-loss findings, log findings, and enforce strict mode. diff --git a/docs/configuration.md b/docs/configuration.md index 0c3cb6ac..2a5acdff 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -227,6 +227,29 @@ attaching `nullable` directly to one branch of a nullable multi-type union, so the result is not just structurally valid but actually accepts a `null` value under real OpenAPI 3.0 tooling. +A `type` array without `null` (for example `type: [string, integer]`, which +validation producers use for error locations) is rewritten to an `anyOf` of +single-type schemas, and sibling keywords stay on the schema. An `array` branch +gains an empty `items`, because OpenAPI 3.0 requires one; a nullable union +carries `nullable` on its first branch, so `null` is still accepted; and a +`$ref` beside the array moves into `allOf`, because OpenAPI 3.0 ignores the +siblings of a reference. A multi-type `type` array that conversion cannot +rewrite faithfully, such as one beside a `discriminator`, stays in the document. +Like an `anyOf` with a `null` branch that conversion could not normalize, it is +reported as a structured `version-downgrade-drop` +warning (visible via `generate_openapi_report()` or the CLI's +`--fail-on-warnings`) and raises `OpenAPISpecConfigError` under `strict=True`. +These rewrites and the check cover every inline schema, including the schemas +of response headers. Other JSON Schema 2020-12 keywords that OpenAPI 3.0 does +not define, for example `if`/`then`/`else` or `contains`, are not checked. + +A schema that accepts only `null` (`type: "null"`, or a union of nothing but null +members) becomes `enum: [null]` with `nullable: true`. A type-less `nullable` +schema would accept every value in OpenAPI 3.0, and a `not` over it would then +reject every value. An `enum` or `const` beside it is intersected with that: a +list that includes `null` narrows to it, and one that excludes `null` leaves a +schema that accepts nothing, as the original does. + A heterogeneous `tuple[...]` (differing item types per position) has no positional representation in OpenAPI 3.0 at all: it is approximated as `items.anyOf`, which accepts the right set of values but can no longer diff --git a/src/azure_functions_openapi/_schema_version_conversion.py b/src/azure_functions_openapi/_schema_version_conversion.py index 05da5bd6..63fdc007 100644 --- a/src/azure_functions_openapi/_schema_version_conversion.py +++ b/src/azure_functions_openapi/_schema_version_conversion.py @@ -10,7 +10,7 @@ from __future__ import annotations -from collections.abc import Callable +from collections.abc import Callable, Iterable, Iterator from typing import Any from azure_functions_openapi.exceptions import OpenAPISpecConfigError @@ -228,6 +228,9 @@ def _collapse_nullable_combinator( one branch in the union actually accepts null, plus the same sibling ``nullable: true`` for discoverability (ignored by validation once a branch already accepts null, but documents the field's shape). + * null members alone -> the combinator is dropped and the schema stays + null-only (an ``enum`` of ``null`` plus ``nullable: true``), because + ``nullable`` without a ``type`` accepts every value. Nodes without a null-bearing ``anyOf``/``oneOf`` are returned unchanged. """ @@ -292,6 +295,8 @@ def _collapse_nullable_combinator( result.setdefault(k, v) else: result[key] = non_null + elif not non_null: + _keep_null_only(result) elif len(non_null) > 1: patched_members: list[Any] = [] patched_one = False @@ -337,6 +342,65 @@ def _tighter_bound(key: str, first: Any, second: Any) -> Any: return first +def _expand_type_union(schema: dict[str, Any], types: list[str]) -> None: + """Replace the multi-type ``type`` array of *schema* with an ``anyOf`` of single-type schemas. + + Sibling keywords stay where they are: type-specific ones (``minLength``, ``items``, + ...) only constrain instances of their own type, so the result accepts the same + values. Three details keep each branch valid and faithful under OpenAPI 3.0: + + * ``nullable`` only takes effect beside the ``type`` it exempts, so a nullable + *schema* hands it to the first branch, the idiom the nullable-union collapse + already uses. + * Every ``array`` schema needs ``items``, so an ``array`` branch gets an empty one; + the sibling ``items`` of *schema* still applies as before. + * An ``anyOf`` that *schema* already carries is kept by adding the new union to its + ``allOf``. + """ + del schema["type"] + nullable = schema.get("nullable") is True + union: list[dict[str, Any]] = [] + for index, name in enumerate(types): + branch: dict[str, Any] = {"type": name} + if name == "array": + branch["items"] = {} + if nullable and index == 0: + branch["nullable"] = True + union.append(branch) + if "anyOf" in schema: + all_of = schema.get("allOf") + schema["allOf"] = [*(all_of if isinstance(all_of, list) else []), {"anyOf": union}] + else: + schema["anyOf"] = union + + +def _wrap_ref_siblings(schema: dict[str, Any]) -> None: + """Move the ``$ref`` of *schema* into ``allOf``. + + OpenAPI 3.0 ignores every sibling of a Reference Object, so keywords that conversion + just wrote beside a ``$ref`` would silently stop constraining anything. + """ + all_of = schema.get("allOf") + schema["allOf"] = [{"$ref": schema.pop("$ref")}, *(all_of if isinstance(all_of, list) else [])] + + +def _keep_null_only(schema: dict[str, Any]) -> None: + """Keep *schema*, which just lost a null-only ``type``, accepting nothing but ``null``. + + ``nullable`` without a ``type`` accepts every value, so the schema gets an ``enum`` of + ``null`` instead. An ``enum`` it already carries (a ``const`` was turned into one) is + intersected with that: a list that includes ``null`` narrows to it, and one that + excludes it can be met by no instance, so the null-only predicate joins ``allOf`` and + the two together still accept nothing. + """ + enum = schema.get("enum") + if isinstance(enum, list) and None not in enum: + all_of = schema.get("allOf") + schema["allOf"] = [*(all_of if isinstance(all_of, list) else []), {"enum": [None]}] + else: + schema["enum"] = [None] + + def _convert_schema_to_3_0( schema: dict[str, Any], schemas: dict[str, Any] | None = None, @@ -404,6 +468,7 @@ def _convert_schema_to_3_0( # requires ``type`` to be a single string, so a multi-type union is expressed # as ``anyOf`` of single-type schemas rather than a (3.0-invalid) type array. type_val = result.get("type") + type_array_rewritten = False if isinstance(type_val, list) and "null" in type_val: non_null_types = [t for t in type_val if t != "null"] result["nullable"] = True @@ -411,9 +476,25 @@ def _convert_schema_to_3_0( result["type"] = non_null_types[0] elif not non_null_types: del result["type"] + _keep_null_only(result) else: - del result["type"] - result["anyOf"] = [{"type": t} for t in non_null_types] + _expand_type_union(result, non_null_types) + type_array_rewritten = True + elif isinstance(type_val, list) and type_val and all(isinstance(t, str) for t in type_val): + # A type array without ``null`` takes the same shape. Type-specific keywords + # (minLength, items, properties, ...) only constrain instances of their own + # type, so leaving them beside the ``anyOf`` accepts exactly the same values. + # A sibling ``discriminator`` would start constraining instances once a + # combinator appears, so a multi-type array there is left for the audit to + # report; a single type adds no combinator and converts regardless. + if len(type_val) == 1: + result["type"] = type_val[0] + type_array_rewritten = True + elif "discriminator" not in result: + _expand_type_union(result, type_val) + type_array_rewritten = True + if type_array_rewritten and "$ref" in result: + _wrap_ref_siblings(result) # ``enum: [None, ...]`` -> keep ``null`` IN the enum (``nullable: true`` # alone does not exempt a sibling ``enum`` from its own closed-list check, @@ -423,8 +504,11 @@ def _convert_schema_to_3_0( result["nullable"] = True if result.get("type") == "null": + # Without ``type``, ``nullable`` alone accepts every value, so a null-only schema + # must stay null-only; a ``not`` over it would otherwise reject everything. del result["type"] result["nullable"] = True + _keep_null_only(result) result = _collapse_nullable_combinator(result, schemas, resolving, lossy) @@ -479,6 +563,8 @@ def _convert_schema_to_3_0( result["additionalProperties"] = _convert_schema_to_3_0( result["additionalProperties"], schemas, resolving, lossy ) + if "not" in result and isinstance(result["not"], dict): + result["not"] = _convert_schema_to_3_0(result["not"], schemas, resolving, lossy) if "$defs" in result and isinstance(result["$defs"], dict): result["$defs"] = { k: _convert_schema_to_3_0(v, schemas, resolving, lossy) @@ -505,43 +591,72 @@ def _convert_schemas_to_3_0(schemas: dict[str, Any]) -> tuple[dict[str, Any], li return converted, lossy_messages -def _has_3_1_only_constructs(schema: dict[str, Any]) -> bool: - """Check if a schema contains constructs incompatible with OpenAPI 3.0. +_NESTED_SCHEMA_KEYS = ( + "properties", + "items", + "allOf", + "anyOf", + "oneOf", + "not", + "additionalProperties", + "$defs", +) + + +def _find_3_1_only_construct(schema: dict[str, Any]) -> str | None: + """Name the first construct in *schema* that OpenAPI 3.0 cannot express, if any. Detects: - - ``anyOf`` containing ``{"type": "null"}`` (Pydantic v2 nullable pattern) - ``type`` as a list (JSON Schema 2020-12 / OpenAPI 3.1 syntax) + - ``anyOf`` containing ``{"type": "null"}`` (Pydantic v2 nullable pattern) """ if not isinstance(schema, dict): - return False + return None - # type as list is 3.1-only if isinstance(schema.get("type"), list): - return True + return 'a "type" array' - # anyOf containing {type: "null"} is the Pydantic v2 nullable pattern any_of = schema.get("anyOf") if isinstance(any_of, list): for item in any_of: if isinstance(item, dict) and item.get("type") == "null": - return True + return 'an "anyOf" with a null type' - # Recurse into nested structures - for key in ("properties", "items", "allOf", "anyOf", "oneOf", "additionalProperties", "$defs"): + for key in _NESTED_SCHEMA_KEYS: val = schema.get(key) + children: Iterable[Any] if isinstance(val, dict): - if key == "properties": - for prop_schema in val.values(): - if isinstance(prop_schema, dict) and _has_3_1_only_constructs(prop_schema): - return True - elif _has_3_1_only_constructs(val): - return True + children = val.values() if key in ("properties", "$defs") else [val] elif isinstance(val, list): - for item in val: - if isinstance(item, dict) and _has_3_1_only_constructs(item): - return True + children = val + else: + continue + for child in children: + if (found := _find_3_1_only_construct(child)) is not None: + return found + + return None + + +def _has_3_1_only_constructs(schema: dict[str, Any]) -> bool: + """Check if a schema contains constructs incompatible with OpenAPI 3.0.""" + return _find_3_1_only_construct(schema) is not None + + +def _incompatibility_message(subject: str, construct: str) -> str: + return ( + f"{subject} contains OpenAPI 3.1-only constructs ({construct}) incompatible with " + "OpenAPI 3.0. Use openapi_version='3.1.0' (default) or provide a manual " + "3.0-compatible schema." + ) + - return False +def _report_incompatibilities(findings: list[str], strict: bool) -> list[str]: + if strict and findings: + raise OpenAPISpecConfigError( + "OpenAPI 3.0 compatibility error:\n" + "\n".join(f" - {f}" for f in findings) + ) + return findings def _check_schemas_3_0_compatible(schemas: dict[str, Any], strict: bool) -> list[str]: @@ -550,81 +665,115 @@ def _check_schemas_3_0_compatible(schemas: dict[str, Any], strict: bool) -> list Returns a list of warning messages for incompatible schemas. In strict mode, raises OpenAPISpecConfigError if any are found. """ - warnings: list[str] = [] - for name, schema in schemas.items(): - if _has_3_1_only_constructs(schema): - warnings.append( - f"Schema '{name}' contains OpenAPI 3.1-only constructs " - f"(e.g., anyOf with null type) incompatible with OpenAPI 3.0. " - f"Use openapi_version='3.1.0' (default) or provide a manual " - f"3.0-compatible schema instead of a Pydantic model." - ) + findings = [ + _incompatibility_message(f"Schema '{name}'", construct) + for name, schema in schemas.items() + if (construct := _find_3_1_only_construct(schema)) is not None + ] + return _report_incompatibilities(findings, strict) - if strict and warnings: - raise OpenAPISpecConfigError( - "OpenAPI 3.0 compatibility error:\n" + "\n".join(f" - {w}" for w in warnings) - ) - return warnings +def _operation_schema_slots( + operation: dict[str, Any], +) -> Iterator[tuple[str, dict[str, Any], str]]: + """Yield ``(location, container, key)`` for every inline schema of one operation. + + Covers: + - requestBody.content.*.schema + - responses.*.content.*.schema and .itemSchema + - responses.*.headers.*.schema and .content.*.schema + - parameters[].schema (and querystring parameters' content.*.schema) + """ + request_body = operation.get("requestBody") + if isinstance(request_body, dict): + content = request_body.get("content") + if isinstance(content, dict): + for media, media_obj in content.items(): + if isinstance(media_obj, dict) and "schema" in media_obj: + yield f"requestBody.content.{media}.schema", media_obj, "schema" + + responses = operation.get("responses") + if isinstance(responses, dict): + for status, resp in responses.items(): + if not isinstance(resp, dict): + continue + content = resp.get("content") + if isinstance(content, dict): + for media, media_obj in content.items(): + if not isinstance(media_obj, dict): + continue + for schema_key in _MEDIA_SCHEMA_KEYS: + if schema_key in media_obj: + yield ( + f"responses.{status}.content.{media}.{schema_key}", + media_obj, + schema_key, + ) + headers = resp.get("headers") + if isinstance(headers, dict): + for name, header in headers.items(): + if not isinstance(header, dict): + continue + if "schema" in header: + yield f"responses.{status}.headers.{name}.schema", header, "schema" + header_content = header.get("content") + if isinstance(header_content, dict): + for media, media_obj in header_content.items(): + if isinstance(media_obj, dict) and "schema" in media_obj: + yield ( + f"responses.{status}.headers.{name}.content.{media}.schema", + media_obj, + "schema", + ) + + for index, param in enumerate(operation.get("parameters", [])): + if not isinstance(param, dict): + continue + if "schema" in param: + yield f"parameters[{index}].schema", param, "schema" + # querystring parameters (OpenAPI 3.2) carry a content map instead of a + # bare schema. + param_content = param.get("content") + if isinstance(param_content, dict): + for media, media_obj in param_content.items(): + if isinstance(media_obj, dict) and "schema" in media_obj: + yield f"parameters[{index}].content.{media}.schema", media_obj, "schema" def _convert_operation_schemas( paths: dict[str, Any], converter: Callable[[dict[str, Any]], dict[str, Any]], ) -> dict[str, Any]: - """Apply *converter* to every inline schema in operations, in place. - - Converts schemas in: - - requestBody.content.*.schema - - responses.*.content.*.schema and .itemSchema - - parameters[].schema (and querystring parameters' content.*.schema) - """ + """Apply *converter* to every inline schema in operations, in place.""" for _path, methods in paths.items(): for _method, operation in methods.items(): if not isinstance(operation, dict): continue - - # requestBody - rb = operation.get("requestBody") - if isinstance(rb, dict): - content = rb.get("content") - if isinstance(content, dict): - for _media, media_obj in content.items(): - if isinstance(media_obj, dict) and "schema" in media_obj: - media_obj["schema"] = converter(media_obj["schema"]) - - # responses - responses = operation.get("responses") - if isinstance(responses, dict): - for _status, resp in responses.items(): - if not isinstance(resp, dict): - continue - content = resp.get("content") - if isinstance(content, dict): - for _media, media_obj in content.items(): - if not isinstance(media_obj, dict): - continue - for _schema_key in _MEDIA_SCHEMA_KEYS: - if _schema_key in media_obj: - media_obj[_schema_key] = converter(media_obj[_schema_key]) - - # parameters - for param in operation.get("parameters", []): - if not isinstance(param, dict): - continue - if "schema" in param: - param["schema"] = converter(param["schema"]) - # querystring parameters (OpenAPI 3.2) carry a content map - # instead of a bare schema. - param_content = param.get("content") - if isinstance(param_content, dict): - for _media, media_obj in param_content.items(): - if isinstance(media_obj, dict) and "schema" in media_obj: - media_obj["schema"] = converter(media_obj["schema"]) + for _location, container, key in _operation_schema_slots(operation): + container[key] = converter(container[key]) return paths +def _check_operation_schemas_3_0_compatible(paths: dict[str, Any], strict: bool) -> list[str]: + """Check inline operation schemas for OpenAPI 3.1-only constructs when targeting 3.0. + + Counterpart of :func:`_check_schemas_3_0_compatible` for schemas that sit directly + in an operation rather than in ``components.schemas``. Each message names the + operation and the schema location. In strict mode, raises OpenAPISpecConfigError + if any are found. + """ + findings = [ + _incompatibility_message(f"{method.upper()} {path} {location}", construct) + for path, methods in paths.items() + for method, operation in methods.items() + if isinstance(operation, dict) + for location, container, key in _operation_schema_slots(operation) + if (construct := _find_3_1_only_construct(container[key])) is not None + ] + return _report_incompatibilities(findings, strict) + + def _convert_operation_schemas_to_3_1(paths: dict[str, Any]) -> dict[str, Any]: """Apply 3.1 schema conversion to inline schemas in operations.""" return _convert_operation_schemas(paths, _convert_schema_to_3_1) diff --git a/src/azure_functions_openapi/spec.py b/src/azure_functions_openapi/spec.py index 5f2eaec3..667e38f1 100644 --- a/src/azure_functions_openapi/spec.py +++ b/src/azure_functions_openapi/spec.py @@ -20,6 +20,9 @@ from azure_functions_openapi._schema_version_conversion import ( _STRUCTURAL_MERGE_KEYS as _STRUCTURAL_MERGE_KEYS, ) +from azure_functions_openapi._schema_version_conversion import ( + _check_operation_schemas_3_0_compatible, +) from azure_functions_openapi._schema_version_conversion import ( _check_schemas_3_0_compatible as _check_schemas_3_0_compatible, ) @@ -862,6 +865,7 @@ def _generate_openapi_result( lossy_conversion_warnings.extend( _convert_operation_schemas_to_3_0(paths, components.get("schemas")) ) + lossy_conversion_warnings.extend(_check_operation_schemas_3_0_compatible(paths, strict)) # Top-level and info metadata passthrough (#494). Each field is emitted # only when supplied; contact/license nest under ``info`` while @@ -922,9 +926,9 @@ def _generate_openapi_result( components["schemas"] ) lossy_conversion_warnings.extend(component_lossy_warnings) - compat_warnings = _check_schemas_3_0_compatible(components["schemas"], strict) - for w in compat_warnings: - logger.warning("OpenAPI 3.0 compatibility: %s", w) + lossy_conversion_warnings.extend( + _check_schemas_3_0_compatible(components["schemas"], strict) + ) if components.get("schemas") or components.get("securitySchemes"): spec["components"] = components @@ -950,7 +954,8 @@ def _generate_openapi_result( downgrade_drops = _restructure_additional_operations(spec, openapi_version) downgrade_drops.extend(_drop_unsupported_query(spec, openapi_version)) # Lossy OpenAPI 3.0 schema approximations (#782, e.g. a heterogeneous - # tuple down-converted to items.anyOf) share the same silent-loss + # tuple down-converted to items.anyOf) and residual 3.1-only constructs + # the conversion could not normalize (#858) share the same silent-loss # shape as a custom-method/query drop, so they are reported through # the identical downgrade-drop channel and strict/non-strict gate. downgrade_drops.extend(lossy_conversion_warnings) diff --git a/tests/test_bridge_endpoint.py b/tests/test_bridge_endpoint.py index 776729a9..86be3382 100644 --- a/tests/test_bridge_endpoint.py +++ b/tests/test_bridge_endpoint.py @@ -2,6 +2,8 @@ from typing import Any, cast +from openapi_schema_validator import OAS30Validator +from openapi_spec_validator import validate from pydantic import BaseModel import pytest @@ -969,3 +971,48 @@ def test_endpoint_v1_additive_422_response_is_preserved_in_spec() -> None: # The 422 response schema is carried through, not dropped or flattened away. schema = responses["422"]["content"]["application/json"]["schema"] assert schema["properties"]["detail"]["type"] == "string" + + +@pytest.mark.parametrize("hoist_flat_schemas", [False, True]) +def test_endpoint_v1_422_envelope_with_type_array_is_valid_openapi_3_0( + hoist_flat_schemas: bool, +) -> None: + """The ``422`` envelope a validation producer publishes lists ``loc`` items as + ``type: [string, integer]``, a JSON Schema 2020-12 construct OpenAPI 3.0 cannot + express. The 3.0 document must still validate and keep accepting both (#858).""" + envelope: dict[str, Any] = { + "type": "object", + "properties": { + "detail": { + "type": "array", + "items": { + "type": "object", + "properties": { + "loc": {"type": "array", "items": {"type": ["string", "integer"]}}, + "msg": {"type": "string"}, + }, + "required": ["loc", "msg"], + }, + } + }, + "required": ["detail"], + } + payload: dict[str, Any] = { + "version": 1, + "request_body": {"type": "object", "properties": {"name": {"type": "string"}}}, + "responses": {"422": {"schema": envelope}}, + } + scan_endpoint_metadata(_make_app({"endpoint": payload})) + + spec = generate_openapi_spec( + openapi_version="3.0.0", strict=True, hoist_flat_schemas=hoist_flat_schemas + ) + + validate(spec) + schema = spec["paths"]["/api/users"]["post"]["responses"]["422"]["content"]["application/json"][ + "schema" + ] + validator = OAS30Validator({**schema, "components": spec.get("components", {})}) + assert validator.is_valid({"detail": [{"loc": ["body", 0], "msg": "invalid"}]}) + assert not validator.is_valid({"detail": [{"loc": [None], "msg": "invalid"}]}) + assert collect_spec_warnings(spec) == () diff --git a/tests/test_openapi_3_0_nullable.py b/tests/test_openapi_3_0_nullable.py index 785853c7..0ef42651 100644 --- a/tests/test_openapi_3_0_nullable.py +++ b/tests/test_openapi_3_0_nullable.py @@ -88,7 +88,8 @@ def test_type_array_multi_non_null(self) -> None: assert result["nullable"] is True assert "type" not in result - assert result["anyOf"] == [{"type": "string"}, {"type": "integer"}] + # ``nullable`` only takes effect beside a ``type``, so the first branch carries it. + assert result["anyOf"] == [{"type": "string", "nullable": True}, {"type": "integer"}] assert not _has_null_type(result) assert not _has_type_array(result) @@ -97,6 +98,8 @@ def test_type_array_only_null(self) -> None: assert result["nullable"] is True assert "type" not in result + # A type-less ``nullable`` schema accepts every value, so it stays null-only. + assert result["enum"] == [None] def test_enum_with_null(self) -> None: # Regression (#782): ``None`` must stay IN the enum -- a sibling diff --git a/tests/test_spec_conversion_characterization.py b/tests/test_spec_conversion_characterization.py index 5afb6ce0..ec1113ad 100644 --- a/tests/test_spec_conversion_characterization.py +++ b/tests/test_spec_conversion_characterization.py @@ -5,9 +5,16 @@ import copy from openapi_schema_validator import OAS30Validator, OAS31Validator -from pydantic import JsonValue +from openapi_spec_validator import validate +from pydantic import BaseModel, Field, JsonValue import pytest +from azure_functions_openapi._schema_version_conversion import ( + _find_3_1_only_construct, + _operation_schema_slots, +) +from azure_functions_openapi._warnings import WarningCode +from azure_functions_openapi.decorator import register_openapi_metadata from azure_functions_openapi.exceptions import OpenAPISpecConfigError from azure_functions_openapi.registry import OpenAPIRegistry from azure_functions_openapi.spec import collect_spec_warnings, generate_openapi_spec @@ -70,8 +77,276 @@ [{"choice": "fixed", "score": 2, "labels": [None, "x"]}], [{"choice": "other"}, {"score": 1}, {"labels": [42]}], ), + ( + {"type": ["string"]}, + {"type": "string"}, + {"type": ["string"]}, + ["yes"], + [42, None], + ), + ( + {"type": ["string", "integer"]}, + {"anyOf": [{"type": "string"}, {"type": "integer"}]}, + {"type": ["string", "integer"]}, + ["yes", 42], + [[1], 1.5, None], + ), + ( + {"type": ["string", "integer"], "minLength": 2, "minimum": 0}, + { + "anyOf": [{"type": "string"}, {"type": "integer"}], + "minLength": 2, + "minimum": 0, + }, + {"type": ["string", "integer"], "minLength": 2, "minimum": 0}, + ["ab", 0, 5], + ["a", -1, 1.5], + ), + ( + { + "type": ["string", "integer"], + "anyOf": [ + {"type": "string", "minLength": 2}, + {"type": "integer", "minimum": 5}, + ], + }, + { + "anyOf": [ + {"type": "string", "minLength": 2}, + {"type": "integer", "minimum": 5}, + ], + "allOf": [{"anyOf": [{"type": "string"}, {"type": "integer"}]}], + }, + { + "type": ["string", "integer"], + "anyOf": [ + {"type": "string", "minLength": 2}, + {"type": "integer", "minimum": 5}, + ], + }, + ["ab", 6], + ["a", 4, 1.5, None], + ), + ( + { + "type": "object", + "properties": {"loc": {"type": "array", "items": {"type": ["string", "integer"]}}}, + }, + { + "type": "object", + "properties": { + "loc": { + "type": "array", + "items": {"anyOf": [{"type": "string"}, {"type": "integer"}]}, + } + }, + }, + { + "type": "object", + "properties": {"loc": {"type": "array", "items": {"type": ["string", "integer"]}}}, + }, + [{"loc": ["body", 0]}], + [{"loc": [None]}, {"loc": [1.5]}], + ), + ( + {"not": {"type": ["string", "integer"]}}, + {"not": {"anyOf": [{"type": "string"}, {"type": "integer"}]}}, + {"not": {"type": ["string", "integer"]}}, + [[1], None, 1.5], + ["x", 5], + ), + ( + {"type": ["string", "integer", "null"]}, + { + "nullable": True, + "anyOf": [{"type": "string", "nullable": True}, {"type": "integer"}], + }, + {"type": ["string", "integer", "null"]}, + [None, "yes", 42], + [1.5, [1]], + ), + ( + {"anyOf": [{"type": ["string", "integer"]}, {"type": "object"}, {"type": "null"}]}, + { + "nullable": True, + "anyOf": [ + { + "nullable": True, + "anyOf": [{"type": "string", "nullable": True}, {"type": "integer"}], + }, + {"type": "object"}, + ], + }, + {"anyOf": [{"type": ["string", "integer"]}, {"type": "object"}, {"type": "null"}]}, + [None, "yes", 42, {}], + [1.5, [1]], + ), + ( + {"type": ["array", "string"], "items": {"type": "integer"}}, + { + "anyOf": [{"type": "array", "items": {}}, {"type": "string"}], + "items": {"type": "integer"}, + }, + {"type": ["array", "string"], "items": {"type": "integer"}}, + [[1], "yes"], + [["a"], 1, None], + ), + ( + { + "type": ["object"], + "discriminator": {"propertyName": "kind"}, + "properties": {"kind": {"type": "string"}}, + "required": ["kind"], + }, + { + "type": "object", + "discriminator": {"propertyName": "kind"}, + "properties": {"kind": {"type": "string"}}, + "required": ["kind"], + }, + { + "type": ["object"], + "discriminator": {"propertyName": "kind"}, + "properties": {"kind": {"type": "string"}}, + "required": ["kind"], + }, + [{"kind": "a"}], + [{}, "a"], + ), + ( + {"type": "null"}, + {"enum": [None], "nullable": True}, + {"type": "null"}, + [None], + ["x", 1, {}], + ), + ( + {"type": "null", "minLength": 1, "x-label": "a"}, + {"enum": [None], "nullable": True, "minLength": 1, "x-label": "a"}, + {"type": "null", "minLength": 1, "x-label": "a"}, + [None], + ["x", 1, {}], + ), + ( + {"const": None, "type": "null"}, + {"enum": [None], "nullable": True}, + {"const": None, "type": "null"}, + [None], + ["x", 1, {}], + ), + ( + {"anyOf": [{"type": "null"}]}, + {"enum": [None], "nullable": True}, + {"anyOf": [{"type": "null"}]}, + [None], + ["x", 1, {}], + ), + ( + {"properties": {"a": {"type": "null"}}}, + {"properties": {"a": {"enum": [None], "nullable": True}}}, + {"properties": {"a": {"type": "null"}}}, + [{}, {"a": None}], + [{"a": 1}], + ), + ( + {"not": {"type": "null"}}, + {"not": {"enum": [None], "nullable": True}}, + {"not": {"type": "null"}}, + ["x", 1, {}], + [None], + ), + ( + {"not": {"anyOf": [{"type": ["null"]}, {"type": "string"}]}}, + {"not": {"anyOf": [{"enum": [None], "nullable": True}, {"type": "string"}]}}, + {"not": {"anyOf": [{"type": ["null"]}, {"type": "string"}]}}, + [1, {}, [1]], + [None, "x"], + ), + ( + {"type": "null", "enum": [None, "x"]}, + {"enum": [None], "nullable": True}, + {"type": "null", "enum": [None, "x"]}, + [None], + ["x", 1, {}], + ), + ( + {"type": ["null"], "enum": [None, "x"]}, + {"enum": [None], "nullable": True}, + {"type": ["null"], "enum": [None, "x"]}, + [None], + ["x", 1, {}], + ), + ( + {"anyOf": [{"type": "null"}], "enum": [None, "x"]}, + {"enum": [None], "nullable": True}, + {"anyOf": [{"type": "null"}], "enum": [None, "x"]}, + [None], + ["x", 1, {}], + ), + ( + {"properties": {"a": {"type": "null", "enum": [None, "x"]}}}, + {"properties": {"a": {"enum": [None], "nullable": True}}}, + {"properties": {"a": {"type": "null", "enum": [None, "x"]}}}, + [{}, {"a": None}], + [{"a": "x"}], + ), + ( + {"type": "null", "enum": ["x"]}, + {"enum": ["x"], "nullable": True, "allOf": [{"enum": [None], "nullable": True}]}, + {"type": "null", "enum": ["x"]}, + [], + [None, "x", 1, {}], + ), + ( + {"type": "null", "const": "x"}, + {"enum": ["x"], "nullable": True, "allOf": [{"enum": [None], "nullable": True}]}, + {"type": "null", "const": "x"}, + [], + [None, "x", 1], + ), + ( + {"not": {"type": "null", "const": "x"}}, + { + "not": { + "enum": ["x"], + "nullable": True, + "allOf": [{"enum": [None], "nullable": True}], + } + }, + {"not": {"type": "null", "const": "x"}}, + [None, "x", 1, {}], + [], + ), + ], + ids=[ + "legacy-nullable", + "nullable-union", + "nested-keywords", + "single-type-array", + "type-array", + "type-array-with-sibling-keywords", + "type-array-beside-any-of", + "nested-type-array", + "negated-type-array", + "nullable-type-array", + "nullable-union-of-type-array", + "type-array-with-array-member", + "single-type-array-beside-discriminator", + "null-only", + "null-only-with-sibling-keywords", + "null-literal", + "only-null-member", + "null-only-in-a-property", + "negated-null-only", + "negated-null-list-member", + "null-only-beside-a-broader-enum", + "null-list-beside-a-broader-enum", + "only-null-member-beside-a-broader-enum", + "null-only-property-beside-a-broader-enum", + "null-only-beside-an-enum-without-null", + "null-only-beside-a-const", + "negated-null-only-beside-a-const", ], - ids=["legacy-nullable", "nullable-union", "nested-keywords"], ) def test_schema_conversion_at_every_operation_position( version: str, @@ -255,3 +530,351 @@ def test_mutual_nullable_reference_cycle(version: str, strict: bool) -> None: assert validator.is_valid({"next": None}) # Current 3.0 fallback loses nullability at a deeper recursive edge. assert validator.is_valid({"next": {"next": {"next": None}}}) == (version != "3.0.0") + + +@pytest.mark.parametrize( + ("schema", "construct"), + [ + ({"type": ["string", "integer"]}, 'a "type" array'), + ({"anyOf": [{"type": "string"}, {"type": "null"}]}, 'an "anyOf" with a null type'), + ( + {"type": "object", "properties": {"a": {"items": {"type": ["string"]}}}}, + 'a "type" array', + ), + ({"allOf": ["junk", {"type": ["string"]}]}, 'a "type" array'), + ({"$defs": {"X": {"type": ["string"]}}}, 'a "type" array'), + ({"not": {"type": ["string", "integer"]}}, 'a "type" array'), + ({"type": "string", "nullable": True}, None), + ], +) +def test_audit_names_the_first_construct_openapi_3_0_cannot_express( + schema: dict[str, JsonValue], construct: str | None +) -> None: + # Given / When / Then: the finding names what is wrong, not just that something is. + assert _find_3_1_only_construct(schema) == construct + + +@pytest.mark.parametrize( + "odd", + [ + {"type": []}, + {"type": ["string", "integer"], "discriminator": {"propertyName": "kind"}}, + ], + ids=["empty-type-list", "type-array-beside-discriminator"], +) +@pytest.mark.parametrize("hoisted", [False, True], ids=["inline", "component"]) +@pytest.mark.parametrize("strict", [False, True]) +def test_unconvertible_construct_is_reported_through_the_downgrade_channel( + odd: dict[str, JsonValue], hoisted: bool, strict: bool +) -> None: + # Given: a ``type`` array no OpenAPI 3.0 conversion can normalize faithfully: an + # empty list, or one beside a ``discriminator`` (a combinator would activate it). + body = {"$ref": "#/$defs/Odd", "$defs": {"Odd": odd}} if hoisted else odd + registry = OpenAPIRegistry() + registry.set("odd", {"route": "odd", "method": "post", "request_body": body}) + subject = ( + "Schema 'Odd'" if hoisted else "POST /api/odd requestBody.content.application/json.schema" + ) + + # When / Then: strict mode refuses the document and names the construct and location. + if strict: + with pytest.raises(OpenAPISpecConfigError, match="3.1-only constructs") as caught: + generate_openapi_spec(registry=registry, openapi_version="3.0.0", strict=True) + assert subject in str(caught.value) + assert '(a "type" array)' in str(caught.value) + return + + # When: non-strict generation still produces a document. + spec = generate_openapi_spec( + registry=registry, openapi_version="3.0.0", hoist_flat_schemas=hoisted + ) + + # Then: the finding is a structured warning that --fail-on-warnings can gate on. + (warning,) = collect_spec_warnings(spec, registry) + assert warning.code is WarningCode.VERSION_DOWNGRADE_DROP + assert warning.message.startswith(f"{subject} contains OpenAPI 3.1-only constructs") + assert '(a "type" array)' in warning.message + + +@pytest.mark.parametrize( + ("original", "expected"), + [ + ( + {"$ref": "#/$defs/Num", "type": ["string", "integer"]}, + { + "allOf": [{"$ref": "#/components/schemas/Num"}], + "anyOf": [{"type": "string"}, {"type": "integer"}], + }, + ), + ( + {"$ref": "#/$defs/Num", "type": ["integer", "null"]}, + {"allOf": [{"$ref": "#/components/schemas/Num"}], "type": "integer", "nullable": True}, + ), + ], + ids=["multi-type", "nullable-single-type"], +) +def test_type_array_beside_a_reference_moves_the_reference_into_all_of( + original: dict[str, JsonValue], expected: dict[str, JsonValue] +) -> None: + # Given: OpenAPI 3.0 ignores every sibling of a ``$ref``, so a converted ``type`` + # must not be left beside one where it would silently stop constraining anything. + body = {"$ref": "#/$defs/Holder", "$defs": {"Holder": original, "Num": {"type": "number"}}} + registry = OpenAPIRegistry() + registry.set("holder", {"route": "holder", "method": "post", "request_body": body}) + + # When + spec = generate_openapi_spec(registry=registry, openapi_version="3.0.0", strict=True) + + # Then: the reference and the converted type constrain the value together. + assert spec["components"]["schemas"]["Holder"] == expected + validate(spec) + validator = OAS30Validator( + {"$ref": "#/components/schemas/Holder", "components": spec["components"]} + ) + assert validator.is_valid(1) + assert not any(validator.is_valid(instance) for instance in (1.5, "x", None)) + assert collect_spec_warnings(spec, registry) == () + + +@pytest.mark.parametrize("hoisted", [False, True], ids=["inline", "component"]) +@pytest.mark.parametrize( + "negated", + [ + {"type": "null"}, + {"type": "null", "minLength": 1}, + {"const": None, "type": "null"}, + {"anyOf": [{"type": ["null"]}, {"type": "string"}]}, + {"type": "string", "properties": {"a": {"type": "null"}}}, + {"allOf": [{"type": "string"}, {"type": "null"}]}, + ], + ids=["null-only", "constrained", "null-literal", "null-list-member", "inapplicable", "all-of"], +) +def test_negation_of_a_null_only_schema_accepts_what_the_original_accepts( + negated: dict[str, JsonValue], hoisted: bool +) -> None: + # Given: schemas an OpenAPI 3.0 reader could only get right if a null-only schema stays + # null-only; as a type-less ``nullable`` schema it would accept every value and the + # negation would reject every value. + odd = {"not": negated} + body = {"$ref": "#/$defs/Odd", "$defs": {"Odd": odd}} if hoisted else odd + registry = OpenAPIRegistry() + registry.set("odd", {"route": "odd", "method": "post", "request_body": body}) + + # When + spec = generate_openapi_spec( + registry=registry, openapi_version="3.0.0", hoist_flat_schemas=hoisted, strict=True + ) + + # Then: nothing is reported, and both versions accept exactly the same instances. + assert collect_spec_warnings(spec, registry) == () + validate(spec) + if hoisted: + root = {"$ref": "#/components/schemas/Odd", "components": spec["components"]} + else: + root = spec["paths"]["/api/odd"]["post"]["requestBody"]["content"]["application/json"][ + "schema" + ] + original = OAS31Validator(odd) + converted = OAS30Validator(root) + probes: list[JsonValue] = [None, "", "x", 0, 1.5, [], [None], {}, {"a": None}, {"a": 1}] + assert [converted.is_valid(probe) for probe in probes] == [ + original.is_valid(probe) for probe in probes + ] + + +@pytest.mark.parametrize("strict", [False, True]) +def test_reference_to_a_null_only_component_stays_null_only(strict: bool) -> None: + # Given: the null-only schema is a component, reached through a ``$ref`` under ``not``. + body = { + "$ref": "#/$defs/Odd", + "$defs": {"Odd": {"not": {"$ref": "#/$defs/Nothing"}}, "Nothing": {"type": "null"}}, + } + registry = OpenAPIRegistry() + registry.set("odd", {"route": "odd", "method": "post", "request_body": body}) + + # When + spec = generate_openapi_spec(registry=registry, openapi_version="3.0.0", strict=strict) + + # Then: the component keeps accepting only ``null``, so the negation accepts the rest. + assert spec["components"]["schemas"]["Nothing"] == {"enum": [None], "nullable": True} + validate(spec) + validator = OAS30Validator( + {"$ref": "#/components/schemas/Odd", "components": spec["components"]} + ) + rest: list[JsonValue] = ["x", 1, {}] + assert all(validator.is_valid(instance) for instance in rest) + assert not validator.is_valid(None) + assert collect_spec_warnings(spec, registry) == () + + +def test_none_typed_field_with_a_broader_enum_accepts_only_null() -> None: + # Given: Pydantic keeps both ``type: "null"`` and the enum that an extra supplies. + class Payload(BaseModel): + value: None = Field(default=None, json_schema_extra={"enum": [None, "x"]}) + + registry = OpenAPIRegistry() + register_openapi_metadata("/payload", "POST", request_model=Payload, registry=registry) + + # When + spec = generate_openapi_spec(registry=registry, openapi_version="3.0.0", strict=True) + + # Then: the 3.0 property accepts exactly what the original property does. + original = OAS31Validator(Payload.model_json_schema()) + converted = OAS30Validator( + {"$ref": "#/components/schemas/Payload", "components": spec["components"]} + ) + probes: list[JsonValue] = [{}, {"value": None}, {"value": "x"}] + assert [converted.is_valid(probe) for probe in probes] == [True, True, False] + assert [original.is_valid(probe) for probe in probes] == [True, True, False] + assert collect_spec_warnings(spec, registry) == () + + +@pytest.mark.parametrize( + ("version", "header", "expected"), + [ + ( + "3.0.0", + {"type": ["string", "integer"]}, + {"anyOf": [{"type": "string"}, {"type": "integer"}]}, + ), + ( + "3.1.0", + {"type": "string", "nullable": True, "example": "abc"}, + {"type": ["string", "null"], "examples": ["abc"]}, + ), + ( + "3.2.0", + {"type": "string", "nullable": True, "example": "abc"}, + {"type": ["string", "null"], "examples": ["abc"]}, + ), + ], +) +@pytest.mark.parametrize("through_content", [False, True], ids=["schema", "content"]) +def test_response_header_schemas_are_converted_for_every_version( + version: str, + header: dict[str, JsonValue], + expected: dict[str, JsonValue], + through_content: bool, +) -> None: + # Given: a raw response whose header declares its schema directly or through ``content``. + declaration = ( + {"content": {"application/json": {"schema": header}}} + if through_content + else {"schema": header} + ) + registry = OpenAPIRegistry() + registry.set( + "headers", + { + "route": "headers", + "method": "get", + "response": {200: {"description": "ok", "headers": {"X-Rate": declaration}}}, + }, + ) + + # When + spec = generate_openapi_spec(registry=registry, openapi_version=version, strict=True) + + # Then: the header schema is converted like every other inline schema. + converted = spec["paths"]["/api/headers"]["get"]["responses"]["200"]["headers"]["X-Rate"] + if through_content: + assert converted == {"content": {"application/json": {"schema": expected}}} + else: + assert converted == {"schema": expected} + validate(spec) + assert collect_spec_warnings(spec, registry) == () + + +@pytest.mark.parametrize("strict", [False, True]) +def test_unconvertible_response_header_schema_is_audited(strict: bool) -> None: + # Given: a header schema conversion cannot rewrite (a multi-type array beside a discriminator). + schema = {"type": ["string", "integer"], "discriminator": {"propertyName": "kind"}} + registry = OpenAPIRegistry() + registry.set( + "headers", + { + "route": "headers", + "method": "get", + "response": {200: {"description": "ok", "headers": {"X-Rate": {"schema": schema}}}}, + }, + ) + subject = "GET /api/headers responses.200.headers.X-Rate.schema" + + # When / Then: strict mode names the header; otherwise it is a structured warning. + if strict: + with pytest.raises(OpenAPISpecConfigError, match="3.1-only constructs") as caught: + generate_openapi_spec(registry=registry, openapi_version="3.0.0", strict=True) + assert subject in str(caught.value) + return + spec = generate_openapi_spec(registry=registry, openapi_version="3.0.0") + (warning,) = collect_spec_warnings(spec, registry) + assert warning.code is WarningCode.VERSION_DOWNGRADE_DROP + assert warning.message.startswith(f"{subject} contains OpenAPI 3.1-only constructs") + + +def test_has_3_1_only_constructs_stays_importable_from_spec() -> None: + # Given / When: the compatibility re-export that predates the construct-naming audit. + from azure_functions_openapi.spec import _has_3_1_only_constructs + + # Then: it still answers the yes/no question it always did. + assert _has_3_1_only_constructs({"type": ["string", "integer"]}) is True + assert _has_3_1_only_constructs({"type": "string"}) is False + + +def test_operation_schema_slots_label_each_schema_and_skip_malformed_entries() -> None: + # Given: an operation mixing well-formed schema positions with malformed raw entries. + operation = { + "requestBody": {"content": {"application/json": {"schema": {"type": "object"}}}}, + "responses": { + "200": "junk", + "201": {"content": {"application/json": "junk"}}, + "202": { + "content": {"application/json": {"schema": {"type": "string"}}}, + "headers": { + "X-Junk": "junk", + "X-Plain": {"schema": {"type": "integer"}}, + "X-Wrapped": {"content": {"text/plain": {"schema": {"type": "string"}}}}, + "X-Empty": {"content": {"text/plain": "junk"}}, + }, + }, + "203": {"headers": "junk"}, + }, + "parameters": [ + "junk", + {"name": "q", "in": "query", "schema": {"type": "string"}}, + { + "name": "qs", + "in": "querystring", + "content": {"application/x-www-form-urlencoded": {"schema": {"type": "object"}}}, + }, + ], + } + + # When + locations = [location for location, _container, _key in _operation_schema_slots(operation)] + + # Then: every location names where the schema sits, and malformed entries are skipped. + assert locations == [ + "requestBody.content.application/json.schema", + "responses.202.content.application/json.schema", + "responses.202.headers.X-Plain.schema", + "responses.202.headers.X-Wrapped.content.text/plain.schema", + "parameters[1].schema", + "parameters[2].content.application/x-www-form-urlencoded.schema", + ] + + +@pytest.mark.parametrize( + "operation", + [ + {"requestBody": {"content": "junk"}, "responses": "junk"}, + {"requestBody": {"content": {"application/json": "junk"}}, "parameters": []}, + {"requestBody": "junk", "responses": {"200": {"content": "junk"}}}, + {"parameters": [{"name": "qs", "in": "querystring", "content": {"x": "junk"}}]}, + ], +) +def test_operation_schema_slots_yield_nothing_for_unusable_containers( + operation: dict[str, JsonValue], +) -> None: + # Given / When / Then + assert list(_operation_schema_slots(operation)) == []