-
-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable convert from ActiveModel to Model #725
Enable convert from ActiveModel to Model #725
Conversation
Hi, I'm new comer and chose #606 as my first issue to work on. As @billy1624 suggested, I introduce a new trait For now, error[E0277]: the trait bound `i32: From<sea_query::Value>` is not satisfied
--> /home/sunhengke/Development/rust/sea-orm/src/tests_cfg/cake_filling_price.rs:17:48
|
17 | #[derive(Clone, Debug, PartialEq, DeriveModel, DeriveActiveModel)]
| ^^^^^^^^^^^^^^^^^ the trait `From<sea_query::Value>` is not implemented for `i32`
|
= help: the following implementations were found:
<i32 as From<NonZeroI32>>
<i32 as From<bool>>
<i32 as From<i16>>
<i32 as From<i8>>
and 86 others
= note: required because of the requirements on the impl of `Into<i32>` for `sea_query::Value`
= note: this error originates in the derive macro `DeriveActiveModel` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0063]: missing field `ignored_attr` in initializer of `cake_filling_price::Model`
--> /home/sunhengke/Development/rust/sea-orm/src/tests_cfg/cake_filling_price.rs:17:48
|
17 | #[derive(Clone, Debug, PartialEq, DeriveModel, DeriveActiveModel)]
| ^^^^^^^^^^^^^^^^^ missing `ignored_attr`
|
= note: this error originates in the derive macro `DeriveActiveModel` (in Nightly builds, run with -Z macro-backtrace for more info) The first error is because #[derive(Clone, Debug, PartialEq, DeriveModel, DeriveActiveModel)]
pub struct Model {
pub id: i32,
pub name: String,
pub vendor_id: Option<i32>,
#[sea_orm(ignore)]
pub ignored_attr: i32,
} the field |
I got an idea for the second err, if there's ignored field in a For the first err, I'm still not sure what to do. Am I supposed to call functions like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @greenhandatsjtu, I'm sorry for the delay.
the second error is because the model is: the field ignored_attr is ignored in ActiveModel, so we can never get the value of ignored_attr when trying to convert ActiveModel back to Model.
Ignored fields are assumed to have Default
implemented. So, you can simply write Default::default()
Just like what we did in DeriveModel
procedural macros:
sea-orm/sea-orm-macros/src/derives/model.rs
Lines 113 to 121 in fff738a
if *ignore { | |
quote! { | |
Default::default() | |
} | |
} else { | |
quote! { | |
row.try_get(pre, sea_orm::IdenStatic::as_str(&<<Self as sea_orm::ModelTrait>::Entity as sea_orm::entity::EntityTrait>::Column::#column_ident).into())? | |
} | |
} |
Self { | ||
#(#field: a.#field.into_value().unwrap().into()),* | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first error is because
Value
doesn't implementInto<xxx>
I guess we can a.#field.unwrap().unwrap()
Hi @billy1624 , thanks for your advice. I closed this PR because I'm quite busy recently and had no idea on how to handle these errors 😢. Now that you give me some advice, I think it's worthy to give it another try :) I will continue working on this PR in the next few days. Thank you again! |
49f42bd
to
5eaa58f
Compare
Hi @billy1624 , I just had some free time last weekend and worked on it according to your suggestions, and I think I made it! Thank you so much for guiding me~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @greenhandatsjtu, sorry for the delay. Could you please add some test cases to test the conversion from ActiveModel
to Model
?
Better include tests where some model field are annotated with #[sea_orm(ignore)]
.
https://www.sea-ql.org/SeaORM/docs/generate-entity/entity-structure/#ignore-attribute
Many thanks!!
Hey @billy1624 , sure I will add some tests. However, I need some instructions on following two problems:
#[derive(Clone, Debug, PartialEq, DeriveModel, DeriveActiveModel)]
pub struct Model {
pub id: i32,
pub name: String,
}
let pineapple = ActiveModel {
id: Set(1),
name: Set("Pineapple".to_owned()),
};
let model: Model = pineapple.try_into_model().unwrap();
assert_eq!(model.id, 1);
assert_eq!(model.name, "Pineapple".to_owned()); I would appreciate it if you could give me some advice! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @greenhandatsjtu, if you execute cargo expand
command. You will see:
#[automatically_derived]
impl std::convert::TryFrom<ActiveModel> for <Entity as EntityTrait>::Model {
type Error = DbErr;
fn try_from(a: ActiveModel) -> Result<Self, DbErr> {
if match a.id {
sea_orm::ActiveValue::NotSet => true,
_ => false,
} {
return Err(DbErr::Custom({
let res = ::alloc::fmt::format(::core::fmt::Arguments::new_v1(
&["field ", " is NotSet"],
&[::core::fmt::ArgumentV1::new_display(&"id")],
));
res
}));
}
if match a.name {
sea_orm::ActiveValue::NotSet => true,
_ => false,
} {
return Err(DbErr::Custom({
let res = ::alloc::fmt::format(::core::fmt::Arguments::new_v1(
&["field ", " is NotSet"],
&[::core::fmt::ArgumentV1::new_display(&"name")],
));
res
}));
}
Ok(Self {
id: a.id.into_value().unwrap().unwrap(),
name: a.name.into_value().unwrap().unwrap(),
})
}
}
#[automatically_derived]
impl sea_orm::TryIntoModel<<Entity as EntityTrait>::Model> for ActiveModel {
fn try_into_model(self) -> Result<<Entity as EntityTrait>::Model, DbErr> {
self.try_into()
}
}
When converting from ActiveModel
to Model
. A field is missing.
Ok(Self {
id: a.id.into_value().unwrap().unwrap(),
name: a.name.into_value().unwrap().unwrap(),
// `cake_id` is missing here
})
EDIT: I fixed the error on #975. I consider it's a misuse of DeriveActiveModel
that cause the error.
You can put test under here: sea-orm/src/entity/active_model.rs Line 841 in d262501
Basically, we want to test the success and fail cases. Also the special case, where some fields are being ignored. |
Hi @billy1624 , thanks for your guidance, I've add some tests for converting from ActiveModel to Model, please review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @greenhandatsjtu, the test cases looks nice!
I'd like to refactor the code, please check greenhandatsjtu#1
Hi @billy1624 , the PR looks good to me and I've merged it, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @greenhandatsjtu, thanks again for the contributions!
I will merge this into a working branch and do a rebase before it'll be merged into master. :)
* feat: enable convert from ActiveModel to Model * feat: add tests for converting from ActiveModel to Model * cargo fmt * Refactoring Co-authored-by: Billy Chan <[email protected]>
* Changelog * Enable convert from ActiveModel to Model (#725) * feat: enable convert from ActiveModel to Model * feat: add tests for converting from ActiveModel to Model * cargo fmt * Refactoring Co-authored-by: Billy Chan <[email protected]> * Fix clippy warnings * Use error type Co-authored-by: Chris Tsang <[email protected]> Co-authored-by: greenhandatsjtu <[email protected]>
PR Info
Related issue: #606
Adds
Changes