Skip to content
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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "folioclient"
version = "1.0.3"
version = "1.0.4"
description = "An API wrapper over the FOLIO LSP API Suite (Formerly OKAPI)."
authors = [
{ name = "Theodor Tolstoy", email = "github.teddes@tolstoy.se" },
Expand Down
3 changes: 2 additions & 1 deletion src/folioclient/FolioClient.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,11 @@ def _clear_single_cached_property(self, prop_name: str) -> None:
prop_name (str): Name of the property to clear.
"""
# Verify it's actually a cached property before deleting
# Use __dict__ to check if value is cached without triggering property evaluation
if (
hasattr(self.__class__, prop_name)
and self._is_cached_property(prop_name)
and hasattr(self, prop_name)
and prop_name in self.__dict__
):
delattr(self, prop_name)

Expand Down
2 changes: 1 addition & 1 deletion src/folioclient/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def should_retry_auth_error(exc):
returns:
True if the request should be retried, False otherwise
"""
return exc.response.status_code == HTTPStatus.FORBIDDEN
return exc.response.status_code == HTTPStatus.FORBIDDEN if hasattr(exc, "response") else False


# Custom retry condition classes for tenacity
Expand Down
94 changes: 72 additions & 22 deletions tests/test_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import inspect
import time
from types import SimpleNamespace
import types
import httpx
import pytest
Expand All @@ -22,21 +21,30 @@
use_client_session,
use_client_session_with_generator,
)
from folioclient.exceptions import FolioConnectionError


def _create_mock_response(status_code: int) -> Mock:
"""Helper to create a mock httpx.Response with a status code."""
mock_response = Mock(spec=httpx.Response)
mock_response.status_code = status_code
return mock_response


acceptable_errors_side_effect = [
httpx.HTTPStatusError("error 502", request=None, response=SimpleNamespace(status_code=502)),
httpx.HTTPStatusError("error 503", request=None, response=SimpleNamespace(status_code=503)),
httpx.HTTPStatusError("error 504", request=None, response=SimpleNamespace(status_code=504)),
httpx.HTTPStatusError("error 502", request=None, response=_create_mock_response(502)),
httpx.HTTPStatusError("error 503", request=None, response=_create_mock_response(503)),
httpx.HTTPStatusError("error 504", request=None, response=_create_mock_response(504)),
"test",
]

all_errors_side_effect = acceptable_errors_side_effect.copy()
all_errors_side_effect.insert(
3, httpx.HTTPStatusError("error 500", request=None, response=SimpleNamespace(status_code=500))
3, httpx.HTTPStatusError("error 500", request=None, response=_create_mock_response(500))
)

acceptable_auth_errors_side_effect = [
httpx.HTTPStatusError("error 403", request=None, response=SimpleNamespace(status_code=403)),
httpx.HTTPStatusError("error 403", request=None, response=_create_mock_response(403)),
"test",
]

Expand Down Expand Up @@ -71,7 +79,7 @@ def test_fails_default_auth(_):
internal_fn = Mock(
return_value="test",
side_effect=[
httpx.HTTPStatusError("test", request=None, response=SimpleNamespace(status_code=401)),
httpx.HTTPStatusError("test", request=None, response=_create_mock_response(401)),
"test",
],
)
Expand All @@ -90,7 +98,7 @@ def test_fails_default(_):
internal_fn = Mock(
return_value="test",
side_effect=[
httpx.HTTPStatusError("test", request=None, response=SimpleNamespace(status_code=502)),
httpx.HTTPStatusError("test", request=None, response=_create_mock_response(502)),
"test",
],
)
Expand All @@ -112,7 +120,7 @@ def test_handles_single_fail(_):
internal_fn = Mock(
return_value="test",
side_effect=[
httpx.HTTPStatusError("test", request=None, response=SimpleNamespace(status_code=502)),
httpx.HTTPStatusError("test", request=None, response=_create_mock_response(502)),
"test",
],
)
Expand All @@ -135,7 +143,7 @@ def test_handles_single_fail_auth(_):
internal_fn = Mock(
return_value="test",
side_effect=[
httpx.HTTPStatusError("test", request=None, response=SimpleNamespace(status_code=403)),
httpx.HTTPStatusError("test", request=None, response=_create_mock_response(403)),
"test",
],
)
Expand Down Expand Up @@ -636,21 +644,38 @@ async def test_method(self):


def test_should_retry_server_error_and_auth():
mock_request = Mock(spec=httpx.Request)

# ConnectError should retry
assert should_retry_server_error(httpx.ConnectError("c", request=None)) is True
assert should_retry_server_error(httpx.ConnectError("c", request=mock_request)) is True

# HTTP status 502/503/504 should retry
assert should_retry_server_error(httpx.HTTPStatusError("", request=None, response=SimpleNamespace(status_code=502))) is True
assert should_retry_server_error(httpx.HTTPStatusError("", request=None, response=SimpleNamespace(status_code=503))) is True
assert should_retry_server_error(httpx.HTTPStatusError("", request=None, response=SimpleNamespace(status_code=504))) is True
assert should_retry_server_error(
httpx.HTTPStatusError("", request=mock_request, response=_create_mock_response(502))
) is True
assert should_retry_server_error(
httpx.HTTPStatusError("", request=mock_request, response=_create_mock_response(503))
) is True
assert should_retry_server_error(
httpx.HTTPStatusError("", request=mock_request, response=_create_mock_response(504))
) is True

# Other HTTP statuses should not retry
assert should_retry_server_error(httpx.HTTPStatusError("", request=None, response=SimpleNamespace(status_code=500))) is False
assert should_retry_server_error(
httpx.HTTPStatusError("", request=mock_request, response=_create_mock_response(500))
) is False
assert should_retry_server_error(ValueError("boom")) is False

# Auth retry helper
assert should_retry_auth_error(SimpleNamespace(response=SimpleNamespace(status_code=403))) is True
assert should_retry_auth_error(SimpleNamespace(response=SimpleNamespace(status_code=401))) is False
# Auth retry helper - test with actual httpx.HTTPStatusError
auth_error_403 = httpx.HTTPStatusError(
"forbidden", request=mock_request, response=_create_mock_response(403)
)
assert should_retry_auth_error(auth_error_403) is True

auth_error_401 = httpx.HTTPStatusError(
"unauthorized", request=mock_request, response=_create_mock_response(401)
)
assert should_retry_auth_error(auth_error_401) is False


def test_retry_condition_classes():
Expand All @@ -673,12 +698,38 @@ def __init__(self, outcome, args=()):
assert cond(FakeState(FakeOutcome(False))) is False

# When failed with ConnectError -> True
assert cond(FakeState(FakeOutcome(True, httpx.ConnectError("x", request=None)))) is True
mock_request = Mock(spec=httpx.Request)
assert cond(FakeState(FakeOutcome(True, httpx.ConnectError("x", request=mock_request)))) is True

# AuthErrorRetryCondition
auth_cond = AuthErrorRetryCondition()
assert auth_cond(FakeState(FakeOutcome(False))) is False
assert auth_cond(FakeState(FakeOutcome(True, httpx.HTTPStatusError("", request=None, response=SimpleNamespace(status_code=403))))) is True

auth_error = httpx.HTTPStatusError(
"forbidden", request=mock_request, response=_create_mock_response(403)
)
assert auth_cond(FakeState(FakeOutcome(True, auth_error))) is True


def test_should_retry_auth_error_with_folio_connection_error():
"""Test that should_retry_auth_error handles FolioConnectionError without AttributeError.

FolioConnectionError has a 'request' attribute but no 'response' attribute.
This test ensures the hasattr check prevents AttributeError when checking
for response.status_code.
"""
# Create a mock request
mock_request = Mock(spec=httpx.Request)

# Create a FolioConnectionError (has request but no response)
folio_error = FolioConnectionError("Connection failed", request=mock_request)

# This should return False without raising AttributeError
assert should_retry_auth_error(folio_error) is False

# Verify the exception doesn't have a response attribute
assert not hasattr(folio_error, 'response')
assert hasattr(folio_error, 'request')


def test_auth_refresh_callback_invokes_login(monkeypatch):
Expand All @@ -688,7 +739,7 @@ def __init__(self, attempt_number, args):
self.attempt_number = attempt_number
self.args = args

client = SimpleNamespace()
client = Mock()
client.login = Mock()

state = FakeState(2, (client,))
Expand Down Expand Up @@ -1497,4 +1548,3 @@ def test_get_auth_retry_config_numeric_and_none(monkeypatch):
monkeypatch.setenv("FOLIOCLIENT_AUTH_ERROR_MAX_WAIT", "inf")
cfg2 = get_auth_retry_config()
assert "stop" in cfg2 and "wait" in cfg2

5 changes: 5 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,17 @@ def folio_auth_patcher():
# Instead of trying to mock the whole class, patch the problematic methods
# This is more reliable as it doesn't deal with import timing issues
from datetime import datetime, timezone, timedelta
import httpx

mock_token = Mock()
mock_token.auth_token = "test-token"
mock_token.refresh_token = "test-refresh-token"
# Set expires_at to a future datetime to avoid expiration issues
mock_token.expires_at = datetime.now(timezone.utc) + timedelta(hours=1)
# Mock cookies as an httpx.Cookies object (which is iterable)
mock_token.cookies = httpx.Cookies()
mock_token.cookies.set("folioAccessToken", "test-token")
mock_token.cookies.set("folioRefreshToken", "test-refresh-token")

with patch('folioclient._httpx.FolioAuth._do_sync_auth', return_value=mock_token), \
patch('folioclient._httpx.FolioAuth._token_is_expiring', return_value=False):
Expand Down