Update how we handle axis availability information 401 response (PP-1081

) (#1746)

* Fix token refresh mechanism
* Update axis retry logic

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

tests/api/files/axis/availability_invalid_token.xml

@@ -0,0 +1 @@
+<?xml version="1.0" encoding="utf-8"?><availabilityResponse xmlns="http://axis360api.baker-taylor.com/vendorAPI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"><status><code>1001</code><statusMessage>Authorization token is invalid</statusMessage></status></availabilityResponse>

tests/api/files/axis/availability_patron_not_found.xml

@@ -0,0 +1 @@
+<?xml version="1.0" encoding="utf-8"?><availabilityResponse xmlns="http://axis360api.baker-taylor.com/vendorAPI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"><status><code>3122</code><statusMessage>Patron information is not found.</statusMessage></status></availabilityResponse>

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,25 +296,49 @@ 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")))
         axis360.api.queue_response(401)
 
         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):