Better error handling in from_row functions

oxidizer-entity-macro/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "oxidizer-entity-macro"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Gustavo Sampaio <[email protected]>", "davysson <[email protected]>", "Vandre Leal <[email protected]>"]
 edition = "2018"
 keywords = ["orm", "tokio-postgres", "refinery", "postgres", "sql"]

oxidizer-entity-macro/src/entity_builder.rs

@@ -30,7 +30,7 @@ impl EntityBuilder {
 
                     let ty_ident = format_ident!("{}", ty);
 
-                    return quote! { &<#ty_ident>::from(&self.#name) };
+                    return quote! { &<#ty_ident>::try_from(&self.#name)? };
                 }
 
                 quote! { &self.#name }
@@ -86,7 +86,7 @@ impl EntityBuilder {
                             query,
                             &[#( #fields_plain_value_acessors ),*]
                         ).await?;
-                        let first_row = rows.first().ok_or(oxidizer::db::Error::Other)?;
+                        let first_row = rows.first().ok_or(oxidizer::db::Error::Other("Error while saving entity".to_string()))?;
                         self.#primary_key_ident = first_row.get::<&str, #primary_key_type>(stringify!(#primary_key_ident));
                         1
                     },
@@ -122,14 +122,16 @@ impl EntityBuilder {
                 let ty = field.get_type();
 
                 let mut converter = quote! {};
+                let mut converter_pos = quote! {};
 
                 if let Some(_) = field.parse_custom_type() {
                     let custom_ty = &field.ty;
-                    converter = quote! { <#custom_ty>::from };
+                    converter = quote! { <#custom_ty>::try_from };
+                    converter_pos  = quote! {?};
                 }
 
                 quote! {
-                    #name: #converter(row.get::<&str, #ty>(concat!(stringify!(#name)))),
+                    #name: #converter(row.get::<&str, #ty>(concat!(stringify!(#name))))#converter_pos,
                 }
             })
             .collect();
@@ -142,14 +144,14 @@ impl EntityBuilder {
             props.get_ignored_fields().map(|field| &field.ty).collect();
 
         quote! {
-            fn from_row(row: &oxidizer::tokio_postgres::Row) -> Self {
+            fn from_row(row: &oxidizer::tokio_postgres::Row) -> oxidizer::db::DBResult<Self> {
                 let mut obj: Self = Self{
                     #( #fields_all_loaders )*
                     #(
                         #fields_ignored_names: <#fields_ignored_types>::default(),
                     )*
                 };
-                obj
+                Ok(obj)
             }
         }
     }
@@ -205,7 +207,13 @@ impl EntityBuilder {
             async fn find(db: &oxidizer::db::DB, condition: &str, params: &'_ [&'_ (dyn oxidizer::db_types::ToSql + Sync)]) -> oxidizer::db::DBResult<Vec<#name>> {
                 let query_str = format!("SELECT * FROM {} WHERE {}", #table_name, condition);
                 let rows = db.query(&query_str, params).await?;
-                let results: Vec<#name> = rows.iter().map(|row| Self::from_row(row)).collect();
+
+                let mut results: Vec<#name> = Vec::with_capacity(rows.len());
+
+                for row in rows.iter() {
+                    results.push(Self::from_row(row)?);
+                }
+
                 Ok(results)
             }
         }
@@ -218,7 +226,12 @@ impl EntityBuilder {
             async fn first(db: &oxidizer::db::DB, condition: &str, params: &'_ [&'_ (dyn oxidizer::db_types::ToSql + Sync)]) -> oxidizer::db::DBResult<std::option::Option<#name>> {
                 let query_str = format!("SELECT * FROM {} WHERE {} LIMIT 1", #table_name, condition);
                 let rows = db.query(&query_str, params).await?;
-                let mut results: Vec<#name> = rows.iter().map(|row| Self::from_row(row)).collect();
+
+                let mut results: Vec<#name> = Vec::with_capacity(rows.len());
+                for row in rows.iter() {
+                    results.push(Self::from_row(row)?);
+                }
+
                 match results.len() {
                     0 => Ok(None),
                     _ => Ok(Some(results.remove(0))),
@@ -307,7 +320,7 @@ impl EntityBuilder {
                             return Err(oxidizer::db::Error::DoesNotExist);
                         }
 
-                        Ok(#model::from_row(&results[0]))
+                        #model::from_row(&results[0])
                     }
 
                     async fn #set_ident(&mut self, db: &oxidizer::db::DB, v: &#model) -> oxidizer::db::DBResult<()> {

oxidizer-tests/Cargo.toml

@@ -1,11 +1,11 @@
 [package]
 name = "oxidizer-tests"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Gustavo Sampaio <[email protected]>"]
 edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
 tokio = { version = "0.2", features = ["full"] }
-oxidizer = { version = "0.2.0", path = "../oxidizer" }
+oxidizer = { version = "0.2.1", path = "../oxidizer" }

oxidizer-tests/src/main.rs

@@ -1,6 +1,6 @@
 use oxidizer::*;
 
-// mod test_2;
+// mod tmp;
 
 #[derive(PartialEq, Debug)]
 pub enum MyEnum {
@@ -14,21 +14,35 @@ impl Default for MyEnum {
     }
 }
 
-impl std::convert::From<&MyEnum> for i32 {
-    fn from(v: &MyEnum) -> Self {
+pub enum ConvertError {
+    Error,
+}
+
+impl std::fmt::Display for ConvertError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("Error trying to convert")
+    }
+}
+
+impl TryFrom<&MyEnum> for i32 {
+    type Error = ConvertError;
+
+    fn try_from(v: &MyEnum) -> Result<Self, Self::Error> {
         match v {
-            MyEnum::Item1 => 0,
-            MyEnum::Item2 => 1,
+            MyEnum::Item1 => Ok(0),
+            MyEnum::Item2 => Ok(1),
         }
     }
 }
 
-impl std::convert::From<i32> for MyEnum {
-    fn from(v: i32) -> Self {
+impl TryFrom<i32> for MyEnum {
+    type Error = ConvertError;
+
+    fn try_from(v: i32) -> Result<Self, Self::Error> {
         match v {
-            0 => MyEnum::Item1,
-            1 => MyEnum::Item2,
-            _ => unimplemented!(),
+            0 => Ok(MyEnum::Item1),
+            1 => Ok(MyEnum::Item2),
+            _ => Err(ConvertError::Error),
         }
     }
 }

oxidizer/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "oxidizer"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Gustavo Sampaio <[email protected]>", "davysson <[email protected]>", "Vandre Leal <[email protected]>"]
 edition = "2018"
 keywords = ["orm", "tokio-postgres", "refinery", "postgres", "sql"]
@@ -23,4 +23,4 @@ barrel = { version = "0.6.5", features = ["pg"] }
 refinery = { version = "0.3.0", features = ["tokio-postgres"]}
 openssl = { version = "0.10", features = ["vendored"] }
 
-oxidizer-entity-macro = { version = "0.2.0", path = "../oxidizer-entity-macro" }
+oxidizer-entity-macro = { version = "0.2.1", path = "../oxidizer-entity-macro" }

oxidizer/src/db/db.rs

@@ -7,6 +7,7 @@ use refinery::{Report, Runner};
 use std::str::FromStr;
 
 use super::super::migration::Migration;
+use super::error::*;
 
 use barrel::backend::Pg;
 use tokio_postgres::{
@@ -51,19 +52,6 @@ where
     }
 }
 
-#[derive(Debug)]
-pub enum Error {
-    PostgresError(tokio_postgres::Error),
-    OpensslError(openssl::error::ErrorStack),
-    MobcError(mobc::Error<tokio_postgres::Error>),
-    RefineryError(refinery::Error),
-    DoesNotExist,
-    ReferencedModelIsNotInDB,
-    Other,
-}
-
-pub type DBResult<T> = std::result::Result<T, Error>;
-
 #[derive(Clone)]
 enum ConnectionPool {
     TLS(Pool<ConnectionManager<MakeTlsConnector>>),

oxidizer/src/db/error.rs

@@ -0,0 +1,21 @@
+#[derive(Debug)]
+pub enum Error {
+    PostgresError(tokio_postgres::Error),
+    OpensslError(openssl::error::ErrorStack),
+    MobcError(mobc::Error<tokio_postgres::Error>),
+    RefineryError(refinery::Error),
+    DoesNotExist,
+    ReferencedModelIsNotInDB,
+    Other(String),
+}
+
+pub type DBResult<T> = std::result::Result<T, Error>;
+
+impl<R> std::convert::From<R> for Error
+where
+    R: std::fmt::Display,
+{
+    fn from(v: R) -> Self {
+        Error::Other(v.to_string())
+    }
+}

oxidizer/src/db/mod.rs

@@ -1,5 +1,5 @@
 pub mod db;
-pub use db::DBResult;
-pub use db::Error;
 pub use db::DB;
+pub mod error;
+pub use error::*;
 pub mod test_utils;

oxidizer/src/entity.rs

@@ -13,7 +13,7 @@ pub trait IEntity: Sized {
 
     fn is_synced_with_db(&self) -> bool;
 
-    fn from_row(row: &Row) -> Self;
+    fn from_row(row: &Row) -> DBResult<Self>;
     fn create_migration() -> DBResult<Migration>;
     fn get_table_name() -> String;
 

oxidizer/src/lib.rs

@@ -151,21 +151,35 @@
 //!     Item2,
 //! }
 //!
-//! impl std::convert::From<&MyEnum> for i32 {
-//!     fn from(v: &MyEnum) -> Self {
+//! pub enum ConvertError {
+//!     Error
+//! }
+//!
+//! impl std::fmt::Display for ConvertError {
+//!     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+//!         f.write_str("Error trying to convert")
+//!     }
+//! }
+//!
+//! impl TryFrom<&MyEnum> for i32 {
+//!     type Error = ConvertError;
+//!
+//!     fn try_from(v: &MyEnum) -> Result<Self, Self::Error> {
 //!         match v {
-//!             MyEnum::Item1 => 0,
-//!             MyEnum::Item2 => 1,
+//!             MyEnum::Item1 => Ok(0),
+//!             MyEnum::Item2 => Ok(1),
 //!         }
 //!     }
 //! }
 //!
-//! impl std::convert::From<i32> for MyEnum {
-//!     fn from(v: i32) -> Self {
+//! impl TryFrom<i32> for MyEnum {
+//!     type Error = ConvertError;
+//!
+//!     fn try_from(v: i32) -> Result<Self, Self::Error> {
 //!         match v {
-//!             0 => MyEnum::Item1,
-//!             1 => MyEnum::Item2,
-//!             _ => unimplemented!(),
+//!             0 => Ok(MyEnum::Item1),
+//!             1 => Ok(MyEnum::Item2),
+//!             _ => Err(ConvertError::Error),
 //!         }
 //!     }
 //! }
@@ -179,7 +193,8 @@
 //!     my_enum: MyEnum,
 //! }
 //! ```
-//! The custom type requires you to explicity implement the related `From` functions to convert between the actual type and the overriden type
+//! The custom type requires you to explicity implement the related `TryFrom` trait functions to convert between the
+//! actual type and the overriden type. The error type from the `TryFrom` trait must implement the `std::fmt::Display` trait
 //!
 //!
 //! ## Relations
@@ -313,3 +328,5 @@ mod tests_macro;
 
 #[cfg(test)]
 mod migrations;
+
+pub use std::convert::TryFrom;