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: Add embed field attribute for AsChangeset #2529

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
36 changes: 23 additions & 13 deletions diesel_derives/src/as_changeset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,18 @@ fn field_changeset_ty(
treat_none_as_null: bool,
lifetime: Option<proc_macro2::TokenStream>,
) -> syn::Type {
let column_name = field.column_name();
if !treat_none_as_null && is_option_ty(&field.ty) {
let field_ty = inner_of_option_ty(&field.ty);
parse_quote!(std::option::Option<diesel::dsl::Eq<#table_name::#column_name, #lifetime #field_ty>>)
} else {
if field.has_flag("embed") {
let field_ty = &field.ty;
parse_quote!(diesel::dsl::Eq<#table_name::#column_name, #lifetime #field_ty>)
parse_quote!(#lifetime #field_ty)
Copy link
Member

Choose a reason for hiding this comment

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

From looking again at this here I think we must call <#lifetime #field_ty as AsChangeSet>::Changeset here`

} else {
let column_name = field.column_name();
if !treat_none_as_null && is_option_ty(&field.ty) {
let field_ty = inner_of_option_ty(&field.ty);
parse_quote!(std::option::Option<diesel::dsl::Eq<#table_name::#column_name, #lifetime #field_ty>>)
} else {
let field_ty = &field.ty;
parse_quote!(diesel::dsl::Eq<#table_name::#column_name, #lifetime #field_ty>)
}
}
}

Expand All @@ -109,14 +114,19 @@ fn field_changeset_expr(
lifetime: Option<proc_macro2::TokenStream>,
) -> syn::Expr {
let field_access = field.name.access();
let column_name = field.column_name();
if !treat_none_as_null && is_option_ty(&field.ty) {
if lifetime.is_some() {
parse_quote!(self#field_access.as_ref().map(|x| #table_name::#column_name.eq(x)))
if field.has_flag("embed") {
parse_quote!(#lifetime self#field_access)
Copy link
Member

Choose a reason for hiding this comment

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

That should then be <#lifetime #field_ty as AsChangeset>::as_changeset(self#field_access)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll try that this weekend.

} else {
let column_name = field.column_name();
let column: syn::Expr = parse_quote!(#table_name::#column_name);
if !treat_none_as_null && is_option_ty(&field.ty) {
if lifetime.is_some() {
parse_quote!(self#field_access.as_ref().map(|x| #column.eq(x)))
} else {
parse_quote!(self#field_access.map(|x| #column.eq(x)))
}
} else {
parse_quote!(self#field_access.map(|x| #table_name::#column_name.eq(x)))
parse_quote!(#column.eq(#lifetime self#field_access))
}
} else {
parse_quote!(#table_name::#column_name.eq(#lifetime self#field_access))
}
}
9 changes: 8 additions & 1 deletion diesel_derives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ use diagnostic_shim::*;
/// annotate your struct with `#[changeset_options(treat_none_as_null =
/// "true")]`.
///
/// Your struct can also contain fields which implement `AsChangeset`. This is
/// useful when you want to have one field map to more than one column (for
/// example, an enum that maps to a label and a value column). Add
/// `#[diesel(embed)]` to any such fields.
///
/// # Attributes
///
/// ## Optional type attributes
Expand All @@ -88,9 +93,11 @@ use diagnostic_shim::*;
/// * `#[column_name = "some_column_name"]`, overrides the column name
/// of the current field to `some_column_name`. By default the field
/// name is used as column name.
/// * `#[diesel(embed)]`, specifies that the current field maps not only
/// to single database field, but is a type that implements `AsChangeset`
#[proc_macro_derive(
AsChangeset,
attributes(table_name, primary_key, column_name, changeset_options)
attributes(diesel, table_name, primary_key, column_name, changeset_options)
)]
pub fn derive_as_changeset(input: TokenStream) -> TokenStream {
expand_proc_macro(input, as_changeset::derive)
Expand Down
94 changes: 94 additions & 0 deletions diesel_tests/tests/update.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use crate::schema::*;
use diesel::*;
use diesel::{
expression::{bound::Bound, grouped::Grouped, operators},
sql_types::{Nullable, Text},
};
heyrict marked this conversation as resolved.
Show resolved Hide resolved

#[test]
fn test_updating_single_column() {
Expand Down Expand Up @@ -301,3 +305,93 @@ fn upsert_with_sql_literal_for_target() {
];
assert_eq!(Ok(expected_data), data);
}

#[derive(AsChangeset)]
#[table_name = "users"]
struct UpdateUser {
name: Option<String>,
#[diesel(embed)]
hair_color: HairColor,
}

#[allow(dead_code)]
enum HairColor {
Bald,
Black,
White,
Other(String),
}

impl AsChangeset for HairColor {
type Target = users::table;
type Changeset =
Option<Grouped<operators::Eq<users::hair_color, Bound<Nullable<Text>, String>>>>;

fn as_changeset(self) -> Self::Changeset {
match self {
HairColor::Black => Some(users::hair_color.eq("Black".to_string())),
HairColor::White => Some(users::hair_color.eq("White".to_string())),
HairColor::Other(x) => Some(users::hair_color.eq(x)),
HairColor::Bald => None,
}
}
}

impl AsChangeset for &HairColor {
type Target = users::table;
type Changeset =
Option<Grouped<operators::Eq<users::hair_color, Bound<Nullable<Text>, String>>>>;

fn as_changeset(self) -> Self::Changeset {
match self {
HairColor::Black => Some(users::hair_color.eq("Black".to_string())),
HairColor::White => Some(users::hair_color.eq("White".to_string())),
HairColor::Other(x) => Some(users::hair_color.eq(x.clone())),
HairColor::Bald => None,
}
}
}

#[test]
fn update_user_with_embed() {
let connection = connection_with_sean_and_tess_in_users_table();

let expected_user = User {
id: 1,
name: "Sean".to_string(),
hair_color: Some("Black".to_string()),
};

let updated_user: User = diesel::update(users::table)
.filter(users::id.eq(1))
.set(UpdateUser {
name: None,
hair_color: HairColor::Black,
})
.get_result(&connection)
Copy link
Member

Choose a reason for hiding this comment

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

Better use a separate query here to load the results, as both Sqlite and Mysql do not support queries containing RETURNING clauses. (Or use the UpdateAndFetchResult trait impls.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint! I was wandering about the error messages here.

.unwrap();

assert_eq!(expected_user, updated_user);
}

#[test]
fn update_user_with_embed_that_sets_null() {
let connection = connection_with_sean_and_tess_in_users_table();

let expected_user = User {
id: 1,
name: "Sean".to_string(),
hair_color: None,
};

let updated_user: User = diesel::update(users::table)
.filter(users::id.eq(1))
.set(UpdateUser {
name: None,
hair_color: HairColor::Bald,
})
.get_result(&connection)
.unwrap();

assert_eq!(expected_user, updated_user);
}