From 5e3c633f1994329297999899790e69d28516934b Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sun, 5 May 2024 03:46:35 +0200 Subject: [PATCH] Refactor save and remove methods of BaseObject subclasses. --- plomtask/conditions.py | 9 +----- plomtask/days.py | 4 --- plomtask/db.py | 23 ++++++++++---- plomtask/http.py | 2 +- plomtask/processes.py | 25 ++++------------ plomtask/todos.py | 30 +++++-------------- plomtask/versioned_attributes.py | 4 +++ scripts/pre-commit | 3 +- tests/__pycache__/conditions.cpython-311.pyc | Bin 9478 -> 9550 bytes tests/__pycache__/todos.cpython-311.pyc | Bin 31688 -> 31699 bytes tests/conditions.py | 1 + tests/todos.py | 4 +-- 12 files changed, 40 insertions(+), 65 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index cba606d..a6e9c97 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -11,6 +11,7 @@ class Condition(BaseModel[int]): """Non-Process dependency for ProcessSteps and Todos.""" table_name = 'conditions' to_save = ['is_active'] + to_save_versioned = ['title', 'description'] def __init__(self, id_: int | None, is_active: bool = False) -> None: super().__init__(id_) @@ -30,12 +31,6 @@ class Condition(BaseModel[int]): getattr(condition, name).history_from_row(row_) return condition - def save(self, db_conn: DatabaseConnection) -> None: - """Save self and its VersionedAttributes to DB and cache.""" - self.save_core(db_conn) - self.title.save(db_conn) - self.description.save(db_conn) - def remove(self, db_conn: DatabaseConnection) -> None: """Remove from DB, with VersionedAttributes. @@ -49,8 +44,6 @@ class Condition(BaseModel[int]): table_name = f'{item}_{attr}' for _ in db_conn.row_where(table_name, 'condition', self.id_): raise HandledException('cannot remove Condition in use') - db_conn.delete_where('condition_titles', 'parent', self.id_) - db_conn.delete_where('condition_descriptions', 'parent', self.id_) super().remove(db_conn) diff --git a/plomtask/days.py b/plomtask/days.py index 8dd3843..0e07bf7 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -94,7 +94,3 @@ class Day(BaseModel[str]): """Return date succeeding date of this Day.""" next_datetime = self.datetime + timedelta(days=1) return next_datetime.strftime(DATE_FORMAT) - - def save(self, db_conn: DatabaseConnection) -> None: - """Add (or re-write) self to DB and cache.""" - self.save_core(db_conn) diff --git a/plomtask/db.py b/plomtask/db.py index 982ddfe..e4d5f6e 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -75,7 +75,7 @@ class DatabaseConnection: """Close DB connection.""" self.conn.close() - def rewrite_relations(self, table_name: str, key: str, target: int, + def rewrite_relations(self, table_name: str, key: str, target: int | str, rows: list[list[Any]]) -> None: """Rewrite relations in table_name to target, with rows values.""" self.delete_where(table_name, key, target) @@ -121,6 +121,8 @@ class BaseModel(Generic[BaseModelId]): """Template for most of the models we use/derive from the DB.""" table_name = '' to_save: list[str] = [] + to_save_versioned: list[str] = [] + to_save_relations: list[tuple[str, str, str]] = [] id_: None | BaseModelId cache_: dict[BaseModelId, Self] @@ -245,11 +247,11 @@ class BaseModel(Generic[BaseModelId]): items[item.id_] = item return list(items.values()) - def save_core(self, db_conn: DatabaseConnection) -> None: - """Write bare-bones self (sans connected items), ensuring self.id_. + def save(self, db_conn: DatabaseConnection) -> None: + """Write self to DB and cache and ensure .id_. Write both to DB, and to cache. To DB, write .id_ and attributes - listed in cls.to_save. + listed in cls.to_save[_versioned|_relations]. Ensure self.id_ by setting it to what the DB command returns as the last saved row's ID (cursor.lastrowid), EXCEPT if self.id_ already @@ -265,10 +267,21 @@ class BaseModel(Generic[BaseModelId]): if not isinstance(self.id_, str): self.id_ = cursor.lastrowid # type: ignore[assignment] self.cache() + for attr_name in self.to_save_versioned: + getattr(self, attr_name).save(db_conn) + for table, column, attr_name in self.to_save_relations: + assert isinstance(self.id_, (int, str)) + db_conn.rewrite_relations(table, column, self.id_, + [[i.id_] for i + in getattr(self, attr_name)]) def remove(self, db_conn: DatabaseConnection) -> None: - """Remove from DB and cache.""" + """Remove from DB and cache, including dependencies.""" if self.id_ is None or self.__class__.get_cached(self.id_) is None: raise HandledException('cannot remove unsaved item') + for attr_name in self.to_save_versioned: + getattr(self, attr_name).remove(db_conn) + for table, column, attr_name in self.to_save_relations: + db_conn.delete_where(table, column, self.id_) self.uncache() db_conn.delete_where(self.table_name, 'id', self.id_) diff --git a/plomtask/http.py b/plomtask/http.py index f5160f6..adac957 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -263,7 +263,7 @@ class TaskHandler(BaseHTTPRequestHandler): self.form_data.get_all_int('condition')) process.set_enables(self.conn, self.form_data.get_all_int('enables')) process.set_disables(self.conn, self.form_data.get_all_int('disables')) - process.save_core(self.conn) + process.save(self.conn) process.explicit_steps = [] steps: list[tuple[int | None, int, int | None]] = [] for step_id in self.form_data.get_all_int('steps'): diff --git a/plomtask/processes.py b/plomtask/processes.py index c4ccfa8..1778e4f 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -23,6 +23,10 @@ class ProcessStepsNode: class Process(BaseModel[int], ConditionsRelations): """Template for, and metadata for, Todos, and their arrangements.""" table_name = 'processes' + to_save_versioned = ['title', 'description', 'effort'] + to_save_relations = [('process_conditions', 'process', 'conditions'), + ('process_enables', 'process', 'enables'), + ('process_disables', 'process', 'disables')] # pylint: disable=too-many-instance-attributes @@ -155,17 +159,8 @@ class Process(BaseModel[int], ConditionsRelations): def save(self, db_conn: DatabaseConnection) -> None: """Add (or re-write) self and connected items to DB.""" - self.save_core(db_conn) + super().save(db_conn) assert isinstance(self.id_, int) - self.title.save(db_conn) - self.description.save(db_conn) - self.effort.save(db_conn) - db_conn.rewrite_relations('process_conditions', 'process', self.id_, - [[c.id_] for c in self.conditions]) - db_conn.rewrite_relations('process_enables', 'process', self.id_, - [[c.id_] for c in self.enables]) - db_conn.rewrite_relations('process_disables', 'process', self.id_, - [[c.id_] for c in self.disables]) db_conn.delete_where('process_steps', 'owner', self.id_) for step in self.explicit_steps: step.save(db_conn) @@ -180,14 +175,8 @@ class Process(BaseModel[int], ConditionsRelations): raise HandledException('cannot remove Process in use') for _ in db_conn.row_where('todos', 'process', self.id_): raise HandledException('cannot remove Process in use') - db_conn.delete_where('process_conditions', 'process', self.id_) - db_conn.delete_where('process_enables', 'process', self.id_) - db_conn.delete_where('process_disables', 'process', self.id_) for step in self.explicit_steps: step.remove(db_conn) - db_conn.delete_where('process_titles', 'parent', self.id_) - db_conn.delete_where('process_descriptions', 'parent', self.id_) - db_conn.delete_where('process_efforts', 'parent', self.id_) super().remove(db_conn) @@ -203,10 +192,6 @@ class ProcessStep(BaseModel[int]): self.step_process_id = step_process_id self.parent_step_id = parent_step_id - def save(self, db_conn: DatabaseConnection) -> None: - """Default to simply calling self.save_core for simple cases.""" - self.save_core(db_conn) - def remove(self, db_conn: DatabaseConnection) -> None: """Remove from DB, and owner's .explicit_steps.""" owner = Process.by_id(db_conn, self.owner_id) diff --git a/plomtask/todos.py b/plomtask/todos.py index ecdd599..fcb8617 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -25,10 +25,17 @@ class Todo(BaseModel[int], ConditionsRelations): # pylint: disable=too-many-instance-attributes table_name = 'todos' to_save = ['process_id', 'is_done', 'date'] + to_save_relations = [('todo_conditions', 'todo', 'conditions'), + ('todo_enables', 'todo', 'enables'), + ('todo_disables', 'todo', 'disables'), + ('todo_children', 'parent', 'children'), + ('todo_children', 'child', 'parents')] def __init__(self, id_: int | None, process: Process, is_done: bool, date: str) -> None: super().__init__(id_) + if process.id_ is None: + raise NotFoundException('Process of Todo without ID (not saved?)') self.process = process self._is_done = is_done self.date = date @@ -252,35 +259,12 @@ class Todo(BaseModel[int], ConditionsRelations): self.children.remove(child) child.parents.remove(self) - def save(self, db_conn: DatabaseConnection) -> None: - """Write self and children to DB and its cache.""" - if self.process.id_ is None: - raise NotFoundException('Process of Todo without ID (not saved?)') - self.save_core(db_conn) - assert isinstance(self.id_, int) - db_conn.rewrite_relations('todo_children', 'child', self.id_, - [[p.id_] for p in self.parents]) - db_conn.rewrite_relations('todo_children', 'parent', self.id_, - [[c.id_] for c in self.children]) - db_conn.rewrite_relations('todo_conditions', 'todo', self.id_, - [[c.id_] for c in self.conditions]) - db_conn.rewrite_relations('todo_enables', 'todo', self.id_, - [[c.id_] for c in self.enables]) - db_conn.rewrite_relations('todo_disables', 'todo', self.id_, - [[c.id_] for c in self.disables]) - def remove(self, db_conn: DatabaseConnection) -> None: """Remove from DB, including relations.""" - assert isinstance(self.id_, int) children_to_remove = self.children[:] parents_to_remove = self.parents[:] for child in children_to_remove: self.remove_child(child) for parent in parents_to_remove: parent.remove_child(self) - db_conn.delete_where('todo_children', 'parent', self.id_) - db_conn.delete_where('todo_children', 'child', self.id_) - db_conn.delete_where('todo_conditions', 'todo', self.id_) - db_conn.delete_where('todo_enables', 'todo', self.id_) - db_conn.delete_where('todo_disables', 'todo', self.id_) super().remove(db_conn) diff --git a/plomtask/versioned_attributes.py b/plomtask/versioned_attributes.py index ab39df0..1810a31 100644 --- a/plomtask/versioned_attributes.py +++ b/plomtask/versioned_attributes.py @@ -66,3 +66,7 @@ class VersionedAttribute: db_conn.rewrite_relations(self.table_name, 'parent', self.parent.id_, [[item[0], item[1]] for item in self.history.items()]) + + def remove(self, db_conn: DatabaseConnection) -> None: + """Remove from DB.""" + db_conn.delete_where(self.table_name, 'parent', self.parent.id_) diff --git a/scripts/pre-commit b/scripts/pre-commit index 2aaccb0..6f84c41 100755 --- a/scripts/pre-commit +++ b/scripts/pre-commit @@ -1,7 +1,6 @@ #!/bin/sh set -e -# for dir in $(echo '.' 'plomtask' 'tests'); do -for dir in $(echo 'tests'); do +for dir in $(echo '.' 'plomtask' 'tests'); do echo "Running mypy on ${dir}/ …." python3 -m mypy --strict ${dir}/*.py echo "Running flake8 on ${dir}/ …" diff --git a/tests/__pycache__/conditions.cpython-311.pyc b/tests/__pycache__/conditions.cpython-311.pyc index 252bf75240a44a184ae2e5476e2b5b779fdb807c..4dd00fd37a9985baa01f9ce133463f14ef0be350 100644 GIT binary patch delta 364 zcmZqkI_Jf^oR^o20SKl)HcM08$ZIUhXfoMRG?S%*`sI-klt++AJaYZH|f&)n0V$Vn{PAn-YntVZGD&vmHX_C4kH#m6uIlDL~s7%qh z$RU4)L;eDX{N~A$^^Af?M5JA;S&vw=xY%fVx#V94js{DZgE3r9TW;HWg zCeWhd%aa3@3^rRR2s1I3O!iS)$e6bIozhxHBX34VplulF_X`#SONf9 Cx?Dd1 delta 327 zcmX@-)#k;!oR^o20SJtRjMG>*@*0aWGE8<9&15QJpZq}7kdbq;keD)%G!s*0WSwlt zEW%R5ki|AxL0poVWdW+d)hCFGfb7ZtUa)y^mm-FUayylSNf60eTls_5c6? diff --git a/tests/__pycache__/todos.cpython-311.pyc b/tests/__pycache__/todos.cpython-311.pyc index 0369c07cb56c3d56c293f4d8e70348cbc33c60d8..6aa08151a43688a894e37888188609077b68efdc 100644 GIT binary patch delta 480 zcmX@{o$>N_M&9MTyj%=GaP+ZRTHZ$95LPaBAdeY{Ki`_1&8o~&!?1vHvZH_qQwhst zK^+;f5|BI;q%hVnrZCkoWC5kL#2J|;_iGwX4repOuV{amDacXf9=VV(y z@yR9-z{_*1nE~y6`;@3GQU*;4PzRNE>!DWX23eycL zTeL3n+g{U2_vN2l#V#>fiz890h!yCJB4HqLizl(TIJKxG zC^54*wYZ3L@=}ia`k#yp3=IrVr4$#a%+#Hu+rfXAUwDGc4E+_R8&tMvUF5gD!f)Hb za)Vd!Ig-3g7=h&E Mdop61o4ErD0L|NiApigX diff --git a/tests/conditions.py b/tests/conditions.py index 9c95206..faaeb87 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -83,6 +83,7 @@ class TestsWithDB(TestCaseWithDB): self.check_remove() c = Condition(None) proc = Process(None) + proc.save(self.db_conn) todo = Todo(None, proc, False, '2024-01-01') for depender in (proc, todo): assert hasattr(depender, 'save') diff --git a/tests/todos.py b/tests/todos.py index a5dafd1..7fabfdb 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -24,10 +24,10 @@ class TestsWithDB(TestCaseWithDB): def test_Todo_by_id(self) -> None: """Test creation and findability of Todos.""" process_unsaved = Process(None) - todo = Todo(None, process_unsaved, False, self.date1) with self.assertRaises(NotFoundException): - todo.save(self.db_conn) + todo = Todo(None, process_unsaved, False, self.date1) process_unsaved.save(self.db_conn) + todo = Todo(None, process_unsaved, False, self.date1) todo.save(self.db_conn) self.assertEqual(Todo.by_id(self.db_conn, 1), todo) with self.assertRaises(NotFoundException): -- 2.30.2