Patron activity task

src/palace/manager/api/circulation_manager.py

@@ -40,6 +40,9 @@
 )
 from palace.manager.service.analytics.analytics import Analytics
 from palace.manager.service.container import Services
+from palace.manager.service.integration_registry.license_providers import (
+    LicenseProvidersRegistry,
+)
 from palace.manager.service.logging.configuration import LogLevel
 from palace.manager.sqlalchemy.model.collection import Collection
 from palace.manager.sqlalchemy.model.discovery_service_registration import (
@@ -259,10 +262,12 @@ def load_settings(self):
             message_prefix="load_settings - create collection apis",
         ):
             collection_apis = {}
-            registry = self.services.integration_registry.license_providers()
+            registry: LicenseProvidersRegistry = (
+                self.services.integration_registry.license_providers()
+            )
             for collection in collections:
                 try:
-                    api = registry[collection.protocol](self._db, collection)
+                    api = registry.from_collection(self._db, collection)
                     collection_apis[collection.id] = api
                 except CannotLoadConfiguration as exception:
                     self.log.exception(

src/palace/manager/api/controller/circulation_manager.py

@@ -1,12 +1,8 @@
 from __future__ import annotations
 
-from datetime import datetime
-from email.utils import parsedate_to_datetime
 from typing import TypeVar
 
 import flask
-import pytz
-from flask import Response
 from flask_babel import lazy_gettext as _
 from sqlalchemy import select
 from sqlalchemy.orm import Session, eagerload
@@ -23,6 +19,7 @@
 )
 from palace.manager.core.problem_details import INVALID_INPUT
 from palace.manager.search.external_search import ExternalSearchIndex
+from palace.manager.service.redis.redis import Redis
 from palace.manager.sqlalchemy.model.collection import Collection
 from palace.manager.sqlalchemy.model.identifier import Identifier
 from palace.manager.sqlalchemy.model.integration import (
@@ -100,46 +97,9 @@ def search_engine(self) -> ExternalSearchIndex | ProblemDetail:
             )
         return search_engine  # type: ignore[no-any-return]
 
-    def handle_conditional_request(
-        self, last_modified: datetime | None = None
-    ) -> Response | None:
-        """Handle a conditional HTTP request.
-
-        :param last_modified: A datetime representing the time this
-           resource was last modified.
-
-        :return: a Response, if the incoming request can be handled
-            conditionally. Otherwise, None.
-        """
-        if not last_modified:
-            return None
-
-        # If-Modified-Since values have resolution of one second. If
-        # last_modified has millisecond resolution, change its
-        # resolution to one second.
-        if last_modified.microsecond:
-            last_modified = last_modified.replace(microsecond=0)
-
-        if_modified_since = flask.request.headers.get("If-Modified-Since")
-        if not if_modified_since:
-            return None
-
-        try:
-            parsed_if_modified_since = parsedate_to_datetime(if_modified_since)
-        except ValueError:
-            return None
-        if not parsed_if_modified_since:
-            return None
-
-        # "[I]f the date is conforming to the RFCs it will represent a
-        # time in UTC but with no indication of the actual source
-        # timezone of the message the date comes from."
-        if parsed_if_modified_since.tzinfo is None:
-            parsed_if_modified_since = parsed_if_modified_since.replace(tzinfo=pytz.UTC)
-
-        if parsed_if_modified_since >= last_modified:
-            return Response(status=304)
-        return None
+    @property
+    def redis_client(self) -> Redis:
+        return self.manager.services.redis.client()  # type: ignore[no-any-return]
 
     def load_lane(self, lane_identifier: int | None) -> Lane | WorkList | ProblemDetail:
         """Turn user input into a Lane object."""

src/palace/manager/api/controller/loan.py

@@ -24,8 +24,10 @@
     NO_ACTIVE_LOAN_OR_HOLD,
     NO_LICENSES,
 )
+from palace.manager.celery.tasks.patron_activity import sync_patron_activity
 from palace.manager.core.problem_details import INTERNAL_SERVER_ERROR
 from palace.manager.feed.acquisition import OPDSAcquisitionFeed
+from palace.manager.service.redis.models.patron_activity import PatronActivity
 from palace.manager.sqlalchemy.model.datasource import DataSource
 from palace.manager.sqlalchemy.model.library import Library
 from palace.manager.sqlalchemy.model.licensing import (
@@ -58,32 +60,17 @@ def sync(self) -> Response:
             self.log.exception(f"Could not parse refresh query parameter.")
             refresh = True
 
-        # Save some time if we don't believe the patron's loans or holds have
-        # changed since the last time the client requested this feed.
-        response = self.handle_conditional_request(patron.last_loan_activity_sync)
-        if isinstance(response, Response):
-            return response
-
-        # TODO: SimplyE used to make a HEAD request to the bookshelf feed
-        # as a quick way of checking authentication. Does this still happen?
-        # It shouldn't -- the patron profile feed should be used instead.
-        # If it's not used, we can take this out.
-        if flask.request.method == "HEAD":
-            return Response()
-
-        # First synchronize our local list of loans and holds with all
-        # third-party loan providers.
+        # Queue up tasks to sync the patron's activity with any third-party providers,
+        # that need to be synced. We don't wait for the task to complete, so we can return
+        # the feed immediately. If our knowledge of the loans is out of date, the patron will
+        # see the updated information when they refresh the page.
         if patron.authorization_identifier and refresh:
             header = self.authorization_header()
             credential = self.manager.auth.get_credential_from_header(header)
-            try:
-                self.circulation.sync_bookshelf(patron, credential)
-            except Exception as e:
-                # If anything goes wrong, omit the sync step and just
-                # display the current active loans, as we understand them.
-                self.manager.log.error(
-                    "ERROR DURING SYNC for %s: %r", patron.id, e, exc_info=e
-                )
+            for collection in PatronActivity.collections_ready_for_sync(
+                self.redis_client, patron
+            ):
+                sync_patron_activity.apply_async((collection.id, patron.id, credential))
 
         # Then make the feed.
         feed = OPDSAcquisitionFeed.active_loans_for(self.circulation, patron)
@@ -93,9 +80,6 @@ def sync(self) -> Response:
             mime_types=flask.request.accept_mimetypes,
         )
 
-        last_modified = patron.last_loan_activity_sync
-        if last_modified:
-            response.last_modified = last_modified
         return response
 
     def borrow(
@@ -141,11 +125,22 @@ def borrow(
                 return loan_or_hold_or_pd
             loan_or_hold = loan_or_hold_or_pd
 
-        # At this point we have either a loan or a hold. If a loan, serve
-        # a feed that tells the patron how to fulfill the loan. If a hold,
-        # serve a feed that talks about the hold.
-        # We also need to drill in the Accept header, so that it eventually
-        # gets sent to core.feed.opds.BaseOPDSFeed.entry_as_response
+        # At this point we have either a loan or a hold.
+
+        # If it is a new loan or hold, queue up a task to sync the patron's activity with the remote.
+        # This way we are sure we have the most up-to-date information.
+        if is_new and self.circulation.supports_patron_activity(
+            loan_or_hold.license_pool
+        ):
+            sync_patron_activity.apply_async(
+                (loan_or_hold.license_pool.collection.id, patron.id, credential),
+                {"force": True},
+                countdown=5,
+            )
+
+        # If we have a loan, serve a feed that tells the patron how to fulfill the loan. If a hold,
+        # serve a feed that talks about the hold. We also need to drill in the Accept header, so that
+        # it eventually gets sent to core.feed.opds.BaseOPDSFeed.entry_as_response
         response_kwargs = {
             "status": 201 if is_new else 200,
             "mime_types": flask.request.accept_mimetypes,
@@ -511,6 +506,16 @@ def revoke(self, license_pool_id: int) -> OPDSEntryResponse | ProblemDetail:
             except (CirculationException, RemoteInitiatedServerError) as e:
                 return e.problem_detail
 
+        # At this point we have successfully revoked the loan or hold.
+        # If the api supports it, queue up a task to sync the patron's activity with the remote.
+        # That way we are sure we have the most up-to-date information.
+        if self.circulation.supports_patron_activity(pool):
+            sync_patron_activity.apply_async(
+                (pool.collection.id, patron.id, credential),
+                {"force": True},
+                countdown=5,
+            )
+
         work = pool.work
         annotator = self.manager.annotator(None)
         single_entry_feed = OPDSAcquisitionFeed.single_entry(work, annotator)

src/palace/manager/api/opds_for_distributors.py

@@ -9,11 +9,7 @@
 from flask_babel import lazy_gettext as _
 from sqlalchemy.orm import Session
 
-from palace.manager.api.circulation import (
-    FulfillmentInfo,
-    LoanInfo,
-    PatronActivityCirculationAPI,
-)
+from palace.manager.api.circulation import BaseCirculationAPI, FulfillmentInfo, LoanInfo
 from palace.manager.api.circulation_exceptions import (
     CannotFulfill,
     DeliveryMechanismError,
@@ -80,9 +76,7 @@ class OPDSForDistributorsLibrarySettings(BaseSettings):
 
 
 class OPDSForDistributorsAPI(
-    PatronActivityCirculationAPI[
-        OPDSForDistributorsSettings, OPDSForDistributorsLibrarySettings
-    ],
+    BaseCirculationAPI[OPDSForDistributorsSettings, OPDSForDistributorsLibrarySettings],
     HasCollectionSelfTests,
 ):
     BEARER_TOKEN_CREDENTIAL_TYPE = "OPDS For Distributors Bearer Token"
@@ -356,29 +350,6 @@ def fulfill(
             content_expires=credential.expires,
         )
 
-    def patron_activity(
-        self, patron: Patron, pin: str | None
-    ) -> list[LoanInfo | HoldInfo]:
-        # Look up loans for this collection in the database.
-        _db = Session.object_session(patron)
-        loans = (
-            _db.query(Loan)
-            .join(Loan.license_pool)
-            .filter(LicensePool.collection_id == self.collection_id)
-            .filter(Loan.patron == patron)
-        )
-        return [
-            LoanInfo(
-                loan.license_pool.collection,
-                loan.license_pool.data_source.name,
-                loan.license_pool.identifier.type,
-                loan.license_pool.identifier.identifier,
-                loan.start,
-                loan.end,
-            )
-            for loan in loans
-        ]
-
     def release_hold(self, patron: Patron, pin: str, licensepool: LicensePool) -> None:
         # All the books for this integration are available as simultaneous
         # use, so there's no need to release a hold.