From b9bf6839f2e948d92d6fa1672e807ca84d742aa7 Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:17:37 +0100 Subject: [PATCH 1/5] fix(generator): resolve allOf members that restate the same union An allOf whose members are the same union, once annotations are ignored, failed with "intersects multiple union members". The current Cloudflare spec types its Workers Observability query filters as allOf: [$ref filter_node, anyOf: [group, leaf]], where the inline anyOf repeats filter_node's branches with added descriptions. That error stopped the whole spec from generating. Repeated union members are now dropped before composition, comparing assertions only: annotation keywords are stripped recursively, member names under properties are kept, and `type: object` on the named union is ignored. A restatement next to a single $ref becomes an alias of that schema. Unions that differ still report the diagnostic. The pinned corpus has no such schema, so its output is unchanged. --- src/analysis.rs | 81 +++++++++++++++++++ tests/allof_object_union_preservation_test.rs | 80 ++++++++++++++++++ 2 files changed, 161 insertions(+) diff --git a/src/analysis.rs b/src/analysis.rs index a0a460c..b7c4f46 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1415,6 +1415,46 @@ fn schema_is_annotation_only(schema: &crate::openapi::Schema) -> bool { }) } +fn is_annotation_keyword(key: &str) -> bool { + matches!( + key, + "title" + | "description" + | "deprecated" + | "readOnly" + | "writeOnly" + | "examples" + | "example" + | "externalDocs" + | "xml" + | "$comment" + ) || key.starts_with("x-") +} + +/// Remove annotation keywords from a schema and its subschemas, leaving the +/// assertions. Member names under `properties` and similar maps are kept. +fn strip_schema_annotations(value: &mut Value) { + let Some(object) = value.as_object_mut() else { + return; + }; + object.retain(|key, _| !is_annotation_keyword(key)); + for (key, child) in object.iter_mut() { + match key.as_str() { + "properties" | "patternProperties" | "$defs" | "definitions" | "dependentSchemas" => { + if let Some(members) = child.as_object_mut() { + members.values_mut().for_each(strip_schema_annotations); + } + } + "enum" | "const" | "default" | "required" | "discriminator" => {} + _ => match child { + Value::Array(items) => items.iter_mut().for_each(strip_schema_annotations), + Value::Object(_) => strip_schema_annotations(child), + _ => {} + }, + } + } +} + /// Load an extension file and parse it into the JSON representation used by /// the analyzer. YAML extensions follow the same conversion policy as YAML /// OpenAPI documents; every other extension is parsed as JSON. @@ -4278,6 +4318,9 @@ impl SchemaAnalyzer { all_of_schemas: &[Schema], dependencies: &mut HashSet, ) -> Result { + let without_restated = self.without_restated_unions(all_of_schemas); + let all_of_schemas = without_restated.as_deref().unwrap_or(all_of_schemas); + // A scalar/array carrier intersected only with annotation-only // siblings keeps its wire type. This is deliberately narrower than // "pick the first non-object": multiple carriers or assertion-bearing @@ -4646,6 +4689,44 @@ impl SchemaAnalyzer { } } + /// Drop `allOf` members that repeat an earlier member's union, compared + /// without annotations. Returns `None` when nothing repeats. + fn without_restated_unions(&self, members: &[Schema]) -> Option> { + let mut seen = Vec::new(); + let mut kept = Vec::with_capacity(members.len()); + for member in members { + if let Some(signature) = self.union_signature(member) { + if seen.contains(&signature) { + continue; + } + seen.push(signature); + } + kept.push(member.clone()); + } + (kept.len() < members.len()).then_some(kept) + } + + /// The assertions of a member that is only a union, following one `$ref`. + /// `type: object` is ignored so a named union matches an inline restatement + /// that omits it. + fn union_signature(&self, member: &Schema) -> Option { + let schema = match member.reference() { + Some(reference) => self.schemas.get(self.extract_schema_name(reference)?)?, + None => member, + }; + let mut value = serde_json::to_value(schema).ok()?; + strip_schema_annotations(&mut value); + let object = value.as_object_mut()?; + if object.get("type").and_then(Value::as_str) == Some("object") { + object.remove("type"); + } + let is_union = object.contains_key("anyOf") || object.contains_key("oneOf"); + let only_union = object + .keys() + .all(|key| matches!(key.as_str(), "anyOf" | "oneOf" | "discriminator")); + (is_union && only_union).then_some(value) + } + /// Property names declared by any object reachable through the branches /// of an analyzed union, including nested unions and flattened variants. fn union_declared_property_names(&mut self, target: &str) -> Result> { diff --git a/tests/allof_object_union_preservation_test.rs b/tests/allof_object_union_preservation_test.rs index 6ce2704..3d94cfa 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -1,3 +1,5 @@ +#![recursion_limit = "256"] + use openapi_to_rust::analysis::SchemaType; use openapi_to_rust::{CodeGenerator, GeneratorConfig, SchemaAnalyzer}; use serde_json::{Value, json}; @@ -188,6 +190,61 @@ fn object_union_allof_spec() -> Value { { "$ref": "#/components/schemas/RecordWithData" } ] }, + "FilterLeaf": { + "type": "object", + "required": ["key", "operation"], + "properties": { + "key": { "type": "string" }, + "operation": { "type": "string" }, + "value": { "type": "string" } + } + }, + "FilterNode": { + "description": "A leaf filter or a nested group.", + "type": "object", + "anyOf": [ + { + "type": "object", + "required": ["kind", "filters"], + "properties": { + "kind": { "type": "string", "enum": ["group"] }, + "filters": { + "type": "array", + "items": { "$ref": "#/components/schemas/FilterNode" } + } + } + }, + { "$ref": "#/components/schemas/FilterLeaf" } + ] + }, + "RestatedFilter": { + "allOf": [ + { "$ref": "#/components/schemas/FilterNode" }, + { + "description": "Supports nested groups.", + "anyOf": [ + { + "description": "A group of child filters.", + "type": "object", + "required": ["kind", "filters"], + "properties": { + "kind": { + "description": "Marks a group.", + "type": "string", + "enum": ["group"] + }, + "filters": { + "description": "Child filters.", + "type": "array", + "items": { "$ref": "#/components/schemas/FilterNode" } + } + } + }, + { "$ref": "#/components/schemas/FilterLeaf" } + ] + } + ] + }, "GitpodOutputSpec": { "type": "object", "additionalProperties": false, @@ -445,6 +502,17 @@ fn main() { "boolean": {} })); round_trip::(serde_json::json!({})); + round_trip::(serde_json::json!({ + "kind": "group", + "filters": [ + { "key": "service", "operation": "eq", "value": "api" }, + { "kind": "group", "filters": [] } + ] + })); + round_trip::(serde_json::json!({ + "key": "status", + "operation": "exists" + })); } "##, ) @@ -468,6 +536,18 @@ fn main() { ); } +#[test] +fn allof_restating_a_union_up_to_annotations_resolves_to_that_union() { + let analysis = analyze(); + let SchemaType::Reference { target } = &analysis.schemas["RestatedFilter"].schema_type else { + panic!( + "RestatedFilter should alias FilterNode, got {:?}", + analysis.schemas["RestatedFilter"].schema_type + ); + }; + assert_eq!(target, "FilterNode"); +} + #[test] fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { let spec = json!({ From 5cfa18f9108df34cdb5fef2fb78c24d1013ceeb3 Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:17:37 +0100 Subject: [PATCH 2/5] docs: changelog entry for restated allOf unions --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63cd447..0792ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ when correcting output that was wrong or incomplete on the wire. optionality. Cloudflare's DNS record responses list `data` as required although only some record types carry it, so A and MX records failed to deserialize, and `name`, `content` and `ttl` were untyped. +- An `allOf` whose members repeat the same union, differing only in + annotations such as `description`, resolves to that union instead of failing + with "intersects multiple union members". The current Cloudflare spec + restates its Workers Observability filter union inline next to a `$ref` to + it, which stopped the whole spec from generating. Members that are different + unions still fail with that diagnostic. ## [0.22.0] - 2026-10-06 From 0ac4b337c89052fc78eae1fa4ccddaa29bed066e Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:21:03 +0100 Subject: [PATCH 3/5] fix: preserve assertions when deduplicating allOf unions --- CHANGELOG.md | 4 +- src/analysis.rs | 67 +++++++++++++++++-- tests/allof_object_union_preservation_test.rs | 53 +++++++++++++++ 3 files changed, 116 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0792ea3..91dac65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,9 @@ when correcting output that was wrong or incomplete on the wire. with "intersects multiple union members". The current Cloudflare spec restates its Workers Observability filter union inline next to a `$ref` to it, which stopped the whole spec from generating. Members that are different - unions still fail with that diagnostic. + unions still fail with that diagnostic. Assertion data such as + `dependentRequired` is preserved, and `type: object` is ignored only when + every union branch explicitly requires an object. ## [0.22.0] - 2026-10-06 diff --git a/src/analysis.rs b/src/analysis.rs index b7c4f46..5fabaac 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1445,12 +1445,29 @@ fn strip_schema_annotations(value: &mut Value) { members.values_mut().for_each(strip_schema_annotations); } } - "enum" | "const" | "default" | "required" | "discriminator" => {} - _ => match child { + "allOf" | "anyOf" | "oneOf" | "prefixItems" => { + if let Some(items) = child.as_array_mut() { + items.iter_mut().for_each(strip_schema_annotations); + } + } + "items" => match child { Value::Array(items) => items.iter_mut().for_each(strip_schema_annotations), - Value::Object(_) => strip_schema_annotations(child), - _ => {} + _ => strip_schema_annotations(child), }, + "additionalProperties" + | "additionalItems" + | "unevaluatedProperties" + | "unevaluatedItems" + | "contains" + | "propertyNames" + | "not" + | "if" + | "then" + | "else" + | "contentSchema" => strip_schema_annotations(child), + // Other keywords contain data, not schemas (notably dependentRequired). + // Preserve unknown keywords conservatively as well. + _ => {} } } } @@ -4707,8 +4724,8 @@ impl SchemaAnalyzer { } /// The assertions of a member that is only a union, following one `$ref`. - /// `type: object` is ignored so a named union matches an inline restatement - /// that omits it. + /// `type: object` is ignored only when every branch already requires an + /// object, so a named object union can match an inline restatement. fn union_signature(&self, member: &Schema) -> Option { let schema = match member.reference() { Some(reference) => self.schemas.get(self.extract_schema_name(reference)?)?, @@ -4717,7 +4734,19 @@ impl SchemaAnalyzer { let mut value = serde_json::to_value(schema).ok()?; strip_schema_annotations(&mut value); let object = value.as_object_mut()?; - if object.get("type").and_then(Value::as_str) == Some("object") { + if object.get("type").and_then(Value::as_str) == Some("object") + && ["anyOf", "oneOf"].iter().any(|keyword| { + object + .get(*keyword) + .and_then(Value::as_array) + .is_some_and(|branches| { + !branches.is_empty() + && branches + .iter() + .all(|branch| self.union_branch_requires_object(branch)) + }) + }) + { object.remove("type"); } let is_union = object.contains_key("anyOf") || object.contains_key("oneOf"); @@ -4727,6 +4756,30 @@ impl SchemaAnalyzer { (is_union && only_union).then_some(value) } + /// A conservative proof: require an explicit object type, optionally + /// following one reference. Do not infer a type from object-only keywords + /// such as properties/required, which also accept scalars in JSON Schema. + fn union_branch_requires_object(&self, branch: &Value) -> bool { + let resolved; + let branch = if let Some(reference) = branch.get("$ref").and_then(Value::as_str) { + let Some(schema) = self + .extract_schema_name(reference) + .and_then(|name| self.schemas.get(name)) + else { + return false; + }; + let Ok(value) = serde_json::to_value(schema) else { + return false; + }; + resolved = value; + &resolved + } else { + branch + }; + branch.get("type").and_then(Value::as_str) == Some("object") + && branch.get("nullable").and_then(Value::as_bool) != Some(true) + } + /// Property names declared by any object reachable through the branches /// of an analyzed union, including nested unions and flattened variants. fn union_declared_property_names(&mut self, target: &str) -> Result> { diff --git a/tests/allof_object_union_preservation_test.rs b/tests/allof_object_union_preservation_test.rs index 3d94cfa..4258c6f 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -577,3 +577,56 @@ fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { "unexpected error: {error}" ); } + +fn assert_distinct_unions_are_rejected(first: Value, second: Value, witness: Value) { + assert!(jsonschema::is_valid(&first, &witness)); + assert!(!jsonschema::is_valid(&second, &witness)); + for members in [json!([first, second]), json!([second, first])] { + let spec = json!({ + "openapi": "3.1.0", + "info": { "title": "distinct unions", "version": "1" }, + "paths": {}, + "components": { "schemas": { "Intersection": { "allOf": members } } } + }); + let error = SchemaAnalyzer::new(spec) + .unwrap() + .analyze() + .expect_err("different union assertions must not be deduplicated"); + assert!( + error + .to_string() + .contains("intersects multiple union members"), + "unexpected error: {error}" + ); + } +} + +#[test] +fn union_restatements_preserve_dependent_required_data() { + for keyword in ["anyOf", "oneOf"] { + let first = json!({keyword: [ + {"type": "object", "dependentRequired": {"description": ["kind"]}}, + {"type": "string"} + ]}); + let second = json!({keyword: [ + {"type": "object", "dependentRequired": {"description": ["value"]}}, + {"type": "string"} + ]}); + assert_distinct_unions_are_rejected( + first, + second, + json!({"description": "x", "kind": "y"}), + ); + } +} + +#[test] +fn union_restatements_preserve_object_type_for_mixed_branches() { + for keyword in ["anyOf", "oneOf"] { + let first = json!({keyword: [{"type": "object"}, {"type": "string"}]}); + let second = json!({"type": "object", keyword: [ + {"type": "object"}, {"type": "string"} + ]}); + assert_distinct_unions_are_rejected(first, second, json!("scalar")); + } +} From 50301d1877088b5cadecaed4a97194d7dc9137de Mon Sep 17 00:00:00 2001 From: James Lal Date: Fri, 16 Oct 2026 20:39:33 -0600 Subject: [PATCH 4/5] fix(generator): preserve owner fields when collapsing restated unions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep deduplicated allOf compositions with owner properties or required names on the merge path instead of returning a bare alias. Add an anyOf/oneOf member-order matrix and generated recursive-filter round trips with missing-required rejection. Continues Giles Westwood’s work in PR #104; his original commits remain in the branch history. --- CHANGELOG.md | 3 +- src/analysis.rs | 22 +++- tests/allof_object_union_preservation_test.rs | 106 +++++++++++++++++- 3 files changed, 126 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47af617..0a65747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,8 @@ when correcting output that was wrong or incomplete on the wire. it, which stopped the whole spec from generating. Members that are different unions still fail with that diagnostic. Assertion data such as `dependentRequired` is preserved, and `type: object` is ignored only when - every union branch explicitly requires an object. + every union branch explicitly requires an object. Fields and required names + declared beside `allOf` survive when deduplication leaves a single member. ## [0.22.0] - 2026-10-06 diff --git a/src/analysis.rs b/src/analysis.rs index 5fabaac..e33bbfd 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -4338,6 +4338,20 @@ impl SchemaAnalyzer { let without_restated = self.without_restated_unions(all_of_schemas); let all_of_schemas = without_restated.as_deref().unwrap_or(all_of_schemas); + // Deduplicating unions can leave a single member, but the owner may + // still contribute fields. Keep those compositions on the merge path + // instead of returning an alias before the owner's fields are merged. + let owner_details = owner_schema.details(); + let preserve_owner_members = without_restated.is_some() + && (owner_details + .properties + .as_ref() + .is_some_and(|properties| !properties.is_empty()) + || owner_details + .required + .as_ref() + .is_some_and(|required| !required.is_empty())); + // A scalar/array carrier intersected only with annotation-only // siblings keeps its wire type. This is deliberately narrower than // "pick the first non-object": multiple carriers or assertion-bearing @@ -4358,7 +4372,10 @@ impl SchemaAnalyzer { let only_reference_and_annotations = all_of_schemas .iter() .all(|schema| schema.reference().is_some() || schema_is_annotation_only(schema)); - if referenced_targets.len() == 1 && only_reference_and_annotations { + if !preserve_owner_members + && referenced_targets.len() == 1 + && only_reference_and_annotations + { let target = referenced_targets[0]; dependencies.insert(target.to_string()); return Ok(SchemaType::Reference { @@ -4369,7 +4386,8 @@ impl SchemaAnalyzer { // A single member composes with nothing: `allOf: [{type: string}]` is // that string. Specs write it to hang a description off a scalar or a // `$ref`, and merging it as an object would lose the type entirely. - if let [only] = all_of_schemas + if !preserve_owner_members + && let [only] = all_of_schemas && !matches!( only.schema_type(), Some(OpenApiSchemaType::Object) | Some(OpenApiSchemaType::Null) diff --git a/tests/allof_object_union_preservation_test.rs b/tests/allof_object_union_preservation_test.rs index 4258c6f..1cfdb40 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -5,7 +5,7 @@ use openapi_to_rust::{CodeGenerator, GeneratorConfig, SchemaAnalyzer}; use serde_json::{Value, json}; fn object_union_allof_spec() -> Value { - json!({ + let mut spec = json!({ "openapi": "3.1.0", "info": { "title": "object union allOf", "version": "1" }, "paths": {}, @@ -288,7 +288,27 @@ fn object_union_allof_spec() -> Value { } } } - }) + }); + let schemas = spec["components"]["schemas"] + .as_object_mut() + .expect("schema map"); + let reference = json!({ "$ref": "#/components/schemas/FilterNode" }); + let inline = schemas["FilterNode"].clone(); + for (name, members) in [ + ("RepeatedFilterWithOwner", json!([reference, reference])), + ("InlineRestatedFilterWithOwner", json!([inline, reference])), + ] { + schemas.insert( + name.into(), + json!({ + "type": "object", + "properties": { "owner_field": { "type": "string" } }, + "required": ["owner_field"], + "allOf": members + }), + ); + } + spec } fn analyze() -> openapi_to_rust::SchemaAnalysis { @@ -513,6 +533,22 @@ fn main() { "key": "status", "operation": "exists" })); + round_trip::(serde_json::json!({ + "owner_field": "request-1", + "key": "status", + "operation": "exists" + })); + round_trip::(serde_json::json!({ + "owner_field": "request-2", + "kind": "group", + "filters": [{ "key": "status", "operation": "exists" }] + })); + assert!(serde_json::from_value::( + serde_json::json!({ "key": "status", "operation": "exists" }) + ).is_err()); + assert!(serde_json::from_value::( + serde_json::json!({ "kind": "group", "filters": [] }) + ).is_err()); } "##, ) @@ -548,6 +584,72 @@ fn allof_restating_a_union_up_to_annotations_resolves_to_that_union() { assert_eq!(target, "FilterNode"); } +#[test] +fn restated_unions_retain_owner_properties_and_required_names() { + for keyword in ["anyOf", "oneOf"] { + let union = json!({keyword: [ + {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, + {"type": "object", "properties": {"b": {"type": "integer"}}, "required": ["b"]} + ]}); + let reference = json!({"$ref": "#/components/schemas/Union"}); + for members in [ + json!([reference, reference]), + json!([reference, union]), + json!([union, reference]), + json!([union, union]), + ] { + for (declares_property, requires_property) in + [(true, true), (true, false), (false, true)] + { + let mut owner = json!({"type": "object", "allOf": members}); + if declares_property { + owner["properties"] = json!({"owner_field": {"type": "string"}}); + } + if requires_property { + owner["required"] = json!(["owner_field"]); + } + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "restated union owner", "version": "1"}, + "paths": {}, + "components": {"schemas": {"Union": union, "Owner": owner}} + }); + let analysis = SchemaAnalyzer::new(spec).unwrap().analyze().unwrap(); + let SchemaType::Object { + properties, + required, + variant, + .. + } = &analysis.schemas["Owner"].schema_type + else { + panic!( + "owner fields were dropped for {owner}: {:?}", + analysis.schemas["Owner"].schema_type + ); + }; + assert!(variant.is_some(), "owner must retain its union for {owner}"); + assert_eq!( + required.contains("owner_field"), + requires_property, + "{owner}" + ); + let property = &properties["owner_field"]; + if declares_property { + assert!( + matches!( + &property.schema_type, + SchemaType::Primitive { rust_type, .. } if rust_type == "String" + ), + "declared owner field must keep its type for {owner}" + ); + } else { + assert!(property.synthesized_required, "{owner}"); + } + } + } + } +} + #[test] fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { let spec = json!({ From e934411772df55427f51a75181c4d83dfbc3ac77 Mon Sep 17 00:00:00 2001 From: James Lal Date: Fri, 16 Oct 2026 21:02:46 -0600 Subject: [PATCH 5/5] fix(generator): retain owner assertions around restated unions Only collapse a deduplicated allOf owner when it has no sibling assertions. Preserve object-type and not constraints around mixed object/scalar unions, with JSON Schema witnesses and compiled scalar-rejection coverage. Keep annotation-only owners eligible for aliases. --- CHANGELOG.md | 5 +- src/analysis.rs | 33 ++++----- tests/allof_object_union_preservation_test.rs | 73 ++++++++++++++++++- 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a65747..b934459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,8 +37,9 @@ when correcting output that was wrong or incomplete on the wire. it, which stopped the whole spec from generating. Members that are different unions still fail with that diagnostic. Assertion data such as `dependentRequired` is preserved, and `type: object` is ignored only when - every union branch explicitly requires an object. Fields and required names - declared beside `allOf` survive when deduplication leaves a single member. + every union branch explicitly requires an object. Object types, fields and + required names declared beside `allOf` survive when deduplication leaves a + single member. ## [0.22.0] - 2026-10-06 diff --git a/src/analysis.rs b/src/analysis.rs index e33bbfd..83adfa5 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -4338,19 +4338,20 @@ impl SchemaAnalyzer { let without_restated = self.without_restated_unions(all_of_schemas); let all_of_schemas = without_restated.as_deref().unwrap_or(all_of_schemas); - // Deduplicating unions can leave a single member, but the owner may - // still contribute fields. Keep those compositions on the merge path - // instead of returning an alias before the owner's fields are merged. - let owner_details = owner_schema.details(); - let preserve_owner_members = without_restated.is_some() - && (owner_details - .properties - .as_ref() - .is_some_and(|properties| !properties.is_empty()) - || owner_details - .required - .as_ref() - .is_some_and(|required| !required.is_empty())); + // After deduplication, only an owner with no assertions beside allOf + // can collapse to a bare member. Otherwise keep the merge path, retaining + // the owner's object shape and fields. + let preserve_owner_shape = without_restated.is_some() + && !serde_json::to_value(owner_schema) + .ok() + .and_then(|value| value.as_object().cloned()) + .is_some_and(|owner| { + owner.keys().all(|key| { + key == "allOf" + || is_annotation_keyword(key) + || matches!(key.as_str(), "default" | "nullable") + }) + }); // A scalar/array carrier intersected only with annotation-only // siblings keeps its wire type. This is deliberately narrower than @@ -4372,9 +4373,7 @@ impl SchemaAnalyzer { let only_reference_and_annotations = all_of_schemas .iter() .all(|schema| schema.reference().is_some() || schema_is_annotation_only(schema)); - if !preserve_owner_members - && referenced_targets.len() == 1 - && only_reference_and_annotations + if !preserve_owner_shape && referenced_targets.len() == 1 && only_reference_and_annotations { let target = referenced_targets[0]; dependencies.insert(target.to_string()); @@ -4386,7 +4385,7 @@ impl SchemaAnalyzer { // A single member composes with nothing: `allOf: [{type: string}]` is // that string. Specs write it to hang a description off a scalar or a // `$ref`, and merging it as an object would lose the type entirely. - if !preserve_owner_members + if !preserve_owner_shape && let [only] = all_of_schemas && !matches!( only.schema_type(), diff --git a/tests/allof_object_union_preservation_test.rs b/tests/allof_object_union_preservation_test.rs index 1cfdb40..0a958c7 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -308,6 +308,26 @@ fn object_union_allof_spec() -> Value { }), ); } + schemas.insert( + "MixedFilter".into(), + json!({"anyOf": [ + {"type": "string"}, + {"type": "object", "properties": {"key": {"type": "string"}}, "required": ["key"]} + ]}), + ); + for (name, mut owner) in [ + ("RepeatedMixedFilterObject", json!({"type": "object"})), + ( + "RepeatedMixedFilterNotString", + json!({"not": {"type": "string"}}), + ), + ] { + owner["allOf"] = json!([ + {"$ref": "#/components/schemas/MixedFilter"}, + {"$ref": "#/components/schemas/MixedFilter"} + ]); + schemas.insert(name.into(), owner); + } spec } @@ -549,6 +569,14 @@ fn main() { assert!(serde_json::from_value::( serde_json::json!({ "kind": "group", "filters": [] }) ).is_err()); + round_trip::(serde_json::json!({ "key": "status" })); + assert!(serde_json::from_value::( + serde_json::json!("scalar") + ).is_err()); + round_trip::(serde_json::json!({ "key": "status" })); + assert!(serde_json::from_value::( + serde_json::json!("scalar") + ).is_err()); } "##, ) @@ -601,7 +629,7 @@ fn restated_unions_retain_owner_properties_and_required_names() { for (declares_property, requires_property) in [(true, true), (true, false), (false, true)] { - let mut owner = json!({"type": "object", "allOf": members}); + let mut owner = json!({"allOf": members}); if declares_property { owner["properties"] = json!({"owner_field": {"type": "string"}}); } @@ -650,6 +678,49 @@ fn restated_unions_retain_owner_properties_and_required_names() { } } +#[test] +fn restated_unions_preserve_owner_assertions_for_mixed_branches() { + for keyword in ["anyOf", "oneOf"] { + let union = json!({keyword: [ + {"type": "object", "properties": {"key": {"type": "string"}}, "required": ["key"]}, + {"type": "string"} + ]}); + assert!(jsonschema::is_valid(&union, &json!("scalar"))); + for mut owner in [ + json!({"type": "object"}), + json!({"not": {"type": "string"}}), + ] { + owner["allOf"] = json!([union, union]); + assert!(!jsonschema::is_valid(&owner, &json!("scalar"))); + owner["allOf"] = json!([ + {"$ref": "#/components/schemas/Union"}, + {"$ref": "#/components/schemas/Union"} + ]); + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "restated union owner type", "version": "1"}, + "paths": {}, + "components": {"schemas": { + "Union": union, + "Owner": owner + }} + }); + let analysis = SchemaAnalyzer::new(spec).unwrap().analyze().unwrap(); + assert!( + matches!( + &analysis.schemas["Owner"].schema_type, + SchemaType::Object { + variant: Some(_), + .. + } + ), + "owner must still require an object: {:?}", + analysis.schemas["Owner"].schema_type + ); + } + } +} + #[test] fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { let spec = json!({