From 5d710f0c186a2d1a464fdd87a85e8cfe8a6c2569 Mon Sep 17 00:00:00 2001 From: s3lph Date: Fri, 31 Aug 2018 21:52:00 +0200 Subject: [PATCH] - Another change to the database schema - Enabled foreign key constraints (!!!) - Fixed a facade bug exposed by foreign key constraints --- matemat/db/facade.py | 12 ++++++++++-- matemat/db/migrations.py | 16 ++++++++++++++++ matemat/db/schemas.py | 4 ++-- matemat/db/test/test_facade.py | 16 +++++++++++++--- matemat/db/wrapper.py | 7 +++++++ 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/matemat/db/facade.py b/matemat/db/facade.py index 09fc4ce..34211c6 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -111,8 +111,12 @@ class MatematDatabase(object): 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, receipt_pref = row - return User(user_id, username, balance, email, is_admin, is_member, ReceiptPreference(receipt_pref)) + user_id, username, email, is_admin, is_member, balance, receipt_p = row + try: + receipt_pref: ReceiptPreference = ReceiptPreference(receipt_p) + except ValueError as e: + raise DatabaseConsistencyError(f'{receipt_p} is not a valid ReceiptPreference') + return User(user_id, username, balance, email, is_admin, is_member, receipt_pref) def create_user(self, username: str, @@ -531,6 +535,10 @@ class MatematDatabase(object): if amount < 0: raise ValueError('Cannot deposit a negative value') with self.db.transaction() as c: + c.execute('''SELECT username FROM users WHERE user_id = ?''', + [user.id]) + if c.fetchone() is None: + raise DatabaseConsistencyError(f'No such user: {user.id}') c.execute(''' INSERT INTO transactions (user_id, value, old_balance) VALUES (:user_id, :value, :old_balance) diff --git a/matemat/db/migrations.py b/matemat/db/migrations.py index cfbc60b..34416d3 100644 --- a/matemat/db/migrations.py +++ b/matemat/db/migrations.py @@ -119,6 +119,22 @@ def migrate_schema_2_to_3(c: sqlite3.Cursor): # Add missing column to users table c.execute('ALTER TABLE users ADD COLUMN receipt_pref INTEGER(1) NOT NULL DEFAULT 0') + # Fix ON DELETE in transactions table + c.execute(''' + CREATE TABLE transactions_new ( + ta_id INTEGER PRIMARY KEY, + user_id INTEGER DEFAULT NULL, + value INTEGER(8) NOT NULL, + old_balance INTEGER(8) NOT NULL, + date INTEGER(8) DEFAULT (STRFTIME('%s', 'now')), + FOREIGN KEY (user_id) REFERENCES users(user_id) + ON DELETE SET NULL ON UPDATE CASCADE + ) + ''') + c.execute('INSERT INTO transactions_new SELECT * FROM transactions') + c.execute('DROP TABLE transactions') + c.execute('ALTER TABLE transactions_new RENAME TO transactions') + # Change modifications table c.execute(''' CREATE TABLE modifications_new ( diff --git a/matemat/db/schemas.py b/matemat/db/schemas.py index 65052c8..999e4ff 100644 --- a/matemat/db/schemas.py +++ b/matemat/db/schemas.py @@ -129,12 +129,12 @@ SCHEMAS[3] = [ ''' CREATE TABLE transactions ( -- "superclass" of the following 3 tables ta_id INTEGER PRIMARY KEY, - user_id INTEGER NOT NULL, + user_id INTEGER DEFAULT NULL, value INTEGER(8) NOT NULL, old_balance INTEGER(8) NOT NULL, date INTEGER(8) DEFAULT (STRFTIME('%s', 'now')), FOREIGN KEY (user_id) REFERENCES users(user_id) - ON DELETE CASCADE ON UPDATE CASCADE + ON DELETE SET NULL ON UPDATE CASCADE ); ''', ''' diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index d4ab500..de225f4 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -4,7 +4,7 @@ import unittest import crypt from matemat.db import MatematDatabase -from matemat.db.primitives import User +from matemat.db.primitives import User, ReceiptPreference from matemat.exceptions import AuthenticationError, DatabaseConsistencyError @@ -24,12 +24,13 @@ class DatabaseTest(unittest.TestCase): self.assertEqual('testuser@example.com', row[2]) self.assertEqual(0, row[5]) self.assertEqual(1, row[6]) + self.assertEqual(ReceiptPreference.NONE.value, row[7]) 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): + with db.transaction() as c: created = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com', admin=True, member=False) user = db.get_user(created.id) @@ -37,8 +38,14 @@ class DatabaseTest(unittest.TestCase): self.assertEqual('testuser@example.com', user.email) self.assertEqual(False, user.is_member) self.assertEqual(True, user.is_admin) + self.assertEqual(ReceiptPreference.NONE, user.receipt_pref) with self.assertRaises(ValueError): db.get_user(-1) + # Write an invalid receipt preference to the database + c.execute('UPDATE users SET receipt_pref = 42 WHERE user_id = ?', + [user.id]) + with self.assertRaises(DatabaseConsistencyError): + db.get_user(user.id) def test_list_users(self) -> None: with self.db as db: @@ -166,18 +173,21 @@ class DatabaseTest(unittest.TestCase): with self.db as db: agent = db.create_user('admin', 'supersecurepassword', 'admin@example.com', True, True) user = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com', True, True) - db.change_user(user, agent, email='newaddress@example.com', is_admin=False, is_member=False, balance=4200) + db.change_user(user, agent, email='newaddress@example.com', is_admin=False, is_member=False, balance=4200, + receipt_pref=ReceiptPreference.MONTHLY) # Changes must be reflected in the passed user object self.assertEqual('newaddress@example.com', user.email) self.assertFalse(user.is_admin) self.assertFalse(user.is_member) self.assertEqual(4200, user.balance) + self.assertEqual(ReceiptPreference.MONTHLY, user.receipt_pref) # Changes must be reflected in the database checkuser = db.get_user(user.id) self.assertEqual('newaddress@example.com', user.email) self.assertFalse(checkuser.is_admin) self.assertFalse(checkuser.is_member) self.assertEqual(4200, checkuser.balance) + self.assertEqual(ReceiptPreference.MONTHLY, checkuser.receipt_pref) # Balance change without an agent must fail with self.assertRaises(ValueError): db.change_user(user, None, balance=0) diff --git a/matemat/db/wrapper.py b/matemat/db/wrapper.py index 112597b..1dcb038 100644 --- a/matemat/db/wrapper.py +++ b/matemat/db/wrapper.py @@ -62,7 +62,14 @@ class DatabaseWrapper(object): return Transaction(self._sqlite_db, exclusive) def _setup(self) -> None: + # Enable foreign key enforcement + cursor = self._sqlite_db.cursor() + cursor.execute('PRAGMA foreign_keys = 1') + + # Create or update schemas if necessary with self.transaction() as c: + # Defer foreign key enforcement in the setup transaction + c.execute('PRAGMA defer_foreign_keys = 1') version: int = self._user_version if version < 1: # Don't use executescript, as it issues a COMMIT first