From: Christian Heller <c.heller@plomlompom.de>
Date: Sat, 22 Jun 2024 02:43:08 +0000 (+0200)
Subject: Empty cache more often to avoid race conditions.
X-Git-Url: https://plomlompom.com/repos/%22https:/validator.w3.org/%7B%7Bdb.prefix%7D%7D/condition?a=commitdiff_plain;h=7d5347d7bbbbfaf0cbec191ef2dc1ff9cda41ac3;p=plomtask

Empty cache more often to avoid race conditions.
---

diff --git a/plomtask/http.py b/plomtask/http.py
index 417c5a6..0626b4c 100644
--- a/plomtask/http.py
+++ b/plomtask/http.py
@@ -177,6 +177,38 @@ class TaskHandler(BaseHTTPRequestHandler):
     @staticmethod
     def _request_wrapper(http_method: str, not_found_msg: str
                          ) -> Callable[..., Callable[[TaskHandler], None]]:
+        """Wrapper for do_GET… and do_POST… handlers, to init and clean up.
+
+        Among other things, conditionally cleans all caches, but only on POST
+        requests, as only those are expected to change the states of objects
+        that may be cached, and certainly only those are expected to write any
+        changes to the database. We want to call them as early though as
+        possible here, either exactly after the specific request handler
+        returns successfully, or right after any exception is triggered –
+        otherwise, race conditions become plausible.
+
+        Note that any POST attempt, even a failed one, may end in problematic
+        inconsistencies:
+
+        - if the POST handler experiences an Exception, changes to objects
+          won't get written to the DB, but the changed objects may remain in
+          the cache and affect other objects despite their possibly illegal
+          state
+
+        - even if an object was just saved to the DB, we cannot be sure its
+          current state is completely identical to what we'd get if loading it
+          fresh from the DB (e.g. currently Process.n_owners is only updated
+          when loaded anew via .from_table_row, nor is its state written to
+          the DB by .save; a questionable design choice, but proof that we
+          have no guarantee that objects' .save stores all their states we'd
+          prefer at their most up-to-date.
+        """
+
+        def clear_caches() -> None:
+            for cls in (Day, Todo, Condition, Process, ProcessStep):
+                assert hasattr(cls, 'empty_cache')
+                cls.empty_cache()
+
         def decorator(f: Callable[..., str | None]
                       ) -> Callable[[TaskHandler], None]:
             def wrapper(self: TaskHandler) -> None:
@@ -193,6 +225,8 @@ class TaskHandler(BaseHTTPRequestHandler):
                     if hasattr(self, handler_name):
                         handler = getattr(self, handler_name)
                         redir_target = f(self, handler)
+                        if 'POST' != http_method:
+                           clear_caches()
                         if redir_target:
                             self.send_response(302)
                             self.send_header('Location', redir_target)
@@ -201,9 +235,8 @@ class TaskHandler(BaseHTTPRequestHandler):
                         msg = f'{not_found_msg}: {self._site}'
                         raise NotFoundException(msg)
                 except HandledException as error:
-                    for cls in (Day, Todo, Condition, Process, ProcessStep):
-                        assert hasattr(cls, 'empty_cache')
-                        cls.empty_cache()
+                    if 'POST' != http_method:
+                       clear_caches()
                     ctx = {'msg': error}
                     self._send_page(ctx, 'msg', error.http_code)
                 finally: