From ac5a85f6d0186d714415ce7e2b51597bf5dca248 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Thu, 25 Apr 2024 05:38:31 +0200 Subject: [PATCH] Re-write caching. --- plomtask/conditions.py | 6 ++-- plomtask/days.py | 4 +-- plomtask/db.py | 73 ++++++++++++++++++++++++++++-------------- plomtask/processes.py | 11 +++---- plomtask/todos.py | 4 +-- tests/todos.py | 5 ++- tests/utils.py | 9 ++++++ 7 files changed, 72 insertions(+), 40 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index cd147cb..4b01249 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -7,7 +7,7 @@ from plomtask.misc import VersionedAttribute from plomtask.exceptions import NotFoundException -class Condition(BaseModel): +class Condition(BaseModel[int]): """Non Process-dependency for ProcessSteps and Todos.""" table_name = 'conditions' to_save = ['is_active'] @@ -35,12 +35,13 @@ class Condition(BaseModel): def all(cls, db_conn: DatabaseConnection) -> list[Condition]: """Collect all Conditions and their VersionedAttributes.""" conditions = {} - for id_, condition in db_conn.cached_conditions.items(): + for id_, condition in cls.cache_.items(): conditions[id_] = condition already_recorded = conditions.keys() for id_ in db_conn.column_all('conditions', 'id'): if id_ not in already_recorded: condition = cls.by_id(db_conn, id_) + assert isinstance(condition.id_, int) conditions[condition.id_] = condition return list(conditions.values()) @@ -65,7 +66,6 @@ class Condition(BaseModel): self.title.save(db_conn) self.description.save(db_conn) assert isinstance(self.id_, int) - db_conn.cached_conditions[self.id_] = self class ConditionsRelations: diff --git a/plomtask/days.py b/plomtask/days.py index d838039..5fe984b 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -24,7 +24,7 @@ def todays_date() -> str: return datetime.now().strftime(DATE_FORMAT) -class Day(BaseModel): +class Day(BaseModel[str]): """Individual days defined by their dates.""" table_name = 'days' to_save = ['comment'] @@ -77,7 +77,7 @@ class Day(BaseModel): if not create: raise NotFoundException(f'Day not found for date: {date}') day = cls(date) - db_conn.cached_days[date] = day + day.cache() assert isinstance(day, Day) return day diff --git a/plomtask/db.py b/plomtask/db.py index dd2ee24..e8a542e 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -1,8 +1,9 @@ """Database management.""" +from __future__ import annotations from os.path import isfile from difflib import Differ from sqlite3 import connect as sql_connect, Cursor, Row -from typing import Any, Dict +from typing import Any, Self, TypeVar, Generic from plomtask.exceptions import HandledException PATH_DB_SCHEMA = 'scripts/init.sql' @@ -61,11 +62,6 @@ class DatabaseConnection: def __init__(self, db_file: DatabaseFile) -> None: self.file = db_file self.conn = sql_connect(self.file.path) - self.cached_todos: Dict[int, Any] = {} - self.cached_days: Dict[str, Any] = {} - self.cached_process_steps: Dict[int, Any] = {} - self.cached_processes: Dict[int, Any] = {} - self.cached_conditions: Dict[int, Any] = {} def commit(self) -> None: """Commit SQL transaction.""" @@ -116,38 +112,41 @@ class DatabaseConnection: return '(' + ','.join(['?'] * len(values)) + ')' -class BaseModel: +X = TypeVar('X', int, str) +T = TypeVar('T', bound='BaseModel[Any]') + + +class BaseModel(Generic[X]): """Template for most of the models we use/derive from the DB.""" table_name = '' to_save: list[str] = [] - id_: None | int | str - id_type: type[Any] = int + id_: None | X + cache_: dict[X, Self] = {} @classmethod - def from_table_row(cls, db_conn: DatabaseConnection, - row: Row | list[Any]) -> Any: + def from_table_row(cls: type[T], + # pylint: disable=unused-argument + db_conn: DatabaseConnection, + row: Row | list[Any]) -> T: """Make from DB row, write to DB cache.""" obj = cls(*row) - assert isinstance(obj.id_, cls.id_type) - cache = getattr(db_conn, f'cached_{cls.table_name}') - cache[obj.id_] = obj + obj.cache() return obj @classmethod def _by_id(cls, db_conn: DatabaseConnection, - id_: int | str) -> tuple[Any, bool]: + id_: X) -> tuple[Self | None, bool]: """Return instance found by ID, or None, and if from cache or not.""" from_cache = False - obj = None - cache = getattr(db_conn, f'cached_{cls.table_name}') - if id_ in cache.keys(): - obj = cache[id_] + obj = cls.get_cached(id_) + if obj: from_cache = True else: for row in db_conn.row_where(cls.table_name, 'id', id_): obj = cls.from_table_row(db_conn, row) - cache[id_] = obj + assert isinstance(obj, cls) + obj.cache() break return obj, from_cache @@ -156,7 +155,7 @@ class BaseModel: if (id_ is not None) and id_ < 1: msg = f'illegal {self.__class__.__name__} ID, must be >=1: {id_}' raise HandledException(msg) - self.id_ = id_ + self.id_ = id_ # type: ignore[assignment] def save_core(self, db_conn: DatabaseConnection, update_with_lastrowid: bool = True) -> None: @@ -168,6 +167,32 @@ class BaseModel: cursor = db_conn.exec(f'REPLACE INTO {table_name} VALUES {q_marks}', values) if update_with_lastrowid: - self.id_ = cursor.lastrowid - cache = getattr(db_conn, f'cached_{table_name}') - cache[self.id_] = self + self.id_ = cursor.lastrowid # type: ignore[assignment] + self.cache() + + @classmethod + def get_cached(cls: type[T], id_: X) -> T | None: + """Get object of id_ from class's cache, or None if not found.""" + # pylint: disable=consider-iterating-dictionary + if id_ in cls.cache_.keys(): + obj = cls.cache_[id_] + assert isinstance(obj, cls) + return obj + return None + + def cache(self) -> None: + """Update object in class's cache.""" + if self.id_ is None: + raise HandledException('Cannot cache object without ID.') + self.__class__.cache_[self.id_] = self + + def uncache(self) -> None: + """Remove self from cache.""" + if self.id_ is None: + raise HandledException('Cannot un-cache object without ID.') + del self.__class__.cache_[self.id_] + + @classmethod + def empty_cache(cls) -> None: + """Empty class's cache.""" + cls.cache_ = {} diff --git a/plomtask/processes.py b/plomtask/processes.py index 6249d48..590c5bc 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -18,7 +18,7 @@ class ProcessStepsNode: seen: bool -class Process(BaseModel, ConditionsRelations): +class Process(BaseModel[int], ConditionsRelations): """Template for, and metadata for, Todos, and their arrangements.""" table_name = 'processes' @@ -38,12 +38,13 @@ class Process(BaseModel, ConditionsRelations): def all(cls, db_conn: DatabaseConnection) -> list[Process]: """Collect all Processes and their connected VersionedAttributes.""" processes = {} - for id_, process in db_conn.cached_processes.items(): + for id_, process in cls.cache_.items(): processes[id_] = process already_recorded = processes.keys() for id_ in db_conn.column_all('processes', 'id'): if id_ not in already_recorded: process = cls.by_id(db_conn, id_) + assert isinstance(process.id_, int) processes[process.id_] = process return list(processes.values()) @@ -165,8 +166,7 @@ class Process(BaseModel, ConditionsRelations): """Set self.explicit_steps in bulk.""" assert isinstance(self.id_, int) for step in self.explicit_steps: - assert isinstance(step.id_, int) - del db_conn.cached_process_steps[step.id_] + step.uncache() self.explicit_steps = [] db_conn.delete_where('process_steps', 'owner', self.id_) for step_tuple in steps: @@ -189,10 +189,9 @@ class Process(BaseModel, ConditionsRelations): db_conn.delete_where('process_steps', 'owner', self.id_) for step in self.explicit_steps: step.save(db_conn) - db_conn.cached_processes[self.id_] = self -class ProcessStep(BaseModel): +class ProcessStep(BaseModel[int]): """Sub-unit of Processes.""" table_name = 'process_steps' to_save = ['owner_id', 'step_process_id', 'parent_step_id'] diff --git a/plomtask/todos.py b/plomtask/todos.py index 80dc97c..0b42d47 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -19,7 +19,7 @@ class TodoStepsNode: seen: bool -class Todo(BaseModel, ConditionsRelations): +class Todo(BaseModel[int], ConditionsRelations): """Individual actionable.""" # pylint: disable=too-many-instance-attributes @@ -71,6 +71,7 @@ class Todo(BaseModel, ConditionsRelations): todo.parents += [cls.by_id(db_conn, t_id)] for name in ('conditions', 'enables', 'disables'): table = f'todo_{name}' + assert isinstance(todo.id_, int) for cond_id in db_conn.column_where(table, 'condition', 'todo', todo.id_): target = getattr(todo, name) @@ -233,7 +234,6 @@ class Todo(BaseModel, ConditionsRelations): raise NotFoundException('Process of Todo without ID (not saved?)') self.save_core(db_conn) assert isinstance(self.id_, int) - db_conn.cached_todos[self.id_] = self 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_, diff --git a/tests/todos.py b/tests/todos.py index 7aed5f8..b47834c 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -31,9 +31,9 @@ class TestsWithDB(TestCaseWithDB): todo.save(self.db_conn) self.assertEqual(Todo.by_id(self.db_conn, 1), todo) with self.assertRaises(NotFoundException): - self.assertEqual(Todo.by_id(self.db_conn, 0), todo) + Todo.by_id(self.db_conn, 0) with self.assertRaises(NotFoundException): - self.assertEqual(Todo.by_id(self.db_conn, 2), todo) + Todo.by_id(self.db_conn, 2) def test_Todo_by_date(self) -> None: """Test findability of Todos by date.""" @@ -301,7 +301,6 @@ class TestsWithServer(TestCaseWithServer): def post_and_reload(form_data: dict[str, object], status: int = 302) -> Todo: self.check_post(form_data, '/todo?id=1', status, '/') - self.db_conn.cached_todos = {} return Todo.by_date(self.db_conn, '2024-01-01')[0] # test minimum form_data = {'title': '', 'description': '', 'effort': 1} diff --git a/tests/utils.py b/tests/utils.py index c80b34d..63b07e9 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -8,12 +8,21 @@ from os import remove as remove_file from typing import Mapping from plomtask.db import DatabaseFile, DatabaseConnection from plomtask.http import TaskHandler, TaskServer +from plomtask.processes import Process, ProcessStep +from plomtask.conditions import Condition +from plomtask.days import Day +from plomtask.todos import Todo class TestCaseWithDB(TestCase): """Module tests not requiring DB setup.""" def setUp(self) -> None: + Condition.empty_cache() + Day.empty_cache() + Process.empty_cache() + ProcessStep.empty_cache() + Todo.empty_cache() timestamp = datetime.now().timestamp() self.db_file = DatabaseFile(f'test_db:{timestamp}') self.db_file.remake() -- 2.30.2