From b39015081d8de2c34c2759f286a9fadac95b03f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-S=C3=A9bastien=20Bour?= Date: Wed, 18 Sep 2024 16:43:19 +0200 Subject: [PATCH] feat: add `preserve_order` feature to keep struct fields order in schema Fixes #538 --- Cargo.toml | 1 + core/Cargo.toml | 2 + core/src/v2/mod.rs | 5 +++ core/src/v2/schema.rs | 13 ++++--- core/src/v3/schema.rs | 4 +- macros/src/core.rs | 8 ++-- src/v2/codegen/emitter.rs | 4 +- src/v2/mod.rs | 2 +- tests/test_app.rs | 82 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 107 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 05416be9b..569347ec1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,7 @@ actix-base = ["v2", "paperclip-macros/actix"] swagger-ui = ["paperclip-actix/swagger-ui"] rapidoc = ["paperclip-actix/rapidoc"] path-in-definition = ["paperclip-macros/path-in-definition"] +preserve_order = ["paperclip-core/preserve_order"] # OpenAPI support (v2 and codegen) cli = ["env_logger", "structopt", "git2", "v2", "codegen"] diff --git a/core/Cargo.toml b/core/Cargo.toml index 69885e91e..23cac59b4 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -18,6 +18,7 @@ actix-session = { version = "0", optional = true } actix-identity = { version = "0", optional = true } actix-files = { version = "0", optional = true } chrono = { version = "0.4", optional = true } +indexmap = { version = "2", optional = true } jiff = { version = "0.1.5", optional = true } heck = { version = "0.4", optional = true } once_cell = "1.4" @@ -57,6 +58,7 @@ nightly = ["paperclip-macros/nightly"] v2 = ["paperclip-macros/v2"] v3 = ["v2", "openapiv3"] codegen = ["v2", "heck", "log"] +preserve_order = ["dep:indexmap", "indexmap/serde", "serde_json/preserve_order"] uuid = ["uuid0"] uuid0 = ["uuid0_dep"] uuid1 = ["uuid1_dep"] diff --git a/core/src/v2/mod.rs b/core/src/v2/mod.rs index 89a95a91f..a8d2e0a29 100644 --- a/core/src/v2/mod.rs +++ b/core/src/v2/mod.rs @@ -29,6 +29,11 @@ use self::resolver::Resolver; #[cfg(feature = "codegen")] use crate::error::ValidationError; +#[cfg(feature = "preserve_order")] +pub type PropertiesMap = indexmap::IndexMap; +#[cfg(not(feature = "preserve_order"))] +pub type PropertiesMap = std::collections::BTreeMap; + #[cfg(feature = "codegen")] impl ResolvableApi { /// Consumes this API schema, resolves the references and returns diff --git a/core/src/v2/schema.rs b/core/src/v2/schema.rs index b882f3e49..f8d822aab 100644 --- a/core/src/v2/schema.rs +++ b/core/src/v2/schema.rs @@ -1,8 +1,11 @@ //! Traits used for code and spec generation. -use super::models::{ - DataType, DataTypeFormat, DefaultOperationRaw, DefaultSchemaRaw, Either, Resolvable, - SecurityScheme, +use super::{ + models::{ + DataType, DataTypeFormat, DefaultOperationRaw, DefaultSchemaRaw, Either, Resolvable, + SecurityScheme, + }, + PropertiesMap, }; use std::collections::{BTreeMap, BTreeSet}; @@ -39,10 +42,10 @@ pub trait Schema: Sized { fn additional_properties_mut(&mut self) -> Option<&mut Either>>; /// Map of names and schema for properties, if it's an object (`properties` field) - fn properties(&self) -> Option<&BTreeMap>>; + fn properties(&self) -> Option<&PropertiesMap>>; /// Mutable access to `properties` field. - fn properties_mut(&mut self) -> Option<&mut BTreeMap>>; + fn properties_mut(&mut self) -> Option<&mut PropertiesMap>>; /// Returns the required properties (if any) for this object. fn required_properties(&self) -> Option<&BTreeSet>; diff --git a/core/src/v3/schema.rs b/core/src/v3/schema.rs index 1f2bd0d21..0b8233d31 100644 --- a/core/src/v3/schema.rs +++ b/core/src/v3/schema.rs @@ -1,4 +1,4 @@ -use crate::v2::models::Either; +use crate::v2::{models::Either, PropertiesMap}; use super::{invalid_referenceor, v2}; use std::ops::Deref; @@ -65,7 +65,7 @@ fn v2_data_type_to_v3( format: &Option, enum_: &[serde_json::Value], items: &Option>, - properties: &std::collections::BTreeMap>, + properties: &PropertiesMap>, extra_properties: &Option>>, required: &std::collections::BTreeSet, ) -> openapiv3::SchemaKind { diff --git a/macros/src/core.rs b/macros/src/core.rs index 3870c0c29..9f9aeef0c 100644 --- a/macros/src/core.rs +++ b/macros/src/core.rs @@ -148,7 +148,7 @@ pub fn emit_v2_schema_struct(input: TokenStream) -> TokenStream { } #[inline] - fn properties(&self) -> Option<&std::collections::BTreeMap>> { + fn properties(&self) -> Option<&paperclip::v2::PropertiesMap>> { if self.properties.is_empty() { None } else { @@ -157,7 +157,7 @@ pub fn emit_v2_schema_struct(input: TokenStream) -> TokenStream { } #[inline] - fn properties_mut(&mut self) -> Option<&mut std::collections::BTreeMap>> { + fn properties_mut(&mut self) -> Option<&mut paperclip::v2::PropertiesMap>> { if self.properties.is_empty() { None } else { @@ -298,8 +298,8 @@ fn schema_fields(name: &Ident, is_ref: bool) -> proc_macro2::TokenStream { )); gen.extend(quote!( - #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")] - pub properties: std::collections::BTreeMap,)); diff --git a/src/v2/codegen/emitter.rs b/src/v2/codegen/emitter.rs index f30763d99..c060c52ed 100644 --- a/src/v2/codegen/emitter.rs +++ b/src/v2/codegen/emitter.rs @@ -565,7 +565,7 @@ where let ty = self .build_def(&schema, ctx.clone().define(false))? .known_type(); - let map = format!("std::collections::BTreeMap", ty); + let map = format!("paperclip::v2::PropertiesMap", ty); Ok(EmittedUnit::Known(map)) } _ => Ok(EmittedUnit::None), @@ -673,7 +673,7 @@ where if let Some(Either::Left(true)) = def.additional_properties() { obj.fields_mut().push(ObjectField { name: EXTRA_PROPS_FIELD.into(), - ty_path: "std::collections::BTreeMap".into(), + ty_path: "paperclip::v2::PropertiesMap".into(), description: None, is_required: false, needs_any: true, diff --git a/src/v2/mod.rs b/src/v2/mod.rs index 28a14e8c1..9b31212a3 100644 --- a/src/v2/mod.rs +++ b/src/v2/mod.rs @@ -102,7 +102,7 @@ pub use paperclip_core::{ v2::{ models::{self, DefaultSchema, ResolvableApi}, schema::{self, Schema}, - serde_json, + serde_json, PropertiesMap, }, }; diff --git a/tests/test_app.rs b/tests/test_app.rs index 148cb2c1e..1febe682f 100644 --- a/tests/test_app.rs +++ b/tests/test_app.rs @@ -5202,6 +5202,88 @@ fn test_schema_with_generics() { ); } +#[test] +fn test_schema_properties_order() { + /// A good boy/girl. + #[derive(Apiv2Schema, Deserialize, Serialize)] + struct Dog { + /// Pick a good one. + name: String, + /// Be sure to chip them! + id: usize, + /// To each their own preferred one. + breed: String, + } + + #[post("/echo")] + #[api_v2_operation] + async fn echo(body: web::Json) -> Result, Error> { + Ok(body) + } + + run_and_check_app( + || { + App::new() + .wrap_api() + .service(echo) + .with_raw_json_spec(|app, spec| { + app.route( + "/api/spec", + web::get().to(move || { + #[cfg(feature = "actix4")] + { + let spec = spec.clone(); + async move { + paperclip::actix::HttpResponseWrapper( + actix_web::HttpResponse::Ok().json(&spec), + ) + } + } + + #[cfg(not(feature = "actix4"))] + actix_web::HttpResponse::Ok().json(&spec) + }), + ) + }) + .build() + }, + |addr| { + let resp = CLIENT + .get(&format!("http://{}/api/spec", addr)) + .send() + .expect("request failed?"); + + assert_eq!(resp.status().as_u16(), 200); + let json = resp.json::().expect("json error"); + + let properties = json + .as_object() + .expect("schema is not an object") + .get("definitions") + .expect("schema does not have definitions") + .as_object() + .expect("definitions is not an object") + .get("Dog") + .expect("no dog definition") + .as_object() + .expect("dog definition is not an object") + .get("properties") + .expect("no dog properties") + .as_object() + .expect("dog properties is not an object") + .keys() + .collect::>(); + + #[cfg(feature = "preserve_order")] + let expected = ["name", "id", "breed"]; + #[cfg(not(feature = "preserve_order"))] + let expected = ["breed", "id", "name"]; + + assert_eq!(properties, expected); + }, + ); +} + #[test] #[cfg(feature = "path-in-definition")] fn test_module_path_in_definition_name() {