diff --git a/Cargo.lock b/Cargo.lock index ff0b9de..3789598 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -212,9 +212,9 @@ checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" [[package]] name = "cc" -version = "1.4.3" +version = "1.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "509591b7bcd67f4ef775afad7662703b4935daaa6ec0e5605cfb1090b32a2b6d" +checksum = "0ad534f4357a5264cce5019c989cf66a4f0dc4e0d1b1d15f8aacec0ff7360273" dependencies = [ "find-msvc-tools", "shlex 2.0.1", @@ -280,7 +280,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -360,7 +360,7 @@ checksum = "c6232dd377dcc64799954cbd3a9bb882e9cdc1308ccd87b1c098f1fb2eaf82a8" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -371,9 +371,9 @@ checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" [[package]] name = "either" -version = "1.17.0" +version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e5e8f6c15a24b9a3ee5efec809ccd006d3b30e8b3bb63c39af737c7f87daa1d" +checksum = "252afb9ae5eaa683babdc6a068b3f5726eb19e05070c731f9b2a23a7c3e8ed34" [[package]] name = "encoding_rs" @@ -561,9 +561,9 @@ dependencies = [ [[package]] name = "hyper" -version = "1.11.0" +version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d22053281f852e11534f5198498373cbb59295120a20771d90f7ed1897490a72" +checksum = "27b501faa50e7a26c3d3560ca625132f4078a17771f4810baf70475ae48cbe43" dependencies = [ "atomic-waker", "bytes", @@ -697,9 +697,9 @@ checksum = "e590f038c1464a96894fd6d10127e90a8be4509f56ff7ecef851b15cee0b7caa" [[package]] name = "icu_provider" -version = "2.3.0" +version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92a7ed671a6aad807a8651a2e1782a6598fda9ce5185dd8158549e95a91c6428" +checksum = "d27bbb9d3abbefac45d55f647c9de1d44aafcd1186eb91879afef17c396c3e73" dependencies = [ "displaydoc", "icu_locale_core", @@ -733,9 +733,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.14.0" +version = "2.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" +checksum = "07aa2048142242915a31d35844fb311e0e53fcca590c3a0a40dcf1b841fa09eb" dependencies = [ "equivalent", "hashbrown", @@ -792,9 +792,9 @@ checksum = "47d9d19d1d6efa0109d2f65ff4c85cddd50bd572e5a00127ab10987290bcefae" [[package]] name = "log" -version = "0.4.33" +version = "0.4.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" +checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" [[package]] name = "matchit" @@ -897,7 +897,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "serde_yaml", - "syn 3.0.3", + "syn 3.0.4", "trycmd", "uuid", ] @@ -937,9 +937,9 @@ dependencies = [ [[package]] name = "owo-colors" -version = "4.3.0" +version = "4.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d211803b9b6b570f68772237e415a029d5a50c65d382910b879fb19d3271f94d" +checksum = "13c45bb4a6ae1280ec0803b1ef9d3455eb50f01efbbe1447ab020f1d54fba9d8" [[package]] name = "percent-encoding" @@ -975,7 +975,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2bfe0f4c752e450fc2faf62654f1c134747922825d5b04ca717b8874f41a40c0" dependencies = [ "proc-macro2", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -1141,7 +1141,7 @@ checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -1229,9 +1229,9 @@ checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" [[package]] name = "similar" -version = "3.1.2" +version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85ee016af5d736b69fc89e19254540fa4b5f5492853fb5503920f084011c78b6" +checksum = "4f66ca1f7aca2474dc10c942eb22feffc897735f54cd1db90138c2fddb490987" dependencies = [ "bstr", ] @@ -1320,9 +1320,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.3" +version = "3.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" dependencies = [ "proc-macro2", "quote", @@ -1423,7 +1423,7 @@ checksum = "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -1598,9 +1598,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.24.1" +version = "1.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" +checksum = "b5772d71c9be8a8a6ac2117d949c5b224c1b72241bb611d9a3012edcf8af7812" dependencies = [ "js-sys", "serde_core", @@ -1861,9 +1861,9 @@ dependencies = [ [[package]] name = "zerovec" -version = "0.11.7" +version = "0.11.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94b5c6b5976d66c1d703c4fd17d3f5e43c8cedaacf604961b171adc7130896d8" +checksum = "bb0464e17806c1d976d5cba29399c7f08e516e279e2ba493f63123b5fca67dd8" dependencies = [ "yoke", "zerofrom", @@ -1872,13 +1872,13 @@ dependencies = [ [[package]] name = "zerovec-derive" -version = "0.11.4" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47402523226a02bfe5230160dc3ccc089aa6f6f19e7fcbb4e6f824bbb1b4aa62" +checksum = "34df6fc39dbd26ddc9c10e6a2984476e13acce22e64e4487636ef494369225da" dependencies = [ "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] diff --git a/crates/oapi-codegen/Cargo.toml b/crates/oapi-codegen/Cargo.toml index 1fd523e..2a3cc7a 100644 --- a/crates/oapi-codegen/Cargo.toml +++ b/crates/oapi-codegen/Cargo.toml @@ -30,9 +30,9 @@ workspace = true anstream = "1.0.0" clap = { version = "4.6.6", features = ["derive"] } http = "1.5.0" -indexmap = "2.14.0" +indexmap = "2.14.1" openapiv3 = "2.2.0" -owo-colors = "4.3.0" +owo-colors = "4.4.0" prettyplease = "0.3.0" proc-macro2 = "1.0.107" quote = "1.0.47" @@ -40,7 +40,7 @@ regex = { version = "1.13.1", default-features = false, features = ["std", "perf serde = { version = "1.0.229", features = ["derive"] } serde_json = "1.0.151" serde_yaml = "0.9.34" -syn = { version = "3.0.3", features = ["full"] } +syn = { version = "3.0.4", features = ["full"] } [dev-dependencies] axum = { version = "0.8.9", features = ["multipart"] } @@ -53,4 +53,4 @@ reqwest = { version = "0.13.4", default-features = false, features = ["blocking" serde = { version = "1.0.229", features = ["derive"] } serde_urlencoded = "0.7.1" trycmd = "1.2.1" -uuid = { version = "1.24.1", features = ["serde"] } +uuid = { version = "1.26.0", features = ["serde"] } diff --git a/crates/oapi-codegen/src/emit/mod.rs b/crates/oapi-codegen/src/emit/mod.rs index a2422d1..5d69842 100644 --- a/crates/oapi-codegen/src/emit/mod.rs +++ b/crates/oapi-codegen/src/emit/mod.rs @@ -11,7 +11,7 @@ mod operation; mod package; mod reqwest; mod servers; -mod usage; +pub(crate) mod usage; use std::collections::HashMap; @@ -300,13 +300,12 @@ fn render_body(items: &[TokenStream]) -> Result { } /// Render a doc attribute, or nothing when there is no documentation. +/// +/// This splits the text on its line breaks, so a multi-line `description` prints +/// as a run of `///` lines and not one `/** */` block. pub(crate) fn doc_attr(doc: &Option) -> TokenStream { let tokens = match doc { - Some(text) => { - // Leading space matches the `/// text` desugaring rustfmt produces. - let spaced = format!(" {text}"); - quote! { #[doc = #spaced] } - } + Some(text) => doc_lines(std::slice::from_ref(text)), None => quote! {}, }; return tokens; diff --git a/crates/oapi-codegen/src/emit/models.rs b/crates/oapi-codegen/src/emit/models.rs index 7868ad8..3c17576 100644 --- a/crates/oapi-codegen/src/emit/models.rs +++ b/crates/oapi-codegen/src/emit/models.rs @@ -516,6 +516,7 @@ fn emit_alias(alias: &Alias) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::ir::Access; use crate::naming::Case; use crate::naming::to_ident; @@ -536,6 +537,7 @@ mod tests { serde_skip: false, default: Some(DefaultValue::Int(10)), constraints: None, + access: Access::ReadWrite, }], additional_properties: None, deny_unknown_fields: false, diff --git a/crates/oapi-codegen/src/emit/usage.rs b/crates/oapi-codegen/src/emit/usage.rs index 9b04593..56d6693 100644 --- a/crates/oapi-codegen/src/emit/usage.rs +++ b/crates/oapi-codegen/src/emit/usage.rs @@ -18,6 +18,7 @@ use std::collections::HashMap; use crate::emit::Targets; use crate::emit::models::ModelDerives; use crate::emit::models::SerdeDerives; +use crate::ir::Direction; use crate::ir::ForeignDerives; use crate::ir::Item; use crate::ir::Module; @@ -32,11 +33,11 @@ use crate::naming::to_ident; /// Whether a model is reachable as a request payload and/or a response payload. #[derive(Debug, Default, Clone, Copy)] -struct Usage { +pub(crate) struct Usage { /// Reachable from a request body or request-input struct. - request: bool, + pub(crate) request: bool, /// Reachable from a response body. - response: bool, + pub(crate) response: bool, } /// Compute the derive set for every generated model, keyed by its logical name. @@ -46,14 +47,7 @@ struct Usage { pub(crate) fn model_derives(module: &Module, service: &Service, targets: Targets) -> HashMap { let adjacency = adjacency(module); let foreign = foreign_derives(module, &adjacency); - let mut usage: HashMap = HashMap::new(); - - for name in request_seeds(service) { - mark(&adjacency, &name, &mut usage, Direction::Request); - } - for name in response_seeds(service) { - mark(&adjacency, &name, &mut usage, Direction::Response); - } + let usage = direction_usage(module, service); // Union of both keys. A model can be constrained by a foreign type without // being reachable from any operation, and the other way round, so taking only @@ -264,11 +258,19 @@ fn item_types(item: &Item) -> Vec { return types; } -/// The direction a seed propagates. -#[derive(Debug, Clone, Copy)] -enum Direction { - Request, - Response, +/// Which direction reaches each model, keyed by its logical name. +/// +/// An absent name is reached by no operation, which happens under `skip-prune`. +pub(crate) fn direction_usage(module: &Module, service: &Service) -> HashMap { + let adjacency = adjacency(module); + let mut usage: HashMap = HashMap::new(); + for name in request_seeds(service) { + mark(&adjacency, &name, &mut usage, Direction::Request); + } + for name in response_seeds(service) { + mark(&adjacency, &name, &mut usage, Direction::Response); + } + return usage; } /// Mark `start` and every model reachable from it with `direction`, following @@ -301,7 +303,7 @@ fn mark( /// Build the model-reference graph: each item name mapped to the names of the /// generated models it references through its fields, variants, or alias target. -fn adjacency(module: &Module) -> HashMap> { +pub(crate) fn adjacency(module: &Module) -> HashMap> { let mut graph = HashMap::with_capacity(module.items.len()); for item in &module.items { graph.insert(item.name().to_owned(), item_references(item)); @@ -427,6 +429,7 @@ fn struct_field_names(strukt: &Struct, out: &mut Vec) { #[cfg(test)] mod tests { use super::*; + use crate::ir::Access; use crate::ir::Alias; use crate::ir::Body; use crate::ir::Field; @@ -457,6 +460,7 @@ mod tests { serde_skip: false, default: None, constraints: None, + access: Access::ReadWrite, }; } diff --git a/crates/oapi-codegen/src/ir.rs b/crates/oapi-codegen/src/ir.rs index bc1c396..a3b6ad2 100644 --- a/crates/oapi-codegen/src/ir.rs +++ b/crates/oapi-codegen/src/ir.rs @@ -100,6 +100,51 @@ pub struct Field { /// The generator checks these on the way in, so the check runs only where /// the code deserializes. A response the server writes is not checked. pub constraints: Option, + /// Which direction of an exchange carries the property, from `readOnly` and + /// `writeOnly`. + pub access: Access, +} + +/// One direction of an exchange. +/// +/// A request travels from the client to the server. A response travels back. +/// The direction does not name a serde trait, because a server deserializes a +/// request and serializes a response, and a client does the opposite. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Direction { + /// The payload an operation reads: parameters and the request body. + Request, + /// The payload an operation writes: the response body and its headers. + Response, +} + +/// Which direction of an exchange carries a property. +/// +/// `readOnly` gives [`Access::ReadOnly`] and `writeOnly` gives +/// [`Access::WriteOnly`]. [`crate::lower::direction`] then drops a property that +/// the direction of a shape does not carry. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[non_exhaustive] +pub enum Access { + /// Both directions carry the property. + #[default] + ReadWrite, + /// A response carries the property, and a request must not. + ReadOnly, + /// A request carries the property, and a response must not. + WriteOnly, +} + +impl Access { + /// Whether `direction` carries a property with this access. + pub fn carried_by(self, direction: Direction) -> bool { + return match (self, direction) { + (Access::ReadWrite, _) => true, + (Access::ReadOnly, Direction::Response) | (Access::WriteOnly, Direction::Request) => true, + (Access::ReadOnly, Direction::Request) | (Access::WriteOnly, Direction::Response) => false, + }; + } } /// A numeric bound, in the form the document writes it. diff --git a/crates/oapi-codegen/src/lib.rs b/crates/oapi-codegen/src/lib.rs index 5bbff09..3213328 100644 --- a/crates/oapi-codegen/src/lib.rs +++ b/crates/oapi-codegen/src/lib.rs @@ -101,6 +101,9 @@ fn lower_spec(spec_path: &Path, config: &Config) -> Result { .unwrap_or(crate::config::DEFAULT_RESPONSE_SUFFIX); let mut service = lower::generate_service(&spec, &config.import_mapping, response_type_suffix)?; lower::rewrite_service(&mut service, names.renames()); + // Runs before the prune pass, which drops a shape no operation reaches, + // and before the name checks, which then see the projected names. + lower::split_by_direction(&mut module, Some(&mut service)); if !config.output_options.skip_prune { lower::prune_unused_models(&mut module, &service); } @@ -128,6 +131,7 @@ fn lower_spec(spec_path: &Path, config: &Config) -> Result { } // Models-only generation prunes nothing, so the module holds every schema and // every collision reports. + lower::split_by_direction(&mut module, None); names.check_emitted(&module)?; lower::check_duplicate_models(&module)?; lower::check_prelude_shadowing(&module, emit::Targets::default())?; @@ -216,6 +220,7 @@ pub fn generate_models_string(spec_path: &Path) -> Result { let spec = Spec::load(spec_path)?; let names = lower::type_renames(&spec, None)?; let mut module = lower::generate_models(&spec, &names)?; + lower::split_by_direction(&mut module, None); // Every schema becomes an item here, so every collision reaches the file. names.check_emitted(&module)?; lower::check_duplicate_models(&module)?; diff --git a/crates/oapi-codegen/src/loader.rs b/crates/oapi-codegen/src/loader.rs index 352b68a..c063827 100644 --- a/crates/oapi-codegen/src/loader.rs +++ b/crates/oapi-codegen/src/loader.rs @@ -16,6 +16,8 @@ use openapiv3::Schema; use crate::error::Error; use crate::error::Result; +use crate::lower::direction::REQUEST_SUFFIX; +use crate::lower::direction::RESPONSE_SUFFIX; /// Maximum `$ref` chain length before bailing out (cycle guard). const MAX_REF_DEPTH: usize = 32; @@ -441,17 +443,33 @@ impl Spec { /// `type-name-suffix` it takes from its own config. This run cannot see that /// config, so it emits the plain name and reaches whichever of the two /// schemas kept it. That builds, and it carries the wrong type. + /// + /// A schema the direction pass splits is an error for the same reason. The + /// other run emits two names there, and this one cannot tell which of the + /// two an `import-mapping` reference means. pub fn external_schema_name(&self, file: &str, name: &str, reference: &str) -> Result { let entry = self.component_schema(Some(file), reference, name)?; let chosen = external_name_of(&entry, name)?; let doc = self.document_for(file)?; let schemas = doc.components.as_ref().map(|components| return &components.schemas); + let ident = crate::naming::to_ident(&chosen, crate::naming::Case::Pascal); + if direction_split_schemas(&doc).contains(name) { + let shape = ident.logical(); + return Err(Error::UnsupportedRef { + reference: reference.to_owned(), + reason: format!( + "`{name}` in `{file}` marks a property `readOnly` or `writeOnly`, so the run that \ + generates that file emits it as `{shape}{REQUEST_SUFFIX}` and `{shape}{RESPONSE_SUFFIX}`. \ + This run cannot tell which of the two an `import-mapping` reference means. Declare \ + the schema in this document instead, or drop the mark" + ), + }); + } for (other, other_entry) in schemas.into_iter().flatten() { if other == name { continue; } let taken = external_name_of(other_entry, other)?; - let ident = crate::naming::to_ident(&chosen, crate::naming::Case::Pascal); if crate::naming::to_ident(&taken, crate::naming::Case::Pascal).logical() == ident.logical() { return Err(Error::UnsupportedRef { reference: reference.to_owned(), @@ -466,6 +484,193 @@ impl Spec { } } +/// The schemas in `doc` that the direction pass splits into a request shape and +/// a response shape. +/// +/// A schema qualifies when a property inside it carries a direction mark, or +/// when it reaches such a schema through a same-document `$ref`. +/// +/// A mark on a schema itself does not qualify that schema. The direction pass +/// splits a model only when one of its properties is directional, so a marked +/// primitive keeps its one name and only its referrers split. +/// +/// This reads the document alone, because both runs that compose a crate must +/// reach the same verdict. Only one of the two holds the operations. +fn direction_split_schemas(doc: &OpenAPI) -> std::collections::BTreeSet { + let mut marked = std::collections::BTreeSet::new(); + let Some(components) = doc.components.as_ref() else { + return marked; + }; + + let mut directional = std::collections::BTreeSet::new(); + for (name, entry) in &components.schemas { + if let ReferenceOr::Item(schema) = entry + && (schema.schema_data.read_only || schema.schema_data.write_only) + { + directional.insert(name.clone()); + } + } + + let mut referrers: HashMap> = HashMap::new(); + for (name, entry) in &components.schemas { + match entry { + ReferenceOr::Item(schema) => { + if schema_marks_a_direction(schema, &directional) { + marked.insert(name.clone()); + } + let mut targets = Vec::new(); + schema_local_refs(schema, &mut targets); + for target in targets { + referrers.entry(target).or_default().push(name.clone()); + } + } + // A component declared as a `$ref` is an alias. An alias to a split + // model splits as well, so it has to reach the closure below. + ReferenceOr::Reference { reference } => { + if let Some(target) = ref_target_name(reference) { + referrers.entry(target.to_owned()).or_default().push(name.clone()); + } + } + } + } + + let mut stack: Vec = marked.iter().cloned().collect(); + while let Some(name) = stack.pop() { + let Some(parents) = referrers.get(&name) else { + continue; + }; + for parent in parents { + if marked.insert(parent.clone()) { + stack.push(parent.clone()); + } + } + } + return marked; +} + +/// Whether a property of `schema`, or of a schema written inside it, carries a +/// direction mark. +/// +/// `directional` holds the component schemas that set `readOnly` or `writeOnly` +/// on themselves. A property that names one of those carries the mark too. +fn schema_marks_a_direction(schema: &Schema, directional: &std::collections::BTreeSet) -> bool { + let mut found = declares_a_directional_property(schema, directional); + walk_inline_schemas(schema, &mut |inner| { + found = found || declares_a_directional_property(inner, directional); + }); + return found; +} + +/// Whether one schema declares a property that only one direction carries. +fn declares_a_directional_property(schema: &Schema, directional: &std::collections::BTreeSet) -> bool { + return object_properties(schema).iter().any(|entry| { + return match entry { + ReferenceOr::Item(inner) => inner.schema_data.read_only || inner.schema_data.write_only, + ReferenceOr::Reference { reference } => { + return ref_file_part(reference).is_none() + && ref_component_name(reference, "schemas").is_some_and(|name| return directional.contains(name)); + } + }; + }); +} + +/// The schemas one schema declares as properties. +fn object_properties(schema: &Schema) -> Vec> { + let unbox = |entry: &ReferenceOr>| { + return match entry { + ReferenceOr::Item(inner) => ReferenceOr::Item((**inner).clone()), + ReferenceOr::Reference { reference } => ReferenceOr::Reference { + reference: reference.clone(), + }, + }; + }; + return match &schema.schema_kind { + openapiv3::SchemaKind::Type(openapiv3::Type::Object(object)) => { + return object.properties.values().map(&unbox).collect(); + } + openapiv3::SchemaKind::Any(any) => return any.properties.values().map(&unbox).collect(), + openapiv3::SchemaKind::Type(_) + | openapiv3::SchemaKind::OneOf { .. } + | openapiv3::SchemaKind::AllOf { .. } + | openapiv3::SchemaKind::AnyOf { .. } + | openapiv3::SchemaKind::Not { .. } => Vec::new(), + }; +} + +/// The same-document component schemas a schema names, at any depth. +fn schema_local_refs(schema: &Schema, out: &mut Vec) { + let mut collect = |entry: &ReferenceOr| { + if let ReferenceOr::Reference { reference } = entry + && ref_file_part(reference).is_none() + && let Some(name) = ref_component_name(reference, "schemas") + { + out.push(name.to_owned()); + } + }; + for entry in inline_members(schema) { + collect(&entry); + } + walk_inline_schemas(schema, &mut |inner| { + for entry in inline_members(inner) { + collect(&entry); + } + }); +} + +/// Apply `visit` to every schema written inline within `schema`, at any depth. +fn walk_inline_schemas(schema: &Schema, visit: &mut impl FnMut(&Schema)) { + for entry in inline_members(schema) { + if let ReferenceOr::Item(inner) = entry { + visit(&inner); + walk_inline_schemas(&inner, visit); + } + } +} + +/// The schemas one schema holds directly: its properties, its element type, its +/// `additionalProperties`, and its composition members. +fn inline_members(schema: &Schema) -> Vec> { + let unbox = |entry: &ReferenceOr>| { + return match entry { + ReferenceOr::Item(inner) => ReferenceOr::Item((**inner).clone()), + ReferenceOr::Reference { reference } => ReferenceOr::Reference { + reference: reference.clone(), + }, + }; + }; + let additional = |entry: &Option| { + return match entry { + Some(openapiv3::AdditionalProperties::Schema(inner)) => vec![(**inner).clone()], + Some(openapiv3::AdditionalProperties::Any(_)) | None => Vec::new(), + }; + }; + let mut members = Vec::new(); + match &schema.schema_kind { + openapiv3::SchemaKind::Type(openapiv3::Type::Object(object)) => { + members.extend(object.properties.values().map(&unbox)); + members.extend(additional(&object.additional_properties)); + } + openapiv3::SchemaKind::Type(openapiv3::Type::Array(array)) => { + members.extend(array.items.as_ref().map(&unbox)); + } + openapiv3::SchemaKind::Type(_) => {} + openapiv3::SchemaKind::OneOf { one_of } => members.extend(one_of.iter().cloned()), + openapiv3::SchemaKind::AllOf { all_of } => members.extend(all_of.iter().cloned()), + openapiv3::SchemaKind::AnyOf { any_of } => members.extend(any_of.iter().cloned()), + openapiv3::SchemaKind::Not { not } => members.push((**not).clone()), + openapiv3::SchemaKind::Any(any) => { + members.extend(any.properties.values().map(&unbox)); + members.extend(additional(&any.additional_properties)); + members.extend(any.items.as_ref().map(&unbox)); + members.extend(any.one_of.iter().cloned()); + members.extend(any.all_of.iter().cloned()); + members.extend(any.any_of.iter().cloned()); + members.extend(any.not.as_ref().map(|not| return (**not).clone())); + } + } + return members; +} + /// The name a schema in a referenced document declares for itself, honouring /// `x-rust-name`. fn external_name_of(entry: &ReferenceOr, name: &str) -> Result { @@ -851,4 +1056,58 @@ mod tests { openapiv3::SchemaKind::Type(openapiv3::Type::Integer(_)) )); } + + /// A mark on a schema itself splits the schemas that name it, not the schema. + /// + /// The direction pass splits a model only when one of its properties is + /// directional, so a marked primitive stays one type alias. Reporting it as + /// split rejects an `import-mapping` reference that resolves. + #[test] + fn a_schema_level_mark_splits_only_the_referrers() { + let doc: OpenAPI = serde_yaml::from_str( + "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths: {}\ncomponents:\n schemas:\n Marked:\n type: string\n readOnly: true\n Holder:\n type: object\n properties:\n a:\n $ref: '#/components/schemas/Marked'\n Bystander:\n type: object\n properties:\n b:\n type: string\n", + ) + .expect("parse doc"); + + let split = direction_split_schemas(&doc); + assert!(split.contains("Holder"), "a property that names a marked schema splits"); + assert!(!split.contains("Marked"), "a marked primitive keeps its one name"); + assert!(!split.contains("Bystander"), "an unrelated schema keeps its one name"); + } + + /// A component declared as a `$ref` is an alias, and an alias to a split + /// model splits as well. + /// + /// Missing one emits a reference to a name the models run never writes, so + /// the composed crate does not build. + #[test] + fn an_alias_to_a_split_model_splits_as_well() { + let doc: OpenAPI = serde_yaml::from_str( + "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths: {}\ncomponents:\n schemas:\n Marked:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n Alias:\n $ref: '#/components/schemas/Marked'\n Plain:\n type: object\n properties:\n a:\n type: string\n PlainAlias:\n $ref: '#/components/schemas/Plain'\n", + ) + .expect("parse doc"); + + let split = direction_split_schemas(&doc); + assert!(split.contains("Marked"), "the model that declares the mark splits"); + assert!(split.contains("Alias"), "an alias to a split model splits"); + assert!(!split.contains("Plain"), "an unmarked model keeps its one name"); + assert!( + !split.contains("PlainAlias"), + "an alias to an unmarked model keeps its one name" + ); + } + + /// A mark on a property splits the model that declares it, and every model + /// that reaches it. + #[test] + fn a_property_mark_splits_the_declaring_model_and_its_referrers() { + let doc: OpenAPI = serde_yaml::from_str( + "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths: {}\ncomponents:\n schemas:\n Leaf:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n Parent:\n type: object\n properties:\n leaf:\n $ref: '#/components/schemas/Leaf'\n", + ) + .expect("parse doc"); + + let split = direction_split_schemas(&doc); + assert!(split.contains("Leaf"), "the model that declares the mark splits"); + assert!(split.contains("Parent"), "a model that reaches the mark splits"); + } } diff --git a/crates/oapi-codegen/src/lower/direction.rs b/crates/oapi-codegen/src/lower/direction.rs new file mode 100644 index 0000000..ff82fb4 --- /dev/null +++ b/crates/oapi-codegen/src/lower/direction.rs @@ -0,0 +1,616 @@ +//! The request shape and the response shape that `readOnly` and `writeOnly` +//! describe. +//! +//! OpenAPI marks a property `readOnly` when a response carries it and a request +//! must not, and `writeOnly` for the opposite. The mark names a direction, not a +//! value, so one struct cannot state both. +//! +//! Each model therefore drops the properties that its direction does not carry. +//! A model that one direction reaches keeps its name. A model that both +//! directions reach becomes `Request` and `Response`, and so does +//! every model that both directions reach and that references it. +//! +//! `docs/design.md` states the rest of the reasoning. + +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::HashMap; + +use crate::emit::usage; +use crate::ir::Alias; +use crate::ir::Direction; +use crate::ir::Enum; +use crate::ir::EnumKind; +use crate::ir::Field; +use crate::ir::Item; +use crate::ir::Module; +use crate::ir::RequestPayload; +use crate::ir::ResponseBody; +use crate::ir::RustType; +use crate::ir::Service; +use crate::ir::Struct; +use crate::ir::UnionVariant; +use crate::naming::Case; +use crate::naming::RustIdent; +use crate::naming::to_ident; + +/// The suffix the request shape of a split model takes. +pub const REQUEST_SUFFIX: &str = "Request"; +/// The suffix the response shape of a split model takes. +pub const RESPONSE_SUFFIX: &str = "Response"; + +/// How one direction-sensitive model is projected. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Projection { + /// Both directions reach the model, so it becomes two items with suffixed + /// names. + Split, + /// One direction reaches the model, so it keeps its name and drops only the + /// properties that direction does not carry. + Single(Direction), +} + +/// Project every model a direction mark reaches onto the direction that carries +/// it, and point each API position at the shape it takes. +/// +/// `service` is `None` for a models-only run, which has no operation to give a +/// direction. The pass splits every marked model there, because the run that +/// consumes those models does know the direction. +/// +/// The pass leaves a document that marks no property untouched. +pub fn split_by_direction(module: &mut Module, service: Option<&mut Service>) { + let projections = projections(module, service.as_deref()); + if projections.is_empty() { + return; + } + let mut items = Vec::with_capacity(module.items.len() + projections.len()); + for item in module.items.drain(..) { + match projections.get(item.name()) { + Some(Projection::Split) => { + items.push(project_item(&item, Direction::Request, &projections)); + items.push(project_item(&item, Direction::Response, &projections)); + } + Some(Projection::Single(direction)) => items.push(project_item(&item, *direction, &projections)), + None => items.push(item), + } + } + module.items = items; + if let Some(service) = service { + project_service(service, &projections); + } +} + +/// The name a model takes in one direction. +pub fn projected_name(name: &str, direction: Direction) -> RustIdent { + let suffix = match direction { + Direction::Request => REQUEST_SUFFIX, + Direction::Response => RESPONSE_SUFFIX, + }; + return to_ident(&format!("{name} {suffix}"), Case::Pascal); +} + +/// How each direction-sensitive model is projected, keyed by its logical name. +fn projections(module: &Module, service: Option<&Service>) -> BTreeMap { + let sensitive = sensitive_models(module); + let Some(service) = service else { + return sensitive + .into_iter() + .map(|name| return (name, Projection::Split)) + .collect(); + }; + + let usage = usage::direction_usage(module, service); + return sensitive + .into_iter() + .map(|name| { + let used = usage.get(&name).copied().unwrap_or_default(); + let projection = match (used.request, used.response) { + (true, false) => Projection::Single(Direction::Request), + (false, true) => Projection::Single(Direction::Response), + _ => Projection::Split, + }; + return (name, projection); + }) + .collect(); +} + +/// The models a direction mark reaches: the ones that mark a property, and +/// every model that references one of those. +/// +/// The walk starts at a marked model and follows the reference graph in +/// reverse. The field type of a holder is what changes per direction. +fn sensitive_models(module: &Module) -> BTreeSet { + let mut marked: BTreeSet = module + .items + .iter() + .filter(|item| return marks_a_direction(item)) + .map(|item| return item.name().to_owned()) + .collect(); + if marked.is_empty() { + return marked; + } + + let mut referrers: HashMap> = HashMap::new(); + for (name, targets) in usage::adjacency(module) { + for target in targets { + referrers.entry(target).or_default().push(name.clone()); + } + } + + let mut stack: Vec = marked.iter().cloned().collect(); + while let Some(name) = stack.pop() { + let Some(parents) = referrers.get(&name) else { + continue; + }; + for parent in parents { + if marked.insert(parent.clone()) { + stack.push(parent.clone()); + } + } + } + return marked; +} + +/// Whether an item declares a property that only one direction carries. +fn marks_a_direction(item: &Item) -> bool { + let Item::Struct(strukt) = item else { + return false; + }; + return strukt.fields.iter().any(|field| { + return field.access != crate::ir::Access::ReadWrite; + }); +} + +/// Build the shape of an item for one direction. +/// +/// The name takes the suffix of the direction only when both directions reach +/// the model. The shape drops the properties that the other direction carries, +/// and points every reference to a split model at the shape of that model. +fn project_item(item: &Item, direction: Direction, projections: &BTreeMap) -> Item { + let split = matches!(projections.get(item.name()), Some(Projection::Split)); + let name_of = |name: &RustIdent| { + if split { + return projected_name(name.logical(), direction); + } + return name.clone(); + }; + return match item { + Item::Struct(strukt) => { + let fields: Vec = strukt + .fields + .iter() + .filter(|field| return field.access.carried_by(direction)) + .map(|field| { + let mut projected = field.clone(); + projected.ty = project_type(&field.ty, direction, projections); + return projected; + }) + .collect(); + let dropped = fields.len() < strukt.fields.len(); + Item::Struct(Struct { + name: name_of(&strukt.name), + doc: projected_doc(&strukt.doc, strukt.name.logical(), direction, split, dropped), + fields, + additional_properties: strukt + .additional_properties + .as_ref() + .map(|ty| return project_type(ty, direction, projections)), + ..strukt.clone() + }) + } + Item::Enum(enom) => Item::Enum(Enum { + name: name_of(&enom.name), + doc: projected_doc(&enom.doc, enom.name.logical(), direction, split, false), + kind: match &enom.kind { + EnumKind::Union(variants) => EnumKind::Union( + variants + .iter() + .map(|variant| { + return UnionVariant { + name: variant.name.clone(), + ty: project_type(&variant.ty, direction, projections), + }; + }) + .collect(), + ), + other => other.clone(), + }, + ..enom.clone() + }), + Item::Alias(alias) => Item::Alias(Alias { + name: name_of(&alias.name), + doc: projected_doc(&alias.doc, alias.name.logical(), direction, split, false), + ty: project_type(&alias.ty, direction, projections), + ..alias.clone() + }), + }; +} + +/// Add a line about the direction, after the schema `description`. +/// +/// A split shape always takes a line, because a reader meets two types where the +/// document declares one schema. The line names the keyword only when the +/// projection drops a property. A holder that splits because it references a +/// split model drops nothing, so its line states the direction alone. +/// +/// A model that keeps its name takes a line only when the projection drops a +/// property. So a mark that costs a shape nothing leaves the generated file as +/// it was. +fn projected_doc(doc: &Option, name: &str, direction: Direction, split: bool, dropped: bool) -> Option { + let note = match (split, dropped, direction) { + (true, true, Direction::Request) => { + format!("The request shape of `{name}`. A `readOnly` property is not part of it.") + } + (true, true, Direction::Response) => { + format!("The response shape of `{name}`. A `writeOnly` property is not part of it.") + } + (true, false, Direction::Request) => format!("The request shape of `{name}`."), + (true, false, Direction::Response) => format!("The response shape of `{name}`."), + (false, false, _) => return doc.clone(), + (false, true, Direction::Request) => { + "Only a request carries this model, so a `readOnly` property is not part of it.".to_owned() + } + (false, true, Direction::Response) => { + "Only a response carries this model, so a `writeOnly` property is not part of it.".to_owned() + } + }; + return Some(match doc { + Some(text) => format!("{text}\n\n{note}"), + None => note, + }); +} + +/// Rewrite a type expression so a reference to a split model names the shape of +/// that model for `direction`. This leaves any other type as it is. +fn project_type(ty: &RustType, direction: Direction, projections: &BTreeMap) -> RustType { + return match ty { + RustType::Named(name) => { + // A `RustType::Named` holds the schema name the document wrote, so the + // lookup canonicalizes it. Without that, a reference to `order-item` + // would miss the `OrderItem` entry and keep the unsplit name. + let canonical = to_ident(name, Case::Pascal); + match projections.get(canonical.logical()) { + Some(Projection::Split) => { + RustType::Named(projected_name(canonical.logical(), direction).logical().to_owned()) + } + _ => ty.clone(), + } + } + RustType::Vec(inner) => RustType::Vec(Box::new(project_type(inner, direction, projections))), + RustType::Map(inner) => RustType::Map(Box::new(project_type(inner, direction, projections))), + RustType::Option(inner) => RustType::Option(Box::new(project_type(inner, direction, projections))), + RustType::Boxed(inner) => RustType::Boxed(Box::new(project_type(inner, direction, projections))), + other => other.clone(), + }; +} + +/// Point every API position at the shape of the direction it carries. +fn project_service(service: &mut Service, projections: &BTreeMap) { + for operation in &mut service.operations { + for param in &mut operation.path_params { + param.ty = project_type(¶m.ty, Direction::Request, projections); + } + if let Some(query) = &mut operation.query { + project_struct(query, Direction::Request, projections); + } + if let Some(headers) = &mut operation.headers { + for param in &mut headers.params { + param.ty = project_type(¶m.ty, Direction::Request, projections); + } + } + if let Some(cookies) = &mut operation.cookies { + for param in &mut cookies.params { + param.ty = project_type(¶m.ty, Direction::Request, projections); + } + } + if let Some(request) = &mut operation.request { + match request { + RequestPayload::Single(body) => body.ty = project_type(&body.ty, Direction::Request, projections), + RequestPayload::Multipart(multipart) => { + for field in &mut multipart.fields { + field.ty = project_type(&field.ty, Direction::Request, projections); + } + } + RequestPayload::Negotiated(negotiated) => { + for variant in &mut negotiated.variants { + variant.body.ty = project_type(&variant.body.ty, Direction::Request, projections); + } + } + } + } + for case in &mut operation.responses { + for header in &mut case.headers { + header.ty = project_type(&header.ty, Direction::Response, projections); + } + match &mut case.body { + Some(ResponseBody::Single(body)) => body.ty = project_type(&body.ty, Direction::Response, projections), + Some(ResponseBody::Negotiated(negotiated)) => { + for variant in &mut negotiated.variants { + variant.body.ty = project_type(&variant.body.ty, Direction::Response, projections); + } + } + None => {} + } + } + } +} + +/// Rewrite the type of every field of a per-operation struct. +fn project_struct(strukt: &mut Struct, direction: Direction, projections: &BTreeMap) { + for field in &mut strukt.fields { + field.ty = project_type(&field.ty, direction, projections); + } + if let Some(additional) = &mut strukt.additional_properties { + *additional = project_type(additional, direction, projections); + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + use crate::ir::Access; + use crate::ir::Field; + use crate::loader::Spec; + + /// Lower the schemas of an inline document and split them, the way a + /// models-only run does. + fn split_yaml(yaml: &str) -> Module { + let doc: openapiv3::OpenAPI = serde_yaml::from_str(yaml).expect("parse spec"); + let spec = Spec::from_parts(doc, PathBuf::from("inline.yaml")); + let names = crate::lower::rename::type_renames(&spec, None).expect("resolve names"); + let mut module = crate::lower::generate_models(&spec, &names).expect("lower schemas"); + split_by_direction(&mut module, None); + return module; + } + + /// Lower an inline document with its operations and split them, the way a + /// server or client run does. + fn split_service_yaml(yaml: &str) -> Module { + let doc: openapiv3::OpenAPI = serde_yaml::from_str(yaml).expect("parse spec"); + let spec = Spec::from_parts(doc, PathBuf::from("inline.yaml")); + let names = crate::lower::rename::type_renames(&spec, None).expect("resolve names"); + let mut module = crate::lower::generate_models(&spec, &names).expect("lower schemas"); + let mut service = + crate::lower::generate_service(&spec, &Default::default(), "Response").expect("lower operations"); + crate::lower::rewrite_service(&mut service, names.renames()); + split_by_direction(&mut module, Some(&mut service)); + return module; + } + + /// The doc text of the item called `name`. + fn doc(module: &Module, name: &str) -> Option { + for item in &module.items { + if item.name() == name { + return match item { + Item::Struct(strukt) => strukt.doc.clone(), + Item::Enum(enom) => enom.doc.clone(), + Item::Alias(alias) => alias.doc.clone(), + }; + } + } + panic!("no item named `{name}`"); + } + + /// A document whose only operation returns `Account`, so no request reaches + /// it. + fn response_only(schemas: &str) -> String { + return format!( + "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths:\n /accounts:\n get:\n operationId: listAccounts\n responses:\n '200':\n description: ok\n content:\n application/json:\n schema:\n $ref: '#/components/schemas/Account'\ncomponents:\n schemas:\n{schemas}" + ); + } + + /// The item names a module declares, in emission order. + fn names(module: &Module) -> Vec { + return module.items.iter().map(|item| return item.name().to_owned()).collect(); + } + + /// The field names of the struct called `name`. + fn fields(module: &Module, name: &str) -> Vec { + for item in &module.items { + if let Item::Struct(strukt) = item + && strukt.name.logical() == name + { + return strukt + .fields + .iter() + .map(|field| return field.name.logical().to_owned()) + .collect(); + } + } + panic!("no struct named `{name}`"); + } + + const PREAMBLE: &str = "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths: {}\ncomponents:\n schemas:\n"; + + #[test] + fn one_direction_keeps_the_name_and_drops_nothing_it_carries() { + let module = split_service_yaml(&response_only( + " Account:\n description: An account.\n type: object\n properties:\n id:\n type: string\n readOnly: true\n email:\n type: string\n", + )); + assert_eq!(names(&module), vec!["Account"]); + assert_eq!(fields(&module, "Account"), vec!["id", "email"]); + assert_eq!(doc(&module, "Account"), Some("An account.".to_owned())); + } + + #[test] + fn one_direction_keeps_the_name_and_drops_what_it_cannot_carry() { + let module = split_service_yaml(&response_only( + " Account:\n type: object\n properties:\n email:\n type: string\n secret:\n type: string\n writeOnly: true\n", + )); + assert_eq!(names(&module), vec!["Account"]); + assert_eq!(fields(&module, "Account"), vec!["email"]); + assert_eq!( + doc(&module, "Account"), + Some("Only a response carries this model, so a `writeOnly` property is not part of it.".to_owned()) + ); + } + + /// A split shape names the keyword only when it drops a property. + /// + /// A holder splits because it references a split model. It drops nothing, so + /// a line about a `readOnly` property would state a fault that is not there. + #[test] + fn a_split_shape_that_drops_nothing_states_the_direction_alone() { + let module = split_yaml(&format!( + "{PREAMBLE} Account:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n Envelope:\n type: object\n properties:\n account:\n $ref: '#/components/schemas/Account'\n" + )); + assert_eq!( + doc(&module, "AccountRequest"), + Some("The request shape of `Account`. A `readOnly` property is not part of it.".to_owned()) + ); + assert_eq!( + doc(&module, "AccountResponse"), + Some("The response shape of `Account`.".to_owned()), + "the response drops nothing, because no property is `writeOnly`", + ); + assert_eq!( + doc(&module, "EnvelopeRequest"), + Some("The request shape of `Envelope`.".to_owned()) + ); + assert_eq!( + doc(&module, "EnvelopeResponse"), + Some("The response shape of `Envelope`.".to_owned()) + ); + } + + #[test] + fn a_holder_that_one_direction_reaches_keeps_its_name_and_names_the_split_shape() { + let yaml = "openapi: 3.0.3\ninfo:\n title: t\n version: '1'\npaths:\n /accounts:\n post:\n operationId: createAccount\n requestBody:\n content:\n application/json:\n schema:\n $ref: '#/components/schemas/Account'\n responses:\n '200':\n description: ok\n content:\n application/json:\n schema:\n $ref: '#/components/schemas/Page'\ncomponents:\n schemas:\n Account:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n email:\n type: string\n Page:\n type: object\n properties:\n items:\n type: array\n items:\n $ref: '#/components/schemas/Account'\n"; + let module = split_service_yaml(yaml); + assert_eq!(names(&module), vec!["AccountRequest", "AccountResponse", "Page"]); + let Some(Item::Struct(page)) = module.items.iter().find(|item| return item.name() == "Page") else { + panic!("no struct named `Page`"); + }; + assert_eq!(page.fields[0].ty.label(), "Option>"); + } + + #[test] + fn a_document_with_no_mark_is_left_alone() { + let module = split_yaml(&format!( + "{PREAMBLE} Widget:\n type: object\n properties:\n name:\n type: string\n" + )); + assert_eq!(names(&module), vec!["Widget"]); + } + + #[test] + fn a_marked_property_leaves_the_shape_the_other_direction_carries() { + let module = split_yaml(&format!( + "{PREAMBLE} Account:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n secret:\n type: string\n writeOnly: true\n email:\n type: string\n" + )); + assert_eq!(names(&module), vec!["AccountRequest", "AccountResponse"]); + assert_eq!(fields(&module, "AccountRequest"), vec!["secret", "email"]); + assert_eq!(fields(&module, "AccountResponse"), vec!["id", "email"]); + } + + #[test] + fn a_mark_on_a_referenced_schema_reaches_the_property_that_names_it() { + let module = split_yaml(&format!( + "{PREAMBLE} Id:\n type: string\n readOnly: true\n Account:\n type: object\n properties:\n id:\n $ref: '#/components/schemas/Id'\n email:\n type: string\n" + )); + assert_eq!(fields(&module, "AccountRequest"), vec!["email"]); + assert_eq!(fields(&module, "AccountResponse"), vec!["id", "email"]); + } + + #[test] + fn an_all_of_member_carries_its_marks_into_the_merged_shape() { + let module = split_yaml(&format!( + "{PREAMBLE} Timestamps:\n type: object\n properties:\n createdAt:\n type: string\n readOnly: true\n Account:\n allOf:\n - $ref: '#/components/schemas/Timestamps'\n - type: object\n properties:\n email:\n type: string\n" + )); + assert_eq!(fields(&module, "AccountRequest"), vec!["email"]); + assert_eq!(fields(&module, "AccountResponse"), vec!["created_at", "email"]); + } + + #[test] + fn a_holder_of_a_split_model_splits_and_names_the_matching_shape() { + let module = split_yaml(&format!( + "{PREAMBLE} Account:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n Envelope:\n type: object\n properties:\n account:\n $ref: '#/components/schemas/Account'\n" + )); + assert_eq!( + names(&module), + vec![ + "AccountRequest", + "AccountResponse", + "EnvelopeRequest", + "EnvelopeResponse" + ] + ); + let request = module + .items + .iter() + .find(|item| return item.name() == "EnvelopeRequest") + .expect("request shape"); + let Item::Struct(strukt) = request else { + panic!("expected a struct"); + }; + assert_eq!( + strukt.fields.first().map(|field| return field.ty.label()), + Some("Option".to_owned()) + ); + } + + #[test] + fn both_marks_on_one_property_are_rejected() { + let doc: openapiv3::OpenAPI = serde_yaml::from_str(&format!( + "{PREAMBLE} Account:\n type: object\n properties:\n secret:\n type: string\n readOnly: true\n writeOnly: true\n" + )) + .expect("parse spec"); + let spec = Spec::from_parts(doc, PathBuf::from("inline.yaml")); + let names = crate::lower::rename::type_renames(&spec, None).expect("resolve names"); + let error = crate::lower::generate_models(&spec, &names).expect_err("both marks must be rejected"); + assert!( + format!("{error}").contains("`readOnly` and `writeOnly` are both set"), + "unexpected message: {error}" + ); + } + + #[test] + fn a_name_that_is_not_pascal_case_still_matches_the_split_set() { + // A `RustType::Named` holds the schema name the document wrote, so the + // lookup canonicalizes it. Without that, a reference to `order-item` + // would miss the `OrderItem` entry and keep the unsplit name. + let module = split_yaml(&format!( + "{PREAMBLE} order-item:\n type: object\n properties:\n id:\n type: string\n readOnly: true\n Basket:\n type: object\n properties:\n item:\n $ref: '#/components/schemas/order-item'\n" + )); + assert!(names(&module).contains(&"OrderItemRequest".to_owned())); + let Some(Item::Struct(basket)) = module.items.iter().find(|item| return item.name() == "BasketRequest") else { + panic!("no request shape for `Basket`"); + }; + assert_eq!( + basket.fields.first().map(|field| return field.ty.label()), + Some("Option".to_owned()) + ); + } + + #[test] + fn access_reports_the_direction_that_carries_the_property() { + let cases = [ + (Access::ReadWrite, true, true), + (Access::ReadOnly, false, true), + (Access::WriteOnly, true, false), + ]; + for (access, request, response) in cases { + assert_eq!(access.carried_by(Direction::Request), request, "{access:?} request"); + assert_eq!(access.carried_by(Direction::Response), response, "{access:?} response"); + } + } + + #[test] + fn a_field_with_no_mark_keeps_the_default_access() { + let field = Field { + name: to_ident("name", Case::Snake), + rename: None, + doc: None, + deprecated: None, + ty: RustType::String, + required: true, + omit_empty: None, + serde_skip: false, + default: None, + constraints: None, + access: Access::default(), + }; + assert_eq!(field.access, Access::ReadWrite); + } +} diff --git a/crates/oapi-codegen/src/lower/mod.rs b/crates/oapi-codegen/src/lower/mod.rs index 0e82332..f6106b0 100644 --- a/crates/oapi-codegen/src/lower/mod.rs +++ b/crates/oapi-codegen/src/lower/mod.rs @@ -10,6 +10,7 @@ pub(crate) mod constraints; /// Lowering of a schema `default`. Used by [`schema`] and [`paths`] only. pub(crate) mod default; +pub mod direction; /// Readers for the `x-` extensions the generator understands. pub(crate) mod extension; pub mod paths; @@ -21,6 +22,7 @@ pub mod security; pub mod servers; pub mod validate; +pub use crate::lower::direction::split_by_direction; pub use crate::lower::paths::generate_service; pub use crate::lower::prune::prune_unused_models; pub use crate::lower::recurse::box_recursive_types; diff --git a/crates/oapi-codegen/src/lower/paths.rs b/crates/oapi-codegen/src/lower/paths.rs index c7b56f7..d28b96d 100644 --- a/crates/oapi-codegen/src/lower/paths.rs +++ b/crates/oapi-codegen/src/lower/paths.rs @@ -50,6 +50,7 @@ use openapiv3::ReferenceOr; use openapiv3::RequestBody; use openapiv3::Response as OasResponse; use openapiv3::Schema; +use openapiv3::SchemaData; use openapiv3::SchemaKind; use openapiv3::StatusCode; use openapiv3::Type; @@ -440,6 +441,9 @@ impl Lowerer<'_> { serde_skip: false, default, constraints, + // A parameter travels in a request only, so a direction mark on its + // schema states nothing the generator can act on. + access: crate::ir::Access::ReadWrite, }; crate::lower::constraints::check_constraints(&field)?; return Ok(field); @@ -1178,7 +1182,12 @@ impl Lowerer<'_> { return name == wire_name; }); let (kind, nullable) = match property { - ReferenceOr::Item(schema) => (schema.schema_kind.clone(), schema.schema_data.nullable), + ReferenceOr::Item(schema) => { + if !multipart_part_is_sent(&schema.schema_data, path, method, wire_name)? { + continue; + } + (schema.schema_kind.clone(), schema.schema_data.nullable) + } ReferenceOr::Reference { reference } => { if ref_file_part(reference).is_some() { return Err(Error::UnsupportedOperation { @@ -1190,6 +1199,9 @@ impl Lowerer<'_> { }); } let resolved = self.spec.resolve_schema(None, reference)?; + if !multipart_part_is_sent(&resolved.schema_data, path, method, wire_name)? { + continue; + } (resolved.schema_kind, resolved.schema_data.nullable) } }; @@ -1551,6 +1563,17 @@ impl Lowerer<'_> { } } +/// Whether a `multipart/form-data` part travels in a request. +/// +/// A multipart body is a request body only, so the generator leaves a `readOnly` +/// part out of the extractor. `writeOnly` states what a multipart part already +/// does. +fn multipart_part_is_sent(data: &SchemaData, path: &str, method: &str, wire_name: &str) -> Result { + let at = format!("{method} {path} multipart field `{wire_name}`"); + let access = crate::lower::schema::access_of(data, &at)?; + return Ok(access.carried_by(crate::ir::Direction::Request)); +} + /// Derive the trait method name: an explicit `x-rust-name`, else the /// `operationId`, else a name synthesised from the method and path (for example /// `get /v1/widgets` -> `get_v1_widgets`). diff --git a/crates/oapi-codegen/src/lower/recurse.rs b/crates/oapi-codegen/src/lower/recurse.rs index 69058c1..a46d738 100644 --- a/crates/oapi-codegen/src/lower/recurse.rs +++ b/crates/oapi-codegen/src/lower/recurse.rs @@ -426,6 +426,7 @@ fn canonical(name: &str) -> String { #[cfg(test)] mod tests { use super::*; + use crate::ir::Access; use crate::ir::Alias; use crate::ir::Enum; use crate::ir::Field; @@ -449,6 +450,7 @@ mod tests { serde_skip: false, default: None, constraints: None, + access: Access::ReadWrite, }], additional_properties: None, deny_unknown_fields: false, diff --git a/crates/oapi-codegen/src/lower/schema.rs b/crates/oapi-codegen/src/lower/schema.rs index cb74c7c..3156b50 100644 --- a/crates/oapi-codegen/src/lower/schema.rs +++ b/crates/oapi-codegen/src/lower/schema.rs @@ -15,6 +15,7 @@ use openapiv3::VariantOrUnknownOrEmpty; use crate::error::Error; use crate::error::Result; +use crate::ir::Access; use crate::ir::Alias; use crate::ir::Deprecation; use crate::ir::Enum; @@ -329,6 +330,15 @@ impl Mapper<'_> { None => to_ident(wire, Case::Snake), }; let rename = crate::naming::rename_for(wire, &ident); + let access = match prop { + ReferenceOr::Item(schema) => access_of(&schema.schema_data, &at)?, + // A `$ref` property carries no sibling keyword in OpenAPI 3.0, so + // the mark can only sit on the target. + ReferenceOr::Reference { reference } => match self.spec.resolve(reference) { + Ok(target) => access_of(&target.schema_data, &at)?, + Err(_) => Access::ReadWrite, + }, + }; let constraints = match prop { ReferenceOr::Item(schema) => crate::lower::constraints::constraints_of(schema), // The alias a `$ref` makes carries no serde attribute, so the field @@ -350,6 +360,7 @@ impl Mapper<'_> { serde_skip, default, constraints, + access, }; crate::lower::constraints::check_constraints(&field)?; return Ok(field); @@ -1064,6 +1075,24 @@ fn verbatim_type(data: &SchemaData, verbatim: &str, path: &str) -> Result Result { + return match (data.read_only, data.write_only) { + (true, true) => Err(Error::UnsupportedSchema { + path: at.to_owned(), + reason: "`readOnly` and `writeOnly` are both set, so no request and no response could \ + carry the property. Set at most one of the two." + .to_owned(), + }), + (true, false) => Ok(Access::ReadOnly), + (false, true) => Ok(Access::WriteOnly), + (false, false) => Ok(Access::ReadWrite), + }; +} + /// Derive a `#[deprecated]` annotation from `deprecated: true` and an optional /// `x-deprecated-reason` note. Returns `None` unless the schema is deprecated, /// so a lone `x-deprecated-reason` is a no-op (matching `oapi-codegen`). diff --git a/crates/oapi-codegen/tests/coverage.rs b/crates/oapi-codegen/tests/coverage.rs index e614917..7017487 100644 --- a/crates/oapi-codegen/tests/coverage.rs +++ b/crates/oapi-codegen/tests/coverage.rs @@ -333,13 +333,18 @@ const TEST_TABLE: &[Feature] = &[ }, Feature { element: "meta.readOnly", - status: Status::Ignored, - fixture: None, + status: Status::Supported, + fixture: Some("read_write_only"), }, Feature { element: "meta.writeOnly", - status: Status::Ignored, - fixture: None, + status: Status::Supported, + fixture: Some("read_write_only"), + }, + Feature { + element: "meta.readOnly.with.writeOnly", + status: Status::Unsupported, + fixture: Some("read_write_only_conflict"), }, Feature { element: "meta.example", @@ -555,6 +560,7 @@ const SERVER_UNSUPPORTED_FIXTURES: &[&str] = &[ "server_unsupported_xfile_missing_component", "server_unsupported_xfile_missing_schema", "server_unsupported_xfile_name_clash", + "server_unsupported_xfile_direction_split", "server_unsupported_xfile_no_import_mapping", "server_unsupported_xfile_object_path_param", "server_unsupported_object_response_header", @@ -601,6 +607,7 @@ const CLIENT_UNSUPPORTED_FIXTURES: &[&str] = &[ /// same per-operation types alongside the component models. const COMBINED_FIXTURES: &[&str] = &[ "combined_keyword_operations", + "combined_read_write_only", "combined_prelude_value_names", "combined_server_client", "combined_response_name_collision", @@ -742,6 +749,7 @@ generated_tests!( compose_shared, primitive_scalars, recursive_schema, + read_write_only, ref_local, integer_enum, string_enum, @@ -777,6 +785,10 @@ fn server_config() -> oapi_codegen::Config { "schemas/compose_clash.yaml".to_owned(), "crate::generated::compose_clash".to_owned(), ); + import_mapping.insert( + "schemas/compose_marked.yaml".to_owned(), + "crate::generated::compose_marked".to_owned(), + ); return oapi_codegen::Config { generate: oapi_codegen::config::Generate { std_http_server: true, @@ -887,6 +899,49 @@ fn server_unsupported_features_are_rejected() { } } +/// A property that sets both direction marks is rejected **for that**, and not +/// for whatever else the lowering pass trips over first. +/// +/// `unsupported_features_are_rejected` asserts `is_err()` only, so it passes on +/// any error at all. The OpenAPI specification calls the combination invalid, so +/// the message must name both keywords and leave the author with one remedy. +#[test] +fn a_property_that_sets_both_direction_marks_names_both_in_the_message() { + let fixture = tests_dir().join("fixtures").join("read_write_only_conflict.yaml"); + let error = oapi_codegen::generate_models_string(&fixture).expect_err("both marks must be rejected"); + let message = format!("{error}"); + assert!( + message.contains("`readOnly` and `writeOnly` are both set"), + "the message must name both keywords: {message}", + ); +} + +/// A cross-file reference to a schema the direction pass splits is rejected, and +/// the message names the two shapes the other run emits. +/// +/// The run that writes the operations resolves the reference by name alone, so +/// it cannot tell which shape the author meant. Emitting the plain name compiles +/// nothing, because the models crate declares neither. +/// +/// The target carries an `x-rust-name`, so the message must name the shapes that +/// the chosen name builds, not the shapes that the schema name builds. +#[test] +fn a_cross_file_reference_to_a_split_schema_is_rejected() { + let fixture = tests_dir() + .join("fixtures") + .join("server_unsupported_xfile_direction_split.yaml"); + let error = oapi_codegen::generate(&fixture, &server_config()).expect_err("a split target must be rejected"); + let message = format!("{error}"); + assert!( + message.contains("ShipmentRequest") && message.contains("ShipmentResponse"), + "the message must name both shapes: {message}", + ); + assert!( + !message.contains("ParcelRequest") && !message.contains("ParcelResponse"), + "the message must not name a shape the models run never emits: {message}", + ); +} + /// Unused component schemas are pruned by default, but retained when /// `output-options.skip-prune` is set — mirroring `oapi-codegen`'s pruning. #[test] @@ -1276,6 +1331,7 @@ macro_rules! combined_generated_tests { combined_generated_tests!( combined_keyword_operations, + combined_read_write_only, combined_prelude_value_names, combined_server_client, combined_response_name_collision, diff --git a/crates/oapi-codegen/tests/fixtures/combined_read_write_only.yaml b/crates/oapi-codegen/tests/fixtures/combined_read_write_only.yaml new file mode 100644 index 0000000..1b09f14 --- /dev/null +++ b/crates/oapi-codegen/tests/fixtures/combined_read_write_only.yaml @@ -0,0 +1,138 @@ +openapi: 3.0.3 +info: + title: Combined Accounts API + version: 1.0.0 +paths: + /accounts: + post: + operationId: createAccount + summary: Create an account. + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/Account" + responses: + "201": + description: The created account. + content: + application/json: + schema: + $ref: "#/components/schemas/Account" + get: + operationId: listAccounts + summary: List every account. + responses: + "200": + description: The accounts, wrapped in a page. + content: + application/json: + schema: + $ref: "#/components/schemas/AccountPage" + /accounts/search: + post: + operationId: searchAccounts + summary: Search accounts. + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/AccountFilter" + responses: + "204": + description: The search was accepted. + /audit: + get: + operationId: listAudit + summary: List the audit log. + responses: + "200": + description: The audit entries. + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/AuditEntry" + /accounts/{accountId}/avatar: + put: + operationId: uploadAvatar + summary: Replace an account avatar. + parameters: + - name: accountId + in: path + required: true + schema: + type: string + requestBody: + required: true + content: + multipart/form-data: + schema: + $ref: "#/components/schemas/Avatar" + responses: + "204": + description: The avatar was stored. +components: + schemas: + Account: + description: An account, used as a request body and as a response body. + type: object + required: [id, email, password] + properties: + id: + description: Assigned by the server, so only a response carries it. + type: string + readOnly: true + email: + type: string + password: + description: Sent when the account is created, and never read back. + type: string + writeOnly: true + AccountPage: + description: A response-only holder of a split model. + type: object + required: [items] + properties: + items: + type: array + items: + $ref: "#/components/schemas/Account" + AccountFilter: + description: A request-only model that marks a property `readOnly`. + type: object + required: [email] + properties: + email: + type: string + matched: + description: Only a response could carry this, so the request drops it. + type: integer + format: int32 + readOnly: true + AuditEntry: + description: A response-only model that marks a property `writeOnly`. + type: object + required: [action] + properties: + action: + type: string + token: + description: Only a request could carry this, so the response drops it. + type: string + writeOnly: true + Avatar: + description: A multipart body, which only a request carries. + type: object + required: [image] + properties: + image: + type: string + format: binary + checksum: + description: The server computes this, so the extractor leaves it out. + type: string + readOnly: true diff --git a/crates/oapi-codegen/tests/fixtures/read_write_only.yaml b/crates/oapi-codegen/tests/fixtures/read_write_only.yaml new file mode 100644 index 0000000..8ac1394 --- /dev/null +++ b/crates/oapi-codegen/tests/fixtures/read_write_only.yaml @@ -0,0 +1,51 @@ +openapi: "3.0.3" +info: + title: Read-only and write-only properties + version: "1.0.0" +paths: {} +components: + schemas: + Account: + description: A schema whose marks split it into a request shape and a response shape. + type: object + required: [id, email, password, nickname] + properties: + id: + description: Assigned by the server, so only a response carries it. + type: string + format: uuid + readOnly: true + createdAt: + type: string + format: date-time + readOnly: true + email: + type: string + password: + description: Sent when the account is created, and never read back. + type: string + writeOnly: true + nickname: + type: string + Envelope: + description: A holder splits too, because its field type differs per direction. + type: object + required: [account] + properties: + account: + $ref: "#/components/schemas/Account" + accounts: + type: array + items: + $ref: "#/components/schemas/Account" + AccountList: + description: An alias to a split model splits as well. + type: array + items: + $ref: "#/components/schemas/Account" + Untouched: + description: No mark reaches this schema, so it keeps its name. + type: object + properties: + label: + type: string diff --git a/crates/oapi-codegen/tests/fixtures/read_write_only_conflict.yaml b/crates/oapi-codegen/tests/fixtures/read_write_only_conflict.yaml new file mode 100644 index 0000000..37b8d30 --- /dev/null +++ b/crates/oapi-codegen/tests/fixtures/read_write_only_conflict.yaml @@ -0,0 +1,16 @@ +openapi: "3.0.3" +info: + title: Both direction marks on one property + version: "1.0.0" +paths: {} +components: + schemas: + Account: + type: object + required: [secret] + properties: + secret: + description: No request and no response could carry this property. + type: string + readOnly: true + writeOnly: true diff --git a/crates/oapi-codegen/tests/fixtures/schemas/compose_marked.yaml b/crates/oapi-codegen/tests/fixtures/schemas/compose_marked.yaml new file mode 100644 index 0000000..5a89035 --- /dev/null +++ b/crates/oapi-codegen/tests/fixtures/schemas/compose_marked.yaml @@ -0,0 +1,25 @@ +openapi: 3.0.3 +info: + title: shared models that a direction mark splits + version: "1" +paths: {} +components: + schemas: + # `Parcel` marks a property `readOnly`, so the run that writes the models + # emits `ShipmentRequest` and `ShipmentResponse` and no `Shipment`. The + # `x-rust-name` key sets the base name that the two suffixes extend. + # + # The run that writes the operations resolves an `import-mapping` reference + # by name alone. It cannot tell which of the two shapes the reference means, + # so it rejects the reference instead of emitting a name the models crate + # never declares. + Parcel: + x-rust-name: Shipment + type: object + required: [id, weight_kg] + properties: + id: + type: string + readOnly: true + weight_kg: + type: number diff --git a/crates/oapi-codegen/tests/fixtures/server_unsupported_xfile_direction_split.yaml b/crates/oapi-codegen/tests/fixtures/server_unsupported_xfile_direction_split.yaml new file mode 100644 index 0000000..7ee6ee0 --- /dev/null +++ b/crates/oapi-codegen/tests/fixtures/server_unsupported_xfile_direction_split.yaml @@ -0,0 +1,18 @@ +openapi: 3.0.3 +info: + title: unsupported cross-file schema ref (a direction mark splits the target) + version: "1" +paths: + /parcels: + post: + operationId: acceptParcel + summary: Accept a parcel. + requestBody: + required: true + content: + application/json: + schema: + $ref: "schemas/compose_marked.yaml#/components/schemas/Parcel" + responses: + "201": + description: The parcel was accepted. diff --git a/crates/oapi-codegen/tests/generated.rs b/crates/oapi-codegen/tests/generated.rs index df3fcda..84dd363 100644 --- a/crates/oapi-codegen/tests/generated.rs +++ b/crates/oapi-codegen/tests/generated.rs @@ -34,6 +34,8 @@ mod generated { pub mod combined_keyword_operations; #[path = "combined_prelude_value_names.rs"] pub mod combined_prelude_value_names; + #[path = "combined_read_write_only.rs"] + pub mod combined_read_write_only; #[path = "combined_response_name_collision.rs"] pub mod combined_response_name_collision; #[path = "combined_server_client.rs"] @@ -81,6 +83,8 @@ mod generated { pub mod prelude_value_names; #[path = "primitive_scalars.rs"] pub mod primitive_scalars; + #[path = "read_write_only.rs"] + pub mod read_write_only; #[path = "recursive_schema.rs"] pub mod recursive_schema; #[path = "ref_local.rs"] @@ -249,6 +253,102 @@ fn optional_field_is_skipped_when_none() { assert_eq!(json, r#"{"id":"u1"}"#); } +#[test] +fn a_direction_mark_splits_a_model_into_two_shapes() { + use generated::read_write_only; + + // The request shape drops what only a response carries, so a payload the + // client builds cannot state a server-assigned value. + let request = read_write_only::AccountRequest { + email: "a@example.com".to_owned(), + password: "hunter2".to_owned(), + nickname: "ann".to_owned(), + }; + let json = serde_json::to_string(&request).expect("serialize"); + assert_eq!( + json, + r#"{"email":"a@example.com","password":"hunter2","nickname":"ann"}"# + ); + + // The response shape drops what only a request carries, and reading a + // response that still holds it ignores the extra key. + let response: read_write_only::AccountResponse = serde_json::from_str( + r#"{"id":"3a1f9b0e-1c1a-4d0f-8b6f-2f9a4c5d6e70","email":"a@example.com","nickname":"ann","password":"hunter2"}"#, + ) + .expect("deserialize"); + assert_eq!(response.email, "a@example.com"); + let json = serde_json::to_string(&response).expect("serialize"); + assert!( + !json.contains("password"), + "response shape wrote a write-only property: {json}" + ); +} + +#[test] +fn a_holder_of_a_split_model_names_the_shape_of_its_direction() { + use generated::read_write_only; + + let envelope = read_write_only::EnvelopeRequest { + account: read_write_only::AccountRequest { + email: "a@example.com".to_owned(), + password: "hunter2".to_owned(), + nickname: "ann".to_owned(), + }, + accounts: None, + }; + let json = serde_json::to_string(&envelope).expect("serialize"); + assert!( + !json.contains("\"id\""), + "request shape wrote a read-only property: {json}" + ); + + let list: read_write_only::AccountListResponse = serde_json::from_str( + r#"[{"id":"3a1f9b0e-1c1a-4d0f-8b6f-2f9a4c5d6e70","email":"a@example.com","nickname":"ann"}]"#, + ) + .expect("deserialize"); + assert_eq!(list.len(), 1); +} + +#[test] +fn a_model_that_one_direction_reaches_keeps_its_name() { + use generated::combined_read_write_only; + + // `AccountPage` is response-only, so it keeps its name and only its field + // type follows the split of `Account`. + let page = combined_read_write_only::AccountPage { + items: vec![combined_read_write_only::AccountResponse { + id: "3a1f9b0e-1c1a-4d0f-8b6f-2f9a4c5d6e70".to_owned(), + email: "a@example.com".to_owned(), + }], + }; + assert_eq!(page.items.len(), 1); + + // `AuditEntry` is response-only and marks `token` `writeOnly`, so it keeps + // its name and drops that property. + let entry: combined_read_write_only::AuditEntry = + serde_json::from_str(r#"{"action":"login","token":"t"}"#).expect("deserialize"); + let json = serde_json::to_string(&entry).expect("serialize"); + assert_eq!(json, r#"{"action":"login"}"#); + + // `AccountFilter` is request-only and marks `matched` `readOnly`, so it + // keeps its name and drops that property. + let filter = combined_read_write_only::AccountFilter { + email: "a@example.com".to_owned(), + }; + let json = serde_json::to_string(&filter).expect("serialize"); + assert_eq!(json, r#"{"email":"a@example.com"}"#); +} + +#[test] +fn a_read_only_multipart_part_is_left_out_of_the_extractor() { + use generated::combined_read_write_only; + + // `Avatar.checksum` is `readOnly`, and a multipart body only travels in a + // request, so the extractor never reads that part. + let body = combined_read_write_only::UploadAvatarMultipart { image: vec![1, 2, 3] }; + assert_eq!(body.image, vec![1, 2, 3]); +} + #[test] fn generated_server_trait_implements_and_routes() { use generated::server_petstore; diff --git a/crates/oapi-codegen/tests/generated/combined_read_write_only.rs b/crates/oapi-codegen/tests/generated/combined_read_write_only.rs new file mode 100644 index 0000000..7eb09c8 --- /dev/null +++ b/crates/oapi-codegen/tests/generated/combined_read_write_only.rs @@ -0,0 +1,466 @@ +// Code generated by oapi-codegen-rust. DO NOT EDIT. +#![allow( + dead_code, + unused_imports, + clippy::all, + clippy::pedantic, + clippy::nursery, + clippy::restriction, + reason = "generated code, not first-party source" +)] + +/// An account, used as a request body and as a response body. +/// +/// The request shape of `Account`. A `readOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountRequest { + pub email: String, + /// Sent when the account is created, and never read back. + pub password: String, +} + +/// An account, used as a request body and as a response body. +/// +/// The response shape of `Account`. A `writeOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountResponse { + /// Assigned by the server, so only a response carries it. + pub id: String, + pub email: String, +} + +/// A response-only holder of a split model. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountPage { + pub items: Vec, +} + +/// A request-only model that marks a property `readOnly`. +/// +/// Only a request carries this model, so a `readOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountFilter { + pub email: String, +} + +/// A response-only model that marks a property `writeOnly`. +/// +/// Only a response carries this model, so a `writeOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AuditEntry { + pub action: String, +} + +/// List every account. +#[derive(Debug, Clone, PartialEq)] +pub enum ListAccountsResponse { + /// The accounts, wrapped in a page. + Ok(AccountPage), +} + +/// Create an account. +#[derive(Debug, Clone, PartialEq)] +pub enum CreateAccountResponse { + /// The created account. + Created(AccountResponse), +} + +/// Search accounts. +#[derive(Debug, Clone, PartialEq)] +pub enum SearchAccountsResponse { + /// The search was accepted. + NoContent, +} + +/// List the audit log. +#[derive(Debug, Clone, PartialEq)] +pub enum ListAuditResponse { + /// The audit entries. + Ok(Vec), +} + +#[derive(Debug, Clone)] +pub struct UploadAvatarMultipart { + pub image: Vec, +} + +/// Replace an account avatar. +#[derive(Debug, Clone, PartialEq)] +pub enum UploadAvatarResponse { + /// The avatar was stored. + NoContent, +} + +impl axum::extract::FromRequest for UploadAvatarMultipart +where + S: Send + Sync, +{ + type Rejection = (axum::http::StatusCode, String); + async fn from_request( + request: axum::extract::Request, + state: &S, + ) -> Result { + let mut multipart = >::from_request(request, state) + .await + .map_err(|error| { + return (axum::http::StatusCode::BAD_REQUEST, error.to_string()); + })?; + let mut image: Option> = None; + while let Some(field) = multipart + .next_field() + .await + .map_err(|error| { + return (axum::http::StatusCode::BAD_REQUEST, error.to_string()); + })? + { + let field_name = field.name().map(|name| return name.to_owned()); + match field_name.as_deref() { + Some("image") => { + let value = field + .bytes() + .await + .map_err(|error| { + return ( + axum::http::StatusCode::BAD_REQUEST, + error.to_string(), + ); + })?; + image = Some(value.to_vec()); + } + _ => {} + } + } + return Ok(Self { + image: image + .ok_or(( + axum::http::StatusCode::BAD_REQUEST, + "missing required multipart field `image`".to_owned(), + ))?, + }); + } +} + +/// Server behaviour: implement one method per operation. +pub trait Api: Clone + Send + Sync + 'static { + /// List every account. + fn list_accounts( + &self, + ) -> impl std::future::Future + Send; + /// Create an account. + fn create_account( + &self, + body: AccountRequest, + ) -> impl std::future::Future + Send; + /// Search accounts. + fn search_accounts( + &self, + body: AccountFilter, + ) -> impl std::future::Future + Send; + /// List the audit log. + fn list_audit(&self) -> impl std::future::Future + Send; + /// Replace an account avatar. + fn upload_avatar( + &self, + account_id: String, + body: UploadAvatarMultipart, + ) -> impl std::future::Future + Send; +} + +impl axum::response::IntoResponse for ListAccountsResponse { + fn into_response(self) -> axum::response::Response { + match self { + ListAccountsResponse::Ok(body) => { + const STATUS: axum::http::StatusCode = match axum::http::StatusCode::from_u16( + 200, + ) { + Ok(status) => status, + Err(_) => panic!("oapi-codegen emitted an invalid HTTP status code"), + }; + (STATUS, axum::Json(body)).into_response() + } + } + } +} + +impl axum::response::IntoResponse for CreateAccountResponse { + fn into_response(self) -> axum::response::Response { + match self { + CreateAccountResponse::Created(body) => { + const STATUS: axum::http::StatusCode = match axum::http::StatusCode::from_u16( + 201, + ) { + Ok(status) => status, + Err(_) => panic!("oapi-codegen emitted an invalid HTTP status code"), + }; + (STATUS, axum::Json(body)).into_response() + } + } + } +} + +impl axum::response::IntoResponse for SearchAccountsResponse { + fn into_response(self) -> axum::response::Response { + match self { + SearchAccountsResponse::NoContent => { + const STATUS: axum::http::StatusCode = match axum::http::StatusCode::from_u16( + 204, + ) { + Ok(status) => status, + Err(_) => panic!("oapi-codegen emitted an invalid HTTP status code"), + }; + STATUS.into_response() + } + } + } +} + +impl axum::response::IntoResponse for ListAuditResponse { + fn into_response(self) -> axum::response::Response { + match self { + ListAuditResponse::Ok(body) => { + const STATUS: axum::http::StatusCode = match axum::http::StatusCode::from_u16( + 200, + ) { + Ok(status) => status, + Err(_) => panic!("oapi-codegen emitted an invalid HTTP status code"), + }; + (STATUS, axum::Json(body)).into_response() + } + } + } +} + +impl axum::response::IntoResponse for UploadAvatarResponse { + fn into_response(self) -> axum::response::Response { + match self { + UploadAvatarResponse::NoContent => { + const STATUS: axum::http::StatusCode = match axum::http::StatusCode::from_u16( + 204, + ) { + Ok(status) => status, + Err(_) => panic!("oapi-codegen emitted an invalid HTTP status code"), + }; + STATUS.into_response() + } + } + } +} + +/// Build an axum `Router` that dispatches each route to `api`. +pub fn router(api: T) -> axum::Router { + axum::Router::new() + .route( + "/accounts", + axum::routing::get(list_accounts_handler::) + .post(create_account_handler::), + ) + .route("/accounts/search", axum::routing::post(search_accounts_handler::)) + .route("/audit", axum::routing::get(list_audit_handler::)) + .route( + "/accounts/{accountId}/avatar", + axum::routing::put(upload_avatar_handler::), + ) + .with_state(api) +} + +async fn list_accounts_handler( + axum::extract::State(api): axum::extract::State, +) -> ListAccountsResponse { + api.list_accounts().await +} + +async fn create_account_handler( + axum::extract::State(api): axum::extract::State, + axum::Json(body): axum::Json, +) -> CreateAccountResponse { + api.create_account(body).await +} + +async fn search_accounts_handler( + axum::extract::State(api): axum::extract::State, + axum::Json(body): axum::Json, +) -> SearchAccountsResponse { + api.search_accounts(body).await +} + +async fn list_audit_handler( + axum::extract::State(api): axum::extract::State, +) -> ListAuditResponse { + api.list_audit().await +} + +async fn upload_avatar_handler( + axum::extract::State(api): axum::extract::State, + axum::extract::Path(account_id): axum::extract::Path, + body: UploadAvatarMultipart, +) -> UploadAvatarResponse { + api.upload_avatar(account_id, body).await +} + +/// Errors returned by the generated client. +#[derive(Debug)] +pub enum ClientError { + /// The `reqwest` request failed to send or complete, including any + /// body decoding `reqwest` performs internally (such as JSON). + Http(reqwest::Error), + /// The server returned a status code the operation does not declare. + UnexpectedStatus(reqwest::StatusCode), + /// The response `Content-Type` matched none of the representations the + /// operation declares for its status. + UnexpectedContentType(String), + /// The response cannot be decoded: a body that failed to + /// deserialize (for example malformed form-urlencoded content), or a + /// required response header that was missing or unparsable. + Decode(String), +} +impl std::fmt::Display for ClientError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ClientError::Http(error) => return write!(f, "HTTP request failed: {error}"), + ClientError::UnexpectedStatus(status) => { + return write!(f, "unexpected response status: {status}"); + } + ClientError::UnexpectedContentType(content_type) => { + return write!(f, "unexpected response content type: {content_type}"); + } + ClientError::Decode(message) => { + return write!(f, "failed to decode response: {message}"); + } + } + } +} +impl std::error::Error for ClientError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + ClientError::Http(error) => return Some(error), + ClientError::UnexpectedStatus(_) + | ClientError::UnexpectedContentType(_) + | ClientError::Decode(_) => return None, + } + } +} +impl From for ClientError { + fn from(error: reqwest::Error) -> Self { + return ClientError::Http(error); + } +} + +const PATH_PARAM_ENCODE_SET: &percent_encoding::AsciiSet = &percent_encoding::NON_ALPHANUMERIC + .remove(b'-') + .remove(b'.') + .remove(b'_') + .remove(b'~'); + +/// A blocking HTTP client for the API. +/// +/// `base_url` is used as a prefix for every request path and must not +/// carry a trailing slash (for example `https://api.example.com`). +#[derive(Debug, Clone)] +pub struct Client { + base_url: String, + http: reqwest::blocking::Client, +} + +impl Client { + /// Build a client targeting `base_url` with a default blocking + /// `reqwest::blocking::Client`. + pub fn new(base_url: impl Into) -> Result { + let http = reqwest::blocking::Client::builder().build()?; + return Ok(Self { + base_url: base_url.into(), + http, + }); + } + /// Build a client targeting `base_url` with a caller-provided + /// `reqwest::blocking::Client` (for example preconfigured with timeouts). + pub fn with_client( + base_url: impl Into, + http: reqwest::blocking::Client, + ) -> Self { + return Self { + base_url: base_url.into(), + http, + }; + } + /// List every account. + pub fn list_accounts(&self) -> Result { + let url = format!("{}/accounts", self.base_url); + let response = self.http.request(reqwest::Method::GET, url).send()?; + let status = response.status(); + if status.as_u16() == 200 { + let body: AccountPage = response.json()?; + return Ok(ListAccountsResponse::Ok(body)); + } + return Err(ClientError::UnexpectedStatus(status)); + } + /// Create an account. + pub fn create_account( + &self, + body: AccountRequest, + ) -> Result { + let url = format!("{}/accounts", self.base_url); + let mut request = self.http.request(reqwest::Method::POST, url); + request = request.json(&body); + let response = request.send()?; + let status = response.status(); + if status.as_u16() == 201 { + let body: AccountResponse = response.json()?; + return Ok(CreateAccountResponse::Created(body)); + } + return Err(ClientError::UnexpectedStatus(status)); + } + /// Search accounts. + pub fn search_accounts( + &self, + body: AccountFilter, + ) -> Result { + let url = format!("{}/accounts/search", self.base_url); + let mut request = self.http.request(reqwest::Method::POST, url); + request = request.json(&body); + let response = request.send()?; + let status = response.status(); + if status.as_u16() == 204 { + return Ok(SearchAccountsResponse::NoContent); + } + return Err(ClientError::UnexpectedStatus(status)); + } + /// List the audit log. + pub fn list_audit(&self) -> Result { + let url = format!("{}/audit", self.base_url); + let response = self.http.request(reqwest::Method::GET, url).send()?; + let status = response.status(); + if status.as_u16() == 200 { + let body: Vec = response.json()?; + return Ok(ListAuditResponse::Ok(body)); + } + return Err(ClientError::UnexpectedStatus(status)); + } + /// Replace an account avatar. + pub fn upload_avatar( + &self, + account_id: String, + body: UploadAvatarMultipart, + ) -> Result { + let url = format!( + "{}/accounts/{}/avatar", self.base_url, + percent_encoding::utf8_percent_encode(account_id.as_str(), + PATH_PARAM_ENCODE_SET) + ); + let mut request = self.http.request(reqwest::Method::PUT, url); + let mut form = reqwest::blocking::multipart::Form::new(); + form = form + .part( + "image", + reqwest::blocking::multipart::Part::bytes(body.image).file_name("image"), + ); + request = request.multipart(form); + let response = request.send()?; + let status = response.status(); + if status.as_u16() == 204 { + return Ok(UploadAvatarResponse::NoContent); + } + return Err(ClientError::UnexpectedStatus(status)); + } +} diff --git a/crates/oapi-codegen/tests/generated/read_write_only.rs b/crates/oapi-codegen/tests/generated/read_write_only.rs new file mode 100644 index 0000000..f1bba31 --- /dev/null +++ b/crates/oapi-codegen/tests/generated/read_write_only.rs @@ -0,0 +1,71 @@ +// Code generated by oapi-codegen-rust. DO NOT EDIT. +#![allow( + dead_code, + unused_imports, + clippy::all, + clippy::pedantic, + clippy::nursery, + clippy::restriction, + reason = "generated code, not first-party source" +)] + +/// A schema whose marks split it into a request shape and a response shape. +/// +/// The request shape of `Account`. A `readOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountRequest { + pub email: String, + /// Sent when the account is created, and never read back. + pub password: String, + pub nickname: String, +} + +/// A schema whose marks split it into a request shape and a response shape. +/// +/// The response shape of `Account`. A `writeOnly` property is not part of it. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AccountResponse { + /// Assigned by the server, so only a response carries it. + pub id: uuid::Uuid, + #[serde(rename = "createdAt", skip_serializing_if = "Option::is_none")] + pub created_at: Option>, + pub email: String, + pub nickname: String, +} + +/// A holder splits too, because its field type differs per direction. +/// +/// The request shape of `Envelope`. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct EnvelopeRequest { + pub account: AccountRequest, + #[serde(skip_serializing_if = "Option::is_none")] + pub accounts: Option>, +} + +/// A holder splits too, because its field type differs per direction. +/// +/// The response shape of `Envelope`. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct EnvelopeResponse { + pub account: AccountResponse, + #[serde(skip_serializing_if = "Option::is_none")] + pub accounts: Option>, +} + +/// An alias to a split model splits as well. +/// +/// The request shape of `AccountList`. +pub type AccountListRequest = Vec; + +/// An alias to a split model splits as well. +/// +/// The response shape of `AccountList`. +pub type AccountListResponse = Vec; + +/// No mark reaches this schema, so it keeps its name. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct Untouched { + #[serde(skip_serializing_if = "Option::is_none")] + pub label: Option, +} diff --git a/crates/oapi-codegen/tests/generated/server_auth.rs b/crates/oapi-codegen/tests/generated/server_auth.rs index 7651650..37ffbed 100644 --- a/crates/oapi-codegen/tests/generated/server_auth.rs +++ b/crates/oapi-codegen/tests/generated/server_auth.rs @@ -34,9 +34,9 @@ pub enum GetReportsResponse { Ok(Message), } -/** Fetch session data, which an API key cookie protects. - -The cookie is set at login and expires with the session.*/ +/// Fetch session data, which an API key cookie protects. +/// +/// The cookie is set at login and expires with the session. #[derive(Debug, Clone, PartialEq)] pub enum GetSessionResponse { /// The session data. diff --git a/docs/design.md b/docs/design.md index 22fa51f..abb86bc 100644 --- a/docs/design.md +++ b/docs/design.md @@ -512,8 +512,57 @@ does the same. `minProperties` on a schema that names its properties, and names the way out, because a rule the document states and the code drops is worse than no rule at all. -`readOnly` and `writeOnly` are read by nothing yet. They mark a direction, not a -value, and one struct cannot be both. +## A direction mark projects a model onto its direction + +`readOnly` and `writeOnly` mark a direction, not a value. A `readOnly` property +travels in a response, and a request must not send it. A `writeOnly` property +travels in a request, and a response must not send it. A property that sets both +marks is an error, because no direction is left to carry it. + +A serde attribute cannot state this. The same `Order` type can reach a request +body and a response body, so an attribute that is right for one is wrong for the +other. The side does not settle it either. A server reads a request and writes a +response, and a client does the reverse. + +So each model drops the properties that its direction does not carry. When one +direction reaches a model, the model keeps its name. An `Order` that only a +response carries keeps the name `Order` and drops each `writeOnly` property. A +`readOnly` property costs that model nothing, so a read-only API generates what +it generated before the marks were there. + +When both directions reach a model, one name is not enough. `Order` then becomes +`OrderRequest`, which has no `readOnly` property, and `OrderResponse`, which has +no `writeOnly` property. A parameter, a request body, and a multipart part take +the request shape. A response body and a response header take the response +shape. + +The split spreads along model references. A model that holds a split model and +that both directions reach cannot keep one name either, because its field type +differs per direction. A holder that one direction reaches keeps its name, and +only its field type follows the split. So a response-only `Page` keeps the name +`Page` and holds an `OrderResponse`. + +A models-only run splits every marked model, because it has no operation to give +a direction. The run that consumes those models does know the direction and +picks the shape it needs. + +A new operation can rename a model. A `POST` that sends an `Order` gives that +model a second direction, so the one name becomes two. The model gains a second +shape at that point, so the two names report what the document now states. + +The two names go through the same checks as any other type name. A schema named +`GetWidget` gives a `GetWidgetResponse` shape, which the response enum of a +`getWidget` operation also claims. That is reported, and `x-rust-name` or +`output-options.response-type-suffix` gives the way out. + +A `required` property needs no separate rule. The shape that must not send the +property does not declare it at all, so the requirement applies only where the +property exists. This is what the OpenAPI specification states. + +An `import-mapping` reference to a marked schema is an error. Two runs make a +composed crate, and only one of them holds the operations. The other run is a +models-only run, so it emits two names for that schema. A reference by name +alone cannot say which of the two it means. ## A `default` removes the `Option` diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 7243604..b73c15e 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,2 +1,2 @@ [toolchain] -channel = "1.97.1" +channel = "1.98.0"