Fix schema skip issue

Author: owjs3901

crates/vespera/src/validated.rs

@@ -200,8 +200,12 @@ fn build_validation_response(report: &::garde::Report) -> Response {
     // to axum as raw bytes (content-type is overridden to
     // application/json below regardless).  Byte-identical to the previous
     // `to_string` body.
-    let body = ::serde_json::to_vec(&ValidationEnvelope { report })
-        .expect("serializing the 422 validation envelope is infallible");
+    // Serializing the envelope is practically infallible (no I/O, string
+    // keys), but this is a request-time boundary: on the unreachable failure
+    // path emit a minimal valid 422 envelope rather than panicking.
+    let body = ::serde_json::to_vec(&ValidationEnvelope { report }).unwrap_or_else(|_| {
+        br#"{"errors":[{"path":"","message":"request validation failed"}]}"#.to_vec()
+    });
 
     let mut response = (StatusCode::UNPROCESSABLE_ENTITY, body).into_response();
     response.headers_mut().insert(

crates/vespera/tests/multipart_wire.rs

@@ -27,6 +27,10 @@ use ::vespera_inprocess::{dispatch_from_bytes, register_app};
 #[allow(dead_code)]
 struct UploadReq {
     name: String,
+    // This round-trip test intentionally accepts any size (it exercises the
+    // 256 KiB tempfile path with the body limit disabled), so it opts out of
+    // the now-mandatory file-field cap explicitly rather than inheriting one.
+    #[form_data(limit = "unlimited")]
     file: FieldData<NamedTempFile>,
 }
 

crates/vespera_core/src/openapi.rs

@@ -188,15 +188,7 @@ impl OpenApi {
         if let Some(other_components) = other.components
             && has_any_component_map(&other_components)
         {
-            let self_components = self.components.get_or_insert(Components {
-                schemas: None,
-                responses: None,
-                parameters: None,
-                examples: None,
-                request_bodies: None,
-                headers: None,
-                security_schemes: None,
-            });
+            let self_components = self.components.get_or_insert_with(Components::default);
 
             merge_component_map(&mut self_components.schemas, other_components.schemas);
             merge_component_map(&mut self_components.responses, other_components.responses);
@@ -229,15 +221,16 @@ impl OpenApi {
         // with the existing tag names and grows as incoming tags are appended,
         // so an incoming tag is kept only when its name collides with neither
         // an existing tag nor an already-appended incoming one (first-wins,
-        // incoming insertion order preserved).  Tag merging runs at compile
-        // time over a handful of tags, so owning the names (one clone each)
-        // is cheaper to read than the prior borrow-juggling two-pass flag Vec.
+        // incoming insertion order preserved).  A name is cloned only when the
+        // tag is actually kept — a duplicate is detected by borrow and skipped
+        // without cloning.
         if let Some(other_tags) = other.tags {
             let self_tags = self.tags.get_or_insert_with(Vec::new);
             let mut seen: std::collections::HashSet<String> =
                 self_tags.iter().map(|tag| tag.name.clone()).collect();
             for tag in other_tags {
-                if seen.insert(tag.name.clone()) {
+                if !seen.contains(tag.name.as_str()) {
+                    seen.insert(tag.name.clone());
                     self_tags.push(tag);
                 }
             }

crates/vespera_core/src/schema.rs

@@ -3,8 +3,20 @@
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
 
-/// Schema reference or inline schema
-#[derive(Debug, Clone, Serialize, Deserialize)]
+/// Schema reference or inline schema.
+///
+/// Serializes untagged — a bare `{"$ref": ...}` object for
+/// [`SchemaRef::Ref`], the schema object for [`SchemaRef::Inline`].
+///
+/// Deserialization is a hand-written impl rather than
+/// `#[serde(untagged)]`: an untagged `Ref`-first enum greedily matched
+/// **any** object carrying a `$ref` key and silently dropped its
+/// siblings (e.g. a nullable reference's `"nullable": true`).  The
+/// custom impl treats only a *pure* `{"$ref": <string>}` object as a
+/// reference; a `$ref` accompanied by any sibling keyword
+/// (`nullable`, `description`, …) is an inline [`Schema`], so those
+/// siblings survive the round-trip instead of being discarded.
+#[derive(Debug, Clone, Serialize)]
 #[serde(untagged)]
 pub enum SchemaRef {
     /// Schema reference (e.g., "#/components/schemas/User")
@@ -13,6 +25,31 @@ pub enum SchemaRef {
     Inline(Box<Schema>),
 }
 
+impl<'de> Deserialize<'de> for SchemaRef {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        use serde::de::Error as _;
+        // OpenAPI is always JSON; buffer the node so a *pure* reference
+        // can be distinguished from a `$ref` carrying sibling keywords.
+        let value = serde_json::Value::deserialize(deserializer)?;
+        // Pure reference: an object whose ONLY key is `$ref` with a string
+        // value.  A `$ref` with any sibling (`nullable`, `description`, …)
+        // is an inline schema, so the siblings are preserved instead of
+        // being dropped by the prior untagged `Ref`-first match.
+        if let serde_json::Value::Object(map) = &value
+            && map.len() == 1
+            && let Some(serde_json::Value::String(ref_path)) = map.get("$ref")
+        {
+            return Ok(Self::Ref(Reference::new(ref_path.clone())));
+        }
+        serde_json::from_value::<Schema>(value)
+            .map(|schema| Self::Inline(Box::new(schema)))
+            .map_err(D::Error::custom)
+    }
+}
+
 /// Reference definition
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Reference {
@@ -274,73 +311,40 @@ pub struct Schema {
 }
 
 impl Schema {
-    /// Create a new schema
+    /// Create a new schema of the given type.
+    ///
+    /// Every other field starts at its [`Default`] (`None`/empty), so a newly
+    /// added `Schema` field is auto-defaulted here instead of having to be
+    /// appended to a ~40-field manual initializer that drifts out of sync.
     #[must_use]
-    pub const fn new(schema_type: SchemaType) -> Self {
+    pub fn new(schema_type: SchemaType) -> Self {
         Self {
-            ref_path: None,
             schema_type: Some(schema_type),
-            format: None,
-            title: None,
-            description: None,
-            default: None,
-            example: None,
-            examples: None,
-            minimum: None,
-            maximum: None,
-            exclusive_minimum: None,
-            exclusive_maximum: None,
-            multiple_of: None,
-            min_length: None,
-            max_length: None,
-            pattern: None,
-            items: None,
-            prefix_items: None,
-            min_items: None,
-            max_items: None,
-            unique_items: None,
-            properties: None,
-            required: None,
-            additional_properties: None,
-            min_properties: None,
-            max_properties: None,
-            r#enum: None,
-            all_of: None,
-            any_of: None,
-            one_of: None,
-            not: None,
-            discriminator: None,
-            nullable: None,
-            read_only: None,
-            write_only: None,
-            external_docs: None,
-            defs: None,
-            dynamic_anchor: None,
-            dynamic_ref: None,
+            ..Self::default()
         }
     }
 
     /// Create a string schema
     #[must_use]
-    pub const fn string() -> Self {
+    pub fn string() -> Self {
         Self::new(SchemaType::String)
     }
 
     /// Create an integer schema
     #[must_use]
-    pub const fn integer() -> Self {
+    pub fn integer() -> Self {
         Self::new(SchemaType::Integer)
     }
 
     /// Create a number schema
     #[must_use]
-    pub const fn number() -> Self {
+    pub fn number() -> Self {
         Self::new(SchemaType::Number)
     }
 
     /// Create a boolean schema
     #[must_use]
-    pub const fn boolean() -> Self {
+    pub fn boolean() -> Self {
         Self::new(SchemaType::Boolean)
     }
 
@@ -381,6 +385,26 @@ impl Schema {
             ..Self::new(SchemaType::Object)
         }
     }
+
+    /// Reconstruct a [`Schema`] from a compile-time-serialized JSON spec.
+    ///
+    /// This is the bridge the `schema!` proc-macro uses to emit a runtime
+    /// `Schema` value that is **identical** to the one the OpenAPI
+    /// generator produces for the same type: the macro builds the schema
+    /// through the shared `parse_struct_to_schema` path, serializes it to
+    /// JSON at compile time, and emits a call to this constructor — so the
+    /// `schema!` result can never drift from the documented component
+    /// schema (required-by-nullability, doc descriptions,
+    /// flatten/transparent, field constraints, `$ref` references).
+    ///
+    /// The input is always valid JSON (the macro just serialized it via
+    /// `serde_json`), so a parse failure is unreachable in practice; it
+    /// degrades to [`Schema::default`] rather than panicking inside
+    /// generated user code.
+    #[must_use]
+    pub fn from_compiled_json(json: &str) -> Self {
+        serde_json::from_str(json).unwrap_or_default()
+    }
 }
 
 /// External documentation reference
@@ -409,7 +433,7 @@ pub struct Discriminator {
 }
 
 /// `OpenAPI` Components (reusable components)
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Components {
     /// Schema definitions
@@ -691,4 +715,66 @@ mod tests {
             "a nullable reference must not also emit a type: {json}"
         );
     }
+
+    // ── SchemaRef: $ref-sibling preservation ─────────────────────────
+    //
+    // The prior `#[serde(untagged)]` `Ref`-first enum greedily matched
+    // ANY object with a `$ref` key and silently dropped its siblings
+    // (e.g. a nullable reference's `"nullable": true`).  The custom
+    // `Deserialize` treats only a *pure* `{"$ref": <string>}` as a
+    // reference; a `$ref` with any sibling becomes an inline `Schema`
+    // so the siblings round-trip intact.
+
+    #[test]
+    fn schema_ref_pure_ref_deserializes_as_ref() {
+        let v: SchemaRef =
+            serde_json::from_str(r##"{"$ref":"#/components/schemas/User"}"##).unwrap();
+        match v {
+            SchemaRef::Ref(r) => assert_eq!(r.ref_path, "#/components/schemas/User"),
+            SchemaRef::Inline(_) => panic!("a pure $ref must deserialize as SchemaRef::Ref"),
+        }
+    }
+
+    #[test]
+    fn schema_ref_with_nullable_sibling_preserves_fields() {
+        let v: SchemaRef =
+            serde_json::from_str(r##"{"$ref":"#/components/schemas/User","nullable":true}"##)
+                .unwrap();
+        match v {
+            SchemaRef::Inline(schema) => {
+                assert_eq!(
+                    schema.ref_path.as_deref(),
+                    Some("#/components/schemas/User"),
+                    "the $ref must survive as an inline ref_path"
+                );
+                assert_eq!(
+                    schema.nullable,
+                    Some(true),
+                    "the nullable sibling must not be dropped"
+                );
+            }
+            SchemaRef::Ref(_) => panic!("$ref with a sibling must not be matched as a bare Ref"),
+        }
+    }
+
+    #[test]
+    fn schema_ref_inline_object_deserializes_as_inline() {
+        let v: SchemaRef = serde_json::from_str(r#"{"type":"string"}"#).unwrap();
+        assert!(matches!(v, SchemaRef::Inline(_)));
+    }
+
+    #[test]
+    fn schema_ref_nullable_reference_roundtrips() {
+        // Build → serialize → deserialize must keep BOTH `$ref` and `nullable`.
+        let original = Schema::nullable_reference("#/components/schemas/User".to_owned());
+        let json = serde_json::to_string(&SchemaRef::Inline(Box::new(original))).unwrap();
+        let back: SchemaRef = serde_json::from_str(&json).unwrap();
+        match back {
+            SchemaRef::Inline(s) => {
+                assert_eq!(s.ref_path.as_deref(), Some("#/components/schemas/User"));
+                assert_eq!(s.nullable, Some(true));
+            }
+            SchemaRef::Ref(_) => panic!("a nullable reference must round-trip as inline"),
+        }
+    }
 }

crates/vespera_inprocess/src/wire.rs

@@ -142,6 +142,12 @@ mod tests {
             // empty headers object + duplicate header NAMES preserved
             br#"{"v":1,"method":"GET","path":"/p","headers":{}}"#,
             br#"{"v":1,"method":"GET","path":"/p","headers":{"x-a":"1","x-a":"2"}}"#,
+            // VALID but complex values under an UNKNOWN key — the strict
+            // skip must still ACCEPT every JSON-legal form (negative /
+            // float / exponent numbers, escaped strings, nested arrays and
+            // objects, the three literals) so forward-compat fields aren't
+            // over-rejected.
+            br#"{"v":1,"method":"GET","path":"/p","a":-3.14e10,"b":"esc\"d\n","c":[true,null,{"x":1}],"d":0}"#,
         ];
         for case in cases {
             match (parse_wire_header(case), parse_wire_header_serde(case)) {
@@ -177,6 +183,20 @@ mod tests {
             br#"{"v":1,"method":"GET","path":"/p","headers":{"x":1}}"#, // header value not string
             br#"{"v":1,"method":"GET","path":"/p","app":7}"#, // app not string/null
             br#"{"v":1,"method":"GET","path":"/p","headers":[]}"#, // headers not object
+            // Malformed values under UNKNOWN keys must still be rejected
+            // (the skip path validates the full JSON grammar, matching
+            // serde_json — not the prior permissive skip that accepted them).
+            br#"{"v":1,"method":"GET","path":"/p","x":"\q"}"#, // invalid string escape
+            b"{\"v\":1,\"method\":\"GET\",\"path\":\"/p\",\"x\":\"\x01\"}", // unescaped control char
+            br#"{"v":1,"method":"GET","path":"/p","x":tru}"#,               // truncated literal
+            br#"{"v":1,"method":"GET","path":"/p","x":nul}"#,               // truncated null
+            br#"{"v":1,"method":"GET","path":"/p","x":1e+}"#, // exponent without digit
+            br#"{"v":1,"method":"GET","path":"/p","x":1.}"#,  // fraction without digit
+            br#"{"v":1,"method":"GET","path":"/p","x":01}"#,  // leading zero
+            br#"{"v":1,"method":"GET","path":"/p","x":[}"#,   // mismatched container open
+            br#"{"v":1,"method":"GET","path":"/p","x":[1,2}"#, // array closed by '}'
+            br#"{"v":1,"method":"GET","path":"/p","x":{"a":1,}}"#, // trailing comma in object
+            br#"{"v":01,"method":"GET","path":"/p"}"#,        // leading zero in `v`
         ];
         for case in bad {
             assert!(