From 3d25540a1f2791942eaee02d6650efe8f04f3aa4 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sun, 23 Dec 2018 22:43:19 +0100 Subject: [PATCH 1/4] Fixed a crash in receipt generation. --- matemat/db/facade.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matemat/db/facade.py b/matemat/db/facade.py index 1c8061f..feb174a 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -647,7 +647,7 @@ class MatematDatabase(object): else: t = Transaction(ta_id, user, value, old_balance, datetime.fromtimestamp(date)) transactions.append(t) - if write: + if write and len(transactions) > 0: cursor.execute(''' INSERT INTO receipts (user_id, first_ta_id, last_ta_id) VALUES (:user_id, :first_ta, :last_ta) From ea70484d00992f302625468cc991bad8d53b1ae0 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sun, 23 Dec 2018 22:50:26 +0100 Subject: [PATCH 2/4] Added a test case for empty receipts. --- matemat/db/test/test_facade.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index 2ca28bf..27b7e24 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -506,6 +506,10 @@ class DatabaseTest(unittest.TestCase): db.increment_consumption(user, product) db.deposit(user, 1337) receipt1: Receipt = db.create_receipt(user, write=True) + # Attempt to create a receipt with zero transactions. Won't be written to DB. + receipt2: Receipt = db.create_receipt(user, write=True) + self.assertEqual(-1, receipt2.id) + self.assertEqual(0, len(receipt2.transactions)) with db.transaction() as c: c.execute('SELECT COUNT(receipt_id) FROM receipts') @@ -521,7 +525,7 @@ class DatabaseTest(unittest.TestCase): FROM users WHERE user_id = :id ''', [user.id]) - receipt2: Receipt = db.create_receipt(user, write=False) + receipt3: Receipt = db.create_receipt(user, write=False) with db.transaction() as c: c.execute('SELECT COUNT(receipt_id) FROM receipts') @@ -551,27 +555,27 @@ class DatabaseTest(unittest.TestCase): self.assertEqual(1337, t12.value) self.assertEqual(4000, t12.old_balance) - self.assertEqual(user, receipt2.user) - self.assertEqual(3, len(receipt2.transactions)) + self.assertEqual(user, receipt3.user) + self.assertEqual(3, len(receipt3.transactions)) - self.assertIsInstance(receipt2.transactions[0], Consumption) - t20: Consumption = receipt2.transactions[0] - self.assertEqual(user, receipt2.user) + self.assertIsInstance(receipt3.transactions[0], Consumption) + t20: Consumption = receipt3.transactions[0] + self.assertEqual(user, receipt3.user) self.assertEqual(-200, t20.value) self.assertEqual(5337, t20.old_balance) self.assertEqual('Flora Power Mate', t20.product) - self.assertIsInstance(receipt2.transactions[1], Modification) - t21: Modification = receipt2.transactions[1] - self.assertEqual(user, receipt2.user) + self.assertIsInstance(receipt3.transactions[1], Modification) + t21: Modification = receipt3.transactions[1] + self.assertEqual(user, receipt3.user) self.assertEqual(-937, t21.value) self.assertEqual(5137, t21.old_balance) self.assertEqual(admin.name, t21.agent) self.assertEqual(None, t21.reason) - self.assertIs(type(receipt2.transactions[2]), Transaction) - t22: Transaction = receipt2.transactions[2] - self.assertEqual(user, receipt2.user) + self.assertIs(type(receipt3.transactions[2]), Transaction) + t22: Transaction = receipt3.transactions[2] + self.assertEqual(user, receipt3.user) self.assertEqual(500, t22.value) self.assertEqual(4200, t22.old_balance) From 9a41d33ef7df0e1cfd11f81616391db94df71236 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sat, 29 Dec 2018 20:36:59 +0100 Subject: [PATCH 3/4] Added database support for empty receipts (i.e. when a user didn't issue any transactions during a receipt period) --- matemat/db/facade.py | 8 ++-- matemat/db/migrations.py | 38 +++++++++++++++ matemat/db/schemas.py | 77 ++++++++++++++++++++++++++++++ matemat/db/test/test_migrations.py | 33 +++++++++++++ matemat/db/wrapper.py | 8 ++-- 5 files changed, 156 insertions(+), 8 deletions(-) diff --git a/matemat/db/facade.py b/matemat/db/facade.py index feb174a..9ccfd12 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -606,7 +606,7 @@ class MatematDatabase(object): transactions: List[Transaction] = [] with self.db.transaction() as cursor: cursor.execute(''' - SELECT COALESCE(MAX(r.date), u.created), COALESCE(MAX(r.last_ta_id), 0) + SELECT COALESCE(MAX(r.date), u.created), MAX(r.last_ta_id) FROM users AS u LEFT JOIN receipts AS r ON r.user_id = u.user_id @@ -647,14 +647,14 @@ class MatematDatabase(object): else: t = Transaction(ta_id, user, value, old_balance, datetime.fromtimestamp(date)) transactions.append(t) - if write and len(transactions) > 0: + if write: cursor.execute(''' INSERT INTO receipts (user_id, first_ta_id, last_ta_id) VALUES (:user_id, :first_ta, :last_ta) ''', { 'user_id': user.id, - 'first_ta': transactions[0].id, - 'last_ta': transactions[-1].id + 'first_ta': transactions[0].id if len(transactions) != 0 else None, + 'last_ta': transactions[-1].id if len(transactions) != 0 else None }) cursor.execute('''SELECT last_insert_rowid()''') receipt_id: int = int(cursor.fetchone()[0]) diff --git a/matemat/db/migrations.py b/matemat/db/migrations.py index 8983180..584f4ed 100644 --- a/matemat/db/migrations.py +++ b/matemat/db/migrations.py @@ -201,3 +201,41 @@ def migrate_schema_2_to_3(c: sqlite3.Cursor): ON DELETE SET NULL ON UPDATE CASCADE ) ''') + + +def migrate_schema_3_to_4(c: sqlite3.Cursor): + # Change receipts schema to allow null for transaction IDs + c.execute(''' + CREATE TEMPORARY TABLE receipts_temp ( + receipt_id INTEGER PRIMARY KEY, + user_id INTEGER NOT NULL, + first_ta_id INTEGER DEFAULT NULL, + last_ta_id INTEGER DEFAULT NULL, + date INTEGER(8) DEFAULT (STRFTIME('%s', 'now')), + FOREIGN KEY (user_id) REFERENCES users(user_id) + ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (first_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY (last_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE + ); + ''') + c.execute('INSERT INTO receipts_temp SELECT * FROM receipts') + c.execute('DROP TABLE receipts') + c.execute(''' + CREATE TABLE receipts ( + receipt_id INTEGER PRIMARY KEY, + user_id INTEGER NOT NULL, + first_ta_id INTEGER DEFAULT NULL, + last_ta_id INTEGER DEFAULT NULL, + date INTEGER(8) DEFAULT (STRFTIME('%s', 'now')), + FOREIGN KEY (user_id) REFERENCES users(user_id) + ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (first_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY (last_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE + ); + ''') + c.execute('INSERT INTO receipts SELECT * FROM receipts_temp') + c.execute('DROP TABLE receipts_temp') diff --git a/matemat/db/schemas.py b/matemat/db/schemas.py index 3bbb5b9..d47abbd 100644 --- a/matemat/db/schemas.py +++ b/matemat/db/schemas.py @@ -178,3 +178,80 @@ SCHEMAS[3] = [ ON DELETE SET NULL ON UPDATE CASCADE ); '''] + +SCHEMAS[4] = [ + ''' + CREATE TABLE users ( + user_id INTEGER PRIMARY KEY, + username TEXT UNIQUE NOT NULL, + email TEXT DEFAULT NULL, + password TEXT NOT NULL, + touchkey TEXT DEFAULT NULL, + is_admin INTEGER(1) NOT NULL DEFAULT 0, + is_member INTEGER(1) NOT NULL DEFAULT 1, + balance INTEGER(8) NOT NULL DEFAULT 0, + lastchange INTEGER(8) NOT NULL DEFAULT 0, + receipt_pref INTEGER(1) NOT NULL DEFAULT 0, + created INTEGER(8) NOT NULL DEFAULT 0 + ); + ''', + ''' + CREATE TABLE products ( + product_id INTEGER PRIMARY KEY, + name TEXT UNIQUE NOT NULL, + stock INTEGER(8) NOT NULL DEFAULT 0, + price_member INTEGER(8) NOT NULL, + price_non_member INTEGER(8) NOT NULL + ); + ''', + ''' + CREATE TABLE transactions ( -- "superclass" of the following 3 tables + 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 + ); + ''', + ''' + CREATE TABLE consumptions ( -- transactions involving buying a product + ta_id INTEGER PRIMARY KEY, + product TEXT NOT NULL, + FOREIGN KEY (ta_id) REFERENCES transactions(ta_id) + ON DELETE CASCADE ON UPDATE CASCADE + ); + ''', + ''' + CREATE TABLE deposits ( -- transactions involving depositing cash + ta_id INTEGER PRIMARY KEY, + FOREIGN KEY (ta_id) REFERENCES transactions(ta_id) + ON DELETE CASCADE ON UPDATE CASCADE + ); + ''', + ''' + CREATE TABLE modifications ( -- transactions involving balance modification by an admin + ta_id INTEGER NOT NULL, + agent TEXT NOT NULL, + reason TEXT DEFAULT NULL, + PRIMARY KEY (ta_id), + FOREIGN KEY (ta_id) REFERENCES transactions(ta_id) + ON DELETE CASCADE ON UPDATE CASCADE + ); + ''', + ''' + CREATE TABLE receipts ( -- receipts sent to the users + receipt_id INTEGER PRIMARY KEY, + user_id INTEGER NOT NULL, + first_ta_id INTEGER DEFAULT NULL, + last_ta_id INTEGER DEFAULT NULL, + date INTEGER(8) DEFAULT (STRFTIME('%s', 'now')), + FOREIGN KEY (user_id) REFERENCES users(user_id) + ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (first_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY (last_ta_id) REFERENCES transactions(ta_id) + ON DELETE SET NULL ON UPDATE CASCADE + ); + '''] diff --git a/matemat/db/test/test_migrations.py b/matemat/db/test/test_migrations.py index 707709e..796f863 100644 --- a/matemat/db/test/test_migrations.py +++ b/matemat/db/test/test_migrations.py @@ -183,3 +183,36 @@ class TestMigrations(unittest.TestCase): self.assertEqual('Flora Power Mate', cursor.fetchone()[0]) cursor.execute('''SELECT product FROM consumptions WHERE ta_id = 5''') self.assertEqual('', cursor.fetchone()[0]) + + def test_upgrade_3_to_4(self): + # Setup test db with example entries to test schema change + self._initialize_db(3) + cursor: sqlite3.Cursor = self.db._sqlite_db.cursor() + cursor.execute(''' + INSERT INTO users VALUES + (1, 'testadmin', 'a@b.c', '$2a$10$herebehashes', NULL, 1, 1, 1337, 0, 0, 0) + ''') + cursor.execute(''' + INSERT INTO products VALUES + (1, 'Club Mate', 42, 200, 250) + ''') + cursor.execute(''' + INSERT INTO transactions VALUES (1, 1, 4200, 0, 1000) + ''') + cursor.execute(''' + INSERT INTO receipts VALUES (1, 1, 1, 1, 1337) + ''') + cursor.execute('PRAGMA user_version = 3') + + # Kick off the migration + schema_version = self.db.SCHEMA_VERSION + self.db.SCHEMA_VERSION = 4 + self.db._setup() + self.db.SCHEMA_VERSION = schema_version + + # Make sure entries from the receipts table are preserved + cursor.execute('''SELECT COUNT(receipt_id) FROM receipts''') + self.assertEqual(1, cursor.fetchone()[0]) + + # Make sure transaction IDs can be set to NULL + cursor.execute('UPDATE receipts SET first_ta_id = NULL, last_ta_id = NULL') diff --git a/matemat/db/wrapper.py b/matemat/db/wrapper.py index 4460680..81baf04 100644 --- a/matemat/db/wrapper.py +++ b/matemat/db/wrapper.py @@ -2,11 +2,9 @@ from __future__ import annotations from typing import Any, Optional -import sqlite3 - from matemat.exceptions import DatabaseConsistencyError from matemat.db.schemas import SCHEMAS -from matemat.db.migrations import migrate_schema_1_to_2, migrate_schema_2_to_3 +from matemat.db.migrations import * class DatabaseTransaction(object): @@ -43,7 +41,7 @@ class DatabaseTransaction(object): class DatabaseWrapper(object): - SCHEMA_VERSION = 3 + SCHEMA_VERSION = 4 def __init__(self, filename: str) -> None: self._filename: str = filename @@ -86,6 +84,8 @@ class DatabaseWrapper(object): migrate_schema_1_to_2(c) if from_version <= 2 and to_version >= 3: migrate_schema_2_to_3(c) + if from_version <= 3 and to_version >= 4: + migrate_schema_3_to_4(c) def connect(self) -> None: if self.is_connected(): From 07a4d3131ee647e2be7278d43a04705c72a1ab65 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sat, 29 Dec 2018 20:55:53 +0100 Subject: [PATCH 4/4] Fixed a small bug in the SQL queries and the unit tests --- matemat/db/facade.py | 2 +- matemat/db/test/test_facade.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/matemat/db/facade.py b/matemat/db/facade.py index 9ccfd12..41274ad 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -606,7 +606,7 @@ class MatematDatabase(object): transactions: List[Transaction] = [] with self.db.transaction() as cursor: cursor.execute(''' - SELECT COALESCE(MAX(r.date), u.created), MAX(r.last_ta_id) + SELECT COALESCE(MAX(r.date), u.created), COALESCE(MAX(r.last_ta_id), 0) FROM users AS u LEFT JOIN receipts AS r ON r.user_id = u.user_id diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index 27b7e24..29c38e2 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -506,14 +506,14 @@ class DatabaseTest(unittest.TestCase): db.increment_consumption(user, product) db.deposit(user, 1337) receipt1: Receipt = db.create_receipt(user, write=True) - # Attempt to create a receipt with zero transactions. Won't be written to DB. + # Attempt to create a receipt with zero transactions. Will carry NULL transaction IDs receipt2: Receipt = db.create_receipt(user, write=True) - self.assertEqual(-1, receipt2.id) + self.assertNotEqual(-1, receipt2.id) self.assertEqual(0, len(receipt2.transactions)) with db.transaction() as c: c.execute('SELECT COUNT(receipt_id) FROM receipts') - self.assertEqual(1, c.fetchone()[0]) + self.assertEqual(2, c.fetchone()[0]) db.increment_consumption(user, product) db.change_user(user, agent=admin, balance=4200) @@ -529,7 +529,7 @@ class DatabaseTest(unittest.TestCase): with db.transaction() as c: c.execute('SELECT COUNT(receipt_id) FROM receipts') - self.assertEqual(1, c.fetchone()[0]) + self.assertEqual(2, c.fetchone()[0]) self.assertEqual(user, receipt1.user) self.assertEqual(3, len(receipt1.transactions))