Ekir 463 delete expired loans (#175)

* EKIR-463 Delete expired loans daily

Delete all loans that expired yesterday and before that on a daily basis.

* EKIR-463 Delete an expired loan

* EKIR-463 Delete very old loans as well

* EKIR-463 Fix test

Author: natlibfi-kaisa

api/controller/odl_notification.py

@@ -4,7 +4,6 @@
 from flask import Response
 from flask_babel import lazy_gettext as _
 from pydantic import ValidationError
-from sqlalchemy.orm.exc import StaleDataError
 
 from api.controller.circulation_manager import CirculationManagerController
 from api.lcp.status import LoanStatus
@@ -19,7 +18,6 @@
 from core.model.credential import Credential
 from core.model.licensing import License
 from core.model.patron import Loan, Patron
-from core.util.datetime_helpers import utc_now
 from core.util.problem_detail import ProblemDetailException
 
 
@@ -96,14 +94,10 @@ def _process_notification(self, loan: Loan | None) -> Response:
 
             # We might be out of sync with the distributor. Mark the loan as expired and update the license pool.
             if not status_doc.active:
-                try:
-                    with self._db.begin_nested():
-                        loan.end = utc_now()
-                except StaleDataError:
-                    # This can happen if this callback happened while we were returning this
-                    # item. We can fetch the loan, but it's deleted by the time we go to do
-                    # the update. This is not a problem, as we were just marking the loan as
-                    # completed anyway so we just continue.
-                    ...
+                self.log.info(f"Loan status was {status_doc.status}, will delete loan")
+                api = self.manager.circulation_apis[library.id].api_for_license_pool(
+                    loan.license_pool
+                )
+                api.delete_expired_loan(loan)
 
         return Response(_("Success"), 200)

api/odl.py

@@ -833,6 +833,16 @@ def patron_activity(self, patron: Patron, pin: str) -> list[LoanInfo | HoldInfo]
     def update_availability(self, licensepool: LicensePool) -> None:
         licensepool.update_availability_from_licenses()
 
+    def delete_expired_loan(self, loan: Loan) -> None:
+        """
+        Delete a loan we know to be expired and update the license pool.
+        """
+        _db = Session.object_session(loan)
+        loan.license.checkin()
+        self.log.info(f"Deleting loan #{loan.id}")
+        _db.delete(loan)
+        self.update_licensepool_and_hold_queue(loan.license_pool)
+
 
 class ODLAPI(
     BaseODLAPI[ODLSettings, ODLLibrarySettings],

api/odl2.py

@@ -1,12 +1,14 @@
 from __future__ import annotations
 
+import datetime
 import logging
 from collections.abc import Callable
 from typing import TYPE_CHECKING, Any
 
 from flask_babel import lazy_gettext as _
 from pydantic import PositiveInt
 from sqlalchemy.orm import Session
+from sqlalchemy.sql.expression import and_, or_
 from webpub_manifest_parser.odl import ODLFeedParserFactory
 from webpub_manifest_parser.opds2.registry import OPDS2LinkRelationsRegistry
 
@@ -18,17 +20,18 @@
     ConfigurationFormItemType,
     FormField,
 )
-from core.metadata_layer import FormatData
-from core.model import Edition, RightsStatus
+from core.metadata_layer import FormatData, TimestampData
+from core.model import Edition, LicensePool, Loan, RightsStatus
 from core.model.configuration import ExternalIntegration
+from core.monitor import CollectionMonitor
 from core.opds2_import import (
     OPDS2Importer,
     OPDS2ImporterSettings,
     OPDS2ImportMonitor,
     RWPMManifestParser,
 )
 from core.util import first_or_default
-from core.util.datetime_helpers import to_utc
+from core.util.datetime_helpers import to_utc, utc_now
 
 if TYPE_CHECKING:
     from webpub_manifest_parser.core.ast import Metadata
@@ -316,3 +319,62 @@ def __init__(
         super().__init__(
             _db, collection, import_class, force_reimport=True, **import_class_kwargs
         )
+
+
+class ODL2LoanReaper(CollectionMonitor):
+    """Check for loans that have expired and delete them, and update
+    the holds queues for their pools."""
+
+    SERVICE_NAME = "ODL2 Loan Reaper"
+    PROTOCOL = ODL2API.label()
+
+    def __init__(
+        self,
+        _db: Session,
+        collection: Collection,
+        api: ODL2API | None = None,
+        **kwargs: Any,
+    ):
+        super().__init__(_db, collection, **kwargs)
+        self.api = api or ODL2API(_db, collection)
+
+    def run_once(self, progress: TimestampData) -> TimestampData:
+        # Find loans that have expired.
+        self.log.info("Loan Reaper Job started")
+        now = utc_now()
+        expired_loans = (
+            self._db.query(Loan)
+            .join(Loan.license_pool)
+            .filter(
+                and_(
+                    LicensePool.open_access == False,
+                    or_(
+                        Loan.end
+                        < now
+                        - datetime.timedelta(
+                            days=1
+                        ),  # Loans that ended before yesterday
+                        Loan.start < now - datetime.timedelta(days=90),
+                        Loan.end == None,
+                    ),  # Loans that started more than 90 days ago and have no end date
+                )
+            )
+        )
+
+        changed_pools = set()
+        total_deleted_loans = 0
+        for loan in expired_loans:
+            changed_pools.add(loan.license_pool)
+            loan.license.checkin()
+            self._db.delete(loan)
+            total_deleted_loans += 1
+
+        for pool in changed_pools:
+            self.api.update_licensepool_and_hold_queue(pool)
+
+        message = "Loans deleted: %d. License pools updated: %d" % (
+            total_deleted_loans,
+            len(changed_pools),
+        )
+        progress = TimestampData(achievements=message)
+        return progress

bin/odl2_loan_reaper

@@ -0,0 +1,12 @@
+#!/usr/bin/env python
+"""Check for loans that have expired and delete them."""
+import os
+import sys
+
+bin_dir = os.path.split(__file__)[0]
+package_dir = os.path.join(bin_dir, "..")
+sys.path.append(os.path.abspath(package_dir))
+from api.odl2 import ODL2LoanReaper
+from core.scripts import RunCollectionMonitorScript
+
+RunCollectionMonitorScript(ODL2LoanReaper).run()

docker/services/cron/cron.d/circulation

@@ -90,9 +90,10 @@ HOME=/var/www/circulation
 15 * * * * root core/bin/run odl_import_monitor >> /var/log/cron.log 2>&1
 0 */4 * * * root core/bin/run odl_hold_reaper >> /var/log/cron.log 2>&1
 
-# OPDS 2.x + ODL import
+# OPDS 2.x + ODL
 #
 0 3 * * * root core/bin/run odl2_import_monitor >> /var/log/cron.log 2>&1
+0 4 * * * root core/bin/run odl2_loan_reaper >> /var/log/cron.log 2>&1
 
 # SAML
 #

tests/api/controller/test_odl_notify.py

@@ -237,14 +237,11 @@ def test_notify_success(
             )
             assert odl_fixture.license.identifier is not None
             assert 200 == response.status_code
-
-        # Since we had a loan and it's not active, we're out of sync with the remote. We've set the loan to end now.
-        assert loan.end == utc_now()
-
-        # The pool's availability has been updated.
-        api = controller_fixture.manager.circulation_apis[
-            db.default_library().id
-        ].api_for_license_pool(loan.license_pool)
+            # The pool's availability has been updated.
+            api = controller_fixture.manager.circulation_apis[
+                db.default_library().id
+            ].api_for_license_pool(loan.license_pool)
+            assert [loan.license_pool] == api.availability_updated_for
 
     def test_notify_errors(
         self, controller_fixture: ControllerFixture, odl_fixture: ODLFixture

tests/api/mockapi/circulation.py

@@ -82,7 +82,8 @@ def release_hold(self, patron, pin, licensepool):
     def internal_format(self, delivery_mechanism):
         return delivery_mechanism
 
-    def update_loan(self, loan, status_doc):
+    # Only called TestODLNotificationController
+    def delete_expired_loan(self, loan):
         self.availability_updated_for.append(loan.license_pool)
 
     def queue_checkout(self, response):

tests/api/test_odl.py

@@ -1371,6 +1371,27 @@ def test_patron_activity_loan(
         assert 1 == pool2.licenses_available
         assert 0 == pool2.licenses_reserved
 
+    def test_delete_expired_loan(
+        self,
+        db: DatabaseTransactionFixture,
+        opds2_with_odl_api_fixture: OPDS2WithODLApiFixture,
+    ) -> None:
+        opds2_with_odl_api_fixture.setup_license(concurrency=1, available=0)
+        # This loan expired a few days ago
+        loan, _ = opds2_with_odl_api_fixture.license.loan_to(
+            opds2_with_odl_api_fixture.patron,
+            end=utc_now() - datetime.timedelta(days=3),
+        )
+        assert 1 == db.session.query(Loan).count()
+
+        opds2_with_odl_api_fixture.api.delete_expired_loan(loan)
+
+        # There's no more loans
+        assert 0 == db.session.query(Loan).count()
+        # The pool's and license's availability has increased
+        assert 1 == opds2_with_odl_api_fixture.pool.licenses_available
+        assert 1 == opds2_with_odl_api_fixture.license.checkouts_available
+
 
 class TestODLImporter:
     @freeze_time("2019-01-01T00:00:00+00:00")

tests/api/test_odl2.py

@@ -9,21 +9,25 @@
 )
 
 from api.circulation_exceptions import PatronHoldLimitReached, PatronLoanLimitReached
-from api.odl2 import ODL2Importer
+from api.odl2 import ODL2Importer, ODL2LoanReaper
 from core.coverage import CoverageFailure
 from core.model import (
+    Collection,
     Contribution,
     Contributor,
+    DataSource,
     DeliveryMechanism,
     Edition,
     EditionConstants,
     LicensePool,
     LicensePoolDeliveryMechanism,
+    Loan,
     MediaTypes,
     Work,
 )
 from core.model.constants import IdentifierConstants
 from core.model.resource import Hyperlink
+from core.util.datetime_helpers import utc_now
 from tests.fixtures.api_odl import (
     LicenseHelper,
     LicenseInfoHelper,
@@ -440,3 +444,53 @@ def test_hold_limit(
         with pytest.raises(PatronHoldLimitReached) as exc:
             odl2_api_fixture.place_hold(odl2_api_fixture.patron, pool)
         assert exc.value.limit == 1
+
+
+class TestODL2LoanReaper:
+    def test_run_once(
+        self, odl2_api_fixture: ODL2ApiFixture, db: DatabaseTransactionFixture
+    ):
+        data_source = DataSource.lookup(db.session, "Feedbooks", autocreate=True)
+        DatabaseTransactionFixture.set_settings(
+            odl2_api_fixture.collection.integration_configuration,
+            **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name},
+        )
+        reaper = ODL2LoanReaper(
+            db.session, odl2_api_fixture.collection, api=odl2_api_fixture.api
+        )
+
+        now = utc_now()
+        yesterday = now - datetime.timedelta(days=1)
+        tomorrow = now + datetime.timedelta(days=1)
+
+        # License with all its checkouts on loan to 4 patrons.
+        odl2_api_fixture.setup_license(concurrency=4, available=0)
+        expired_loan1, ignore = odl2_api_fixture.license.loan_to(
+            db.patron(), end=yesterday
+        )
+        expired_loan2, ignore = odl2_api_fixture.license.loan_to(
+            db.patron(), end=yesterday
+        )
+        current_loan, ignore = odl2_api_fixture.license.loan_to(
+            db.patron(), end=tomorrow
+        )
+        very_old_loan, ignore = odl2_api_fixture.license.loan_to(
+            db.patron(), start=now - datetime.timedelta(days=90), end=None
+        )
+        assert 4 == db.session.query(Loan).count()
+        assert 0 == odl2_api_fixture.pool.licenses_available
+
+        progress = reaper.run_once(reaper.timestamp().to_data())
+
+        # The expired loans have been deleted and the current loan remains.
+        assert 1 == db.session.query(Loan).count()
+
+        assert 3 == odl2_api_fixture.pool.licenses_available
+
+        # The TimestampData returned reflects what work was done.
+        assert "Loans deleted: 3. License pools updated: 1" == progress.achievements
+
+        # The TimestampData does not include any timing information --
+        # that will be applied by run().
+        assert None == progress.start
+        assert None == progress.finish