Merge branch 'dbfacade-staging-merge' into staging-unstable

matemat/db/facade.py

@@ -85,15 +85,17 @@ class MatematDatabase(object):
 
     def get_user(self, uid: int) -> User:
         """
-        Return a user identified by its user ID
+        Return a user identified by its user ID.
-        :param uid: The user's ID
+        :param uid: The user's ID.
         """
         with self.db.transaction(exclusive=False) as c:
+            # Fetch all values to construct the user
             c.execute('SELECT user_id, username, email, is_admin, is_member, balance FROM users WHERE user_id = ?',
                       [uid])
             row = c.fetchone()
             if row is None:
                 raise ValueError(f'No user with user ID {uid} exists.')
+            # Unpack the row and construct the user
             user_id, username, email, is_admin, is_member, balance = row
             return User(user_id, username, balance, email, is_admin, is_member)
 
@@ -230,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.
+        Commit changes to the user in the database.  If writing the requested changes succeeded, the values are updated
-        :param user: The user object to update in the database.
+        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
@@ -248,16 +261,22 @@ class MatematDatabase(object):
             WHERE user_id = :user_id
             ''', {
                 'user_id': user.id,
-                'username': user.name,
+                'username': name,
-                'email': user.email,
+                'email': email,
-                'balance': user.balance,
+                'balance': balance,
-                'is_admin': user.is_admin,
+                'is_admin': is_admin,
-                'is_member': user.is_member
+                '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:
         """
@@ -292,10 +311,11 @@ class MatematDatabase(object):
 
     def get_product(self, pid: int) -> Product:
         """
-        Return a product identified by its product ID
+        Return a product identified by its product ID.
-        :param pid: The products's ID
+        :param pid: The products's ID.
         """
         with self.db.transaction(exclusive=False) as c:
+            # Fetch all values to construct the product
             c.execute('''
                 SELECT product_id, name, price_member, price_non_member, stock
                 FROM products
@@ -304,6 +324,7 @@ class MatematDatabase(object):
             row = c.fetchone()
             if row is None:
                 raise ValueError(f'No product with product ID {pid} exists.')
+            # Unpack the row and construct the product
             product_id, name, price_member, price_non_member, stock = row
             return Product(product_id, name, price_member, price_non_member, stock)
 
@@ -333,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
@@ -345,15 +380,20 @@ class MatematDatabase(object):
             WHERE product_id = :product_id
             ''', {
                 'product_id': product.id,
-                'name': product.name,
+                'name': name,
-                'price_member': product.price_member,
+                'price_member': price_member,
-                'price_non_member': product.price_non_member,
+                'price_non_member': price_non_member,
-                'stock': product.stock
+                '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:
         """

matemat/db/test/test_facade.py

@@ -26,6 +26,19 @@ class DatabaseTest(unittest.TestCase):
                 with self.assertRaises(ValueError):
                     db.create_user('testuser', 'supersecurepassword2', 'testuser2@example.com')
 
+    def test_get_user(self) -> None:
+        with self.db as db:
+            with db.transaction(exclusive=False):
+                created = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com',
+                                         admin=True, member=False)
+                user = db.get_user(created.id)
+                self.assertEqual('testuser', user.name)
+                self.assertEqual('testuser@example.com', user.email)
+                self.assertEqual(False, user.is_member)
+                self.assertEqual(True, user.is_admin)
+                with self.assertRaises(ValueError):
+                    db.get_user(-1)
+
     def test_list_users(self) -> None:
         with self.db as db:
             users = db.list_users()
@@ -170,6 +183,17 @@ class DatabaseTest(unittest.TestCase):
             with self.assertRaises(ValueError):
                 db.create_product('Club Mate', 250, 250)
 
+    def test_get_product(self) -> None:
+        with self.db as db:
+            with db.transaction(exclusive=False):
+                created = db.create_product('Club Mate', 150, 250)
+                product = db.get_product(created.id)
+                self.assertEqual('Club Mate', product.name)
+                self.assertEqual(150, product.price_member)
+                self.assertEqual(250, product.price_non_member)
+                with self.assertRaises(ValueError):
+                    db.get_product(-1)
+
     def test_list_products(self) -> None:
         with self.db as db:
             # Test empty list

matemat/primitives/Product.py

@@ -3,6 +3,10 @@ from typing import Any
 
 
 class Product(object):
+    """
+    Representation of a product offered by the Matemat, with a name, prices for users, and the number of items
+    currently in stock.
+    """
 
     def __init__(self,
                  product_id: int,
@@ -10,6 +14,15 @@ class Product(object):
                  price_member: int,
                  price_non_member: int,
                  stock: int) -> None:
+        """
+        Create  a new product instance with the given arguments.
+
+        :param product_id: The product ID in the database.
+        :param name: The product's name.
+        :param price_member: The price of a unit of this product for users marked as "members".
+        :param price_non_member: The price of a unit of this product for users NOT marked as "members".
+        :param stock: The number of items of this product currently in stock.
+        """
         self._product_id: int = product_id
         self._name: str = name
         self._price_member: int = price_member
@@ -22,7 +35,8 @@ class Product(object):
         return self._product_id == other._product_id \
             and self._name == other._name \
             and self._price_member == other._price_member \
-            and self._price_non_member == other._price_non_member
+            and self._price_non_member == other._price_non_member \
+            and self._stock == other._stock
 
     @property
     def id(self) -> int:

matemat/primitives/User.py

@@ -3,6 +3,11 @@ from typing import Optional, Any
 
 
 class User(object):
+    """
+    Representation of a user registered with the Matemat, with a name, e-mail address (optional), whether the user is a
+    member of the organization the Matemat instance is used in, whether the user is an administrator, and the user's
+    account balance.
+    """
 
     def __init__(self,
                  user_id: int,
@@ -11,6 +16,16 @@ class User(object):
                  email: Optional[str] = None,
                  admin: bool = False,
                  member: bool = True) -> None:
+        """
+        Create  a new user instance with the given arguments.
+
+        :param user_id: The user ID in the database.
+        :param username: The user's name.
+        :param balance: The balance of the user's account.
+        :param email: The user's e-mail address (optional).
+        :param admin: Whether the user is an administrator.
+        :param member: Whether the user is a member.
+        """
         self._user_id: int = user_id
         self._username: str = username
         self._email: Optional[str] = email

matemat/webserver/pagelets/admin.py

@@ -49,15 +49,10 @@ def handle_change(args: RequestArguments, user: User, db: MatematDatabase, confi
             email = str(args.email)
             if len(email) == 0:
                 email = None
-            oldname = user.name
-            oldmail = user.email
             try:
-                user.name = username
+                db.change_user(user, name=username, email=email)
-                user.email = email
-                db.change_user(user)
             except DatabaseConsistencyError:
-                user.name = oldname
+                pass
-                user.email = oldmail
 
         elif change == 'password':
             if 'oldpass' not in args or 'newpass' not in args or 'newpass2' not in args:

matemat/webserver/pagelets/modproduct.py

@@ -62,21 +62,11 @@ def handle_change(args: RequestArguments, product: Product, db: MatematDatabase,
         price_member = int(str(args.pricemember))
         price_non_member = int(str(args.pricenonmember))
         stock = int(str(args.stock))
-        oldname = product.name
-        oldprice_member = product.price_member
-        oldprice_non_member = product.price_non_member
-        oldstock = product.stock
-        product.name = name
-        product.price_member = price_member
-        product.price_non_member = price_non_member
-        product.stock = stock
         try:
-            db.change_product(product)
+            db.change_product(product,
+                              name=name, price_member=price_member, price_non_member=price_non_member, stock=stock)
         except DatabaseConsistencyError:
-            product.name = oldname
+            pass
-            product.email = oldprice_member
-            product.is_member = oldprice_non_member
-            product.stock = oldstock
         if 'image' in args:
             image = bytes(args.image)
             if len(image) > 0:

matemat/webserver/pagelets/moduser.py

@@ -62,26 +62,14 @@ def handle_change(args: RequestArguments, user: User, db: MatematDatabase, confi
         email = str(args.email)
         password = str(args.password)
         balance = int(str(args.balance))
+        is_member = 'ismember' in args
+        is_admin = 'isadmin' in args
         if len(email) == 0:
             email = None
-        oldname = user.name
-        oldmail = user.email
-        oldmember = user.is_member
-        oldadmin = user.is_admin
-        oldbalance = user.balance
-        user.name = username
-        user.email = email
-        user.is_member = 'ismember' in args
-        user.is_admin = 'isadmin' in args
-        user.balance = balance
         try:
-            db.change_user(user)
+            db.change_user(user, name=username, email=email, is_member=is_member, is_admin=is_admin, balance=balance)
         except DatabaseConsistencyError:
-            user.name = oldname
+            pass
-            user.email = oldmail
-            user.is_member = oldmember
-            user.is_admin = oldadmin
-            user.balance = oldbalance
         if len(password) > 0:
             db.change_password(user, '', password, verify_password=False)
         if 'avatar' in args: