From bde43f51f8f47bc6236104e0c57903b0181ac351 Mon Sep 17 00:00:00 2001 From: Jimmy Cuadra Date: Wed, 14 Sep 2022 22:51:05 -0700 Subject: [PATCH 1/6] Implement `IntoActiveValue` for `time` types. I tried to implement a [custom active model](https://www.sea-ql.org/SeaORM/docs/advanced-query/custom-active-model/), and one of the columns was `Option`. I got a compiler error: ``` error[E0277]: the trait bound `std::option::Option: IntoActiveValue<_>` is not satisfied ``` Looking into the source code, it seemed a simple oversight that this trait was implemented for the `chrono` types but not the `time` types, and it was easy enough to fix since there's already a macro to implement it for new types. I also noticed that the `time` types are not accounted for in `src/query/json.rs` while the `chrono` types are, which I assume is also an oversight. However, I don't have a need for that at this point and the fix for that seemed less trivial, so I'm just bringing it to your attention. Thanks for SeaORM! --- src/entity/active_model.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/entity/active_model.rs b/src/entity/active_model.rs index afc8bd28f..cf679d963 100644 --- a/src/entity/active_model.rs +++ b/src/entity/active_model.rs @@ -719,6 +719,22 @@ impl_into_active_value!(crate::prelude::Decimal); #[cfg_attr(docsrs, doc(cfg(feature = "with-uuid")))] impl_into_active_value!(crate::prelude::Uuid); +#[cfg(feature = "with-time")] +#[cfg_attr(docsrs, doc(cfg(feature = "with-time")))] +impl_into_active_value!(crate::prelude::TimeDate); + +#[cfg(feature = "with-time")] +#[cfg_attr(docsrs, doc(cfg(feature = "with-time")))] +impl_into_active_value!(crate::prelude::TimeTime); + +#[cfg(feature = "with-time")] +#[cfg_attr(docsrs, doc(cfg(feature = "with-time")))] +impl_into_active_value!(crate::prelude::TimeDateTime); + +#[cfg(feature = "with-time")] +#[cfg_attr(docsrs, doc(cfg(feature = "with-time")))] +impl_into_active_value!(crate::prelude::TimeDateTimeWithTimeZone); + impl Default for ActiveValue where V: Into, From 8ecb4fccba3bc7eca807899996071069fccf2363 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 15 Sep 2022 15:40:34 +0800 Subject: [PATCH 2/6] Implement `IntoActiveValue` for `Vec` types --- src/entity/active_model.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/entity/active_model.rs b/src/entity/active_model.rs index cf679d963..0603662cc 100644 --- a/src/entity/active_model.rs +++ b/src/entity/active_model.rs @@ -682,6 +682,7 @@ impl_into_active_value!(f32); impl_into_active_value!(f64); impl_into_active_value!(&'static str); impl_into_active_value!(String); +impl_into_active_value!(Vec); #[cfg(feature = "with-json")] #[cfg_attr(docsrs, doc(cfg(feature = "with-json")))] From 0d31a012cc94519efe08b2335f4e9e5a749020bb Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 15 Sep 2022 15:41:13 +0800 Subject: [PATCH 3/6] Add tests to double check and prevent it from happening again --- tests/type_tests.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/type_tests.rs diff --git a/tests/type_tests.rs b/tests/type_tests.rs new file mode 100644 index 000000000..5ceaf5115 --- /dev/null +++ b/tests/type_tests.rs @@ -0,0 +1,52 @@ +pub mod common; + +use sea_orm::{IntoActiveValue, TryFromU64, TryGetable, Value}; + +pub fn it_impl_into_active_value, V: Into>() {} + +pub fn it_impl_try_getable() {} + +pub fn it_impl_try_from_u64() {} + +macro_rules! it_impl_traits { + ( $ty: ty ) => { + it_impl_into_active_value::<$ty, $ty>(); + it_impl_into_active_value::, Option<$ty>>(); + it_impl_into_active_value::>, Option<$ty>>(); + + it_impl_try_getable::<$ty>(); + it_impl_try_getable::>(); + + it_impl_try_from_u64::<$ty>(); + }; +} + +#[sea_orm_macros::test] +fn main() { + it_impl_traits!(i8); + it_impl_traits!(i16); + it_impl_traits!(i32); + it_impl_traits!(i64); + it_impl_traits!(u8); + it_impl_traits!(u16); + it_impl_traits!(u32); + it_impl_traits!(u64); + it_impl_traits!(bool); + it_impl_traits!(f32); + it_impl_traits!(f64); + it_impl_traits!(Vec); + it_impl_traits!(String); + it_impl_traits!(serde_json::Value); + it_impl_traits!(chrono::NaiveDate); + it_impl_traits!(chrono::NaiveTime); + it_impl_traits!(chrono::NaiveDateTime); + it_impl_traits!(chrono::DateTime); + it_impl_traits!(chrono::DateTime); + it_impl_traits!(chrono::DateTime); + it_impl_traits!(time::Date); + it_impl_traits!(time::Time); + it_impl_traits!(time::PrimitiveDateTime); + it_impl_traits!(time::OffsetDateTime); + it_impl_traits!(rust_decimal::Decimal); + it_impl_traits!(uuid::Uuid); +} From 7c591049bee99b0142ff307c9d863acbed4f3d65 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 15 Sep 2022 16:18:13 +0800 Subject: [PATCH 4/6] Add docs --- tests/type_tests.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/type_tests.rs b/tests/type_tests.rs index 5ceaf5115..4225b080d 100644 --- a/tests/type_tests.rs +++ b/tests/type_tests.rs @@ -2,6 +2,18 @@ pub mod common; use sea_orm::{IntoActiveValue, TryFromU64, TryGetable, Value}; +/* + +When supporting a new type in SeaORM we should implement the following traits for it: + - `IntoActiveValue`, given that it implemented `Into` already + - `TryGetable` + - `TryFromU64` + +Also, we need to update `impl FromQueryResult for JsonValue` at `src/query/json.rs` +to correctly serialize the type as `serde_json::Value`. + +*/ + pub fn it_impl_into_active_value, V: Into>() {} pub fn it_impl_try_getable() {} From 1035bbb431efbb12b696f12fdc58ccbb802a0268 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 15 Sep 2022 16:44:31 +0800 Subject: [PATCH 5/6] Fixup --- tests/type_tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/type_tests.rs b/tests/type_tests.rs index 4225b080d..0c9361301 100644 --- a/tests/type_tests.rs +++ b/tests/type_tests.rs @@ -20,6 +20,7 @@ pub fn it_impl_try_getable() {} pub fn it_impl_try_from_u64() {} +#[allow(unused_macros)] macro_rules! it_impl_traits { ( $ty: ty ) => { it_impl_into_active_value::<$ty, $ty>(); @@ -34,6 +35,7 @@ macro_rules! it_impl_traits { } #[sea_orm_macros::test] +#[cfg(feature = "sqlx-dep")] fn main() { it_impl_traits!(i8); it_impl_traits!(i16); From fd93ea7a391c5f94ae0f3d0a4a4b971f6609374e Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 15 Sep 2022 16:21:16 +0800 Subject: [PATCH 6/6] Serialize `time` types as `serde_json::Value` --- Cargo.toml | 2 +- src/query/json.rs | 16 ++++++++++++++++ tests/time_crate_tests.rs | 27 +++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f119bc756..39466ae6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ default = [ ] macros = ["sea-orm-macros"] mock = [] -with-json = ["serde_json", "sea-query/with-json", "chrono/serde", "sqlx?/json"] +with-json = ["serde_json", "sea-query/with-json", "chrono/serde", "time/serde", "sqlx?/json"] with-chrono = ["chrono", "sea-query/with-chrono", "sqlx?/chrono"] with-rust_decimal = ["rust_decimal", "sea-query/with-rust_decimal", "sqlx?/decimal"] with-uuid = ["uuid", "sea-query/with-uuid", "sqlx?/uuid"] diff --git a/src/query/json.rs b/src/query/json.rs index 28aa1146f..e1c1f0bfd 100644 --- a/src/query/json.rs +++ b/src/query/json.rs @@ -54,6 +54,14 @@ impl FromQueryResult for JsonValue { match_mysql_type!(chrono::NaiveDateTime); #[cfg(feature = "with-chrono")] match_mysql_type!(chrono::DateTime); + #[cfg(feature = "with-time")] + match_mysql_type!(time::Date); + #[cfg(feature = "with-time")] + match_mysql_type!(time::Time); + #[cfg(feature = "with-time")] + match_mysql_type!(time::PrimitiveDateTime); + #[cfg(feature = "with-time")] + match_mysql_type!(time::OffsetDateTime); #[cfg(feature = "with-rust_decimal")] match_mysql_type!(rust_decimal::Decimal); #[cfg(feature = "with-json")] @@ -106,6 +114,14 @@ impl FromQueryResult for JsonValue { match_postgres_type!(chrono::NaiveDateTime); #[cfg(feature = "with-chrono")] match_postgres_type!(chrono::DateTime); + #[cfg(feature = "with-time")] + match_postgres_type!(time::Date); + #[cfg(feature = "with-time")] + match_postgres_type!(time::Time); + #[cfg(feature = "with-time")] + match_postgres_type!(time::PrimitiveDateTime); + #[cfg(feature = "with-time")] + match_postgres_type!(time::OffsetDateTime); #[cfg(feature = "with-rust_decimal")] match_postgres_type!(rust_decimal::Decimal); #[cfg(feature = "with-json")] diff --git a/tests/time_crate_tests.rs b/tests/time_crate_tests.rs index f7a4cff05..6419919c9 100644 --- a/tests/time_crate_tests.rs +++ b/tests/time_crate_tests.rs @@ -1,6 +1,7 @@ pub mod common; pub use common::{features::*, setup::*, TestContext}; use sea_orm::{entity::prelude::*, DatabaseConnection, IntoActiveModel}; +use serde_json::json; use time::macros::{date, time}; #[sea_orm_macros::test] @@ -42,5 +43,31 @@ pub async fn create_transaction_log(db: &DatabaseConnection) -> Result<(), DbErr Some(transaction_log.clone()) ); + let json = TransactionLog::find().into_json().one(db).await?.unwrap(); + + #[cfg(feature = "sqlx-postgres")] + assert_eq!( + json, + json!({ + "id": 1, + "date": "2022-03-13", + "time": "16:24:00", + "date_time": "2022-03-13T16:24:00", + "date_time_tz": "2022-03-13T16:24:00+00:00", + }) + ); + + #[cfg(feature = "sqlx-mysql")] + assert_eq!( + json, + json!({ + "id": 1, + "date": "2022-03-13", + "time": "16:24:00", + "date_time": "2022-03-13T16:24:00", + "date_time_tz": "2022-03-13T16:24:00Z", + }) + ); + Ok(()) }