Simplify the structure and codegen of UpdateStatement (#367)

The main goal of this refactoring was to remove the implementations of
`SaveChangesDsl` from codegen and provide them generically instead.
Since I want the code outside of codegen to be as approachable as
possible, I made a few other changes to reduce the where clause of these
implementations. I still would like to reduce the where clause further,
but this is at a decent place. The following tree of changes were in
support of this goal:

- Identifiable needs to have the table. This is so we can generically do
  `table.find(id)` for `T: Identifiable`.
- Structs can now be passed directly to `update` and `delete`
  - This caused us to change `UpdateTarget` to be a struct, and
    introduce an `AsUpdateTarget` trait so that we can abstract over
    whether the fields on that struct are a reference or not

All of this will drastically simplify the non-procedural-macro form of
`#[derive(AsChangeset)]`, and hopefully lead to more refactoring in the
future.

CHANGELOG.md

@@ -24,6 +24,9 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 ### Changed
 
+* Most structs that implement `Queryable` will now also need
+ `#[derive(Identifiable)]`.
+
 * `infer_schema!` on SQLite now accepts a larger range of type names
 
 * `types::VarChar` is now an alias for `types::Text`. Most code should be
@@ -38,6 +41,10 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
  http://docs.diesel.rs/diesel/result/trait.DatabaseErrorInformation.html for
  more information
 
+* Structs which implement `Identifiable` can now be passed to `update` and
+ `delete`. This means you can now write `delete(&user).execute(&connection)`
+ instead of `delete(users.find(user.id)).execute(&connection)`
+
 ### Fixed
 
 * `&&[T]` can now be used in queries. This allows using slices with things like

diesel/src/associations/mod.rs

@@ -1,8 +1,13 @@
 use std::hash::Hash;
 
+use query_dsl::FindDsl;
+use query_source::Table;
+
 pub trait Identifiable {
  type Id: Hash + Eq + Copy;
+ type Table: Table + FindDsl<Self::Id>;
 
+ fn table() -> Self::Table;
  fn id(&self) -> Self::Id;
 }
 

diesel/src/lib.rs

@@ -74,6 +74,14 @@ pub mod helper_types {
 
  /// Represents the return type of `.with(aliased_expr)`
  pub type With<'a, Source, Other> = <Source as WithDsl<'a, Other>>::Output;
+
+ use super::query_builder::{UpdateStatement, IntoUpdateTarget, AsChangeset};
+ /// Represents the return type of `update(lhs).set(rhs)`
+ pub type Update<Target, Changes> = UpdateStatement<
+ <Target as IntoUpdateTarget>::Table,
+ <Target as IntoUpdateTarget>::WhereClause,
+ <Changes as AsChangeset>::Changeset,
+ >;
 }
 
 pub mod prelude {

diesel/src/macros/identifiable.rs

@@ -10,12 +10,14 @@
 ///
 /// ```no_run
 /// # #[macro_use] extern crate diesel;
+/// # table! { users { id -> Integer, } }
 /// struct User {
 /// id: i32,
 /// name: String,
 /// }
 ///
 /// Identifiable! {
+/// (users)
 /// struct User {
 /// id: i32,
 /// name: String,
@@ -40,26 +42,22 @@
 /// ```
 #[macro_export]
 macro_rules! Identifiable {
- // Strip empty argument list if given (Passed by custom_derive macro)
- (() $($body:tt)*) => {
- Identifiable! {
- $($body)*
- }
- };
-
  // Strip meta items, pub (if present) and struct from definition
  (
+ $args:tt
  $(#[$ignore:meta])*
  $(pub)* struct $($body:tt)*
  ) => {
  Identifiable! {
+ $args
  $($body)*
  }
  };
 
  // We found the `id` field, return the final impl
  (
  (
+ table_name = $table_name:ident,
  struct_ty = $struct_ty:ty,
  ),
  fields = [{
@@ -71,6 +69,11 @@ macro_rules! Identifiable {
  ) => {
  impl $crate::associations::Identifiable for $struct_ty {
  type Id = $field_ty;
+ type Table = $table_name::table;
+
+ fn table() -> Self::Table {
+ $table_name::table
+ }
 
  fn id(&self) -> Self::Id {
  self.id
@@ -80,9 +83,7 @@ macro_rules! Identifiable {
 
  // Search for the `id` field and continue
  (
- (
- struct_ty = $struct_ty:ty,
- ),
+ $args:tt,
  fields = [{
  field_name: $field_name:ident,
  column_name: $column_name:ident,
@@ -91,19 +92,21 @@ macro_rules! Identifiable {
  } $($fields:tt)*],
  ) => {
  Identifiable! {
- (struct_ty = $struct_ty,),
+ $args,
  fields = [$($fields)*],
  }
  };
 
 
  // Handle struct with no generics
  (
+ ($table_name:ident)
  $struct_name:ident
  $body:tt $(;)*
  ) => {
  __diesel_parse_struct_body! {
  (
+ table_name = $table_name,
  struct_ty = $struct_name,
  ),
  callback = Identifiable,
@@ -112,6 +115,18 @@ macro_rules! Identifiable {
  };
 }
 
+table! {
+ foos {
+ id -> Integer,
+ }
+}
+
+table! {
+ bars {
+ id -> VarChar,
+ }
+}
+
 #[test]
 fn derive_identifiable_on_simple_struct() {
  use associations::Identifiable;
@@ -123,6 +138,7 @@ fn derive_identifiable_on_simple_struct() {
  }
 
  Identifiable! {
+ (foos)
  struct Foo {
  id: i32,
  foo: i32,
@@ -146,6 +162,7 @@ fn derive_identifiable_when_id_is_not_first_field() {
  }
 
  Identifiable! {
+ (foos)
  struct Foo {
  foo: i32,
  id: i32,
@@ -169,6 +186,7 @@ fn derive_identifiable_on_struct_with_non_integer_pk() {
  }
 
  Identifiable! {
+ (bars)
  struct Foo {
  id: &'static str,
  foo: i32,

diesel/src/macros/mod.rs

@@ -243,6 +243,18 @@ macro_rules! table_body {
  }
  }
 
+ impl IntoUpdateTarget for table {
+ type Table = Self;
+ type WhereClause = ();
+
+ fn into_update_target(self) -> UpdateTarget<Self::Table, Self::WhereClause> {
+ UpdateTarget {
+ table: self,
+ where_clause: None,
+ }
+ }
+ }
+
  impl_query_id!(table);
 
  pub mod columns {

diesel/src/query_builder/delete_statement.rs

@@ -1,45 +1,46 @@
 use backend::Backend;
 use query_builder::*;
+use query_source::Table;
 use result::QueryResult;
 
-#[derive(Debug, Copy, Clone)]
-pub struct DeleteStatement<T>(T);
+#[derive(Debug)]
+pub struct DeleteStatement<T, U>(UpdateTarget<T, U>);
 
-impl<T> DeleteStatement<T> {
+impl<T, U> DeleteStatement<T, U> {
  #[doc(hidden)]
- pub fn new(t: T) -> Self {
+ pub fn new(t: UpdateTarget<T, U>) -> Self {
  DeleteStatement(t)
  }
 }
 
-impl<T, DB> QueryFragment<DB> for DeleteStatement<T> where
+impl<T, U, DB> QueryFragment<DB> for DeleteStatement<T, U> where
  DB: Backend,
- T: UpdateTarget,
- T::WhereClause: QueryFragment<DB>,
+ T: Table,
  T::FromClause: QueryFragment<DB>,
+ U: QueryFragment<DB>,
 {
  fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
  out.push_sql("DELETE FROM ");
- try!(self.0.from_clause().to_sql(out));
- if let Some(clause) = self.0.where_clause() {
+ try!(self.0.table.from_clause().to_sql(out));
+ if let Some(ref clause) = self.0.where_clause {
  out.push_sql(" WHERE ");
  try!(clause.to_sql(out));
  }
  Ok(())
  }
 
  fn collect_binds(&self, out: &mut DB::BindCollector) -> QueryResult<()> {
- try!(self.0.from_clause().collect_binds(out));
- if let Some(clause) = self.0.where_clause() {
+ try!(self.0.table.from_clause().collect_binds(out));
+ if let Some(ref clause) = self.0.where_clause {
  try!(clause.collect_binds(out));
  }
  Ok(())
  }
 
  fn is_safe_to_cache_prepared(&self) -> bool {
- self.0.from_clause().is_safe_to_cache_prepared() &&
- self.0.where_clause().map(|w| w.is_safe_to_cache_prepared()).unwrap_or(true)
+ self.0.table.from_clause().is_safe_to_cache_prepared() &&
+ self.0.where_clause.as_ref().map(|w| w.is_safe_to_cache_prepared()).unwrap_or(true)
  }
 }
 
-impl_query_id!(noop: DeleteStatement<T>);
+impl_query_id!(noop: DeleteStatement<T, U>);

diesel/src/query_builder/functions.rs

@@ -1,5 +1,5 @@
 use expression::Expression;
-use super::{UpdateTarget, IncompleteUpdateStatement, IncompleteInsertStatement, SelectStatement};
+use super::{IntoUpdateTarget, IncompleteUpdateStatement, IncompleteInsertStatement, SelectStatement};
 use super::delete_statement::DeleteStatement;
 use super::insert_statement::Insert;
 
@@ -38,8 +38,8 @@ use super::insert_statement::Insert;
 /// # #[cfg(not(feature = "postgres"))]
 /// # fn main() {}
 /// ```
-pub fn update<T: UpdateTarget>(source: T) -> IncompleteUpdateStatement<T> {
- IncompleteUpdateStatement::new(source)
+pub fn update<T: IntoUpdateTarget>(source: T) -> IncompleteUpdateStatement<T::Table, T::WhereClause> {
+ IncompleteUpdateStatement::new(source.into_update_target())
 }
 
 /// Creates a delete statement. Will delete the records in the given set.
@@ -102,8 +102,8 @@ pub fn update<T: UpdateTarget>(source: T) -> IncompleteUpdateStatement<T> {
 /// # Ok(())
 /// # }
 /// ```
-pub fn delete<T: UpdateTarget>(source: T) -> DeleteStatement<T> {
- DeleteStatement::new(source)
+pub fn delete<T: IntoUpdateTarget>(source: T) -> DeleteStatement<T::Table, T::WhereClause> {
+ DeleteStatement::new(source.into_update_target())
 }
 
 /// Creates an insert statement. Will add the given data to a table. This

diesel/src/query_builder/mod.rs

@@ -27,7 +27,14 @@ pub use self::query_id::QueryId;
 #[doc(hidden)]
 pub use self::select_statement::{SelectStatement, BoxedSelectStatement};
 #[doc(inline)]
-pub use self::update_statement::{IncompleteUpdateStatement, AsChangeset, Changeset, UpdateTarget};
+pub use self::update_statement::{
+ AsChangeset,
+ Changeset,
+ IncompleteUpdateStatement,
+ IntoUpdateTarget,
+ UpdateStatement,
+ UpdateTarget,
+};
 #[doc(inline)]
 pub use self::insert_statement::IncompleteInsertStatement;