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

Refactor circulation exceptions (PP-991) #1694

Merged
merged 1 commit into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 7 additions & 12 deletions api/admin/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
from typing import Any

from api.admin.problem_details import ADMIN_NOT_AUTHORIZED
from core.util.problem_detail import ProblemDetail

from core.util.problem_detail import BaseProblemDetailException, ProblemDetail

class AdminNotAuthorized(Exception):
status_code = 403

def __init__(self, *args: Any) -> None:
self.message = None
if len(args) > 0:
self.message = args[0]
super().__init__(*args)
class AdminNotAuthorized(BaseProblemDetailException):
def __init__(self, message: str | None = None) -> None:
self.message = message
super().__init__(message)

def as_problem_detail_document(self, debug=False) -> ProblemDetail:
@property
def problem_detail(self) -> ProblemDetail:
return (
ADMIN_NOT_AUTHORIZED.detailed(self.message)
if self.message
Expand Down
6 changes: 3 additions & 3 deletions api/admin/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from api.routes import allows_library, has_library, library_route
from core.app_server import ensure_pydantic_after_problem_detail, returns_problem_detail
from core.util.problem_detail import (
BaseProblemDetailException,
ProblemDetail,
ProblemDetailException,
ProblemDetailModel,
)

Expand Down Expand Up @@ -95,8 +95,8 @@ def returns_json_or_response_or_problem_detail(f):
def decorated(*args, **kwargs):
try:
v = f(*args, **kwargs)
except ProblemDetailException as ex:
# A ProblemError is the same as a ProblemDetail
except BaseProblemDetailException as ex:
# A ProblemDetailException just needs to be converted to a ProblemDetail.
v = ex.problem_detail
if isinstance(v, ProblemDetail):
return v.response
Expand Down
30 changes: 25 additions & 5 deletions api/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,31 @@
LoanInfo,
PatronActivityCirculationAPI,
)
from api.circulation_exceptions import *
from api.circulation_exceptions import (
AlreadyCheckedOut,
AlreadyOnHold,
CannotFulfill,
CannotHold,
CannotLoan,
CurrentlyAvailable,
InvalidInputException,
LibraryAuthorizationFailedException,
LibraryInvalidInputException,
NoAcceptableFormat,
NoActiveLoan,
NoAvailableCopies,
NotFoundOnRemote,
NotOnHold,
PatronAuthorizationFailedException,
PatronLoanLimitReached,
RemoteInitiatedServerError,
)
from api.selftest import HasCollectionSelfTests, SelfTestResult
from api.web_publication_manifest import FindawayManifest, SpineItem
from core.analytics import Analytics
from core.config import CannotLoadConfiguration
from core.coverage import BibliographicCoverageProvider, CoverageFailure
from core.exceptions import IntegrationException
from core.integration.settings import (
ConfigurationFormItem,
ConfigurationFormItemType,
Expand Down Expand Up @@ -74,6 +93,7 @@
Subject,
)
from core.monitor import CollectionMonitor, IdentifierSweepMonitor, TimelineMonitor
from core.problem_details import INTEGRATION_ERROR
from core.service.container import Services
from core.util.datetime_helpers import datetime_utc, strptime_utc, utc_now
from core.util.flask_util import Response
Expand Down Expand Up @@ -382,8 +402,8 @@ def checkin(self, patron: Patron, pin: str, licensepool: LicensePool) -> None:
CheckinResponseParser(licensepool.collection).process_first(
response.content
)
except etree.XMLSyntaxError as e:
raise RemoteInitiatedServerError(response.content, self.label())
except etree.XMLSyntaxError:
raise RemoteInitiatedServerError(response.text, self.label())

def _checkin(self, title_id: str | None, patron_id: str | None) -> RequestsResponse:
"""Make a request to the EarlyCheckInTitle endpoint."""
Expand Down Expand Up @@ -424,8 +444,8 @@ def checkout(
if loan_info is None:
raise CannotLoan()
return loan_info
except etree.XMLSyntaxError as e:
raise RemoteInitiatedServerError(response.content, self.label())
except etree.XMLSyntaxError:
raise RemoteInitiatedServerError(response.text, self.label())

def _checkout(
self, title_id: str | None, patron_id: str | None, internal_format: str
Expand Down
52 changes: 29 additions & 23 deletions api/bibliotheca.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,21 @@
LoanInfo,
PatronActivityCirculationAPI,
)
from api.circulation_exceptions import *
from api.circulation_exceptions import (
AlreadyCheckedOut,
AlreadyOnHold,
CannotHold,
CannotLoan,
CannotReleaseHold,
CurrentlyAvailable,
NoAvailableCopies,
NoLicenses,
NotCheckedOut,
NotOnHold,
PatronHoldLimitReached,
PatronLoanLimitReached,
RemoteInitiatedServerError,
)
from api.selftest import HasCollectionSelfTests, SelfTestResult
from api.web_publication_manifest import FindawayManifest, SpineItem
from core.analytics import Analytics
Expand Down Expand Up @@ -79,6 +93,7 @@
from core.util import base64
from core.util.datetime_helpers import datetime_utc, strptime_utc, to_utc, utc_now
from core.util.http import HTTP
from core.util.problem_detail import BaseProblemDetailException
from core.util.xmlparser import XMLParser, XMLProcessor


Expand Down Expand Up @@ -894,23 +909,7 @@ def date_from_subtag(self, tag, key, required=True):
return self.parse_date(value)


class BibliothecaException(Exception):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced this exception with RemoteInitiatedServerError, so that we get problem details from all the exceptions thrown by the Bibliotecha API.

pass


class WorkflowException(BibliothecaException):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't being used anywhere.

def __init__(self, actual_status, statuses_that_would_work):
self.actual_status = actual_status
self.statuses_that_would_work = statuses_that_would_work

def __str__(self):
return "Book status is {}, must be: {}".format(
self.actual_status,
", ".join(self.statuses_that_would_work),
)


class ErrorParser(BibliothecaParser[Exception]):
class ErrorParser(BibliothecaParser[BaseProblemDetailException]):
"""Turn an error document from the Bibliotheca web service into a CheckoutException"""

wrong_status = re.compile(
Expand All @@ -930,13 +929,20 @@ class ErrorParser(BibliothecaParser[Exception]):
def xpath_expression(self) -> str:
return "//Error"

def process_first(self, string: str | bytes) -> Exception:
def process_first(self, string: str | bytes) -> BaseProblemDetailException:
try:
return_val = super().process_first(string)
except Exception as e:
# The server sent us an error with an incorrect or
# nonstandard syntax.
return RemoteInitiatedServerError(string, BibliothecaAPI.SERVICE_NAME)
if isinstance(string, bytes):
try:
debug = string.decode("utf-8")
except UnicodeDecodeError:
debug = "Unreadable error message (Unicode decode error)."
else:
debug = string
return RemoteInitiatedServerError(debug, BibliothecaAPI.SERVICE_NAME)

if return_val is None:
# We were not able to interpret the result as an error.
Expand All @@ -950,7 +956,7 @@ def process_first(self, string: str | bytes) -> Exception:

def process_one(
self, error_tag: _Element, namespaces: dict[str, str] | None
) -> Exception:
) -> BaseProblemDetailException:
message = self.text_of_optional_subtag(error_tag, "Message")
if not message:
return RemoteInitiatedServerError(
Expand Down Expand Up @@ -982,7 +988,7 @@ def process_one(

m = self.wrong_status.search(message)
if not m:
return BibliothecaException(message)
return RemoteInitiatedServerError(message, BibliothecaAPI.SERVICE_NAME)
actual, expected = m.groups()
expected = expected.split(",")

Expand Down Expand Up @@ -1010,7 +1016,7 @@ def process_one(
if "CAN_LOAN" in expected:
return CannotLoan(message)

return BibliothecaException(message)
return RemoteInitiatedServerError(message, BibliothecaAPI.SERVICE_NAME)


class PatronCirculationParser(XMLParser):
Expand Down
21 changes: 20 additions & 1 deletion api/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,25 @@
from pydantic import PositiveInt
from sqlalchemy.orm import Query

from api.circulation_exceptions import *
from api.circulation_exceptions import (
AlreadyCheckedOut,
AlreadyOnHold,
CannotFulfill,
CannotRenew,
CannotReturn,
CurrentlyAvailable,
DeliveryMechanismConflict,
DeliveryMechanismError,
DeliveryMechanismMissing,
NoAcceptableFormat,
NoActiveLoan,
NoAvailableCopies,
NoLicenses,
NotCheckedOut,
NotOnHold,
PatronHoldLimitReached,
PatronLoanLimitReached,
)
from api.util.patron import PatronUtility
from core.analytics import Analytics
from core.integration.base import HasLibraryIntegrationConfiguration
Expand Down Expand Up @@ -45,6 +63,7 @@
from core.model.integration import IntegrationConfiguration
from core.util.datetime_helpers import utc_now
from core.util.log import LoggerMixin
from core.util.problem_detail import ProblemDetail


class CirculationInfo:
Expand Down
Loading