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

lootalot_db/src/lib.rs

@@ -13,10 +13,10 @@ use diesel::prelude::*;
 use diesel::query_dsl::RunQueryDsl;
 use diesel::r2d2::{self, ConnectionManager};
 
-mod actions;
+mod transactions;
 pub mod models;
 mod schema;
-use actions::{ActionResult, ActionStatus, UserAction};
+use transactions::{DbTransaction};
 
 /// The connection used
 pub type DbConnection = SqliteConnection;
@@ -24,7 +24,39 @@ 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>;
+pub type ActionResult<T> = QueryResult<ActionStatus<T>>;
+/// 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(),
+        }
+    }
+}
 /// A wrapper providing an API over the database
 /// It offers a convenient way to deal with connection.
 ///
@@ -129,14 +161,17 @@ 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<S: Into<String>>(self, name: S, price: i32) -> ActionResult<Option<(i32, i32, i32, i32)>> {
-        actions::player::Buy.execute(
+        match transactions::player::Buy.execute(
             self.conn,
-            actions::player::AddLootParams {
+            transactions::player::AddLootParams {
                 player_id: self.id,
                 loot_name: name.into(),
                 loot_price: price,
             },
-        )
+        ) {
+            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
     ///
@@ -149,46 +184,66 @@ impl<'q> AsPlayer<'q> {
         loot_id: i32,
         _price_mod: Option<f32>,
     ) -> ActionResult<Option<(i32, i32, i32, i32)>> {
-        actions::player::Sell.execute(
+        // 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)?;
+        if !exists_and_owned {
+            return Ok(ActionStatus::nop());
+        }
+        transactions::player::Sell.execute(
             self.conn,
-            actions::player::LootParams {
+            transactions::player::LootParams {
                 player_id: self.id,
                 loot_id,
             },
         )
+            .map(|res| ActionStatus { executed: true, response: Some(res.loot_cost) })
+            .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop()) })
     }
 
     /// 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)>> {
-        actions::player::UpdateWealth.execute(
+        transactions::player::UpdateWealth.execute(
             self.conn,
-            actions::player::WealthParams {
+            transactions::player::WealthParams {
                 player_id: self.id,
                 value_in_gp,
             },
         )
+        .map(|res| ActionStatus { executed: true, response: Some(res) })
+        .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop())})
     }
     /// Put a claim on a specific item
     pub fn claim(self, item: i32) -> ActionResult<()> {
-        actions::player::PutClaim.execute(
+        let exists: bool =
+            diesel::select(models::Loot::exists(item)).get_result(self.conn)?;
+        if !exists {
+            return Ok(ActionStatus::nop());
+        };
+        transactions::player::PutClaim.execute(
             self.conn,
-            actions::player::LootParams {
+            transactions::player::LootParams {
                 player_id: self.id,
                 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<()> {
-        actions::player::WithdrawClaim.execute(
+        transactions::player::WithdrawClaim.execute(
             self.conn,
-            actions::player::LootParams {
+            transactions::player::LootParams {
                 player_id: self.id,
                 loot_id: item,
             },
         )
+        .map(|_| ActionStatus { executed: true, response: () })
+        .or_else(|e| { dbg!(&e); Ok(ActionStatus::nop())})
     }
 }
 

lootalot_db/src/transactions.rs

@@ -11,8 +11,10 @@ use diesel::prelude::*;
 //          - Buy
 //          - Sell
 //          - UpdateWealth
+pub type TransactionResult<T> = Result<T, diesel::result::Error>;
 
-pub trait UserAction {
+
+pub trait DbTransaction {
     type Params;
     type Response: serde::Serialize;
 
@@ -20,11 +22,19 @@ pub trait UserAction {
         self,
         conn: &'q DbConnection,
         params: Self::Params,
-    ) -> ActionResult<Self::Response>;
+    ) -> 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>;
+
 }
 
-/// The result of an action provided by DbApi
-pub type ActionResult<R> = QueryResult<ActionStatus<R>>;
 /// Return status of an Action
 #[derive(Serialize, Debug)]
 pub struct ActionStatus<R: serde::Serialize> {
@@ -68,34 +78,62 @@ pub(crate) mod player {
     }
     pub struct Buy;
 
-    impl UserAction for 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 = Option<(i32, i32, i32, i32)>;
+        type Response = LootTransaction;
 
         fn execute<'q>(
             self,
             conn: &'q DbConnection,
             params: Self::Params,
-        ) -> ActionResult<Self::Response> {
+        ) -> 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)
+                    .execute(conn)?
-                .and_then(|r| match r {
+                    // TODO: return ID of inserted item
-                    1 => Ok(UpdateWealth
+            };
-                        .execute(
+            let updated_wealth = UpdateWealth.execute(
                 conn,
                 WealthParams {
                     player_id: params.player_id,
                     value_in_gp: -(params.loot_price as f32),
                 },
-                        )
+            );
-                        .unwrap()),
+            match (added_item, updated_wealth) {
-                    _ => Ok(ActionStatus::nop()),
+                (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!()
         }
     }
 
@@ -104,21 +142,14 @@ pub(crate) mod player {
         pub loot_id: i32,
     }
     pub struct Sell;
-    impl UserAction for Sell {
+    impl DbTransaction for Sell {
         type Params = LootParams;
-        type Response = Option<(i32, i32, i32, i32)>;
+        type Response = LootTransaction;
         fn execute<'q>(
             self,
             conn: &DbConnection,
             params: Self::Params,
-        ) -> ActionResult<Self::Response> {
+        ) -> TransactionResult<Self::Response> {
-            // Check that the item belongs to player
-            let exists_and_owned: bool =
-                diesel::select(models::Loot::owns(params.player_id, params.loot_id))
-                    .get_result(conn)?;
-            if !exists_and_owned {
-                return Ok(ActionStatus::nop());
-            }
             use schema::looted::dsl::*;
             let loot_value = looted
                 .find(params.loot_id)
@@ -147,36 +178,31 @@ pub(crate) mod player {
     }
 
     pub struct PutClaim;
-    impl UserAction for PutClaim {
+    impl DbTransaction for PutClaim {
         type Params = LootParams;
         type Response = ();
         fn execute<'q>(
             self,
             conn: &DbConnection,
             params: Self::Params,
-        ) -> ActionResult<Self::Response> {
+        ) -> TransactionResult<Self::Response> {
-            let exists: bool =
-                diesel::select(models::Loot::exists(params.loot_id)).get_result(conn)?;
-            if !exists {
-                return Ok(ActionStatus::nop());
-            };
             let claim = models::claim::NewClaim::new(params.player_id, params.loot_id);
             diesel::insert_into(schema::claims::table)
                 .values(&claim)
                 .execute(conn)
-                .map(ActionStatus::was_updated)
+                .and_then(|_| Ok(()))
         }
     }
 
     pub struct WithdrawClaim;
-    impl UserAction for WithdrawClaim {
+    impl DbTransaction for WithdrawClaim {
         type Params = LootParams;
         type Response = ();
         fn execute<'q>(
             self,
             conn: &DbConnection,
             params: Self::Params,
-        ) -> ActionResult<Self::Response> {
+        ) -> TransactionResult<Self::Response> {
             use schema::claims::dsl::*;
             diesel::delete(
                 claims
@@ -184,7 +210,7 @@ pub(crate) mod player {
                     .filter(player_id.eq(params.player_id)),
             )
             .execute(conn)
-            .map(ActionStatus::was_updated)
+            .and_then(|_| Ok(()))
         }
     }
 
@@ -194,15 +220,15 @@ pub(crate) mod player {
     }
     pub struct UpdateWealth;
 
-    impl UserAction for UpdateWealth {
+    impl DbTransaction for UpdateWealth {
         type Params = WealthParams;
-        type Response = Option<(i32, i32, i32, i32)>;
+        type Response = (i32, i32, i32, i32);
 
         fn execute<'q>(
             self,
             conn: &'q DbConnection,
             params: WealthParams,
-        ) -> ActionResult<Self::Response> {
+        ) -> TransactionResult<Self::Response> {
             use schema::players::dsl::*;
             let current_wealth = players
                 .find(params.player_id)
@@ -219,12 +245,40 @@ pub(crate) mod player {
                 .filter(id.eq(params.player_id))
                 .set(&updated_wealth)
                 .execute(conn)
-                .map(|r| match r {
+                .and_then(|r| match r {
-                    1 => ActionStatus {
+                    1 => Ok(diff),
-                        executed: true,
+                    _ => panic!("UpdateWealth made no changes !"),
-                        response: Some(diff),
+                })
-                    },
+        }
-                    _ => ActionStatus::nop(),
+    }
+
+    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"),
                 })
         }
     }