Skip to content

Commit

Permalink
Delegate Fulfillment instead of using FulfillmentInfo (PP-1641) (#2035)
Browse files Browse the repository at this point in the history 
Refactor FulfillmentInfo class which just contained information about a fufillment, but had multiple methods to override how fulfillment was done, to be a new class called Fulfillment which the loans controller delegates fulfillment to.

This lets each API control how fulfillment is done, and reduces the amount of if / then we need to have in the loan controller. We define several types of Fulfillment, corresponding to the logic that used to exist in the loans controller, and update the API classes to use them.

The goal of this PR is the have fulfillment for each API act the same way before and after, just refactoring to use the new structure.
  • Loading branch information
@jonathangreen
jonathangreen authored Sep 6, 2024
1 parent aac4204 commit 5fb1411
Show file tree
Hide file tree
Showing 26 changed files with 747 additions and 1,042 deletions.
160 changes: 87 additions & 73 deletions src/palace/manager/api/axis.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@

from palace.manager.api.admin.validator import Validator
from palace.manager.api.circulation import (
APIAwareFulfillmentInfo,
BaseCirculationAPI,
BaseCirculationApiSettings,
BaseCirculationLoanSettings,
CirculationInternalFormatsMixin,
FulfillmentInfo,
Fulfillment,
HoldInfo,
LoanInfo,
PatronActivityCirculationAPI,
UrlFulfillment,
)
from palace.manager.api.circulation_exceptions import (
AlreadyCheckedOut,
Expand Down Expand Up @@ -81,7 +81,6 @@
IdentifierSweepMonitor,
TimelineMonitor,
)
from palace.manager.core.problem_details import INTEGRATION_ERROR
from palace.manager.integration.settings import (
ConfigurationFormItem,
ConfigurationFormItemType,
Expand All @@ -105,9 +104,8 @@
from palace.manager.sqlalchemy.model.resource import Hyperlink, Representation
from palace.manager.util.datetime_helpers import datetime_utc, strptime_utc, utc_now
from palace.manager.util.flask_util import Response
from palace.manager.util.http import HTTP, RequestNetworkException
from palace.manager.util.http import HTTP, RemoteIntegrationException
from palace.manager.util.log import LoggerMixin
from palace.manager.util.problem_detail import ProblemDetail
from palace.manager.util.xmlparser import XMLProcessor


Expand Down Expand Up @@ -478,7 +476,7 @@ def fulfill(
pin: str,
licensepool: LicensePool,
delivery_mechanism: LicensePoolDeliveryMechanism,
) -> FulfillmentInfo:
) -> Fulfillment:
"""Fulfill a patron's request for a specific book."""
identifier = licensepool.identifier
# This should include only one 'activity'.
Expand All @@ -498,8 +496,8 @@ def fulfill(
continue
# We've found the remote loan corresponding to this
# license pool.
fulfillment = loan.fulfillment_info
if not fulfillment or not isinstance(fulfillment, FulfillmentInfo):
fulfillment = loan.fulfillment
if not fulfillment or not isinstance(fulfillment, Fulfillment):
raise CannotFulfill()
return fulfillment
# If we made it to this point, the patron does not have this
Expand Down Expand Up @@ -1614,26 +1612,14 @@ def process_one(
or ""
)

# Arguments common to FulfillmentInfo and
# Axis360FulfillmentInfo.
kwargs = dict(
data_source_name=DataSource.AXIS_360,
identifier_type=self.id_type,
identifier=axis_identifier,
)

fulfillment: FulfillmentInfo | None
fulfillment: Fulfillment | None
if download_url and self.internal_format != self.api.AXISNOW:
# The patron wants a direct link to the book, which we can deliver
# immediately, without making any more API requests.
fulfillment = Axis360AcsFulfillmentInfo(
collection=self.collection,
fulfillment = Axis360AcsFulfillment(
content_link=html.unescape(download_url),
content_type=DeliveryMechanism.ADOBE_DRM,
content=None,
content_expires=None,
verify=self.api.verify_certificate,
**kwargs,
)
elif transaction_id:
# We will eventually need to make a request to the
Expand All @@ -1648,9 +1634,13 @@ def process_one(
# need to make a second request to get the audiobook
# metadata.
#
# Axis360FulfillmentInfo can handle both cases.
fulfillment = Axis360FulfillmentInfo(
api=self.api, key=transaction_id, **kwargs
# Axis360Fulfillment can handle both cases.
fulfillment = Axis360Fulfillment(
data_source_name=DataSource.AXIS_360,
identifier_type=self.id_type,
identifier=axis_identifier,
api=self.api,
key=transaction_id,
)
else:
# We're out of luck -- we can't fulfill this loan.
Expand All @@ -1662,7 +1652,7 @@ def process_one(
identifier=axis_identifier,
start_date=start_date,
end_date=end_date,
fulfillment_info=fulfillment,
fulfillment=fulfillment,
)

elif reserved:
Expand Down Expand Up @@ -1914,35 +1904,70 @@ def __str__(self) -> str:
return json.dumps(data, sort_keys=True)


class Axis360FulfillmentInfo(APIAwareFulfillmentInfo, LoggerMixin):
"""An Axis 360-specific FulfillmentInfo implementation for audiobooks
class Axis360Fulfillment(Fulfillment, LoggerMixin):
"""An Axis 360-specific Fulfillment implementation for audiobooks
and books served through AxisNow.
We use these instead of normal FulfillmentInfo objects because
putting all this information into FulfillmentInfo would require
We use these instead of normal Fulfillment objects because
putting all this information into Fulfillment would require
one or two extra HTTP requests, and there's often no need to make
those requests.
"""

def do_fetch(self) -> None:
def __init__(
self,
api: Axis360API,
data_source_name: str,
identifier_type: str,
identifier: str,
key: str,
):
"""Constructor.
:param api: An Axis360API instance, in case the parsing of
a fulfillment document triggers additional API requests.
:param key: The transaction ID that will be used to fulfill
the request.
"""
self.data_source_name = data_source_name
self.identifier_type = identifier_type
self.identifier = identifier
self.api = api
self.key = key

self.content_type: str | None = None
self.content: str | None = None

def license_pool(self, _db: Session) -> LicensePool:
"""Find the LicensePool model object corresponding to this object."""
collection = self.api.collection
pool, is_new = LicensePool.for_foreign_id(
_db,
self.data_source_name,
self.identifier_type,
self.identifier,
collection=collection,
)
return pool

def do_fetch(self) -> tuple[str, str]:
_db = self.api._db
license_pool = self.license_pool(_db)
transaction_id = self.key
if not isinstance(self.api, Axis360API):
self.log.error(
f"Called with wrong API type {self.api.__class__.__name__} should be {Axis360API.__name__}"
)
raise ValueError("Axis360FulfillmentInfo can only be used with Axis360API")
response = self.api.get_fulfillment_info(transaction_id)
parser = Axis360FulfillmentInfoResponseParser(self.api)
manifest, expires = parser.parse(response.content, license_pool=license_pool)
self._content = str(manifest)
self._content_type = manifest.MEDIA_TYPE
self._content_expires = expires
return str(manifest), manifest.MEDIA_TYPE

def response(self) -> Response:
if self.content is None:
self.content, self.content_type = self.do_fetch()
return Response(response=self.content, content_type=self.content_type)

class Axis360AcsFulfillmentInfo(FulfillmentInfo, LoggerMixin):
"""This implements a Axis 360 specific FulfillmentInfo for ACS content

class Axis360AcsFulfillment(UrlFulfillment, LoggerMixin):
"""This implements a Axis 360 specific Fulfillment for ACS content
served through AxisNow. The AxisNow API gives us a link that we can use
to get the ACSM file that we serve to the mobile apps.
Expand All @@ -1967,27 +1992,22 @@ class Axis360AcsFulfillmentInfo(FulfillmentInfo, LoggerMixin):
and closed the ticket.
https://github.com/urllib3/urllib3/issues/1677
This FulfillmentInfo implementation uses the built in Python urllib
This Fulfillment implementation uses the built in Python urllib
implementation instead of requests (and urllib3) to make this request
to the Axis 360 API, sidestepping the problem, but taking a different
code path than most of our external HTTP requests.
"""

def __init__(self, verify: bool, **kwargs: Any) -> None:
super().__init__(**kwargs)
self.verify: bool = verify

def problem_detail_document(self, error_details: str) -> ProblemDetail:
service_name = urlparse(self.content_link).netloc
self.log.warning(error_details)
return INTEGRATION_ERROR.detailed(
_(RequestNetworkException.detail, service=service_name),
title=RequestNetworkException.title,
debug_message=error_details,
)
def __init__(
self,
content_link: str,
verify: bool,
content_type: str = DeliveryMechanism.ADOBE_DRM,
) -> None:
super().__init__(content_link, content_type)
self.verify = verify

@property
def as_response(self) -> Response | ProblemDetail:
def response(self) -> Response:
service_name = urlparse(str(self.content_link)).netloc
try:
if self.verify:
Expand All @@ -1999,10 +2019,6 @@ def as_response(self) -> Response | ProblemDetail:
else:
# Default context does no ssl verification
ssl_context = ssl.SSLContext()
if self.content_link is None:
return self.problem_detail_document(
f"No content link provided for {service_name}"
)
req = urllib.request.Request(self.content_link)
with urllib.request.urlopen(
req, timeout=20, context=ssl_context
Expand All @@ -2012,21 +2028,19 @@ def as_response(self) -> Response | ProblemDetail:
headers = response.headers

# Mimic the behavior of the HTTP.request_with_timeout class and
# wrap the exceptions thrown by urllib and ssl returning a ProblemDetail document.
# wrap the exceptions thrown by urllib and ssl by raising a RemoteIntegrationException
except urllib.error.HTTPError as e:
return self.problem_detail_document(
"The server received a bad status code ({}) while contacting {}".format(
e.code, service_name
)
)
except TimeoutError:
return self.problem_detail_document(
f"Error connecting to {service_name}. Timeout occurred."
)
message = f"The server received a bad status code ({e.code}) while contacting {service_name}"
self.log.warning(message)
raise RemoteIntegrationException(service_name, message) from e
except TimeoutError as e:
message = f"Error connecting to {service_name}. Timeout occurred."
self.log.warning(message)
raise RemoteIntegrationException(service_name, message) from e
except (urllib.error.URLError, ssl.SSLError) as e:
reason = getattr(e, "reason", e.__class__.__name__)
return self.problem_detail_document(
f"Error connecting to {service_name}. {reason}."
)
message = f"Error connecting to {service_name}. {reason}."
self.log.warning(message)
raise RemoteIntegrationException(service_name, message) from e

return Response(response=content, status=status, headers=headers)
14 changes: 4 additions & 10 deletions src/palace/manager/api/bibliotheca.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
BaseCirculationAPI,
BaseCirculationApiSettings,
BaseCirculationLoanSettings,
FulfillmentInfo,
DirectFulfillment,
HoldInfo,
LoanInfo,
PatronActivityCirculationAPI,
Expand Down Expand Up @@ -488,7 +488,7 @@ def fulfill(
password: str,
pool: LicensePool,
delivery_mechanism: LicensePoolDeliveryMechanism,
) -> FulfillmentInfo:
) -> DirectFulfillment:
"""Get the actual resource file to the patron."""
if (
delivery_mechanism.delivery_mechanism.drm_scheme
Expand All @@ -513,15 +513,9 @@ def fulfill(
response.content,
exc_info=e,
)
return FulfillmentInfo(
pool.collection,
DataSource.BIBLIOTHECA,
pool.identifier.type,
pool.identifier.identifier,
content_link=None,
content_type=content_type or response.headers.get("Content-Type"),
return DirectFulfillment(
content=content,
content_expires=None,
content_type=content_type or response.headers.get("Content-Type"),
)

def get_fulfillment_file(self, patron_id, bibliotheca_id):
Expand Down
Loading

0 comments on commit 5fb1411

Please sign in to comment.