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..8a5de8252 100644 --- a/src/v2/codegen/emitter.rs +++ b/src/v2/codegen/emitter.rs @@ -549,7 +549,7 @@ where self.emit_struct(def, ctx) } - /// Checks if the given definition is a simple map and returns the corresponding `BTreeMap`. + /// Checks if the given definition is a simple map and returns the corresponding `PropertiesMap`. fn try_emit_map( &self, def: &E::Definition, @@ -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/codegen/impls.rs b/src/v2/codegen/impls.rs index 4ce40cd34..709930770 100644 --- a/src/v2/codegen/impls.rs +++ b/src/v2/codegen/impls.rs @@ -470,7 +470,7 @@ where /// Builds the method parameter type using the actual field type. /// /// For example, if a field is `Vec`, then we replace it (in builder method) - /// with `impl Iterator>`, and if we had `BTreeMap`, + /// with `impl Iterator>`, and if we had `PropertiesMap`, /// then we replace it with `impl Iterator` and /// we do this... recursively. // FIXME: Investigate if there's a better way. @@ -489,7 +489,7 @@ where f.write_str("impl Iterator")?; - } else if ty[..i].ends_with("std::collections::BTreeMap") { + } else if ty[..i].ends_with("paperclip::v2::PropertiesMap") { f.write_str("impl Iterator")?; @@ -546,10 +546,10 @@ where f.write_str("value.map(|value| ")?; Self::write_value_map(&ty[i + 1..ty.len() - 1], f)?; f.write_str(").collect::>()")?; - } else if ty[..i].ends_with("std::collections::BTreeMap") { + } else if ty[..i].ends_with("paperclip::v2::PropertiesMap") { f.write_str("value.map(|(key, value)| (key, ")?; Self::write_value_map(&ty[i + 9..ty.len() - 1], f)?; - f.write_str(")).collect::>()")?; + f.write_str(")).collect::>()")?; } } else { f.write_str("value")?; diff --git a/src/v2/codegen/object.rs b/src/v2/codegen/object.rs index 3ddc7aa35..a6bfb67cb 100644 --- a/src/v2/codegen/object.rs +++ b/src/v2/codegen/object.rs @@ -222,8 +222,8 @@ pub struct ObjectField { /// Required fields of the "deepest" child type in the given definition. /// /// Now, what do I mean by "deepest"? For example, if we had `Vec>>` - /// or `Vec>>>`, then "deepest" child - /// type is T (as long as it's not a `Vec` or `BTreeMap`). + /// or `Vec>>>`, then "deepest" child + /// type is T (as long as it's not a `Vec` or `PropertiesMap`). /// /// To understand why we're doing this, see `ApiObjectBuilderImpl::write_builder_ty` /// and `ApiObjectBuilderImpl::write_value_map` functions. @@ -315,7 +315,7 @@ impl ApiObject { if ty[..i].ends_with("Vec") { f.write_str(&ty[..=i])?; Self::write_field_with_any(&ty[i + 1..ty.len() - 1], f)?; - } else if ty[..i].ends_with("std::collections::BTreeMap") { + } else if ty[..i].ends_with("paperclip::v2::PropertiesMap") { f.write_str(&ty[..i + 9])?; Self::write_field_with_any(&ty[i + 9..ty.len() - 1], f)?; } else { 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/snapshots/test_codegen__tests__test_k8s__io__k8s__api__certificates__v1beta1__certificate_signing_request_spec.rs.snap b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__certificates__v1beta1__certificate_signing_request_spec.rs.snap index 13bfbfdb3..096b206e9 100644 --- a/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__certificates__v1beta1__certificate_signing_request_spec.rs.snap +++ b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__certificates__v1beta1__certificate_signing_request_spec.rs.snap @@ -7,7 +7,7 @@ expression: data #[derive(Debug, Default, Clone, Serialize, Deserialize)] pub struct CertificateSigningRequestSpec { /// Extra information about the requesting user. See user.Info interface for details. - pub extra: Option>>, + pub extra: Option>>, /// Group information about the requesting user. See user.Info interface for details. pub groups: Option>, /// Base64-encoded PKCS#10 CSR data @@ -49,7 +49,7 @@ impl CertificateSigningRequestSpecBuilder { /// Extra information about the requesting user. See user.Info interface for details. #[inline] pub fn extra(mut self, value: impl Iterator>)>) -> Self { - self.body.extra = Some(value.map(|(key, value)| (key, value.map(|value| value.into()).collect::>().into())).collect::>().into()); + self.body.extra = Some(value.map(|(key, value)| (key, value.map(|value| value.into()).collect::>().into())).collect::>().into()); self } diff --git a/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__core__v1__config_map.rs.snap b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__core__v1__config_map.rs.snap index b461142c7..cbd5f339f 100644 --- a/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__core__v1__config_map.rs.snap +++ b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__api__core__v1__config_map.rs.snap @@ -11,9 +11,9 @@ pub struct ConfigMap { pub api_version: Option, /// BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet. #[serde(rename = "binaryData")] - pub binary_data: Option>, + pub binary_data: Option>, /// Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process. - pub data: Option>, + pub data: Option>, /// Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds pub kind: Option, /// Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata @@ -94,14 +94,14 @@ impl ConfigMapBuilder { /// BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet. #[inline] pub fn binary_data(mut self, value: impl Iterator)>) -> Self { - self.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } /// Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process. #[inline] pub fn data(mut self, value: impl Iterator)>) -> Self { - self.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } @@ -176,14 +176,14 @@ impl ConfigMapPostBuilder { /// BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet. #[inline] pub fn binary_data(mut self, value: impl Iterator)>) -> Self { - self.inner.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.inner.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } /// Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process. #[inline] pub fn data(mut self, value: impl Iterator)>) -> Self { - self.inner.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.inner.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } @@ -363,14 +363,14 @@ impl ConfigMapPutBuilder1 { /// BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet. #[inline] pub fn binary_data(mut self, value: impl Iterator)>) -> Self { - self.inner.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.inner.body.binary_data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } /// Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process. #[inline] pub fn data(mut self, value: impl Iterator)>) -> Self { - self.inner.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.inner.body.data = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } diff --git a/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__apiextensions_apiserver__pkg__apis__apiextensions__v1beta1__json_schema_props.rs.snap b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__apiextensions_apiserver__pkg__apis__apiextensions__v1beta1__json_schema_props.rs.snap index 92fbc254a..883c742ef 100644 --- a/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__apiextensions_apiserver__pkg__apis__apiextensions__v1beta1__json_schema_props.rs.snap +++ b/tests/snapshots/test_codegen__tests__test_k8s__io__k8s__apiextensions_apiserver__pkg__apis__apiextensions__v1beta1__json_schema_props.rs.snap @@ -19,8 +19,8 @@ pub struct JsonSchemaProps { #[serde(rename = "anyOf")] pub any_of: Option>>, pub default: Option, - pub definitions: Option>>, - pub dependencies: Option>, + pub definitions: Option>>, + pub dependencies: Option>, pub description: Option, #[serde(rename = "enum")] pub enum_: Option>, @@ -56,8 +56,8 @@ pub struct JsonSchemaProps { pub one_of: Option>>, pub pattern: Option, #[serde(rename = "patternProperties")] - pub pattern_properties: Option>>, - pub properties: Option>>, + pub pattern_properties: Option>>, + pub properties: Option>>, pub required: Option>, pub title: Option, #[serde(rename = "type")] @@ -151,13 +151,13 @@ impl JsonSchemaPropsBuilder { #[inline] pub fn definitions(mut self, value: impl Iterator)>) -> Self { - self.body.definitions = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.definitions = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } #[inline] pub fn dependencies(mut self, value: impl Iterator)>) -> Self { - self.body.dependencies = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.dependencies = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } @@ -295,13 +295,13 @@ impl JsonSchemaPropsBuilder { #[inline] pub fn pattern_properties(mut self, value: impl Iterator)>) -> Self { - self.body.pattern_properties = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.pattern_properties = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } #[inline] pub fn properties(mut self, value: impl Iterator)>) -> Self { - self.body.properties = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.properties = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } diff --git a/tests/snapshots/test_codegen__tests__test_pet__pet.rs.snap b/tests/snapshots/test_codegen__tests__test_pet__pet.rs.snap index 81811b7ec..394da0086 100644 --- a/tests/snapshots/test_codegen__tests__test_pet__pet.rs.snap +++ b/tests/snapshots/test_codegen__tests__test_pet__pet.rs.snap @@ -13,7 +13,7 @@ pub struct Pet { pub photo_urls: Option>, pub tags: Option>, #[serde(flatten)] - pub other_fields: Option>, + pub other_fields: Option>, } impl Pet { @@ -107,7 +107,7 @@ impl PetBuilder { #[inline] pub fn other_fields(mut self, value: impl Iterator)>) -> Self { - self.body.other_fields = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.body.other_fields = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } } @@ -189,7 +189,7 @@ impl PetPostBuilder { #[inline] pub fn other_fields(mut self, value: impl Iterator)>) -> Self { - self.inner.body.other_fields = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); + self.inner.body.other_fields = Some(value.map(|(key, value)| (key, value.into())).collect::>().into()); self } } 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() {