Refactor save and remove methods of BaseObject subclasses.

diff --git a/plomtask/conditions.py b/plomtask/conditions.py
index cba606d1a71dd19a0e410677f0ee8fc528d5bdbf..a6e9c97c6bdc05acf654ecda21317df1fbe04e85 100644 (file)
--- 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']
     """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_)
 
     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
 
                 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.
 
     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')
                 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)
 
 
         super().remove(db_conn)
 
 

diff --git a/plomtask/days.py b/plomtask/days.py
index 8dd384349bd1419a16b19971ee4d9aa81deff796..0e07bf7aeb8837fc28031d2d924d65d17b0256bd 100644 (file)
--- 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)
         """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 982ddfe3b96915d4ffc8bd8d4bce264671c09069..e4d5f6e9a42fe4726c5dff35e9488e13b61eea13 100644 (file)
--- a/plomtask/db.py
+++ b/plomtask/db.py
@@ -75,7 +75,7 @@ class DatabaseConnection:
         """Close DB connection."""
         self.conn.close()
 
         """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)
                           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] = []
     """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]
 
     id_: None | BaseModelId
     cache_: dict[BaseModelId, Self]
 


@@ -245,11 +247,11 @@ class BaseModel(Generic[BaseModelId]):
                 items[item.id_] = item
         return list(items.values())
 
                 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
 
         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
 
         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()
         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:
 
     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')
         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_)
         self.uncache()
         db_conn.delete_where(self.table_name, 'id', self.id_)

diff --git a/plomtask/http.py b/plomtask/http.py
index f5160f64d2504a3cf40733668623e1475eaa0cd8..adac957f5a1090267fb7807875eabcec2c8c37d3 100644 (file)
--- 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'))
                                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'):
         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 c4ccfa8fd3926491bf1a52fc3c2c36d6d271cd63..1778e4f73b8eff992322b9b1e166f6987047e05d 100644 (file)
--- 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'
 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
 
 
     # 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."""
 
     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)
         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)
         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')
             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)
         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)
 
 
         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
 
         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)
     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 ecdd599f2cd538acda5494fc5035e72e076b6730..fcb8617d71a055f49549d657b0dcedd294dd9b4d 100644 (file)
--- 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']
     # 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_)
 
     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
         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)
 
         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."""
     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)
         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)
         super().remove(db_conn)

diff --git a/plomtask/versioned_attributes.py b/plomtask/versioned_attributes.py
index ab39df098059e416c7627b5fcefecae2668d7c06..1810a318d3781c96626f40d679c53db42072403d 100644 (file)
--- 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()])
         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 2aaccb027d613c3d960b634e1bf7747ff0627a2b..6f84c41524e31d1aabd689c71dd37550e094ca0c 100755 (executable)
--- a/scripts/pre-commit
+++ b/scripts/pre-commit
@@ -1,7 +1,6 @@
 #!/bin/sh
 set -e
 #!/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}/ …"
     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 (file)
Binary files a/tests/__pycache__/conditions.cpython-311.pyc and b/tests/__pycache__/conditions.cpython-311.pyc differ

diff --git a/tests/__pycache__/todos.cpython-311.pyc b/tests/__pycache__/todos.cpython-311.pyc
index 0369c07cb56c3d56c293f4d8e70348cbc33c60d8..6aa08151a43688a894e37888188609077b68efdc 100644 (file)
Binary files a/tests/__pycache__/todos.cpython-311.pyc and b/tests/__pycache__/todos.cpython-311.pyc differ

diff --git a/tests/conditions.py b/tests/conditions.py
index 9c95206aab63cbdd045aaedf1f415ba6779d8afa..faaeb8722b8722c4fe88e9c10b5257292338be2e 100644 (file)
--- a/tests/conditions.py
+++ b/tests/conditions.py
@@ -83,6 +83,7 @@ class TestsWithDB(TestCaseWithDB):
         self.check_remove()
         c = Condition(None)
         proc = Process(None)
         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')
         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 a5dafd1a8f2b17e58dbc4006f8d6ad54d7bc9b85..7fabfdb4bcb6c0761b40f7159583141e5f38ba09 100644 (file)
--- 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)
     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):
         with self.assertRaises(NotFoundException):

-            todo.save(self.db_conn)
+            todo = Todo(None, process_unsaved, False, self.date1)

         process_unsaved.save(self.db_conn)
         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):
         todo.save(self.db_conn)
         self.assertEqual(Todo.by_id(self.db_conn, 1), todo)
         with self.assertRaises(NotFoundException):