Skip to content

Commit

Permalink
Add treat_none_as_null to changeset_for
Browse files Browse the repository at this point in the history
This allows users to opt into the behavior of `changeset_for` that was
present prior to #47. I still feel that the default behavior of skipping
optional fields is the most common case, as explicitly wanting to set a
column to `NULL` is a pretty niche case to begin with, and much more
frequently the field would be `None` on the struct because you've
populated it from a web form or API endpoint that doesn't specify all
fields.
  • Loading branch information
sgrif committed Jan 19, 2016
1 parent 82dca16 commit 59a25e8
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 13 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
production code is discouraged as it is inherently unsafe and avoids real type
checking.

* Added a `treat_none_as_null` option to `changeset_for`. When set to `true`,
a model will set a field to `Null` when an optional struct field is `None`,
instead of skipping the field entirely. The default value of the option is
`false`, as we think the current behavior is a much more common use case.

### Changed

* Rename both the `#[derive(Queriable)]` attribute and the `Queriable` trait to
Expand Down
4 changes: 4 additions & 0 deletions diesel_codegen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ Any fields which are of the type `Option` will be skipped when their value is
`None`. This makes it easy to support APIs where you may not want to update all
of the fields of a record on every request.

If you'd like `None` to change a field to `NULL`, instead of skipping it, you
can pass the `treat_none_as_null` option like so: `#[changeset_for(posts,
treat_none_as_null="true")]`

If the struct has a field for the primary key, an additional function,
`save_changes<T: Queryable<..>>(&self, connection: &Connection) ->
QueryResult<T>`, will be added to the model. This will persist any changes made,
Expand Down
30 changes: 17 additions & 13 deletions diesel_codegen/src/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub fn expand_changeset_for(
struct ChangesetOptions {
table_name: ast::Ident,
skip_visibility: bool,
treat_none_as_null: bool,
}

fn changeset_options(cx: &mut ExtCtxt, meta_item: &MetaItem) -> Result<ChangesetOptions, ()> {
Expand All @@ -41,9 +42,12 @@ fn changeset_options(cx: &mut ExtCtxt, meta_item: &MetaItem) -> Result<Changeset
let table_name = try!(table_name(cx, &meta_items[0]));
let skip_visibility = try!(boolean_option(cx, &meta_items[1..], "__skip_visibility"))
.unwrap_or(false);
let treat_none_as_null = try!(boolean_option(cx, &meta_items[1..], "treat_none_as_null"))
.unwrap_or(false);
Ok(ChangesetOptions {
table_name: str_to_ident(&table_name),
skip_visibility: skip_visibility,
treat_none_as_null: treat_none_as_null,
})
}
_ => usage_error(cx, meta_item),
Expand Down Expand Up @@ -88,18 +92,17 @@ fn changeset_impl(
options: &ChangesetOptions,
model: &Model,
) -> Option<P<ast::Item>> {
let table: &str = &options.table_name.name.as_str();
let ref struct_name = model.ty;
let pk = model.primary_key_name();
let attrs_for_changeset = model.attrs.iter().filter(|a| a.column_name != pk)
.collect::<Vec<_>>();
let changeset_ty = builder.ty().tuple()
.with_tys(attrs_for_changeset.iter()
.map(|a| changeset_ty(cx, builder, table, a)))
.map(|a| changeset_ty(cx, builder, &options, a)))
.build();
let changeset_body = builder.expr().tuple()
.with_exprs(attrs_for_changeset.iter()
.map(|a| changeset_expr(cx, builder, table, a)))
.map(|a| changeset_expr(cx, builder, &options, a)))
.build();
quote_item!(cx,
impl<'a: 'update, 'update> ::diesel::query_builder::AsChangeset for
Expand Down Expand Up @@ -157,18 +160,19 @@ fn save_changes_impl(
fn changeset_ty(
cx: &mut ExtCtxt,
builder: aster::AstBuilder,
table: &str,
options: &ChangesetOptions,
attr: &Attr,
) -> P<ast::Ty> {
let column = builder.path()
.segment(table).build()
.segment(options.table_name).build()
.segment(attr.column_name).build()
.build();
if let Some(ty) = ty_param_of_option(&attr.ty) {
let inner_ty = inner_changeset_ty(cx, column, &ty);
quote_ty!(cx, Option<$inner_ty>)
} else {
inner_changeset_ty(cx, column, &attr.ty)
match (options.treat_none_as_null, ty_param_of_option(&attr.ty)) {
(false, Some(ty)) => {
let inner_ty = inner_changeset_ty(cx, column, &ty);
quote_ty!(cx, Option<$inner_ty>)
}
_ => inner_changeset_ty(cx, column, &attr.ty),
}
}

Expand All @@ -191,15 +195,15 @@ fn inner_changeset_ty(
fn changeset_expr(
cx: &mut ExtCtxt,
builder: aster::AstBuilder,
table: &str,
options: &ChangesetOptions,
attr: &Attr,
) -> P<ast::Expr> {
let column = builder.path()
.segment(table).build()
.segment(options.table_name).build()
.segment(attr.column_name).build()
.build();
let field_name = &attr.field_name.unwrap();
if is_option_ty(&attr.ty) {
if !options.treat_none_as_null && is_option_ty(&attr.ty) {
quote_expr!(cx, self.$field_name.as_ref().map(|f| $column.eq(f)))
} else {
quote_expr!(cx, $column.eq(&self.$field_name))
Expand Down
24 changes: 24 additions & 0 deletions diesel_tests/tests/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,27 @@ fn can_update_with_struct_containing_single_field() {
let expected_post = Post::new(post.id, sean.id, "Hello".into(), Some("earth".into()));
assert_eq!(expected_post, post);
}

#[test]
fn struct_with_option_fields_treated_as_null() {
#[changeset_for(posts, treat_none_as_null="true", __skip_visibility="true")]
struct UpdatePost {
id: i32,
title: String,
body: Option<String>,
}

let connection = connection_with_sean_and_tess_in_users_table();
let sean = find_user_by_name("Sean", &connection);
let new_post = sean.new_post("Hello", Some("world"));
let post = insert(&new_post).into(posts::table)
.get_result::<Post>(&connection).unwrap();

let changes = UpdatePost { id: post.id, title: "Hello again".into(), body: None };
let expected_post = Post::new(post.id, sean.id, "Hello again".into(), None);
let updated_post = changes.save_changes(&connection);
let post_in_database = connection.find(posts::table, post.id);

assert_eq!(Ok(&expected_post), updated_post.as_ref());
assert_eq!(Ok(&expected_post), post_in_database.as_ref());
}

0 comments on commit 59a25e8

Please sign in to comment.