diff --git a/src/utils/openapi_resolver.py b/src/utils/openapi_resolver.py index 2a396c5..41869f9 100644 --- a/src/utils/openapi_resolver.py +++ b/src/utils/openapi_resolver.py @@ -69,7 +69,7 @@ def resolve_schema(obj: Any, visited: dict[str, int] | None = None) -> Any: result = {} for key, value in obj.items(): result[key] = resolve_schema(value, visited) - return result + return _merge_all_of(result) elif isinstance(obj, list): # Recursively process all items in the list @@ -85,6 +85,70 @@ def resolve_schema(obj: Any, visited: dict[str, int] | None = None) -> Any: return spec +def _merge_all_of(schema: dict[str, Any]) -> dict[str, Any]: + """Fold a plain allOf of object schemas into the schema that holds it. + + springdoc emits each subtype as ``{"required": [...], "allOf": [Parent, {own + fields}]}``, so ``required`` names properties that only exist inside the allOf + branches. Clients that validate tool schemas strictly (Gemini/Vertex AI) reject + that. Merging the branches gives the same constraints with ``properties`` and + ``required`` on one object. Anything that is not a plain object branch, or a + property the branches define differently, leaves the allOf as it is. + """ + branches = schema.get("allOf") + if not isinstance(branches, list) or not branches: + return schema + mergeable = { + "type", + "properties", + "required", + "description", + "title", + "discriminator", + } + if not all( + isinstance(branch, dict) + and set(branch) <= mergeable + and branch.get("type", "object") == "object" + for branch in branches + ): + return schema + if schema.get("type", "object") != "object": + return schema + + properties: dict[str, Any] = {} + required: list[str] = [] + for part in [*branches, schema]: + for name, prop in part.get("properties", {}).items(): + if name in properties: + merged = _merge_property(properties[name], prop) + if merged is None: + return schema + properties[name] = merged + else: + properties[name] = prop + for name in part.get("required", []): + if name not in required: + required.append(name) + + merged_schema = {key: value for key, value in schema.items() if key != "allOf"} + merged_schema["type"] = "object" + merged_schema["properties"] = properties + if required: + merged_schema["required"] = required + return merged_schema + + +def _merge_property(first: Any, second: Any) -> Any: + """Combine two definitions of one property, or None if they disagree.""" + if not (isinstance(first, dict) and isinstance(second, dict)): + return first if first == second else None + for key in set(first) & set(second): + if key not in ("description", "title") and first[key] != second[key]: + return None + return {**first, **second} + + def _without_subtypes(schema: dict[str, Any]) -> dict[str, Any]: return { key: value diff --git a/tests/test_openapi_resolver.py b/tests/test_openapi_resolver.py index c8bb467..9113e18 100644 --- a/tests/test_openapi_resolver.py +++ b/tests/test_openapi_resolver.py @@ -182,13 +182,13 @@ def test_resolve_polymorphic_refs(self): schema = resolved["paths"]["/api/test"]["post"]["requestBody"]["content"]["application/json"]["schema"] assert "$ref" not in json.dumps(schema) - jq_parent, jq_own = schema["oneOf"][0]["allOf"] + jq = schema["oneOf"][0] # The subtype keeps the fields it inherits, but not the parent's list of subtypes - assert jq_parent["properties"]["type"] == {"type": "string"} - assert jq_parent["required"] == ["type"] - assert "oneOf" not in jq_parent - assert "discriminator" not in jq_parent - assert jq_own["properties"]["expression"] == {"type": "string"} + assert jq["properties"] == {"type": {"type": "string"}, "expression": {"type": "string"}} + assert jq["required"] == ["type"] + assert "allOf" not in jq + assert "oneOf" not in jq + assert "discriminator" not in jq def test_resolve_refs_in_arrays(self): """Test resolving $refs inside arrays.""" @@ -342,3 +342,105 @@ def test_original_spec_unchanged(self): # Original should be unchanged assert json.dumps(spec, sort_keys=True) == original_json + + def test_subtype_required_fields_live_next_to_their_properties(self): + """Test that a subtype's required list ends up on the object that defines those fields. + + springdoc puts the subtype's required list next to an allOf of the parent and the + subtype's own fields. Gemini/Vertex AI reject a required name that is not in the same + object's properties, so the allOf is merged into one object. + """ + spec = { + "paths": { + "/api/test": { + "post": { + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "comparison": {"oneOf": [{"$ref": "#/components/schemas/TimeWindow"}]} + }, + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Column": { + "required": ["name", "type"], + "type": "object", + "properties": { + "name": {"type": "string"}, + "ordinal": {"type": "integer", "format": "int32"}, + "type": {"type": "string"}, + }, + "discriminator": {"propertyName": "type"}, + }, + "TimeWindow": { + "required": ["name", "windowSize"], + "type": "object", + "allOf": [ + {"$ref": "#/components/schemas/Column"}, + { + "type": "object", + "properties": { + "ordinal": { + "type": "integer", + "format": "int32", + "description": "Zero-based column position", + }, + "windowSize": {"type": "object"}, + }, + }, + ], + }, + } + }, + } + + resolved = resolve_refs(spec) + + schema = resolved["paths"]["/api/test"]["post"]["requestBody"]["content"]["application/json"]["schema"] + window = schema["properties"]["comparison"]["oneOf"][0] + assert "allOf" not in window + assert "discriminator" not in window + assert window["required"] == ["name", "type", "windowSize"] + assert set(window["required"]) <= set(window["properties"]) + assert window["properties"]["ordinal"] == { + "type": "integer", + "format": "int32", + "description": "Zero-based column position", + } + + def test_all_of_with_conflicting_properties_is_kept(self): + """Test that branches defining a property differently are not merged.""" + spec = { + "paths": { + "/api/test": { + "post": { + "requestBody": { + "content": { + "application/json": { + "schema": { + "allOf": [ + {"type": "object", "properties": {"id": {"type": "string"}}}, + {"type": "object", "properties": {"id": {"type": "integer"}}}, + ] + } + } + } + } + } + } + } + } + + resolved = resolve_refs(spec) + + schema = resolved["paths"]["/api/test"]["post"]["requestBody"]["content"]["application/json"]["schema"] + assert len(schema["allOf"]) == 2