Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delegate Fulfillment instead of using FulfillmentInfo (PP-1641) #2035

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Delegate Fulfillment instead of using FulfillmentInfo (PP-1641) #2035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d166625
Update fulfillment to use fulfillment strategy
jonathangreen Sep 4, 2024
549f98b
Code review feedback: Default content_type to ADOBE_DRM
jonathangreen Sep 6, 2024
86acbb0
Code review feedback: Clean up some missed references to FulfillmentInfo
jonathangreen Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 87 additions & 73 deletions src/palace/manager/api/axis.py
View file
Open in desktop
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
Open in desktop
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