Skip to content

Commit

Permalink
Remove post processors
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonathangreen
jonathangreen committed Oct 17, 2023
1 parent f294bad commit 9a55533
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 145 deletions.
155 changes: 13 additions & 142 deletions api/circulation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -742,40 +742,6 @@ def patron_activity(
...


class CirculationFulfillmentPostProcessor(ABC):
"""Generic interface for a circulation fulfillment post-processor,
i.e., a class adding additional logic to the fulfillment process AFTER the circulation item has been fulfilled.
It takes a FulfillmentInfo object and transforms it according to its internal logic.
"""

@abstractmethod
def __init__(self, collection: Collection) -> None:
...

@abstractmethod
def fulfill(
self,
patron: Patron,
pin: str,
licensepool: LicensePool,
delivery_mechanism: Optional[LicensePoolDeliveryMechanism],
fulfillment: FulfillmentInfo,
) -> FulfillmentInfo:
"""Post-process an existing FulfillmentInfo object.
:param patron: Library's patron
:param pin: The patron's alleged password
:param licensepool: Circulation item's license pool
:param delivery_mechanism: Object containing a delivery mechanism selected by the patron in the UI
(e.g., PDF, EPUB, etc.)
:param fulfillment: Existing FulfillmentInfo describing the circulation item
ready to be downloaded by the patron
:return: Processed FulfillmentInfo object
"""
...


class CirculationAPI:
"""Implement basic circulation logic and abstract away the details
between different circulation APIs behind generic operations like
Expand All @@ -790,49 +756,36 @@ def __init__(
registry: Optional[
IntegrationRegistry[BaseCirculationAPI[BaseSettings, BaseSettings]]
] = None,
fulfillment_post_processors_map: Optional[
Dict[int, Type[CirculationFulfillmentPostProcessor]]
] = None,
):
"""Constructor.
:param db: A database session (probably a scoped session, which is
why we can't derive it from `library`).
:param db: A database session (probably a scoped session, which is
why we can't derive it from `library`).
:param library: A Library object representing the library
whose circulation we're concerned with.
:param library: A Library object representing the library
whose circulation we're concerned with.
:param analytics: An Analytics object for tracking
circulation events.
:param analytics: An Analytics object for tracking
circulation events.
:param registry: An IntegrationRegistry mapping Collection protocols to
API classes that should be instantiated to deal with these
protocols. The default registry will work fine unless you're a
unit test.
:param registry: An IntegrationRegistry mapping Collection protocols to
API classes that should be instantiated to deal with these
protocols. The default registry will work fine unless you're a
unit test.
Since instantiating these API classes may result in API
calls, we only instantiate one CirculationAPI per library,
and keep them around as long as possible.
:param fulfillment_post_processors_map: A dictionary mapping Collection protocols
to fulfillment post-processors.
Since instantiating these API classes may result in API
calls, we only instantiate one CirculationAPI per library,
and keep them around as long as possible.
"""
self._db = db
self.library_id = library.id
self.analytics = analytics
self.initialization_exceptions = dict()
self.registry = registry or LicenseProvidersRegistry()
fulfillment_post_processors_mapping = (
fulfillment_post_processors_map
or self.default_fulfillment_post_processors_map
)

# Each of the Library's relevant Collections is going to be
# associated with an API object.
self.api_for_collection = {}
self._fulfillment_post_processors_map: Dict[
int, CirculationFulfillmentPostProcessor
] = {}

# When we get our view of a patron's loans and holds, we need
# to include loans whose license pools are in one of the
Expand All @@ -858,59 +811,18 @@ def __init__(
if isinstance(api, PatronActivityCirculationAPI):
self.collection_ids_for_sync.append(collection.id)

if (
collection.protocol in fulfillment_post_processors_mapping
and collection.id
):
fulfillment_post_processor = fulfillment_post_processors_mapping[
collection.protocol
](collection)
self._fulfillment_post_processors_map[
collection.id
] = fulfillment_post_processor

@property
def library(self) -> Optional[Library]:
if self.library_id is None:
return None
return Library.by_id(self._db, self.library_id)

@property
def default_fulfillment_post_processors_map(
self,
) -> Dict[str, Type[CirculationFulfillmentPostProcessor]]:
"""Return a default mapping of protocols to fulfillment post-processors.
:return: Mapping of protocols to fulfillment post-processors.
"""
from api.opds2 import TokenAuthenticationFulfillmentProcessor
from api.saml.wayfless import SAMLWAYFlessAcquisitionLinkProcessor
from core.opds2_import import OPDS2Importer
from core.opds_import import OPDSImporter

return {
OPDSImporter.NAME: SAMLWAYFlessAcquisitionLinkProcessor,
OPDS2Importer.NAME: TokenAuthenticationFulfillmentProcessor,
}

def api_for_license_pool(
self, licensepool: LicensePool
) -> Optional[BaseCirculationAPI[BaseSettings, BaseSettings]]:
"""Find the API to use for the given license pool."""
return self.api_for_collection.get(licensepool.collection.id)

def fulfillment_post_processor_for_license_pool(
self, licensepool: LicensePool
) -> Optional[CirculationFulfillmentPostProcessor]:
"""Return a fulfillment post-processor to use for the given license pool.
:param licensepool: License pool for which we need to get a fulfillment post-processor
:return: Fulfillment post-processor to use for the given license pool
"""
if not licensepool.collection.id:
return None
return self._fulfillment_post_processors_map.get(licensepool.collection.id)

def can_revoke_hold(self, licensepool: LicensePool, hold: Hold) -> bool:
"""Some circulation providers allow you to cancel a hold
when the book is reserved to you. Others only allow you to cancel
Expand Down Expand Up @@ -991,43 +903,6 @@ def _collect_checkout_event(self, patron: Patron, licensepool: LicensePool) -> N
patron, licensepool, CirculationEvent.CM_CHECKOUT, include_neighborhood=True
)

def _post_process_fulfillment(
self,
patron: Patron,
pin: str,
licensepool: LicensePool,
delivery_mechanism: Optional[LicensePoolDeliveryMechanism],
fulfillment: FulfillmentInfo,
) -> FulfillmentInfo:
"""Post-process an existing FulfillmentInfo object.
:param patron: Library's patron
:param pin: The patron's alleged password
:param licensepool: Circulation item's license pool
:param delivery_mechanism: Object containing a delivery mechanism selected by the patron in the UI
(e.g., PDF, EPUB, etc.)
:param fulfillment: Existing FulfillmentInfo describing the circulation item
ready to be downloaded by the patron
:return: Processed FulfillmentInfo object
"""
processed_fulfillment = fulfillment
fulfillment_post_processor = self.fulfillment_post_processor_for_license_pool(
licensepool
)

self.log.debug(f"Fulfillment post-processor: {fulfillment_post_processor}")

if fulfillment_post_processor:
processed_fulfillment = fulfillment_post_processor.fulfill(
patron, pin, licensepool, delivery_mechanism, fulfillment
)

self.log.debug(
f"Fulfillment {fulfillment} has been processed into {processed_fulfillment}"
)

return processed_fulfillment

def borrow(
self,
patron: Patron,
Expand Down Expand Up @@ -1471,10 +1346,6 @@ def fulfill(
if not fulfillment or not (fulfillment.content_link or fulfillment.content):
raise NoAcceptableFormat()

fulfillment = self._post_process_fulfillment(
patron, pin, licensepool, delivery_mechanism, fulfillment
)

# Send out an analytics event to record the fact that
# a fulfillment was initiated through the circulation
# manager.
Expand Down
2 changes: 1 addition & 1 deletion api/opds2.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from uritemplate import URITemplate

from api.circulation import CirculationFulfillmentPostProcessor, FulfillmentInfo
from api.circulation import FulfillmentInfo
from api.circulation_exceptions import CannotFulfill
from core.model import ConfigurationSetting, DataSource, ExternalIntegration
from core.model.licensing import LicensePoolDeliveryMechanism
Expand Down
2 changes: 1 addition & 1 deletion api/saml/wayfless.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import sqlalchemy
from sqlalchemy.orm import Session

from api.circulation import CirculationFulfillmentPostProcessor, FulfillmentInfo
from api.circulation import FulfillmentInfo
from api.saml.credential import SAMLCredentialManager
from core.exceptions import BaseError
from core.model import Collection, get_one
Expand Down
4 changes: 3 additions & 1 deletion tests/api/test_circulationapi.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Test the CirculationAPI."""
import datetime
from datetime import timedelta
from typing import cast
from unittest.mock import MagicMock

import flask
Expand All @@ -18,6 +19,7 @@
LoanInfo,
)
from api.circulation_exceptions import *
from core.analytics import Analytics
from core.config import CannotLoadConfiguration
from core.integration.goals import Goals
from core.integration.registry import IntegrationRegistry
Expand Down Expand Up @@ -1086,7 +1088,7 @@ def collect_event(self, library, licensepool, name, neighborhood):
lp1 = circulation_api.db.licensepool(edition=None)
lp2 = circulation_api.db.licensepool(edition=None)

api = CirculationAPI(circulation_api.db.session, l1, analytics) # type: ignore[arg-type]
api = CirculationAPI(circulation_api.db.session, l1, cast(Analytics, analytics))

def assert_event(inp, outp):
# Assert that passing `inp` into the mock _collect_event
Expand Down

0 comments on commit 9a55533

Please sign in to comment.