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.

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

Ekir 197 enable odl loan limited and concurreny licensepools #130

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
3 changes: 1 addition & 2 deletions api/controller/loan.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
NO_ACCEPTABLE_FORMAT,
NO_ACTIVE_LOAN,
NO_ACTIVE_LOAN_OR_HOLD,
NO_COPIES_WHEN_RESERVED,
NO_LICENSES,
NOT_FOUND_ON_REMOTE,
OUTSTANDING_FINES,
Expand Down Expand Up @@ -209,8 +210,6 @@ def _borrow(self, patron, credential, pool, mechanism):
result = e.as_problem_detail_document(debug=False)
except AuthorizationBlocked as e:
result = e.as_problem_detail_document(debug=False)
except CannotLoan as e:
result = CHECKOUT_FAILED.with_debug(str(e))
except CannotLoan as e:
if isinstance(e, NoAvailableCopiesWhenReserved):
result = e.as_problem_detail_document()
Expand Down
62 changes: 53 additions & 9 deletions api/odl.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,11 @@
from core.service.container import Services
from core.util import base64
from core.util.datetime_helpers import to_utc, utc_now
from core.util.http import HTTP, BadResponseException
from core.util.http import HTTP, BadResponseException, RemoteIntegrationException

import logging

logger = logging.getLogger(__name__)

class ODLAPIConstants:
DEFAULT_PASSPHRASE_HINT = "View the help page for more information."
Expand Down Expand Up @@ -242,7 +245,7 @@ def _get_hasher(self) -> Hasher:

return self._hasher_instance

def _get(self, url: str, headers: dict[str, str] | None = None) -> Response:
def _get(self, url: str, headers: dict[str, str] | None = None, allowed_response_codes=None) -> Response:
"""Make a normal HTTP request, but include an authentication
header with the credentials for the collection.
"""
Expand Down Expand Up @@ -322,18 +325,36 @@ def get_license_status_document(self, loan: Loan) -> dict[str, Any]:
hint_url=self.settings.passphrase_hint_url,
)

response = self._get(url)
try:
response = self._get(url, allowed_response_codes=["2xx"])
logger.info("RESPONSE: %s", response)
except BadResponseException as e:
response = e.response
logger.info("Response here: %s", response)
header_string = ", ".join(
{f"{k}: {v}" for k, v in response.headers.items()}
)
response_string = (
response.text
if len(response.text) < 100
else response.text[:100] + "..."
)
logger.error(
f"Error getting License Status Document for loan ({loan.id}): Url '{url}' returned "
f"status code {response.status_code}. Expected 2XX. Response headers: {header_string}. "
f"Response content: {response_string}."
)
raise

try:
status_doc = json.loads(response.content)
except ValueError as e:
raise BadResponseException(
raise RemoteIntegrationException(
url, "License Status Document was not valid JSON."
)
) from e
if status_doc.get("status") not in self.STATUS_VALUES:
raise BadResponseException(
url, "License Status Document had an unknown status value."
)
logger.info("status: %s", status_doc.get("status")) # Tänne ei nyt päädytty, kun homma alkoi toimimaan...
raise BadResponseException(url, "License Status Document had an unknown status value.")
return status_doc # type: ignore[no-any-return]

def checkin(self, patron: Patron, pin: str, licensepool: LicensePool) -> None:
Expand Down Expand Up @@ -468,7 +489,30 @@ def _checkout(
raise NoAvailableCopies()
loan, ignore = license.loan_to(patron)

doc = self.get_license_status_document(loan)
try:
doc = self.get_license_status_document(loan)
except BadResponseException as e:
logger.info("error: %s", e) # ei päätynyt tänne, koska alkoi toimimaan yhtäkkiä
_db.delete(loan)
response = e.response
# DeMarque sends "application/api-problem+json", but the ODL spec says we should
# expect "application/problem+json", so we need to check for both.
if response.headers.get("Content-Type") in [
"application/api-problem+json",
"application/problem+json",
]:
try:
json_response = response.json()
logger.info("type: %s", json_response.get("type"))
except ValueError:
json_response = {}

if (
json_response.get("type")
== "http://opds-spec.org/odl/error/checkout/unavailable"
):
raise NoAvailableCopies()
raise
status = doc.get("status")

if status not in [self.READY_STATUS, self.ACTIVE_STATUS]:
Expand Down
13 changes: 13 additions & 0 deletions core/model/licensing.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,22 @@ def is_inactive(self) -> bool:

@property
def total_remaining_loans(self) -> int | None:
"""
The total number of loans available for this License.

:return: `None` if the license is not limited by the number of checkouts
(=loans), otherwise the number of remaining loans, taking into account
both the total number of remaining checkouts and the concurrency limit.
"""
if self.is_inactive:
return 0
elif self.is_loan_limited:
if self.terms_concurrency is not None:
# We need a type ignore here because mypy doesn't understand that
# `is_loan_limited` implies `checkouts_left` is not None.
# For any loan limited license, there's always the tracker for
# checkouts_left.
return min(self.checkouts_left, self.terms_concurrency) # type: ignore[type-var]
return self.checkouts_left
else:
return self.terms_concurrency
Expand Down
10 changes: 7 additions & 3 deletions core/util/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,19 @@ class BadResponseException(RemoteIntegrationException):
"Got status code %s from external server, cannot continue."
)

def __init__(self, url_or_service, message, debug_message=None, status_code=None):
def __init__(self, url_or_service, message, response=Response, debug_message=None, status_code=None):
"""Indicate that a remote integration has failed.

`param url_or_service` The name of the service that failed
(e.g. "Overdrive"), or the specific URL that had the problem.
"""
if debug_message is None:
debug_message = (
f"Status code: {response.status_code}\nContent: {response.text}"
)

super().__init__(url_or_service, message, debug_message)
# to be set to 500, etc.
self.status_code = status_code
self.response = response

def document_debug_message(self, debug=True):
if debug:
Expand Down
Loading