[PP-1690] Fix the sweeper monitor commit failures. (#2040)

src/palace/manager/api/circulation.py

@@ -164,14 +164,13 @@ def __init__(
         self.resource = resource
 
     def apply(
-        self, loan: Loan, autocommit: bool = True
+        self,
+        loan: Loan,
     ) -> LicensePoolDeliveryMechanism | None:
         """Set an appropriate LicensePoolDeliveryMechanism on the given
         `Loan`, creating a DeliveryMechanism if necessary.
 
         :param loan: A Loan object.
-        :param autocommit: Set this to false if you are in the middle
-            of a nested transaction.
         :return: A LicensePoolDeliveryMechanism if one could be set on the
             given Loan; None otherwise.
         """
@@ -199,17 +198,13 @@ def apply(
         # Look up the LicensePoolDeliveryMechanism for the way the
         # server says this book is available, creating the object if
         # necessary.
-        #
-        # We set autocommit=False because we're probably in the middle
-        # of a nested transaction.
         lpdm = LicensePoolDeliveryMechanism.set(
             pool.data_source,
             pool.identifier,
             self.content_type,
             self.drm_scheme,
             self.rights_uri,
             self.resource,
-            autocommit=autocommit,
         )
         loan.fulfillment = lpdm
         return lpdm
@@ -763,7 +758,7 @@ def sync_loans(
                 # this is the first we've heard of this loan,
                 # it may have been created in another app or through
                 # a library-website integration.
-                loan.locked_to.apply(local_loan, autocommit=False)
+                loan.locked_to.apply(local_loan)
 
         loans_to_delete = [
             local_loans[i] for i in local_loans.keys() - remote_loans.keys()

src/palace/manager/api/odl/api.py

@@ -659,7 +659,6 @@ def _update_hold_end_date(
         self, holdinfo: HoldInfo, pool: LicensePool, library: Library
     ) -> None:
         _db = Session.object_session(pool)
-
         # First make sure the hold position is up-to-date, since we'll
         # need it to calculate the end date.
         original_position = holdinfo.hold_position
@@ -745,6 +744,8 @@ def _update_hold_end_date(
                 days=default_reservation_period
             )
 
+        _db.expire_all()
+
     def _update_hold_position(self, holdinfo: HoldInfo, pool: LicensePool) -> None:
         _db = Session.object_session(pool)
         loans_count = (

src/palace/manager/core/monitor.py

@@ -482,7 +482,6 @@ def run_once(self, *ignore):
         timestamp.start = run_started_at
         total_processed = 0
         while True:
-            old_offset = offset
             batch_started_at = utc_now()
             new_offset, batch_size = self.process_batch(offset)
             total_processed += batch_size
@@ -530,17 +529,14 @@ def process_batch(self, offset):
             items = self.fetch_batch(offset).all()
             if items:
                 self.process_items(items)
-
                 # We've completed a batch. Return the ID of the last item
                 # in the batch so we don't do this work again.
-
                 result = (items[-1].id, len(items))
             else:
                 # There are no more items in this database table, so we
                 # are done with the sweep. Reset the counter.
                 result = (0, 0)
 
-            tx.commit()
             return result
         except Exception as e:
             tx.rollback()

src/palace/manager/sqlalchemy/model/licensing.py

@@ -1482,7 +1482,6 @@ def set(
         drm_scheme,
         rights_uri,
         resource=None,
-        autocommit=True,
     ) -> LicensePoolDeliveryMechanism:
         """Register the fact that a distributor makes a title available in a
         certain format.
@@ -1496,13 +1495,6 @@ def set(
             title.
         :param resource: A Resource representing the book itself in
             a freely redistributable form.
-        :param autocommit: Commit the database session immediately if
-            anything changes in the database. If you're already inside
-            a nested transaction, pass in False here to avoid
-            committing prematurely, but understand that if a
-            LicensePool's open-access status changes as a result of
-            calling this method, the change may not be properly
-            reflected in LicensePool.open_access.
         """
         _db = Session.object_session(data_source)
         delivery_mechanism, ignore = DeliveryMechanism.lookup(
@@ -1524,18 +1516,14 @@ def set(
             dirty = True
 
         if dirty:
-            # TODO: We need to explicitly commit here so that
-            # LicensePool.delivery_mechanisms gets updated. It would be
-            # better if we didn't have to do this, but I haven't been able
-            # to get LicensePool.delivery_mechanisms to notice that it's
-            # out of date.
-            if autocommit:
-                _db.commit()
-
-            # Creating or modifying a LPDM might change the open-access status
-            # of all LicensePools for that DataSource/Identifier.
             for pool in lpdm.license_pools:
+                # expire pool required in order ensure delivery mechanisms are updated.
+                _db.expire(pool)
+
+                # Creating or modifying a LPDM might change the open-access status
+                # of all LicensePools for that DataSource/Identifier.
                 pool.set_open_access_status()
+
         return lpdm
 
     @property

tests/manager/api/controller/test_opds_feed.py

@@ -419,7 +419,9 @@ def test_navigation(self, circulation_fixture: CirculationControllerFixture):
 
         library = circulation_fixture.db.default_library()
         lane = circulation_fixture.manager.top_level_lanes[library.id]
-        lane = circulation_fixture.db.session.merge(lane)
+        session = circulation_fixture.db.session
+        lane = session.merge(lane)
+        session.expire_all()
 
         # Mock NavigationFeed.navigation so we can see the arguments going
         # into it.

tests/manager/api/metadata/test_novelist.py

@@ -2,6 +2,7 @@
 import json
 from unittest.mock import MagicMock, create_autospec
 
+import dateutil.parser
 import pytest
 from pytest import MonkeyPatch
 
@@ -465,7 +466,8 @@ def test_get_items_from_query(self, novelist_fixture: NoveListFixture):
 
         # Set up a book for this library.
         edition = novelist_fixture.db.edition(
-            identifier_type=Identifier.ISBN, publication_date="2012-01-01"
+            identifier_type=Identifier.ISBN,
+            publication_date=dateutil.parser.parse("2012-01-01"),
         )
         pool = novelist_fixture.db.licensepool(
             edition, collection=novelist_fixture.db.default_collection()

tests/manager/api/odl/test_importer.py

@@ -10,6 +10,7 @@
 
 import dateutil
 import pytest
+from dateutil.tz import tzutc
 from freezegun import freeze_time
 from requests import Response
 
@@ -123,7 +124,10 @@ def test_import(
         assert "Feedbooks" == moby_dick_edition.data_source.name
 
         assert "Test Publisher" == moby_dick_edition.publisher
-        assert datetime.date(2015, 9, 29) == moby_dick_edition.published
+        assert (
+            datetime.datetime(2015, 9, 29, tzinfo=tzutc())
+            == moby_dick_edition.published
+        )
 
         assert "http://example.org/cover.jpg" == moby_dick_edition.cover_full_url
         assert (

tests/manager/core/test_opds2_import.py

@@ -5,6 +5,7 @@
 from unittest.mock import MagicMock, patch
 
 import pytest
+from dateutil.tz import tzutc
 from requests import Response
 from webpub_manifest_parser.core.ast import Contributor as WebpubContributor
 from webpub_manifest_parser.opds2 import OPDS2FeedParserFactory
@@ -198,7 +199,10 @@ def test_opds2_importer_correctly_imports_valid_opds2_feed(
         assert data.data_source == moby_dick_edition.data_source
 
         assert "Test Publisher" == moby_dick_edition.publisher
-        assert datetime.date(2015, 9, 29) == moby_dick_edition.published
+        assert (
+            datetime.datetime(2015, 9, 29, tzinfo=tzutc())
+            == moby_dick_edition.published
+        )
 
         assert "http://example.org/cover.jpg" == moby_dick_edition.cover_full_url
         assert (
@@ -255,7 +259,10 @@ def test_opds2_importer_correctly_imports_valid_opds2_feed(
         assert data.data_source == huckleberry_finn_edition.data_source
 
         assert "Test Publisher" == huckleberry_finn_edition.publisher
-        assert datetime.date(2014, 9, 28) == huckleberry_finn_edition.published
+        assert (
+            datetime.datetime(2014, 9, 28, tzinfo=tzutc())
+            == huckleberry_finn_edition.published
+        )
 
         assert "http://example.org/cover.jpg" == moby_dick_edition.cover_full_url