diff --git a/src/palace/manager/api/circulation.py b/src/palace/manager/api/circulation.py index c26151993..ebc2f1bb0 100644 --- a/src/palace/manager/api/circulation.py +++ b/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,9 +198,6 @@ 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, @@ -209,7 +205,6 @@ def apply( 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() diff --git a/src/palace/manager/api/odl/api.py b/src/palace/manager/api/odl/api.py index 514c3bc8b..15893132a 100644 --- a/src/palace/manager/api/odl/api.py +++ b/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 = ( diff --git a/src/palace/manager/core/monitor.py b/src/palace/manager/core/monitor.py index 49e476169..775a22951 100644 --- a/src/palace/manager/core/monitor.py +++ b/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() diff --git a/src/palace/manager/sqlalchemy/model/licensing.py b/src/palace/manager/sqlalchemy/model/licensing.py index 006cc2c75..0097bc492 100644 --- a/src/palace/manager/sqlalchemy/model/licensing.py +++ b/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 diff --git a/tests/manager/api/controller/test_opds_feed.py b/tests/manager/api/controller/test_opds_feed.py index 8a52eca59..a0598c708 100644 --- a/tests/manager/api/controller/test_opds_feed.py +++ b/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. diff --git a/tests/manager/api/metadata/test_novelist.py b/tests/manager/api/metadata/test_novelist.py index 6bd67d7db..401c126b1 100644 --- a/tests/manager/api/metadata/test_novelist.py +++ b/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() diff --git a/tests/manager/api/odl/test_importer.py b/tests/manager/api/odl/test_importer.py index 9a13721a4..3b9aff633 100644 --- a/tests/manager/api/odl/test_importer.py +++ b/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 ( diff --git a/tests/manager/core/test_opds2_import.py b/tests/manager/core/test_opds2_import.py index 92dcc48bd..016da6edd 100644 --- a/tests/manager/core/test_opds2_import.py +++ b/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