diff --git a/AGENTS.md b/AGENTS.md index 3c7400369..9dd35dac0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,6 +14,10 @@ authoritative files named below over copying details into this document. `schema_utils.py`. Portal-facing APIs are primarily `ff_utils.py` (request/auth and metadata functions), `portal_utils.py` (the higher-level `Portal` wrapper), `portal_object_utils.py`, `structured_data.py`, and `submitr/`. +- `redis_utils.py` owns the public Redis error contract: every public operation there and in + `redis_tools.py` is wrapped in `translate_redis_exceptions`, so driver failures surface as + `RedisException` and consumers (notably Snovault) never import `redis.exceptions`. Any new + `RedisBase` method must carry that decorator; `test/test_redis_error_contract.py` enforces it. - Integration modules are grouped by the system named in the file: AWS (`s3_utils.py`, `ecs_utils.py`, `ecr_utils.py`, `cloudformation_utils.py`, `secrets_utils.py`, etc.), search (`es_utils.py`, `opensearch_utils.py`), Redis, Docker, and deployment utilities. diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 084162cd7..6222b08e0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,31 @@ Change Log ---------- +8.20.0 +====== +* willronchetti / 2026-08-07 / branch: fm/dcicutils-redis-error-contract-4w + - Made ``RedisException`` the canonical error contract for every public Redis operation. + ``create_redis_client``, all ``RedisBase`` methods, and all ``RedisSessionToken`` + operations (``from_redis``, ``store_session_token``, ``validate_session_token``, + ``update_session_token``, ``delete_session_token``) now translate redis driver + failures - ``redis.exceptions.RedisError`` and ``redis.exceptions.RedisClusterException`` + and their subclasses - into ``RedisException``, chaining the original as ``__cause__``. + Consumers such as Snovault no longer need to import ``redis.exceptions``. + - Added the public ``dcicutils.redis_utils.translate_redis_exceptions`` decorator and + ``REDIS_DRIVER_EXCEPTIONS`` tuple used to implement this. + - Behavior change: ``RedisSessionToken.store_session_token`` previously caught *any* + ``Exception`` and re-raised a bare ``RedisException``. It now translates only redis + driver failures, so programmer errors (``TypeError``, ``AttributeError``, ...) propagate + unchanged instead of being masked. Absence and unreachability also stay distinct: + ``validate_session_token`` still returns ``False`` for a missing token but raises + ``RedisException`` when Redis cannot be reached. + - ``RedisException`` remains importable from both ``dcicutils.redis_utils`` and + ``dcicutils.redis_tools``; no signatures or return values changed. + - Added ``test/test_redis_error_contract.py``, which injects representative driver + connection/timeout/response failures into every affected public operation. These tests + use mocks and do not require a running redis-server. + + 8.19.0 ====== * ajs/wrr/sn 2026-07-29 / branch: sn_refactor_custom_excel diff --git a/dcicutils/redis_tools.py b/dcicutils/redis_tools.py index ba132fb67..86ab50ac4 100644 --- a/dcicutils/redis_tools.py +++ b/dcicutils/redis_tools.py @@ -2,8 +2,12 @@ import datetime import structlog import jwt -from dcicutils.redis_utils import RedisBase, RedisException +from dcicutils.redis_utils import ( + RedisBase, RedisException, REDIS_DRIVER_EXCEPTIONS, translate_redis_exceptions, +) +# RedisException is re-exported here on purpose: consumers of the session token API +# (notably Snovault) import it from this module and must not need redis.exceptions. log = structlog.getLogger(__name__) @@ -88,13 +92,16 @@ def get_email(self) -> str: return self.email @classmethod + @translate_redis_exceptions def from_redis(cls, *, redis_handler: RedisBase, namespace: str, token: str): """ Builds a RedisSessionToken from an existing record - allows extracting JWT given a session token internally. :param redis_handler: handle to Redis API :param namespace: namespace to search under :param token: value of the token - :return: A RedisSessionToken object built from an existing record in Redis + :return: A RedisSessionToken object built from an existing record in Redis, + or None if no such record exists + :raises RedisException: if Redis cannot be reached or reports a failure """ redis_key = f'{namespace}:session:{token}' redis_entry = redis_handler.get(redis_key) @@ -116,34 +123,44 @@ def decode_jwt(self, audience: str, secret: str, leeway: int = 30, algorithms: l return jwt.decode(self.jwt, secret, audience=audience, leeway=leeway, options={'verify_signature': True}, algorithms=algorithms) + @translate_redis_exceptions def store_session_token(self, *, redis_handler: RedisBase) -> bool: """ Stores the created session token object as an hset in Redis :param redis_handler: handle to Redis API - :return: True if successful, raise Exception otherwise + :return: True if successful + :raises RedisException: if Redis cannot be reached or reports a failure """ try: redis_handler.set(self.redis_key, f'{self.jwt}:{self.email or ""}', exp=self.expiration) - except Exception as e: + except (RedisException, *REDIS_DRIVER_EXCEPTIONS) as e: + # A raw driver exception can only reach here if the caller passed something other than + # a RedisBase; either way the enclosing decorator normalizes it to RedisException. log.error(str(e)) - raise RedisException() + raise return True + @translate_redis_exceptions def validate_session_token(self, *, redis_handler: RedisBase) -> bool: """ Validates the given session token against that stored in redis :param redis_handler: handle to Redis API - :return: True if token matches that in Redis and is not expired + :return: True if token matches that in Redis and is not expired, False if no such + token is stored - note that an unreachable Redis raises rather than + returning False, so callers can distinguish absence from failure + :raises RedisException: if Redis cannot be reached or reports a failure """ redis_token = redis_handler.get(self.redis_key) if not redis_token: return False # if it doesn't exist it's not valid return True # if it does exist it must be valid since we always send with TTL + @translate_redis_exceptions def update_session_token(self, *, redis_handler: RedisBase, jwt: str, email: str) -> bool: """ Refreshes the session token, jwt (if different) and expiration stored in Redis :param redis_handler: handle to Redis API :param jwt: jwt of user :param email: email of user - :return: True if successful, raise Exception otherwise + :return: True if successful + :raises RedisException: if Redis cannot be reached or reports a failure """ # remove old token self.delete_session_token(redis_handler=redis_handler) @@ -155,9 +172,11 @@ def update_session_token(self, *, redis_handler: RedisBase, jwt: str, email: str self.email = email return self.store_session_token(redis_handler=redis_handler) + @translate_redis_exceptions def delete_session_token(self, *, redis_handler: RedisBase) -> bool: """ Deletes the session token from redis, effectively logging out :param redis_handler: handle to Redis API - :return: True if successful, False otherwise + :return: True if a token was removed, False if there was nothing to remove + :raises RedisException: if Redis cannot be reached or reports a failure """ return 1 == redis_handler.delete(self.redis_key) diff --git a/dcicutils/redis_utils.py b/dcicutils/redis_utils.py index 75cb535ea..3541c7850 100644 --- a/dcicutils/redis_utils.py +++ b/dcicutils/redis_utils.py @@ -1,13 +1,59 @@ +import functools import redis +import redis.exceptions import datetime from typing import Union # Low level utilities for working with Redis +class RedisException(Exception): + """ Canonical exception raised by this library for any operational failure reported by the + underlying Redis driver. + + This is the public error contract: callers of create_redis_client, RedisBase and + dcicutils.redis_tools should catch this exception and never need to import + redis.exceptions themselves. The originating driver exception is always attached as + __cause__ for callers that want to inspect it. + """ + pass + + +# Driver exceptions that represent an operational failure talking to Redis (connection refused, +# socket timeout, protocol/response error, cluster failure, ...). redis.exceptions.RedisError is +# the root of nearly all of them; RedisClusterException deliberately sits outside that tree, so +# it is named explicitly. Python-level programmer errors (TypeError, AttributeError, ValueError, +# ...) are intentionally NOT listed and propagate unchanged. +REDIS_DRIVER_EXCEPTIONS = (redis.exceptions.RedisError, redis.exceptions.RedisClusterException) + + +def translate_redis_exceptions(fn): + """ Decorator translating Redis driver failures into the canonical RedisException. + + Applied to every public operation in this module and in dcicutils.redis_tools so that + consumers have a single exception type to handle. Exceptions that are not driver + failures - including RedisException itself, so nesting decorated calls is harmless - + are re-raised untouched. + + :param fn: function issuing one or more Redis commands + :return: wrapped function raising RedisException in place of driver exceptions + """ + @functools.wraps(fn) + def wrapper(*args, **kwargs): + try: + return fn(*args, **kwargs) + except REDIS_DRIVER_EXCEPTIONS as e: + raise RedisException(f'Redis operation {fn.__name__} failed:' + f' {e.__class__.__name__}: {e}') from e + return wrapper + + +@translate_redis_exceptions def create_redis_client(*, url: str, ping=True) -> redis.Redis: """ Creates a Redis client connecting from a host:port or a URL Because this creation pings Redis should re-use this as much as possible, but you can skip the ping if you're confident it will come up + + Raises RedisException if the driver cannot connect (or the ping fails). """ r = redis.from_url(url) if ping: @@ -15,10 +61,6 @@ def create_redis_client(*, url: str, ping=True) -> redis.Redis: return r -class RedisException(Exception): - pass - - class RedisBase(object): """ This class contains low level methods meant to implement useful Redis APIs. The idea is these functions are used to implement the methods needed in Redis Tools. @@ -54,12 +96,14 @@ def _decode_value(value: Union[bytes, int, float]) -> str: """ return value.decode('utf-8') if (value and isinstance(value, bytes)) else value + @translate_redis_exceptions def info(self) -> dict: """ Returns info about the Redis server https://redis.io/commands/info/ :return: a dictionary of information about the redis server """ return self.redis.info() + @translate_redis_exceptions def set(self, key: str, value: Union[str, int, float], exp: Union[int, datetime.timedelta] = None) -> str: """ Sets the given key to the given value https://redis.io/commands/set/ :param key: string to store value under @@ -72,6 +116,7 @@ def set(self, key: str, value: Union[str, int, float], exp: Union[int, datetime. kwargs['ex'] = exp return self.redis.set(self._encode_value(key), self._encode_value(value), **kwargs) + @translate_redis_exceptions def get(self, key: str) -> str: """ Gets the given key from Redis https://redis.io/commands/get/ :param key: key to check for a value store in Redis @@ -82,6 +127,7 @@ def get(self, key: str) -> str: val = self._decode_value(val) return val + @translate_redis_exceptions def set_expiration(self, key: str, t: Union[int, datetime.time]) -> bool: """ Sets the TTL of the given key manually :param key: key to set TTL @@ -90,6 +136,7 @@ def set_expiration(self, key: str, t: Union[int, datetime.time]) -> bool: """ return self.redis.expire(key, t, gt=True) + @translate_redis_exceptions def ttl(self, key: str) -> datetime.time: """ Gets the TTL of the given key :param key: key to get TTL for @@ -97,6 +144,7 @@ def ttl(self, key: str) -> datetime.time: """ return self.redis.ttl(key) + @translate_redis_exceptions def delete(self, key: str) -> int: """ Deletes the given key from Redis https://redis.io/commands/del/ :param key: key to delete @@ -104,6 +152,7 @@ def delete(self, key: str) -> int: """ return self.redis.delete(self._encode_value(key)) + @translate_redis_exceptions def hget(self, key: str, field: str) -> str: """ Gets the value of field from hash key https://redis.io/commands/hget/ :param key: hash key to retrieve field value from @@ -112,6 +161,7 @@ def hget(self, key: str, field: str) -> str: """ return self._decode_value(self.redis.hget(self._encode_value(key), self._encode_value(field))) + @translate_redis_exceptions def hgetall(self, key: str) -> dict: """ Gets all values of the given hash https://redis.io/commands/hgetall/ :param key: hash key to grab all values from @@ -122,6 +172,7 @@ def hgetall(self, key: str) -> dict: encoded_vals = {self._decode_value(k): self._decode_value(v) for k, v in encoded_vals.items()} return encoded_vals + @translate_redis_exceptions def hset(self, key: str, field: str, value: Union[str, int, float]) -> int: """ Sets a single field on a hash key https://redis.io/commands/hset/ :param key: hash key to set field -> value mapping on @@ -131,6 +182,7 @@ def hset(self, key: str, field: str, value: Union[str, int, float]) -> int: """ return self.redis.hset(self._encode_value(key), self._encode_value(field), self._encode_value(value)) + @translate_redis_exceptions def hset_multiple(self, key: str, items: dict) -> int: """ Sets all k,v pairs in items on hash key https://redis.io/commands/hset/ (variadic form) :param key: hash key to store items under @@ -140,6 +192,7 @@ def hset_multiple(self, key: str, items: dict) -> int: encoded_dict = {self._encode_value(k): self._encode_value(v) for k, v in items.items()} return self.redis.hset(key, mapping=encoded_dict) + @translate_redis_exceptions def dbsize(self) -> int: """ Returns number of keys in redis https://redis.io/commands/dbsize/ """ return self.redis.dbsize() diff --git a/pyproject.toml b/pyproject.toml index 031c2913a..4f27c972a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "dcicutils" -version = "8.19.0" +version = "8.20.0" description = "Utility package for interacting with the 4DN Data Portal and other 4DN resources" authors = ["4DN-DCIC Team "] license = "MIT" diff --git a/test/test_redis_error_contract.py b/test/test_redis_error_contract.py new file mode 100644 index 000000000..44f093f64 --- /dev/null +++ b/test/test_redis_error_contract.py @@ -0,0 +1,335 @@ +import datetime +import pytest +import redis.exceptions +from unittest import mock + +from dcicutils.redis_utils import RedisBase, RedisException, create_redis_client, translate_redis_exceptions +from dcicutils.redis_tools import RedisSessionToken +from dcicutils import redis_tools + + +pytestmark = [pytest.mark.unit] + + +# Representative operational failures the redis driver raises. ConnectionError/TimeoutError cover +# an unreachable or slow server, ResponseError covers a server-side protocol failure, +# BusyLoadingError covers a server that is up but not yet serving, and RedisClusterException is +# deliberately included because it does NOT descend from redis.exceptions.RedisError. +DRIVER_FAILURES = [ + redis.exceptions.ConnectionError('Connection refused'), + redis.exceptions.TimeoutError('Timeout reading from socket'), + redis.exceptions.ResponseError('WRONGTYPE Operation against a key holding the wrong kind of value'), + redis.exceptions.BusyLoadingError('Redis is loading the dataset in memory'), + redis.exceptions.RedisClusterException('Redis Cluster cannot be connected'), +] + +# Errors that indicate a bug in the caller rather than a Redis failure. These must NOT be +# translated - swallowing them into RedisException would hide real defects from consumers. +PROGRAMMER_ERRORS = [ + TypeError('takes 2 positional arguments but 3 were given'), + AttributeError("'NoneType' object has no attribute 'get'"), + ValueError('not a valid value'), + KeyError('missing'), +] + +# Every public RedisBase operation, with arguments that succeed against a healthy handle. +REDIS_BASE_OPERATIONS = [ + ('info', ()), + ('set', ('key', 'value')), + ('get', ('key',)), + ('set_expiration', ('key', 60)), + ('ttl', ('key',)), + ('delete', ('key',)), + ('hget', ('key', 'field')), + ('hgetall', ('key',)), + ('hset', ('key', 'field', 'value')), + ('hset_multiple', ('key', {'field': 'value'})), + ('dbsize', ()), +] + + +class FailingRedisHandle: + """ Stand-in for a redis.Redis client where every command raises the given exception. + Used to inject driver failures without needing a live (or dead) Redis server. + """ + + def __init__(self, error): + self.error = error + + def __getattr__(self, item): + def raise_error(*args, **kwargs): + raise self.error + return raise_error + + +def failing_redis_base(error) -> RedisBase: + """ Builds a RedisBase whose underlying driver always fails with the given exception """ + return RedisBase(FailingRedisHandle(error)) + + +def new_session_token() -> RedisSessionToken: + return RedisSessionToken(namespace='dcicutils-unit-test', jwt='example-jwt', email='test@example.com') + + +class TestRedisBaseErrorContract: + """ Every public RedisBase operation must surface driver failures as RedisException. """ + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + @pytest.mark.parametrize('operation,args', REDIS_BASE_OPERATIONS, ids=lambda v: v if isinstance(v, str) else '') + def test_driver_failures_become_redis_exception(self, operation, args, error): + rd = failing_redis_base(error) + with pytest.raises(RedisException): + getattr(rd, operation)(*args) + + @pytest.mark.parametrize('error', PROGRAMMER_ERRORS, ids=lambda e: type(e).__name__) + @pytest.mark.parametrize('operation,args', REDIS_BASE_OPERATIONS, ids=lambda v: v if isinstance(v, str) else '') + def test_programmer_errors_are_not_translated(self, operation, args, error): + rd = failing_redis_base(error) + with pytest.raises(type(error)): + getattr(rd, operation)(*args) + + def test_original_driver_exception_is_chained(self): + """ Consumers that want the underlying cause can still reach it via __cause__ """ + original = redis.exceptions.ConnectionError('Connection refused') + rd = failing_redis_base(original) + with pytest.raises(RedisException) as exc: + rd.get('key') + assert exc.value.__cause__ is original + assert 'ConnectionError' in str(exc.value) + + def test_successful_operations_are_unchanged(self): + """ The translation layer must be transparent when Redis is healthy """ + handle = mock.MagicMock() + handle.get.return_value = b'hello' + handle.set.return_value = True + handle.delete.return_value = 1 + handle.ttl.return_value = 300 + handle.hgetall.return_value = {b'foo': b'bar'} + handle.hget.return_value = b'bar' + handle.dbsize.return_value = 7 + handle.hset.return_value = 1 + handle.expire.return_value = True + handle.info.return_value = {'redis_version': '7.0.0'} + rd = RedisBase(handle) + assert rd.set('key', 'value') is True + assert rd.get('key') == 'hello' + assert rd.delete('key') == 1 + assert rd.ttl('key') == 300 + assert rd.hgetall('key') == {'foo': 'bar'} + assert rd.hget('key', 'foo') == 'bar' + assert rd.hset('key', 'foo', 'bar') == 1 + assert rd.hset_multiple('key', {'foo': 'bar'}) == 1 + assert rd.dbsize() == 7 + assert rd.info() == {'redis_version': '7.0.0'} + # set_expiration must still pass gt=True through to the driver + assert rd.set_expiration('key', 60) is True + assert handle.expire.call_args.args == ('key', 60) + assert handle.expire.call_args.kwargs == {'gt': True} + + def test_get_of_missing_key_still_returns_none(self): + handle = mock.MagicMock() + handle.get.return_value = None + assert RedisBase(handle).get('nope') is None + + def test_docstrings_survive_decoration(self): + """ The decorator uses functools.wraps, so introspection is preserved """ + assert RedisBase.get.__name__ == 'get' + assert 'https://redis.io/commands/get/' in RedisBase.get.__doc__ + + +class TestCreateRedisClientErrorContract: + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_ping_failure_becomes_redis_exception(self, error): + handle = mock.MagicMock() + handle.ping.side_effect = error + with mock.patch.object(redis, 'from_url', return_value=handle): + with pytest.raises(RedisException): + create_redis_client(url='redis://localhost:6379') + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_connect_failure_becomes_redis_exception(self, error): + with mock.patch.object(redis, 'from_url', side_effect=error): + with pytest.raises(RedisException): + create_redis_client(url='redis://localhost:6379') + + def test_successful_creation_is_unchanged(self): + handle = mock.MagicMock() + with mock.patch.object(redis, 'from_url', return_value=handle): + assert create_redis_client(url='redis://localhost:6379') is handle + handle.ping.assert_called_once() + + +class TestSessionTokenErrorContract: + """ Every session operation - creation, lookup, validation, update and revocation - must + raise RedisException when the driver fails, so consumers such as Snovault never need + to import redis.exceptions. + """ + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_store_session_token(self, error): + with pytest.raises(RedisException): + new_session_token().store_session_token(redis_handler=failing_redis_base(error)) + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_validate_session_token(self, error): + with pytest.raises(RedisException): + new_session_token().validate_session_token(redis_handler=failing_redis_base(error)) + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_update_session_token(self, error): + with pytest.raises(RedisException): + new_session_token().update_session_token(redis_handler=failing_redis_base(error), + jwt='new-jwt', email='test@example.com') + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_delete_session_token(self, error): + with pytest.raises(RedisException): + new_session_token().delete_session_token(redis_handler=failing_redis_base(error)) + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_from_redis_get_failure(self, error): + with pytest.raises(RedisException): + RedisSessionToken.from_redis(redis_handler=failing_redis_base(error), + namespace='dcicutils-unit-test', token='some-token') + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_from_redis_ttl_failure(self, error): + """ The record is found but reading its TTL fails - still a RedisException, not a + half-built session object. + """ + handle = mock.MagicMock() + handle.get.return_value = b'example-jwt:test@example.com' + handle.ttl.side_effect = error + with pytest.raises(RedisException): + RedisSessionToken.from_redis(redis_handler=RedisBase(handle), + namespace='dcicutils-unit-test', token='some-token') + + @pytest.mark.parametrize('error', DRIVER_FAILURES, ids=lambda e: type(e).__name__) + def test_session_ops_translate_driver_errors_from_a_raw_handler(self, error): + """ Session operations are decorated in their own right, so a driver exception raised by + a handler that is not a RedisBase is translated too. + """ + token = new_session_token() + raw = FailingRedisHandle(error) + for call in [lambda: token.store_session_token(redis_handler=raw), + lambda: token.validate_session_token(redis_handler=raw), + lambda: token.delete_session_token(redis_handler=raw), + lambda: token.update_session_token(redis_handler=raw, jwt='j', email='e')]: + with pytest.raises(RedisException): + call() + + @pytest.mark.parametrize('error', PROGRAMMER_ERRORS, ids=lambda e: type(e).__name__) + def test_session_ops_do_not_translate_programmer_errors(self, error): + token = new_session_token() + rd = failing_redis_base(error) + for call in [lambda: token.store_session_token(redis_handler=rd), + lambda: token.validate_session_token(redis_handler=rd), + lambda: token.delete_session_token(redis_handler=rd), + lambda: token.update_session_token(redis_handler=rd, jwt='j', email='e'), + lambda: RedisSessionToken.from_redis(redis_handler=rd, namespace='n', token='t')]: + with pytest.raises(type(error)): + call() + + def test_validate_distinguishes_absence_from_failure(self): + """ A missing token is False; an unreachable Redis raises. These must not collapse. """ + handle = mock.MagicMock() + handle.get.return_value = None + assert new_session_token().validate_session_token(redis_handler=RedisBase(handle)) is False + with pytest.raises(RedisException): + new_session_token().validate_session_token( + redis_handler=failing_redis_base(redis.exceptions.ConnectionError('nope'))) + + def test_from_redis_missing_entry_still_returns_none(self): + handle = mock.MagicMock() + handle.get.return_value = None + assert RedisSessionToken.from_redis(redis_handler=RedisBase(handle), + namespace='dcicutils-unit-test', token='some-token') is None + + def test_successful_session_lifecycle_is_unchanged(self): + """ Store/validate/update/delete against a healthy handle behave exactly as before """ + handle = mock.MagicMock() + handle.set.return_value = True + handle.get.return_value = b'example-jwt:test@example.com' + handle.delete.return_value = 1 + handle.ttl.return_value = 300 + rd = RedisBase(handle) + token = new_session_token() + assert token.store_session_token(redis_handler=rd) is True + assert token.validate_session_token(redis_handler=rd) is True + old_key = token.get_redis_key() + assert token.update_session_token(redis_handler=rd, jwt='new-jwt', email='test@example.com') is True + assert token.get_redis_key() != old_key + assert token.get_jwt() == 'new-jwt' + assert token.delete_session_token(redis_handler=rd) is True + handle.delete.return_value = 0 + assert token.delete_session_token(redis_handler=rd) is False + + def test_from_redis_round_trip_is_unchanged(self): + handle = mock.MagicMock() + handle.get.return_value = b'example-jwt:test@example.com' + handle.ttl.return_value = 300 + restored = RedisSessionToken.from_redis(redis_handler=RedisBase(handle), + namespace='dcicutils-unit-test', token='some-token') + assert restored.get_jwt() == 'example-jwt' + assert restored.get_email() == 'test@example.com' + assert restored.get_expiration() == 300 + + +class TestRedisExceptionContractSurface: + """ Public API compatibility guarantees consumers (notably Snovault) rely on. """ + + def test_redis_exception_is_importable_from_both_modules(self): + assert redis_tools.RedisException is RedisException + + def test_redis_exception_is_not_a_driver_exception(self): + """ RedisException must be catchable without importing redis.exceptions """ + assert issubclass(RedisException, Exception) + assert not issubclass(RedisException, redis.exceptions.RedisError) + + def test_redis_exception_can_be_raised_without_arguments(self): + """ Backwards compatibility - older code constructs RedisException() bare """ + with pytest.raises(RedisException): + raise RedisException() + + def test_translation_is_idempotent(self): + """ Nesting decorated calls must not re-wrap an already-canonical RedisException """ + original = RedisException('already translated') + + @translate_redis_exceptions + def inner(): + raise original + + @translate_redis_exceptions + def outer(): + return inner() + + with pytest.raises(RedisException) as exc: + outer() + assert exc.value is original + + def test_translation_preserves_return_value(self): + @translate_redis_exceptions + def op(a, b=2): + """ some docstring """ + return a + b + + assert op(1) == 3 + assert op(1, b=10) == 11 + assert op.__name__ == 'op' + assert 'some docstring' in op.__doc__ + + def test_every_public_redis_base_operation_is_covered(self): + """ Guards against a new RedisBase method being added without error translation """ + public_methods = {name for name in vars(RedisBase) + if not name.startswith('_') and callable(vars(RedisBase)[name])} + assert public_methods == {name for name, _ in REDIS_BASE_OPERATIONS} + + def test_datetime_expiration_still_accepted(self): + """ Sanity check that decoration did not disturb keyword handling """ + handle = mock.MagicMock() + handle.set.return_value = True + rd = RedisBase(handle) + exp = datetime.timedelta(seconds=30) + assert rd.set('key', 'value', exp=exp) is True + assert handle.set.call_args.kwargs['ex'] == exp