home · contact · privacy
Refactor save and remove methods of BaseObject subclasses.
authorChristian Heller <c.heller@plomlompom.de>
Sun, 5 May 2024 01:46:35 +0000 (03:46 +0200)
committerChristian Heller <c.heller@plomlompom.de>
Sun, 5 May 2024 01:46:35 +0000 (03:46 +0200)
12 files changed:
plomtask/conditions.py
plomtask/days.py
plomtask/db.py
plomtask/http.py
plomtask/processes.py
plomtask/todos.py
plomtask/versioned_attributes.py
scripts/pre-commit
tests/__pycache__/conditions.cpython-311.pyc
tests/__pycache__/todos.cpython-311.pyc
tests/conditions.py
tests/todos.py

index cba606d1a71dd19a0e410677f0ee8fc528d5bdbf..a6e9c97c6bdc05acf654ecda21317df1fbe04e85 100644 (file)
@@ -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)
 
 
index 8dd384349bd1419a16b19971ee4d9aa81deff796..0e07bf7aeb8837fc28031d2d924d65d17b0256bd 100644 (file)
@@ -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)
index 982ddfe3b96915d4ffc8bd8d4bce264671c09069..e4d5f6e9a42fe4726c5dff35e9488e13b61eea13 100644 (file)
@@ -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_)
index f5160f64d2504a3cf40733668623e1475eaa0cd8..adac957f5a1090267fb7807875eabcec2c8c37d3 100644 (file)
@@ -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'):
index c4ccfa8fd3926491bf1a52fc3c2c36d6d271cd63..1778e4f73b8eff992322b9b1e166f6987047e05d 100644 (file)
@@ -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)
index ecdd599f2cd538acda5494fc5035e72e076b6730..fcb8617d71a055f49549d657b0dcedd294dd9b4d 100644 (file)
@@ -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)
index ab39df098059e416c7627b5fcefecae2668d7c06..1810a318d3781c96626f40d679c53db42072403d 100644 (file)
@@ -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_)
index 2aaccb027d613c3d960b634e1bf7747ff0627a2b..6f84c41524e31d1aabd689c71dd37550e094ca0c 100755 (executable)
@@ -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}/ …"
index 252bf75240a44a184ae2e5476e2b5b779fdb807c..4dd00fd37a9985baa01f9ce133463f14ef0be350 100644 (file)
Binary files a/tests/__pycache__/conditions.cpython-311.pyc and b/tests/__pycache__/conditions.cpython-311.pyc differ
index 0369c07cb56c3d56c293f4d8e70348cbc33c60d8..6aa08151a43688a894e37888188609077b68efdc 100644 (file)
Binary files a/tests/__pycache__/todos.cpython-311.pyc and b/tests/__pycache__/todos.cpython-311.pyc differ
index 9c95206aab63cbdd045aaedf1f415ba6779d8afa..faaeb8722b8722c4fe88e9c10b5257292338be2e 100644 (file)
@@ -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')
index a5dafd1a8f2b17e58dbc4006f8d6ad54d7bc9b85..7fabfdb4bcb6c0761b40f7159583141e5f38ba09 100644 (file)
@@ -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):