From c4018156f5576b4e58e14b64c0a68a2a73b71c2b Mon Sep 17 00:00:00 2001
From: s3lph <account-gitlab-ideynizv@kernelpanic.lol>
Date: Thu, 19 Jul 2018 21:51:35 +0200
Subject: [PATCH] Changed the change_user and change_product API to get rid of
 the update -> write -> undo on failure pattern.

---
 matemat/db/facade.py | 62 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/matemat/db/facade.py b/matemat/db/facade.py
index b3d53ae..67f1c5f 100644
--- a/matemat/db/facade.py
+++ b/matemat/db/facade.py
@@ -232,12 +232,23 @@ class MatematDatabase(object):
                 'tkhash': tkhash
             })
 
-    def change_user(self, user: User) -> None:
+    def change_user(self, user: User, **kwargs)\
+            -> None:
         """
-        Write changes in the User object to the database.
-        :param user: The user object to update in the database.
+        Commit changes to the user in the database.  If writing the requested changes succeeded, the values are updated
+        in the provided user object.  Otherwise the user object is left untouched.  The user to update is identified by
+        the ID field in the provided user object.
+
+        :param user: The user object to update and to identify the requested user by.
+        :param kwargs: The properties to change.
         :raises DatabaseConsistencyError: If the user represented by the object does not exist.
         """
+        # Resolve the values to change
+        name: str = kwargs['name'] if 'name' in kwargs else user.name
+        email: str = kwargs['email'] if 'email' in kwargs else user.email
+        balance: int = kwargs['balance'] if 'balance' in kwargs else user.balance
+        is_admin: bool = kwargs['is_admin'] if 'is_admin' in kwargs else user.is_admin
+        is_member: bool = kwargs['is_member'] if 'is_member' in kwargs else user.is_member
         with self.db.transaction() as c:
             c.execute('''
             UPDATE users SET
@@ -250,16 +261,22 @@ class MatematDatabase(object):
             WHERE user_id = :user_id
             ''', {
                 'user_id': user.id,
-                'username': user.name,
-                'email': user.email,
-                'balance': user.balance,
-                'is_admin': user.is_admin,
-                'is_member': user.is_member
+                'username': name,
+                'email': email,
+                'balance': balance,
+                'is_admin': is_admin,
+                'is_member': is_member
             })
             affected = c.execute('SELECT changes()').fetchone()[0]
             if affected != 1:
                 raise DatabaseConsistencyError(
                     f'change_user should affect 1 users row, but affected {affected}')
+        # Only update the actual user object after the changes in the database succeeded
+        user.name = name
+        user.email = email
+        user.balance = balance
+        user.is_admin = is_admin
+        user.is_member = is_member
 
     def delete_user(self, user: User) -> None:
         """
@@ -337,7 +354,21 @@ class MatematDatabase(object):
             product_id = int(c.fetchone()[0])
         return Product(product_id, name, price_member, price_non_member, 0)
 
-    def change_product(self, product: Product) -> None:
+    def change_product(self, product: Product, **kwargs) -> None:
+        """
+        Commit changes to the product in the database.  If writing the requested changes succeeded, the values are
+        updated in the provided product object.  Otherwise the product object is left untouched.  The product to update
+        is identified by the ID field in the provided product object.
+
+        :param product: The product object to update and to identify the requested product by.
+        :param kwargs: The properties to change.
+        :raises DatabaseConsistencyError: If the product represented by the object does not exist.
+        """
+        # Resolve the values to change
+        name: str = kwargs['name'] if 'name' in kwargs else product.name
+        price_member: int = kwargs['price_member'] if 'price_member' in kwargs else product.price_member
+        price_non_member: int = kwargs['price_non_member'] if 'price_non_member' in kwargs else product.price_non_member
+        stock: int = kwargs['stock'] if 'stock' in kwargs else product.stock
         with self.db.transaction() as c:
             c.execute('''
             UPDATE products
@@ -349,15 +380,20 @@ class MatematDatabase(object):
             WHERE product_id = :product_id
             ''', {
                 'product_id': product.id,
-                'name': product.name,
-                'price_member': product.price_member,
-                'price_non_member': product.price_non_member,
-                'stock': product.stock
+                'name': name,
+                'price_member': price_member,
+                'price_non_member': price_non_member,
+                'stock': stock
             })
             affected = c.execute('SELECT changes()').fetchone()[0]
             if affected != 1:
                 raise DatabaseConsistencyError(
                     f'change_product should affect 1 products row, but affected {affected}')
+        # Only update the actual product object after the changes in the database succeeded
+        product.name = name
+        product.price_member = price_member
+        product.price_non_member = price_non_member
+        product.stock = stock
 
     def delete_product(self, product: Product) -> None:
         """