Skip to content
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

feat: Allow nesting of structs deriving FromQueryResult (and DerivePartialModel) #2179

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 96 additions & 23 deletions sea-orm-macros/src/derives/from_query_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,81 @@ use syn::{
ext::IdentExt, punctuated::Punctuated, token::Comma, Data, DataStruct, Fields, Generics, Meta,
};

pub struct FromQueryResultItem {
pub skip: bool,
enum ItemType {
Normal,
Skipped,
Nested,
}

struct FromQueryResultItem {
pub typ: ItemType,
pub ident: Ident,
}
impl ToTokens for FromQueryResultItem {

/// Initially, we try to obtain the value for each field and check if it is an ordinary DB error
/// (which we return immediatly), or a null error.
///
/// ### Background
///
/// Null errors do not necessarily mean that the deserialization as a whole fails,
/// since structs embedding the current one might have wrapped the current one in an `Option`.
/// In this case, we do not want to swallow other errors, which are very likely to actually be
/// programming errors that should be noticed (and fixed).
struct TryFromQueryResultCheck<'a>(&'a FromQueryResultItem);

impl<'a> ToTokens for TryFromQueryResultCheck<'a> {
fn to_tokens(&self, tokens: &mut TokenStream) {
let FromQueryResultItem { ident, typ } = self.0;

match typ {
ItemType::Normal => {
let name = ident.unraw().to_string();
tokens.extend(quote! {
let #ident = match row.try_get_nullable(pre, #name) {
Err(v @ sea_orm::TryGetError::DbErr(_)) => {
return Err(v);
}
v => v,
};
});
}
ItemType::Skipped => {
tokens.extend(quote! {
let #ident = std::default::Default::default();
});
}
ItemType::Nested => {
let name = ident.unraw().to_string();
tokens.extend(quote! {
let #ident = match sea_orm::FromQueryResult::from_query_result_nullable(row, &format!("{pre}{}-", #name)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the package name to the prefix makes the whole thing unusable unless you are using it with a DerivePartialModel because the base model don't add the entity name in front of the column. An integration test would have caught that easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This is not the package name but a prefix computed based on the location (property name) of the nested struct in the parent struct.
  2. There are integration tests.
  3. nested only makes sense when used with DerivePartialModel.
  4. (I have specified this in the comments explicitly as well) This is required to avoid name collisions.

Copy link
Contributor Author

@jreppnow jreppnow Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is about using a struct with nested in it in into_model::<..>(), I guess? Yup, that is most likely going to break, but due to the name collision thing above there is not really anything I can do about it. The relationship between FromQueryResult and DerivePartialModel is weird and they should arguably be mutually exclusive, but that would involve copying the entire code from FromQueryResult into DerivePartialModel. That would also mean doing things like the skip I proposed elsewhere twice: #2167

Err(v @ sea_orm::TryGetError::DbErr(_)) => {
return Err(v);
}
v => v,
};
});
}
}
}
}

struct TryFromQueryResultAssignment<'a>(&'a FromQueryResultItem);

impl<'a> ToTokens for TryFromQueryResultAssignment<'a> {
fn to_tokens(&self, tokens: &mut TokenStream) {
let Self { ident, skip } = self;
if *skip {
tokens.extend(quote! {
#ident: std::default::Default::default(),
});
} else {
let name = ident.unraw().to_string();
tokens.extend(quote! {
#ident: row.try_get(pre, #name)?,
});
let FromQueryResultItem { ident, typ, .. } = self.0;

match typ {
ItemType::Normal | ItemType::Nested => {
tokens.extend(quote! {
#ident: #ident?,
});
}
ItemType::Skipped => {
tokens.extend(quote! {
#ident,
});
}
}
}
}
Expand All @@ -31,7 +90,7 @@ pub fn expand_derive_from_query_result(
data: Data,
generics: Generics,
) -> syn::Result<TokenStream> {
let fields = match data {
let parsed_fields = match data {
Data::Struct(DataStruct {
fields: Fields::Named(named),
..
Expand All @@ -42,40 +101,54 @@ pub fn expand_derive_from_query_result(
})
}
};
let mut field = Vec::with_capacity(fields.len());

for parsed_field in fields.into_iter() {
let mut skip = false;
let mut fields = Vec::with_capacity(parsed_fields.len());
for parsed_field in parsed_fields.into_iter() {
let mut typ = ItemType::Normal;
for attr in parsed_field.attrs.iter() {
if !attr.path().is_ident("sea_orm") {
continue;
}
if let Ok(list) = attr.parse_args_with(Punctuated::<Meta, Comma>::parse_terminated) {
for meta in list.iter() {
skip = meta.exists("skip");
if meta.exists("skip") {
typ = ItemType::Skipped;
} else if meta.exists("nested") {
typ = ItemType::Nested;
}
}
}
}
let ident = format_ident!("{}", parsed_field.ident.unwrap().to_string());
field.push(FromQueryResultItem { skip, ident });
fields.push(FromQueryResultItem { typ, ident });
}
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

let ident_try_init: Vec<_> = fields.iter().map(TryFromQueryResultCheck).collect();
let ident_try_assign: Vec<_> = fields.iter().map(TryFromQueryResultAssignment).collect();

Ok(quote!(
#[automatically_derived]
impl #impl_generics sea_orm::FromQueryResult for #ident #ty_generics #where_clause {
fn from_query_result(row: &sea_orm::QueryResult, pre: &str) -> std::result::Result<Self, sea_orm::DbErr> {
fn from_query_result(row: &sea_orm::QueryResult, pre: &str) -> Result<Self, sea_orm::DbErr> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use std::result::Result

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid criticism. The correct way is to use ::std::.. or ::sea_orm::.. everywhere, so I will adjust it to that.

Ok(Self::from_query_result_nullable(row, pre)?)
}

fn from_query_result_nullable(row: &sea_orm::QueryResult, pre: &str) -> Result<Self, sea_orm::TryGetError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, use std::result::Result

#(#ident_try_init)*

Ok(Self {
#(#field)*
#(#ident_try_assign)*
})
}
}
))
}
mod util {

pub(super) mod util {
use syn::Meta;

pub(super) trait GetMeta {
pub trait GetMeta {
fn exists(&self, k: &str) -> bool;
}

Expand Down
100 changes: 75 additions & 25 deletions sea-orm-macros/src/derives/partial_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,25 @@ use proc_macro2::TokenStream;
use quote::format_ident;
use quote::quote;
use quote::quote_spanned;
use syn::ext::IdentExt;
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::token::Comma;
use syn::Expr;

use syn::Meta;
use syn::Type;

use super::from_query_result::util::GetMeta;

use self::util::GetAsKVMeta;

#[derive(Debug)]
enum Error {
InputNotStruct,
EntityNotSpecific,
EntityNotSpecified,
NotSupportGeneric(Span),
BothFromColAndFromExpr(Span),
OverlappingAttributes(Span),
Syn(syn::Error),
}
#[derive(Debug, PartialEq, Eq)]
Expand All @@ -29,6 +33,8 @@ enum ColumnAs {
ColAlias { col: syn::Ident, field: String },
/// from an expr
Expr { expr: syn::Expr, field_name: String },
/// nesting another struct
Nested { typ: Type, field_name: String },
}

struct DerivePartialModel {
Expand Down Expand Up @@ -78,6 +84,7 @@ impl DerivePartialModel {

let mut from_col = None;
let mut from_expr = None;
let mut nested = false;

for attr in field.attrs.iter() {
if !attr.path().is_ident("sea_orm") {
Expand All @@ -94,35 +101,37 @@ impl DerivePartialModel {
.get_as_kv("from_expr")
.map(|s| syn::parse_str::<Expr>(&s).map_err(Error::Syn))
.transpose()?;
nested = meta.exists("nested");
}
}
}

let field_name = field.ident.unwrap();

let col_as = match (from_col, from_expr) {
(None, None) => {
let col_as = match (from_col, from_expr, nested) {
(Some(col), None, false) => {
if entity.is_none() {
return Err(Error::EntityNotSpecific);
return Err(Error::EntityNotSpecified);
}
ColumnAs::Col(format_ident!(
"{}",
field_name.to_string().to_upper_camel_case()
))

let field = field_name.to_string();
ColumnAs::ColAlias { col, field }
}
(None, Some(expr)) => ColumnAs::Expr {
(None, Some(expr), false) => ColumnAs::Expr {
expr,
field_name: field_name.to_string(),
},
(Some(col), None) => {
(None, None, true) => ColumnAs::Nested {
typ: field.ty,
field_name: field_name.unraw().to_string(),
},
(None, None, false) => {
if entity.is_none() {
return Err(Error::EntityNotSpecific);
return Err(Error::EntityNotSpecified);
}

let field = field_name.to_string();
ColumnAs::ColAlias { col, field }
ColumnAs::Col(field_name)
}
(Some(_), Some(_)) => return Err(Error::BothFromColAndFromExpr(field_span)),
(_, _, _) => return Err(Error::OverlappingAttributes(field_span)),
};
column_as_list.push(col_as);
}
Expand All @@ -148,23 +157,64 @@ impl DerivePartialModel {
let select_col_code_gen = fields.iter().map(|col_as| match col_as {
ColumnAs::Col(ident) => {
let entity = entity.as_ref().unwrap();
let col_value = quote!( <#entity as sea_orm::EntityTrait>::Column:: #ident);
quote!(let #select_ident = sea_orm::SelectColumns::select_column(#select_ident, #col_value);)
let uppercase_ident = format_ident!(
"{}",
ident.to_string().to_upper_camel_case()
);
let col_value = quote!( <#entity as sea_orm::EntityTrait>::Column:: #uppercase_ident);
let ident_stringified = ident.unraw().to_string();
quote!(let #select_ident =
if let Some(prefix) = pre {
let ident = format!("{prefix}{}", #ident_stringified);
sea_orm::SelectColumns::select_column_as(#select_ident, #col_value, ident)
} else {
sea_orm::SelectColumns::select_column_as(#select_ident, #col_value, #ident_stringified)
};
)
},
ColumnAs::ColAlias { col, field } => {
let entity = entity.as_ref().unwrap();
let col_value = quote!( <#entity as sea_orm::EntityTrait>::Column:: #col);
quote!(let #select_ident = sea_orm::SelectColumns::select_column_as(#select_ident, #col_value, #field);)
quote!(let #select_ident =
if let Some(prefix) = pre {
let ident = format!("{prefix}{}", #field);
sea_orm::SelectColumns::select_column_as(#select_ident, #col_value, ident)
} else {
sea_orm::SelectColumns::select_column_as(#select_ident, #col_value, #field)
};
)
},
ColumnAs::Expr { expr, field_name } => {
quote!(let #select_ident = sea_orm::SelectColumns::select_column_as(#select_ident, #expr, #field_name);)
quote!(let #select_ident =
if let Some(prefix) = pre {
let ident = format!("{prefix}{}", #field_name);
sea_orm::SelectColumns::select_column_as(#select_ident, #expr, ident)
} else {
sea_orm::SelectColumns::select_column_as(#select_ident, #expr, #field_name)
};
)
},
ColumnAs::Nested { typ, field_name } => {
quote!(let #select_ident =
<#typ as sea_orm::PartialModelTrait>::select_cols_nested(#select_ident,
Some(&if let Some(prefix) = pre {
format!("{prefix}{}-", #field_name) }
else {
format!("{}-", #field_name)
}
));
)
},
});

quote! {
#[automatically_derived]
impl sea_orm::PartialModelTrait for #ident{
fn select_cols<S: sea_orm::SelectColumns>(#select_ident: S) -> S{
fn select_cols<S: sea_orm::SelectColumns>(#select_ident: S) -> S {
Self::select_cols_nested(#select_ident, None)
}

fn select_cols_nested<S: sea_orm::SelectColumns>(#select_ident: S, pre: Option<&str>) -> S {
#(#select_col_code_gen)*
#select_ident
}
Expand All @@ -181,10 +231,10 @@ pub fn expand_derive_partial_model(input: syn::DeriveInput) -> syn::Result<Token
Err(Error::NotSupportGeneric(span)) => Ok(quote_spanned! {
span => compile_error!("you can only derive `DerivePartialModel` on named struct");
}),
Err(Error::BothFromColAndFromExpr(span)) => Ok(quote_spanned! {
span => compile_error!("you can only use one of `from_col` or `from_expr`");
Err(Error::OverlappingAttributes(span)) => Ok(quote_spanned! {
span => compile_error!("you can only use one of `from_col`, `from_expr`, `nested`");
}),
Err(Error::EntityNotSpecific) => Ok(quote_spanned! {
Err(Error::EntityNotSpecified) => Ok(quote_spanned! {
ident_span => compile_error!("you need specific which entity you are using")
}),
Err(Error::InputNotStruct) => Ok(quote_spanned! {
Expand Down Expand Up @@ -258,7 +308,7 @@ struct PartialModel{
assert_eq!(middle.fields.len(), 3);
assert_eq!(
middle.fields[0],
ColumnAs::Col(format_ident!("DefaultField"))
ColumnAs::Col(format_ident!("default_field"))
);
assert_eq!(
middle.fields[1],
Expand Down
25 changes: 23 additions & 2 deletions sea-orm-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ pub fn derive_from_json_query_result(input: TokenStream) -> TokenStream {
/// }
/// ```
///
/// If all fields in the partial model is `from_expr`, the `entity` can be ignore.
/// If all fields in the partial model is `from_expr`, the specifying the `entity` can be skipped.
/// ```
/// use sea_orm::{entity::prelude::*, sea_query::Expr, DerivePartialModel, FromQueryResult};
///
Expand All @@ -773,7 +773,28 @@ pub fn derive_from_json_query_result(input: TokenStream) -> TokenStream {
/// }
/// ```
///
/// A field cannot have attributes `from_col` and `from_expr` at the same time.
/// It is possible to nest structs deriving `FromQueryResult` and `DerivePartialModel`, including
/// optionally, which is useful for specifying columns from tables added via left joins, as well as
/// when building up complicated queries programmatically.
/// ```
/// use sea_orm::{entity::prelude::*, sea_query::Expr, DerivePartialModel, FromQueryResult};
///
/// #[derive(Debug, FromQueryResult, DerivePartialModel)]
/// struct Inner {
/// #[sea_orm(from_expr = "Expr::val(1).add(1)")]
/// sum: i32,
/// }
///
/// #[derive(Debug, FromQueryResult, DerivePartialModel)]
/// struct Outer {
/// #[sea_orm(nested)]
/// inner: Inner,
/// #[sea_orm(nested)]
/// inner_opt: Option<Inner>,
/// }
/// ```
///
/// A field cannot have attributes `from_col`, `from_expr` or `nested` at the same time.
/// Or, it will result in a compile error.
///
/// ```compile_fail
Expand Down
Loading