From c7ad4f0a3aa7d56ae99ddabeb56fcc7b56993983 Mon Sep 17 00:00:00 2001 From: Sean Lynch <42618346+swlynch99@users.noreply.github.com> Date: Thu, 7 Dec 2023 14:22:22 -0800 Subject: [PATCH 1/2] Add JsonSchemaAs impls for all Duration and Timestamp adaptors --- serde_with/src/schemars_0_8.rs | 188 +++++++++++++++++- serde_with/tests/schemars_0_8.rs | 122 +++++++++++- .../schemars_0_8/snapshots/duration.json | 53 +++++ 3 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 serde_with/tests/schemars_0_8/snapshots/duration.json diff --git a/serde_with/src/schemars_0_8.rs b/serde_with/src/schemars_0_8.rs index 38d80282..4f2f17e1 100644 --- a/serde_with/src/schemars_0_8.rs +++ b/serde_with/src/schemars_0_8.rs @@ -6,7 +6,7 @@ //! see [`JsonSchemaAs`]. use crate::{ - formats::{Flexible, Separator, Strict}, + formats::{Flexible, Format, Separator, Strict}, prelude::{Schema as WrapSchema, *}, }; use ::schemars_0_8::{ @@ -609,3 +609,189 @@ where { forward_schema!(Vec>); } + +mod timespan { + use super::*; + + /// Internal helper trait used to constrain which types we implement + /// `JsonSchemaAs` for. + pub trait TimespanSchemaTarget { + /// Whether F is signed. + const SIGNED: bool = true; + + /// Whether F is String + const STRING: bool; + } + + macro_rules! is_string { + (String) => { + true + }; + ($name:ty) => { + false + }; + } + + macro_rules! declare_timespan_target { + ( $target:ty { $($format:ty),* $(,)? } )=> { + $( + impl TimespanSchemaTarget<$format> for $target { + const STRING: bool = is_string!($format); + } + )* + } + } + + impl TimespanSchemaTarget for Duration { + const SIGNED: bool = false; + const STRING: bool = false; + } + + impl TimespanSchemaTarget for Duration { + const SIGNED: bool = false; + const STRING: bool = false; + } + + impl TimespanSchemaTarget for Duration { + const SIGNED: bool = false; + const STRING: bool = true; + } + + declare_timespan_target!(SystemTime { i64, f64, String }); + + #[cfg(feature = "chrono_0_4")] + declare_timespan_target!(::chrono_0_4::Duration { i64, f64, String }); + #[cfg(feature = "chrono_0_4")] + declare_timespan_target!(::chrono_0_4::DateTime<::chrono_0_4::Utc> { i64, f64, String }); + #[cfg(feature = "chrono_0_4")] + declare_timespan_target!(::chrono_0_4::DateTime<::chrono_0_4::Local> { i64, f64, String }); + #[cfg(feature = "chrono_0_4")] + declare_timespan_target!(::chrono_0_4::NaiveDateTime { i64, f64, String }); + + #[cfg(feature = "time_0_3")] + declare_timespan_target!(::time_0_3::Duration { i64, f64, String }); + #[cfg(feature = "time_0_3")] + declare_timespan_target!(::time_0_3::OffsetDateTime { i64, f64, String }); + #[cfg(feature = "time_0_3")] + declare_timespan_target!(::time_0_3::PrimitiveDateTime { i64, f64, String }); +} + +use self::timespan::TimespanSchemaTarget; + +/// Internal type used for the base impls on DurationXXX and TimestampYYY types. +/// +/// This allows the JsonSchema impls that are Strict to be generic without +/// committing to it as part of the public API. +struct Timespan(PhantomData<(Format, Strictness)>); + +impl JsonSchemaAs for Timespan +where + T: TimespanSchemaTarget, + F: Format + JsonSchema, +{ + forward_schema!(F); +} + +fn flexible_timespan_schema(signed: bool, is_string: bool) -> Schema { + let mut number = SchemaObject { + instance_type: Some(InstanceType::Number.into()), + number: (!signed).then(|| { + Box::new(NumberValidation { + minimum: Some(0.0), + ..Default::default() + }) + }), + ..Default::default() + }; + + let mut string = SchemaObject { + instance_type: Some(InstanceType::String.into()), + ..Default::default() + }; + + if is_string { + number.metadata().write_only = true; + } else { + string.metadata().write_only = true; + } + + SchemaObject { + subschemas: Some(Box::new(SubschemaValidation { + one_of: Some(std::vec![number.into(), string.into()]), + ..Default::default() + })), + ..Default::default() + } + .into() +} + +impl JsonSchemaAs for Timespan +where + T: TimespanSchemaTarget, + F: Format + JsonSchema, +{ + fn schema_name() -> String { + match >::STRING { + true => "FlexibleStringTimespan".into(), + false => "FlexibleTimespan".into(), + } + } + + fn schema_id() -> Cow<'static, str> { + match >::STRING { + true => "serde_with::FlexibleStringTimespan".into(), + false => "serde_with::FlexibleTimespan".into(), + } + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + flexible_timespan_schema( + >::SIGNED, + >::STRING, + ) + } + + fn is_referenceable() -> bool { + false + } +} + +macro_rules! forward_duration_schema { + ($ty:ident) => { + impl JsonSchemaAs for $ty + where + T: TimespanSchemaTarget, + F: Format + JsonSchema + { + forward_schema!(WrapSchema>); + } + + impl JsonSchemaAs for $ty + where + T: TimespanSchemaTarget, + F: Format + JsonSchema + { + forward_schema!(WrapSchema>); + } + }; +} + +forward_duration_schema!(DurationSeconds); +forward_duration_schema!(DurationMilliSeconds); +forward_duration_schema!(DurationMicroSeconds); +forward_duration_schema!(DurationNanoSeconds); + +forward_duration_schema!(DurationSecondsWithFrac); +forward_duration_schema!(DurationMilliSecondsWithFrac); +forward_duration_schema!(DurationMicroSecondsWithFrac); +forward_duration_schema!(DurationNanoSecondsWithFrac); + +forward_duration_schema!(TimestampSeconds); +forward_duration_schema!(TimestampMilliSeconds); +forward_duration_schema!(TimestampMicroSeconds); +forward_duration_schema!(TimestampNanoSeconds); + +forward_duration_schema!(TimestampSecondsWithFrac); +forward_duration_schema!(TimestampMilliSecondsWithFrac); +forward_duration_schema!(TimestampMicroSecondsWithFrac); +forward_duration_schema!(TimestampNanoSecondsWithFrac); diff --git a/serde_with/tests/schemars_0_8.rs b/serde_with/tests/schemars_0_8.rs index 19be9714..fe02b63b 100644 --- a/serde_with/tests/schemars_0_8.rs +++ b/serde_with/tests/schemars_0_8.rs @@ -180,7 +180,7 @@ mod test_std { mod snapshots { use super::*; - use serde_with::formats::CommaSeparator; + use serde_with::formats::*; use std::collections::BTreeSet; declare_snapshot_test! { @@ -239,6 +239,22 @@ mod snapshots { data: BTreeSet, } } + + duration { + struct Test { + #[serde_as(as = "DurationSeconds")] + seconds: std::time::Duration, + + #[serde_as(as = "DurationSecondsWithFrac")] + frac: std::time::Duration, + + #[serde_as(as = "DurationSeconds")] + seconds_u64_strict: std::time::Duration, + + #[serde_as(as = "TimestampSeconds")] + time_i64: std::time::SystemTime, + } + } } } @@ -439,6 +455,110 @@ mod bytes_or_string { } } +mod duration { + use super::*; + use serde_with::formats::{Flexible, Strict}; + use std::time::{Duration, SystemTime}; + + #[serde_as] + #[derive(Serialize, JsonSchema)] + struct DurationTest { + #[serde_as(as = "DurationSeconds")] + strict_u64: Duration, + + #[serde_as(as = "DurationSeconds")] + strict_str: Duration, + + #[serde_as(as = "DurationSecondsWithFrac")] + strict_f64: Duration, + + #[serde_as(as = "DurationSeconds")] + flexible_u64: Duration, + + #[serde_as(as = "DurationSeconds")] + flexible_f64: Duration, + + #[serde_as(as = "DurationSeconds")] + flexible_str: Duration, + } + + #[test] + fn test_serialized_is_valid() { + check_valid_json_schema(&DurationTest { + strict_u64: Duration::from_millis(2500), + strict_str: Duration::from_millis(2500), + strict_f64: Duration::from_millis(2500), + flexible_u64: Duration::from_millis(2500), + flexible_f64: Duration::from_millis(2500), + flexible_str: Duration::from_millis(2500), + }); + } + + #[serde_as] + #[derive(Serialize, JsonSchema)] + struct FlexibleU64Duration(#[serde_as(as = "DurationSeconds")] Duration); + + #[serde_as] + #[derive(Serialize, JsonSchema)] + struct FlexibleStringDuration(#[serde_as(as = "DurationSeconds")] Duration); + + #[serde_as] + #[derive(Serialize, JsonSchema)] + struct FlexibleTimestamp(#[serde_as(as = "TimestampSeconds")] SystemTime); + + #[test] + fn test_string_as_flexible_u64() { + check_matches_schema::(&json!("32")); + } + + #[test] + fn test_integer_as_flexible_u64() { + check_matches_schema::(&json!(16)); + } + + #[test] + fn test_number_as_flexible_u64() { + check_matches_schema::(&json!(54.1)); + } + + #[test] + #[should_panic] + fn test_negative_as_flexible_u64() { + check_matches_schema::(&json!(-5)); + } + + #[test] + fn test_string_as_flexible_string() { + check_matches_schema::(&json!("32")); + } + + #[test] + fn test_integer_as_flexible_string() { + check_matches_schema::(&json!(16)); + } + + #[test] + fn test_number_as_flexible_string() { + check_matches_schema::(&json!(54.1)); + } + + #[test] + #[should_panic] + fn test_negative_as_flexible_string() { + check_matches_schema::(&json!(-5)); + } + + #[test] + fn test_negative_as_flexible_timestamp() { + check_matches_schema::(&json!(-50000)); + } + + #[test] + fn test_negative_string_as_flexible_timestamp() { + check_matches_schema::(&json!("-50000")); + } +} + #[test] fn test_borrow_cow() { use std::borrow::Cow; diff --git a/serde_with/tests/schemars_0_8/snapshots/duration.json b/serde_with/tests/schemars_0_8/snapshots/duration.json new file mode 100644 index 00000000..f0c11396 --- /dev/null +++ b/serde_with/tests/schemars_0_8/snapshots/duration.json @@ -0,0 +1,53 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Test", + "type": "object", + "required": [ + "frac", + "seconds", + "seconds_u64_strict", + "time_i64" + ], + "properties": { + "frac": { + "oneOf": [ + { + "type": "number", + "minimum": 0.0 + }, + { + "writeOnly": true, + "type": "string" + } + ] + }, + "seconds": { + "oneOf": [ + { + "type": "number", + "minimum": 0.0 + }, + { + "writeOnly": true, + "type": "string" + } + ] + }, + "seconds_u64_strict": { + "type": "integer", + "format": "uint64", + "minimum": 0.0 + }, + "time_i64": { + "oneOf": [ + { + "type": "number" + }, + { + "writeOnly": true, + "type": "string" + } + ] + } + } +} From cf295734f59e9fbabc63ddbf114b04ca1905009e Mon Sep 17 00:00:00 2001 From: Sean Lynch <42618346+swlynch99@users.noreply.github.com> Date: Mon, 29 Jan 2024 15:32:37 -0800 Subject: [PATCH 2/2] Add a pattern key for the duration string The string representation of the date actually has only a restricted set of values that can be used. By using the pattern property we can have the json schema itself validate the string. The regex used here for the pattern is a more lenient version of that used to match numbers in JSON. --- serde_with/src/schemars_0_8.rs | 150 ++++++++++++------ serde_with/tests/schemars_0_8.rs | 3 + .../schemars_0_8/snapshots/duration.json | 23 ++- 3 files changed, 122 insertions(+), 54 deletions(-) diff --git a/serde_with/src/schemars_0_8.rs b/serde_with/src/schemars_0_8.rs index 4f2f17e1..4e54e17d 100644 --- a/serde_with/src/schemars_0_8.rs +++ b/serde_with/src/schemars_0_8.rs @@ -613,48 +613,76 @@ where mod timespan { use super::*; + // #[non_exhaustive] is not actually necessary here but it should + // help avoid warnings about semver breakage if this ever changes. + #[non_exhaustive] + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + pub enum TimespanTargetType { + String, + F64, + U64, + I64, + } + + impl TimespanTargetType { + pub const fn is_signed(self) -> bool { + !matches!(self, Self::U64) + } + } + /// Internal helper trait used to constrain which types we implement /// `JsonSchemaAs` for. pub trait TimespanSchemaTarget { - /// Whether F is signed. + /// The underlying type. + /// + /// This is mainly used to decide which variant of the resulting schema + /// should be marked as `write_only: true`. + const TYPE: TimespanTargetType; + + /// Whether the target type is signed. + /// + /// This is only true for `std::time::Duration`. const SIGNED: bool = true; - - /// Whether F is String - const STRING: bool; } - macro_rules! is_string { + macro_rules! timespan_type_of { (String) => { - true + TimespanTargetType::String }; - ($name:ty) => { - false + (f64) => { + TimespanTargetType::F64 + }; + (i64) => { + TimespanTargetType::I64 + }; + (u64) => { + TimespanTargetType::U64 }; } macro_rules! declare_timespan_target { - ( $target:ty { $($format:ty),* $(,)? } )=> { + ( $target:ty { $($format:ident),* $(,)? } ) => { $( impl TimespanSchemaTarget<$format> for $target { - const STRING: bool = is_string!($format); + const TYPE: TimespanTargetType = timespan_type_of!($format); } )* } } impl TimespanSchemaTarget for Duration { + const TYPE: TimespanTargetType = TimespanTargetType::U64; const SIGNED: bool = false; - const STRING: bool = false; } impl TimespanSchemaTarget for Duration { + const TYPE: TimespanTargetType = TimespanTargetType::F64; const SIGNED: bool = false; - const STRING: bool = false; } impl TimespanSchemaTarget for Duration { + const TYPE: TimespanTargetType = TimespanTargetType::String; const SIGNED: bool = false; - const STRING: bool = true; } declare_timespan_target!(SystemTime { i64, f64, String }); @@ -676,7 +704,7 @@ mod timespan { declare_timespan_target!(::time_0_3::PrimitiveDateTime { i64, f64, String }); } -use self::timespan::TimespanSchemaTarget; +use self::timespan::{TimespanSchemaTarget, TimespanTargetType}; /// Internal type used for the base impls on DurationXXX and TimestampYYY types. /// @@ -692,37 +720,61 @@ where forward_schema!(F); } -fn flexible_timespan_schema(signed: bool, is_string: bool) -> Schema { - let mut number = SchemaObject { - instance_type: Some(InstanceType::Number.into()), - number: (!signed).then(|| { - Box::new(NumberValidation { - minimum: Some(0.0), +impl TimespanTargetType { + pub(crate) fn to_flexible_schema(self, signed: bool) -> Schema { + use ::schemars_0_8::schema::StringValidation; + + let mut number = SchemaObject { + instance_type: Some(InstanceType::Number.into()), + number: (!signed).then(|| { + Box::new(NumberValidation { + minimum: Some(0.0), + ..Default::default() + }) + }), + ..Default::default() + }; + + // This is a more lenient version of the regex used to determine + // whether JSON numbers are valid. Specifically, it allows multiple + // leading zeroes whereas that is illegal in JSON. + let regex = r#"[0-9]+(\.[0-9]+)?([eE][+-]?[0-9]+)?"#; + let mut string = SchemaObject { + instance_type: Some(InstanceType::String.into()), + string: Some(Box::new(StringValidation { + pattern: Some(match signed { + true => std::format!("^-?{regex}$"), + false => std::format!("^{regex}$"), + }), ..Default::default() - }) - }), - ..Default::default() - }; + })), + ..Default::default() + }; - let mut string = SchemaObject { - instance_type: Some(InstanceType::String.into()), - ..Default::default() - }; + if self == Self::String { + number.metadata().write_only = true; + } else { + string.metadata().write_only = true; + } - if is_string { - number.metadata().write_only = true; - } else { - string.metadata().write_only = true; + SchemaObject { + subschemas: Some(Box::new(SubschemaValidation { + one_of: Some(std::vec![number.into(), string.into()]), + ..Default::default() + })), + ..Default::default() + } + .into() } - SchemaObject { - subschemas: Some(Box::new(SubschemaValidation { - one_of: Some(std::vec![number.into(), string.into()]), - ..Default::default() - })), - ..Default::default() + pub(crate) fn schema_id(self) -> &'static str { + match self { + Self::String => "serde_with::FlexibleStringTimespan", + Self::F64 => "serde_with::FlexibleF64Timespan", + Self::U64 => "serde_with::FlexibleU64Timespan", + Self::I64 => "serde_with::FlexibleI64Timespan", + } } - .into() } impl JsonSchemaAs for Timespan @@ -731,24 +783,20 @@ where F: Format + JsonSchema, { fn schema_name() -> String { - match >::STRING { - true => "FlexibleStringTimespan".into(), - false => "FlexibleTimespan".into(), - } + >::TYPE + .schema_id() + .strip_prefix("serde_with::") + .expect("schema id did not start with `serde_with::` - this is a bug") + .into() } fn schema_id() -> Cow<'static, str> { - match >::STRING { - true => "serde_with::FlexibleStringTimespan".into(), - false => "serde_with::FlexibleTimespan".into(), - } + >::TYPE.schema_id().into() } fn json_schema(_: &mut SchemaGenerator) -> Schema { - flexible_timespan_schema( - >::SIGNED, - >::STRING, - ) + >::TYPE + .to_flexible_schema(>::SIGNED) } fn is_referenceable() -> bool { diff --git a/serde_with/tests/schemars_0_8.rs b/serde_with/tests/schemars_0_8.rs index fe02b63b..d9f8b30e 100644 --- a/serde_with/tests/schemars_0_8.rs +++ b/serde_with/tests/schemars_0_8.rs @@ -248,6 +248,9 @@ mod snapshots { #[serde_as(as = "DurationSecondsWithFrac")] frac: std::time::Duration, + #[serde_as(as = "DurationSeconds")] + flexible_string: std::time::Duration, + #[serde_as(as = "DurationSeconds")] seconds_u64_strict: std::time::Duration, diff --git a/serde_with/tests/schemars_0_8/snapshots/duration.json b/serde_with/tests/schemars_0_8/snapshots/duration.json index f0c11396..cb828a27 100644 --- a/serde_with/tests/schemars_0_8/snapshots/duration.json +++ b/serde_with/tests/schemars_0_8/snapshots/duration.json @@ -3,12 +3,26 @@ "title": "Test", "type": "object", "required": [ + "flexible_string", "frac", "seconds", "seconds_u64_strict", "time_i64" ], "properties": { + "flexible_string": { + "oneOf": [ + { + "writeOnly": true, + "type": "number", + "minimum": 0.0 + }, + { + "type": "string", + "pattern": "^[0-9]+(\\.[0-9]+)?([eE][+-]?[0-9]+)?$" + } + ] + }, "frac": { "oneOf": [ { @@ -17,7 +31,8 @@ }, { "writeOnly": true, - "type": "string" + "type": "string", + "pattern": "^[0-9]+(\\.[0-9]+)?([eE][+-]?[0-9]+)?$" } ] }, @@ -29,7 +44,8 @@ }, { "writeOnly": true, - "type": "string" + "type": "string", + "pattern": "^[0-9]+(\\.[0-9]+)?([eE][+-]?[0-9]+)?$" } ] }, @@ -45,7 +61,8 @@ }, { "writeOnly": true, - "type": "string" + "type": "string", + "pattern": "^-?[0-9]+(\\.[0-9]+)?([eE][+-]?[0-9]+)?$" } ] }