this looks this overdoing it... going back to master

lootalot_db/src/lib.rs

@@ -13,8 +13,10 @@ use diesel::prelude::*;
 use diesel::query_dsl::RunQueryDsl;
 use diesel::r2d2::{self, ConnectionManager};
 
+mod transactions;
 pub mod models;
 mod schema;
+use transactions::{DbTransaction};
 
 /// The connection used
 pub type DbConnection = SqliteConnection;
@@ -22,10 +24,8 @@ pub type DbConnection = SqliteConnection;
 pub type Pool = r2d2::Pool<ConnectionManager<DbConnection>>;
 /// The result of a query on DB
 pub type QueryResult<T> = Result<T, diesel::result::Error>;
-/// The result of an action provided by DbApi
+pub type ActionResult<T> = QueryResult<ActionStatus<T>>;
-pub type ActionResult<R> = QueryResult<ActionStatus<R>>;
+/// Return status of an Action
-
-/// Return status of an API Action
 #[derive(Serialize, Debug)]
 pub struct ActionStatus<R: serde::Serialize> {
     /// Has the action made changes ?
@@ -35,13 +35,13 @@ pub struct ActionStatus<R: serde::Serialize> {
 }
 
 impl ActionStatus<()> {
-    fn was_updated(updated_lines: usize) -> Self {
+    pub fn was_updated(updated_lines: usize) -> Self {
         match updated_lines {
             1 => Self::ok(),
             _ => Self::nop(),
         }
     }
-    fn ok() -> ActionStatus<()> {
+    pub fn ok() -> ActionStatus<()> {
         Self {
             executed: true,
             response: (),
@@ -50,7 +50,7 @@ impl ActionStatus<()> {
 }
 
 impl<T: Default + serde::Serialize> ActionStatus<T> {
-    fn nop() -> ActionStatus<T> {
+    pub fn nop() -> ActionStatus<T> {
         Self {
             executed: false,
             response: Default::default(),
@@ -160,15 +160,18 @@ impl<'q> AsPlayer<'q> {
     ///
     /// This currently panics if player wealth fails to be updated, as this is
     /// a serious error. TODO: handle deletion of bought item in case of wealth update failure.
-    pub fn buy<'a>(self, name: &'a str, price: i32) -> ActionResult<Option<(i32, i32, i32, i32)>> {
+    pub fn buy<S: Into<String>>(self, name: S, price: i32) -> ActionResult<Option<(i32, i32, i32, i32)>> {
-        let new_item = models::item::NewLoot::to_player(self.id, (name, price));
+        match transactions::player::Buy.execute(
-        diesel::insert_into(schema::looted::table)
+            self.conn,
-            .values(&new_item)
+            transactions::player::AddLootParams {
-            .execute(self.conn)
+                player_id: self.id,
-            .and_then(|r| match r {
+                loot_name: name.into(),
-                1 => Ok(self.update_wealth(-(price as f32)).unwrap()),
+                loot_price: price,
-                _ => Ok(ActionStatus::nop()),
+            },
-            })
+        ) {
+            Ok(res) => Ok(ActionStatus { executed: true, response: Some(res.loot_cost) }),
+            Err(e) => { dbg!(&e); Ok(ActionStatus { executed: false, response: None}) },
+        }
     }
     /// Sell an item from this player chest
     ///
@@ -183,77 +186,64 @@ impl<'q> AsPlayer<'q> {
     ) -> ActionResult<Option<(i32, i32, i32, i32)>> {
         // Check that the item belongs to player
         let exists_and_owned: bool =
-            diesel::select(models::Loot::owns(self.id, loot_id)).get_result(self.conn)?;
+            diesel::select(models::Loot::owns(self.id, loot_id))
+                .get_result(self.conn)?;
         if !exists_and_owned {
             return Ok(ActionStatus::nop());
         }
-        use schema::looted::dsl::*;
+        transactions::player::Sell.execute(
-        let loot_value = looted
+            self.conn,
-            .find(loot_id)
+            transactions::player::LootParams {
-            .select(base_price)
+                player_id: self.id,
-            .first::<i32>(self.conn)?;
+                loot_id,
-        let sell_value = (loot_value / 2) as f32;
+            },
-        diesel::delete(looted.find(loot_id))
+        )
-            .execute(self.conn)
+            .map(|res| ActionStatus { executed: true, response: Some(res.loot_cost) })
-            .and_then(|r| match r {
+            .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop()) })
-                // On deletion, update this player wealth
-                1 => Ok(self.update_wealth(sell_value).unwrap()),
-                _ => Ok(ActionStatus {
-                    executed: false,
-                    response: None,
-                }),
-            })
     }
 
     /// Adds the value in gold to the player's wealth.
     ///
     /// Value can be negative to substract wealth.
     pub fn update_wealth(self, value_in_gp: f32) -> ActionResult<Option<(i32, i32, i32, i32)>> {
-        use schema::players::dsl::*;
+        transactions::player::UpdateWealth.execute(
-        let current_wealth = players
+            self.conn,
-            .find(self.id)
+            transactions::player::WealthParams {
-            .select((cp, sp, gp, pp))
+                player_id: self.id,
-            .first::<models::Wealth>(self.conn)?;
+                value_in_gp,
-        // TODO: improve thisdiesel dependant transaction
+            },
-        //       should be move inside a WealthUpdate method
+        )
-        let updated_wealth = models::Wealth::from_gp(current_wealth.to_gp() + value_in_gp);
+        .map(|res| ActionStatus { executed: true, response: Some(res) })
-        // Difference in coins that is sent back
+        .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop())})
-        let (old, new) = (current_wealth.as_tuple(), updated_wealth.as_tuple());
-        let diff = (new.0 - old.0, new.1 - old.1, new.2 - old.2, new.3 - old.3);
-        diesel::update(players)
-            .filter(id.eq(self.id))
-            .set(&updated_wealth)
-            .execute(self.conn)
-            .map(|r| match r {
-                1 => ActionStatus {
-                    executed: true,
-                    response: Some(diff),
-                },
-                _ => ActionStatus::nop(),
-            })
     }
     /// Put a claim on a specific item
     pub fn claim(self, item: i32) -> ActionResult<()> {
-        let exists: bool = diesel::select(models::Loot::exists(item)).get_result(self.conn)?;
+        let exists: bool =
+            diesel::select(models::Loot::exists(item)).get_result(self.conn)?;
         if !exists {
             return Ok(ActionStatus::nop());
         };
-        let claim = models::claim::NewClaim::new(self.id, item);
+        transactions::player::PutClaim.execute(
-        diesel::insert_into(schema::claims::table)
+            self.conn,
-            .values(&claim)
+            transactions::player::LootParams {
-            .execute(self.conn)
+                player_id: self.id,
-            .map(ActionStatus::was_updated)
+                loot_id: item,
+            },
+        )
+        .map(|_| ActionStatus { executed: true, response: () })
+        .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop())})
     }
     /// Withdraw claim
     pub fn unclaim(self, item: i32) -> ActionResult<()> {
-        use schema::claims::dsl::*;
+        transactions::player::WithdrawClaim.execute(
-        diesel::delete(
+            self.conn,
-            claims
+            transactions::player::LootParams {
-                .filter(loot_id.eq(item))
+                player_id: self.id,
-                .filter(player_id.eq(self.id)),
+                loot_id: item,
+            },
         )
-        .execute(self.conn)
+        .map(|_| ActionStatus { executed: true, response: () })
-        .map(ActionStatus::was_updated)
+        .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop())})
     }
 }
 

lootalot_db/src/transactions.rs

@@ -0,0 +1,295 @@
+//!  TODO:
+//!     Extract actions provided by API into their dedicated module.
+//!     Will allow more flexibilty to combinate them inside API methods.
+//!     Should make it easier to add a new feature : Reverting an action
+//!
+use crate::models;
+use crate::schema;
+use crate::DbConnection;
+use diesel::prelude::*;
+// TODO: revertable actions :
+//          - Buy
+//          - Sell
+//          - UpdateWealth
+pub type TransactionResult<T> = Result<T, diesel::result::Error>;
+
+
+pub trait DbTransaction {
+    type Params;
+    type Response: serde::Serialize;
+
+    fn execute<'q>(
+        self,
+        conn: &'q DbConnection,
+        params: Self::Params,
+    ) -> TransactionResult<Self::Response>;
+}
+
+pub trait Revertable : DbTransaction {
+    fn revert<'q>(
+        self,
+        conn: &'q DbConnection,
+        player_id: i32,
+        params: <Self as DbTransaction>::Response,
+    ) -> TransactionResult<<Self as DbTransaction>::Response>;
+
+}
+
+/// Return status of an Action
+#[derive(Serialize, Debug)]
+pub struct ActionStatus<R: serde::Serialize> {
+    /// Has the action made changes ?
+    pub executed: bool,
+    /// Response payload
+    pub response: R,
+}
+
+impl ActionStatus<()> {
+    pub fn was_updated(updated_lines: usize) -> Self {
+        match updated_lines {
+            1 => Self::ok(),
+            _ => Self::nop(),
+        }
+    }
+    pub fn ok() -> ActionStatus<()> {
+        Self {
+            executed: true,
+            response: (),
+        }
+    }
+}
+
+impl<T: Default + serde::Serialize> ActionStatus<T> {
+    pub fn nop() -> ActionStatus<T> {
+        Self {
+            executed: false,
+            response: Default::default(),
+        }
+    }
+}
+// Or a module ?
+pub(crate) mod player {
+    use super::*;
+
+    pub struct AddLootParams {
+        pub player_id: i32,
+        pub loot_name: String,
+        pub loot_price: i32,
+    }
+    pub struct Buy;
+
+    enum LootTransactionKind {
+        Buy,
+        Sell,
+    }
+    #[derive(Serialize, Debug)]
+    pub struct LootTransaction {
+        player_id: i32,
+        loot_id: i32,
+        kind: LootTransactionKind,
+        pub loot_cost: (i32, i32, i32, i32),
+    }
+
+    impl DbTransaction for Buy {
+        type Params = AddLootParams;
+        type Response = LootTransaction;
+
+        fn execute<'q>(
+            self,
+            conn: &'q DbConnection,
+            params: Self::Params,
+        ) -> TransactionResult<Self::Response> {
+            let added_item = {
+                let new_item = models::item::NewLoot::to_player(
+                    params.player_id,
+                    (&params.loot_name, params.loot_price),
+                );
+                diesel::insert_into(schema::looted::table)
+                    .values(&new_item)
+                    .execute(conn)?
+                    // TODO: return ID of inserted item
+            };
+            let updated_wealth = UpdateWealth.execute(
+                conn,
+                WealthParams {
+                    player_id: params.player_id,
+                    value_in_gp: -(params.loot_price as f32),
+                },
+            );
+            match (added_item, updated_wealth) {
+                (1, Ok(loot_cost)) => Ok(LootTransaction {
+                    kind: LootTransactionKind::Buy,
+                    player_id: params.player_id,
+                    loot_id: 0, //TODO:  find added item ID
+                    loot_cost,
+                }),
+                // TODO: Handle other cases
+                _ => panic!()
+            }
+
+        }
+    }
+
+    impl Revertable for Buy {
+        fn revert<'q>(self, conn: &'q DbConnection, player_id: i32, params: <Self as DbTransaction>::Response)
+        -> TransactionResult<<Self as DbTransaction>::Response> {
+            unimplemented!()
+        }
+    }
+
+    pub struct LootParams {
+        pub player_id: i32,
+        pub loot_id: i32,
+    }
+    pub struct Sell;
+    impl DbTransaction for Sell {
+        type Params = LootParams;
+        type Response = LootTransaction;
+        fn execute<'q>(
+            self,
+            conn: &DbConnection,
+            params: Self::Params,
+        ) -> TransactionResult<Self::Response> {
+            use schema::looted::dsl::*;
+            let loot_value = looted
+                .find(params.loot_id)
+                .select(base_price)
+                .first::<i32>(conn)?;
+            let sell_value = (loot_value / 2) as f32;
+            diesel::delete(looted.find(params.loot_id))
+                .execute(conn)
+                .and_then(|r| match r {
+                    // On deletion, update this player wealth
+                    1 => Ok(UpdateWealth
+                        .execute(
+                            conn,
+                            WealthParams {
+                                player_id: params.player_id,
+                                value_in_gp: sell_value as f32,
+                            },
+                        )
+                        .unwrap()),
+                    _ => Ok(ActionStatus {
+                        executed: false,
+                        response: None,
+                    }),
+                })
+        }
+    }
+
+    pub struct PutClaim;
+    impl DbTransaction for PutClaim {
+        type Params = LootParams;
+        type Response = ();
+        fn execute<'q>(
+            self,
+            conn: &DbConnection,
+            params: Self::Params,
+        ) -> TransactionResult<Self::Response> {
+            let claim = models::claim::NewClaim::new(params.player_id, params.loot_id);
+            diesel::insert_into(schema::claims::table)
+                .values(&claim)
+                .execute(conn)
+                .and_then(|_| Ok(()))
+        }
+    }
+
+    pub struct WithdrawClaim;
+    impl DbTransaction for WithdrawClaim {
+        type Params = LootParams;
+        type Response = ();
+        fn execute<'q>(
+            self,
+            conn: &DbConnection,
+            params: Self::Params,
+        ) -> TransactionResult<Self::Response> {
+            use schema::claims::dsl::*;
+            diesel::delete(
+                claims
+                    .filter(loot_id.eq(params.loot_id))
+                    .filter(player_id.eq(params.player_id)),
+            )
+            .execute(conn)
+            .and_then(|_| Ok(()))
+        }
+    }
+
+    pub struct WealthParams {
+        pub player_id: i32,
+        pub value_in_gp: f32,
+    }
+    pub struct UpdateWealth;
+
+    impl DbTransaction for UpdateWealth {
+        type Params = WealthParams;
+        type Response = (i32, i32, i32, i32);
+
+        fn execute<'q>(
+            self,
+            conn: &'q DbConnection,
+            params: WealthParams,
+        ) -> TransactionResult<Self::Response> {
+            use schema::players::dsl::*;
+            let current_wealth = players
+                .find(params.player_id)
+                .select((cp, sp, gp, pp))
+                .first::<models::Wealth>(conn)?;
+            // TODO: improve thisdiesel dependant transaction
+            //       should be move inside a WealthUpdate method
+            let updated_wealth =
+                models::Wealth::from_gp(current_wealth.to_gp() + params.value_in_gp);
+            // Difference in coins that is sent back
+            let (old, new) = (current_wealth.as_tuple(), updated_wealth.as_tuple());
+            let diff = (new.0 - old.0, new.1 - old.1, new.2 - old.2, new.3 - old.3);
+            diesel::update(players)
+                .filter(id.eq(params.player_id))
+                .set(&updated_wealth)
+                .execute(conn)
+                .and_then(|r| match r {
+                    1 => Ok(diff),
+                    _ => panic!("UpdateWealth made no changes !"),
+                })
+        }
+    }
+
+    impl Revertable for UpdateWealth {
+        fn revert<'q>(
+            self,
+            conn: &'q DbConnection,
+            player_id: i32,
+            params: <Self as DbTransaction>::Response,
+        ) -> TransactionResult<<Self as DbTransaction>::Response> {
+            use schema::players::dsl::*;
+            let cur_wealth = players
+                .find(player_id)
+                .select((cp, sp, gp, pp))
+                .first::<models::Wealth>(conn)?;
+            let reverted_wealth = models::player::Wealth {
+                cp: cur_wealth.cp - params.0,
+                sp: cur_wealth.cp - params.1,
+                gp: cur_wealth.cp - params.2,
+                pp: cur_wealth.cp - params.3,
+            };
+            // Difference in coins that is sent back
+            let diff = ( -params.0, -params.1, -params.2, -params.3);
+            diesel::update(players)
+                .filter(id.eq(params.0))
+                .set(&reverted_wealth)
+                .execute(conn)
+                .and_then(|r| match r {
+                    1 => Ok(diff),
+                    _ => panic!("RevertableWealthUpdate made no changes"),
+                })
+        }
+    }
+}
+
+
+pub(crate) mod admin {
+    pub struct AddPlayer;
+    pub struct AddLoot;
+    pub struct SellLoot;
+    pub struct ResolveClaims;
+
+
+}