diff --git a/api/axis.py b/api/axis.py index c5535fcdf2..407c1d37fb 100644 --- a/api/axis.py +++ b/api/axis.py @@ -17,7 +17,7 @@ from dependency_injector.wiring import Provide, inject from flask_babel import lazy_gettext as _ from lxml import etree -from lxml.etree import _Element +from lxml.etree import _Element, _ElementTree from pydantic import validator from requests import Response as RequestsResponse @@ -310,7 +310,7 @@ def request( extra_headers: dict[str, str] | None = None, data: Mapping[str, Any] | None = None, params: Mapping[str, Any] | None = None, - exception_on_401: bool = False, + request_retried: bool = False, **kwargs: Any, ) -> RequestsResponse: """Make an HTTP request, acquiring/refreshing a bearer token @@ -323,36 +323,33 @@ def request( headers = dict(extra_headers) headers["Authorization"] = "Bearer " + self.token headers["Library"] = self.library_id - if exception_on_401: - disallowed_response_codes = ["401"] - else: - disallowed_response_codes = None response = self._make_request( url=url, method=method, headers=headers, data=data, params=params, - disallowed_response_codes=disallowed_response_codes, **kwargs, ) - if response.status_code == 401: - # This must be our first 401, since our second 401 will - # make _make_request raise a RemoteIntegrationException. - # - # The token has expired. Get a new token and try again. - self.token = None - return self.request( - url=url, - method=method, - extra_headers=extra_headers, - data=data, - params=params, - exception_on_401=True, - **kwargs, - ) - else: - return response + if response.status_code == 401 and not request_retried: + parsed = StatusResponseParser().process_first(response.content) + if parsed is None or parsed[0] in [1001, 1002]: + # The token is probably expired. Get a new token and try again. + # Axis 360's status codes mean: + # 1001: Invalid token + # 1002: Token expired + self.token = None + return self.request( + url=url, + method=method, + extra_headers=extra_headers, + data=data, + params=params, + request_retried=True, + **kwargs, + ) + + return response def availability( self, @@ -940,6 +937,35 @@ def _xpath1_date( return self._pd(value) +class StatusResponseParser(Axis360Parser[tuple[int, str]]): + @property + def xpath_expression(self) -> str: + # Sometimes the status tag is overloaded, so we want to only + # look for the status tag that contains the code tag. + return "//axis:status/axis:code/.." + + def process_one( + self, tag: _Element, namespaces: dict[str, str] | None + ) -> tuple[int, str] | None: + status_code = self.int_of_subtag(tag, "axis:code", namespaces) + message = self.text_of_subtag(tag, "axis:statusMessage", namespaces) + return status_code, message + + def process_first( + self, + xml: str | bytes | _ElementTree | None, + ) -> tuple[int, str] | None: + if not xml: + return None + + # Since this is being used to parse error codes, we want to generally be + # very forgiving of errors in the XML, and return None if we can't parse it. + try: + return super().process_first(xml) + except (etree.XMLSyntaxError, AssertionError, ValueError): + return None + + class BibliographicParser(Axis360Parser[tuple[Metadata, CirculationData]], LoggerMixin): DELIVERY_DATA_FOR_AXIS_FORMAT = { "Blio": None, # Legacy format, handled the same way as AxisNow diff --git a/tests/api/files/axis/availability_invalid_token.xml b/tests/api/files/axis/availability_invalid_token.xml new file mode 100644 index 0000000000..0185b745ac --- /dev/null +++ b/tests/api/files/axis/availability_invalid_token.xml @@ -0,0 +1 @@ +1001Authorization token is invalid \ No newline at end of file diff --git a/tests/api/files/axis/availability_patron_not_found.xml b/tests/api/files/axis/availability_patron_not_found.xml new file mode 100644 index 0000000000..2c38973e06 --- /dev/null +++ b/tests/api/files/axis/availability_patron_not_found.xml @@ -0,0 +1 @@ +3122Patron information is not found. \ No newline at end of file diff --git a/tests/api/test_axis.py b/tests/api/test_axis.py index 97c5df94b1..5092e09819 100644 --- a/tests/api/test_axis.py +++ b/tests/api/test_axis.py @@ -30,6 +30,7 @@ HoldReleaseResponseParser, HoldResponseParser, JSONResponseParser, + StatusResponseParser, ) from api.circulation import FulfillmentInfo, HoldInfo, LoanInfo from api.circulation_exceptions import ( @@ -295,9 +296,9 @@ def test_refresh_bearer_token_error(self, axis360: Axis360Fixture): in str(excinfo.value) ) - def test_exception_after_401_with_fresh_token(self, axis360: Axis360Fixture): - # If we get a 401 immediately after refreshing the token, we will - # raise an exception. + def test_bearer_token_only_refreshed_once_after_401(self, axis360: Axis360Fixture): + # If we get a 401 immediately after refreshing the token, we just + # return the response instead of refreshing the token again. axis360.api.queue_response(401) axis360.api.queue_response(200, content=json.dumps(dict(access_token="foo"))) @@ -305,15 +306,39 @@ def test_exception_after_401_with_fresh_token(self, axis360: Axis360Fixture): axis360.api.queue_response(301) - with pytest.raises(RemoteIntegrationException) as excinfo: - axis360.api.request("http://url/") - assert "Got status code 401 from external server, cannot continue." in str( - excinfo.value - ) + response = axis360.api.request("http://url/") + assert response.status_code == 401 # The fourth request never got made. assert [301] == [x.status_code for x in axis360.api.responses] + def test_bearer_token_not_refreshed_for_patron_not_found( + self, axis360: Axis360Fixture + ): + axis360.api.queue_response( + 401, content=axis360.sample_data("availability_patron_not_found.xml") + ) + axis360.api.queue_response(301) + + # This request will fail with a 401, but the bearer token will not be refreshed because it has + # an axis:code set in the response XML, and the code does not indicate that the token is invalid. + response = axis360.api.request("http://url/") + assert response.status_code == 401 + + # Only a single request was made. + assert len(axis360.api.requests) == 1 + + def test_refresh_bearer_token_on_invalid_token_status( + self, axis360: Axis360Fixture + ): + axis360.api.queue_response( + 401, content=axis360.sample_data("availability_invalid_token.xml") + ) + axis360.api.queue_response(200, content=json.dumps(dict(access_token="foo"))) + axis360.api.queue_response(200, content="The data") + response = axis360.api.request("http://url/") + assert b"The data" == response.content + def test_update_availability(self, axis360: Axis360Fixture): # Test the Axis 360 implementation of the update_availability method # defined by the CirculationAPI interface. @@ -998,6 +1023,45 @@ def test_instantiate(self, axis360: Axis360Fixture): class TestParsers: + def test_status_parser(self, axis360: Axis360Fixture): + data = axis360.sample_data("availability_patron_not_found.xml") + parser = StatusResponseParser() + parsed = parser.process_first(data) + assert parsed is not None + status, message = parsed + assert status == 3122 + assert message == "Patron information is not found." + + data = axis360.sample_data("availability_with_loans.xml") + parsed = parser.process_first(data) + assert parsed is not None + status, message = parsed + assert status == 0 + assert message == "Availability Data is Successfully retrieved." + + data = axis360.sample_data("availability_with_ebook_fulfillment.xml") + parsed = parser.process_first(data) + assert parsed is not None + status, message = parsed + assert status == 0 + assert message == "Availability Data is Successfully retrieved." + + data = axis360.sample_data("checkin_failure.xml") + parsed = parser.process_first(data) + assert parsed is not None + status, message = parsed + assert status == 3103 + assert message == "Invalid Title Id" + + data = axis360.sample_data("invalid_error_code.xml") + assert parser.process_first(data) is None + + data = axis360.sample_data("missing_error_code.xml") + assert parser.process_first(data) is None + assert parser.process_first(None) is None + assert parser.process_first(b"") is None + assert parser.process_first(b"not xml") is None + def test_bibliographic_parser(self, axis360: Axis360Fixture): # Make sure the bibliographic information gets properly # collated in preparation for creating Edition objects. @@ -1490,6 +1554,13 @@ def test_parse_ebook_availability(self, axis360parsers: Axis360FixturePlusParser # make that extra request. assert axis360parsers.api == fulfillment.api + def test_patron_not_found(self, axis360parsers: Axis360FixturePlusParsers): + # If the patron is not found, the parser will return an empty list, since + # that patron can't have any loans or holds. + data = axis360parsers.sample_data("availability_patron_not_found.xml") + parser = AvailabilityResponseParser(axis360parsers.api) + assert list(parser.process_all(data)) == [] + class TestJSONResponseParser: def test__required_key(self):