Fixes some of the remaining issues

src/onegov/core/orm/__init__.py

@@ -1,7 +1,7 @@
 import psycopg2
 
 from markupsafe import escape, Markup
-from onegov.core.orm.cache import orm_cached
+from onegov.core.orm.cache import orm_cached, request_cached
 from onegov.core.orm.observer import observes
 from onegov.core.orm.session_manager import SessionManager, query_schemas
 from onegov.core.orm.sql import as_selectable, as_selectable_from_path
@@ -221,5 +221,6 @@ def share_session_manager(query: 'Query[Any]') -> None:
     'find_models',
     'observes',
     'orm_cached',
-    'query_schemas'
+    'query_schemas',
+    'request_cached'
 ]

src/onegov/core/orm/cache.py

@@ -32,6 +32,7 @@ def users(self):
 from typing import cast, overload, Any, Generic, TypeVar, TYPE_CHECKING
 if TYPE_CHECKING:
     from collections.abc import Callable, Iterator
+    from morepath.request import Request
     from onegov.core.framework import Framework
     from typing import Protocol
     from typing import Self
@@ -101,6 +102,7 @@ class OrmCacheApp:
         @property
         def cache(self) -> RedisCacheRegion: ...
         request_cache: dict[str, Any]
+        request_class: type[Request]
 
     def configure_orm_cache(self, **cfg: Any) -> None:
         self.is_orm_cache_setup = getattr(self, 'is_orm_cache_setup', False)
@@ -174,6 +176,11 @@ def orm_cache_descriptors(self) -> 'Iterator[OrmCacheDescriptor[Any]]':
             if isinstance(member, OrmCacheDescriptor):
                 yield member
 
+        # some descriptors are installed on the linked request instead
+        for member_name, member in inspect.getmembers(self.request_class):
+            if isinstance(member, OrmCacheDescriptor):
+                yield member
+
 
 class OrmCacheDescriptor(Generic[_T]):
     """ The descriptor implements the protocol for fetching the objects
@@ -279,6 +286,14 @@ def load(self, instance: 'OrmCacheApp | _HasApp') -> _T:
         else:
             app = instance.app
 
+        # before accessing any cached values we need to make sure that all
+        # pending changes are properly flushed -> this leads to some extra cpu
+        # cycles spent but eliminates the chance of accessing a stale entry
+        # after a change
+        session = app.session_manager.session()
+        if session.dirty:
+            session.flush()
+
         cache_key = self.cache_key(instance)
         self.used_cache_keys.add(cache_key)
 
@@ -369,6 +384,15 @@ def request_cached(
 
     @wraps(appmethod)
     def wrapper(self: '_FrameworkT') -> _T:
+        session = self.session()
+
+        # before accessing any cached values we need to make sure that all
+        # pending changes are properly flushed -> this leads to some extra cpu
+        # cycles spent but eliminates the chance of accessing a stale entry
+        # after a change
+        if session.dirty:
+            session.flush()
+
         if cache_key in self.request_cache:
             return maybe_merge(self.session(), self.request_cache[cache_key])
 

src/onegov/feriennet/models/activity.py

@@ -80,12 +80,13 @@ def ordered_tags(
         tags = [request.translate(_(tag)) for tag in self.tags]
 
         if durations is None:
+            period = request.app.active_period
             durations = sum(o.duration for o in (
                 request.session.query(Occasion)
                 .with_entities(Occasion.duration)
                 .distinct()
                 .filter_by(activity_id=self.id)
-                .filter_by(period=request.app.active_period)
+                .filter_by(period_id=period.id if period else None)
             ))
 
         if DAYS.has(durations, DAYS.half):

src/onegov/org/request.py

@@ -155,7 +155,7 @@ def auto_accept(self, ticket: 'Ticket') -> bool:
             return self.has_role(*roles)
         return ticket.handler_code in (self.app.org.ticket_auto_accepts or ())
 
-    @orm_cached(policy='on-table-change:pages')
+    @orm_cached(policy='on-table-change:pages', by_role=True)
     def pages_tree(self) -> tuple[PageMeta, ...]:
         """
         This is the entire pages tree preloaded into the individual
@@ -205,7 +205,7 @@ def generate_subtree(
         # the child pages
         return generate_subtree(None, None)
 
-    @orm_cached(policy='on-table-change:pages')
+    @orm_cached(policy='on-table-change:pages', by_role=True)
     def root_pages(self) -> tuple[PageMeta, ...]:
 
         def include(page: PageMeta) -> bool:
@@ -216,7 +216,7 @@ def include(page: PageMeta) -> bool:
 
         return tuple(p for p in self.pages_tree if include(p))
 
-    @orm_cached(policy='on-table-change:pages')
+    @orm_cached(policy='on-table-change:pages', by_role=True)
     def homepage_pages(self) -> dict[int, list[PageMeta]]:
 
         def visit_topics(

tests/onegov/agency/test_app.py

@@ -13,6 +13,7 @@ class DummyRequest():
     is_logged_in = False
     is_manager = False
     is_admin = False
+    root_pages = ()
     current_user = Bunch(id=Bunch(hex='abcd'))
     path = ''
     url = ''
@@ -30,7 +31,7 @@ def include(self, asset):
         pass
 
     def exclude_invisible(self, items):
-        return []
+        return
 
 
 def test_app_custom(agency_app):

tests/onegov/core/test_orm.py

@@ -22,7 +22,7 @@
 from onegov.core.orm.mixins import dict_markup_property
 from onegov.core.orm.mixins import ContentMixin
 from onegov.core.orm.mixins import TimestampMixin
-from onegov.core.orm import orm_cached
+from onegov.core.orm import orm_cached, request_cached
 from onegov.core.orm.types import HSTORE, JSON, UTCDateTime, UUID
 from onegov.core.orm.types import LowercaseText, MarkupText
 from onegov.core.security import Private
@@ -1458,7 +1458,10 @@ class App(Framework):
 
         @orm_cached(policy='on-table-change:documents')
         def documents(self):
-            return self.session().query(Document)
+            return self.session().query(Document).with_entities(
+                Document.id,
+                Document.title
+            )
 
         @orm_cached(policy='on-table-change:documents')
         def untitled_documents(self):
@@ -1478,12 +1481,11 @@ def first_document(self):
         @orm_cached(policy=lambda o: o.title == 'Secret')
         def secret_document(self):
             q = self.session().query(Document)
+            q = q.with_entities(Document.id)
             q = q.filter(Document.title == 'Secret')
 
-            return q.first()
-
-    # get dill to pickle the following inline class
-    global Document
+            doc = q.first()
+            return doc.id if doc else None
 
     class Document(Base):
         __tablename__ = 'documents'
@@ -1545,35 +1547,174 @@ class Document(Base):
     assert app.secret_document is None
     assert app.first_document.title == 'Public'
     assert app.untitled_documents == []
-    assert app.documents[0].body == 'Lorem Ipsum'
+    assert app.documents[0].title == 'Public'
 
     # if we add a secret document all caches change
     app.session().add(Document(id=2, title='Secret', body='Geheim'))
     transaction.commit()
 
     assert app.request_cache == {}
-    assert app.secret_document.body == "Geheim"
+    assert app.secret_document == 2
     assert app.first_document.title == 'Public'
     assert app.untitled_documents == []
     assert len(app.documents) == 2
 
+
+def test_orm_cache_flush(postgres_dsn, redis_url):
+
+    Base = declarative_base(cls=ModelBase)
+
+    class App(Framework):
+
+        @property
+        def foo(self):
+            return self.session().query(Document).one()
+
+        @orm_cached(policy='on-table-change:documents')
+        def bar(self):
+            return self.session().query(Document)\
+                .with_entities(Document.title).one()
+
+    class Document(Base):
+        __tablename__ = 'documents'
+
+        id = Column(Integer, primary_key=True)
+        title = Column(Text, nullable=True)
+
+    scan_morepath_modules(App)
+
+    app = App()
+    app.namespace = 'foo'
+    app.configure_application(
+        dsn=postgres_dsn,
+        base=Base,
+        redis_url=redis_url
+    )
+    # remove ORMBase
+    app.session_manager.bases.pop()
+    app.set_application_id('foo/bar')
+    app.clear_request_cache()
+
+    app.session().add(Document(id=1, title='Yo'))
+    transaction.commit()
+
+    # both instances get cached
+    assert app.foo.title == 'Yo'
+    assert app.bar.title == 'Yo'
+
+    # one instance changes without an explicit flush
+    app.foo.title = 'Sup'
+
+    # accessing the bar instance *first* fetches it from the cache which at
+    # this point would contain stale entries because we didn't flush explicitly
+    # but thanks to our autoflush mechanism this doesn't happen
+    assert app.session().dirty
+    assert app.bar.title == 'Sup'
+    assert app.foo.title == 'Sup'
+
+
+def test_request_cache(postgres_dsn, redis_url):
+
+    Base = declarative_base(cls=ModelBase)
+
+    class App(Framework):
+
+        @request_cached
+        def untitled_documents(self):
+            q = self.session().query(Document)
+            q = q.with_entities(Document.id, Document.title)
+            q = q.filter(Document.title == None)
+
+            return q.all()
+
+        @request_cached
+        def first_document(self):
+            q = self.session().query(Document)
+            q = q.with_entities(Document.id, Document.title)
+
+            return q.first()
+
+        @request_cached
+        def secret_document(self):
+            q = self.session().query(Document)
+            q = q.filter(Document.title == 'Secret')
+
+            return q.first()
+
+    # get dill to pickle the following inline class
+    global Document
+
+    class Document(Base):
+        __tablename__ = 'documents'
+
+        id = Column(Integer, primary_key=True)
+        title = Column(Text, nullable=True)
+        body = Column(Text, nullable=True)
+
+    # this is required for the transactions to actually work, usually this
+    # would be onegov.server's job
+    scan_morepath_modules(App)
+
+    app = App()
+    app.namespace = 'foo'
+    app.configure_application(
+        dsn=postgres_dsn,
+        base=Base,
+        redis_url=redis_url
+    )
+    # remove ORMBase
+    app.session_manager.bases.pop()
+    app.set_application_id('foo/bar')
+
+    # ensure that no results work
+    app.clear_request_cache()
+    assert app.untitled_documents == []
+    assert app.first_document is None
+    assert app.secret_document is None
+
+    assert app.request_cache == {
+        'test_request_cache.<locals>.App.first_document': None,
+        'test_request_cache.<locals>.App.secret_document': None,
+        'test_request_cache.<locals>.App.untitled_documents': []
+    }
+
+    app.session().add(Document(id=1, title='Public', body='Lorem Ipsum'))
+    app.session().add(Document(id=2, title='Secret', body='Geheim'))
+    transaction.commit()
+    # no influence on same request
+    assert app.request_cache == {
+        'test_request_cache.<locals>.App.first_document': None,
+        'test_request_cache.<locals>.App.secret_document': None,
+        'test_request_cache.<locals>.App.untitled_documents': []
+    }
+    assert app.untitled_documents == []
+    assert app.first_document is None
+    assert app.secret_document is None
+    app.clear_request_cache()
+
+    assert app.request_cache == {}
+    assert app.secret_document.body == "Geheim"
+    assert app.first_document.title == 'Public'
+    assert app.untitled_documents == []
+
     # if we change something in a cached object it is reflected
+    # in the next request
     app.secret_document.title = None
     transaction.commit()
 
-    assert 'test_orm_cache.<locals>.App.secret_document' in app.request_cache
-    assert app.untitled_documents[0].title is None
-
     # the object in the request cache is now detached
     with pytest.raises(DetachedInstanceError):
-        key = 'test_orm_cache.<locals>.App.secret_document'
+        key = 'test_request_cache.<locals>.App.secret_document'
         assert app.request_cache[key].title
 
     # which we transparently undo
     assert app.secret_document.title is None
 
+    app.clear_request_cache()
+    assert app.untitled_documents[0].title is None
 
-def test_orm_cache_flush(postgres_dsn, redis_url):
+
+def test_request_cache_flush(postgres_dsn, redis_url):
 
     Base = declarative_base(cls=ModelBase)
 
@@ -1622,7 +1763,7 @@ class Document(Base):
     app.foo.title = 'Sup'
 
     # accessing the bar instance *first* fetches it from the cache which at
-    # this point would contain stale entries because we didn't flush eplicitly,
+    # this point would contain stale entries because we didn't flush explicitly
     # but thanks to our autoflush mechanism this doesn't happen
     assert app.session().dirty
     assert app.bar.title == 'Sup'