diff --git a/allianceauth/authentication/task_statistics/event_series.py b/allianceauth/authentication/task_statistics/event_series.py index 03865103..9ac1f15d 100644 --- a/allianceauth/authentication/task_statistics/event_series.py +++ b/allianceauth/authentication/task_statistics/event_series.py @@ -5,59 +5,27 @@ import logging from typing import List, Optional from pytz import utc -from redis import Redis, RedisError +from redis import Redis -from allianceauth.utils.cache import get_redis_client +from .helpers import get_redis_client_or_stub logger = logging.getLogger(__name__) -class _RedisStub: - """Stub of a Redis client. - - It's purpose is to prevent EventSeries objects from trying to access Redis - when it is not available. e.g. when the Sphinx docs are rendered by readthedocs.org. - """ - - def delete(self, *args, **kwargs): - pass - - def incr(self, *args, **kwargs): - return 0 - - def zadd(self, *args, **kwargs): - pass - - def zcount(self, *args, **kwargs): - pass - - def zrangebyscore(self, *args, **kwargs): - pass - - class EventSeries: """API for recording and analyzing a series of events.""" _ROOT_KEY = "ALLIANCEAUTH_EVENT_SERIES" - def __init__(self, key_id: str, redis: Redis = None) -> None: - self._redis = get_redis_client() if not redis else redis - try: - if not self._redis.ping(): - raise RuntimeError() - except (AttributeError, RedisError, RuntimeError): - logger.exception( - "Failed to establish a connection with Redis. " - "This EventSeries object is disabled.", - ) - self._redis = _RedisStub() + def __init__(self, key_id: str, redis: Optional[Redis] = None) -> None: + self._redis = get_redis_client_or_stub() if not redis else redis self._key_id = str(key_id) self.clear() @property def is_disabled(self): """True when this object is disabled, e.g. Redis was not available at startup.""" - return isinstance(self._redis, _RedisStub) + return hasattr(self._redis, "IS_STUB") @property def _key_counter(self): @@ -97,7 +65,7 @@ class EventSeries: self._redis.delete(self._key_counter) def count(self, earliest: dt.datetime = None, latest: dt.datetime = None) -> int: - """Count of events, can be restricted to given timeframe. + """Count of events, can be restricted to given time frame. Args: - earliest: Date of first events to count(inclusive), or -infinite if not specified diff --git a/allianceauth/authentication/task_statistics/helpers.py b/allianceauth/authentication/task_statistics/helpers.py index 7b887be7..1766a372 100644 --- a/allianceauth/authentication/task_statistics/helpers.py +++ b/allianceauth/authentication/task_statistics/helpers.py @@ -1,21 +1,62 @@ """Helpers for Task Statistics.""" +import logging from typing import Optional +from app_utils.allianceauth import get_redis_client +from redis import Redis, RedisError + from django.core.cache import cache +logger = logging.getLogger(__name__) + + +class _RedisStub: + """Stub of a Redis client. + + It's purpose is to prevent EventSeries objects from trying to access Redis + when it is not available. e.g. when the Sphinx docs are rendered by readthedocs.org. + """ + + IS_STUB = True + + def delete(self, *args, **kwargs): + pass + + def incr(self, *args, **kwargs): + return 0 + + def zadd(self, *args, **kwargs): + pass + + def zcount(self, *args, **kwargs): + pass + + def zrangebyscore(self, *args, **kwargs): + pass + class ItemCounter: - """A process safe item counter.""" + """A process safe item counter. + + Args: + - name: Unique name for the counter + - minimum: Counter can not go below the minimum, when set + - redis: A Redis client. Will use AA's cache client by default + """ CACHE_KEY_BASE = "allianceauth-item-counter" DEFAULT_CACHE_TIMEOUT = 24 * 3600 - def __init__(self, name: str) -> None: + def __init__( + self, name: str, minimum: Optional[int] = None, redis: Optional[Redis] = None + ) -> None: if not name: raise ValueError("Must define a name") self._name = str(name) + self._minimum = minimum + self._redis = redis if redis else get_redis_client() @property def _cache_key(self) -> str: @@ -23,6 +64,9 @@ class ItemCounter: def reset(self, init_value: int = 0): """Reset counter to initial value.""" + if self._minimum is not None and init_value < self._minimum: + raise ValueError("Can not reset below minimum") + cache.set(self._cache_key, init_value, self.DEFAULT_CACHE_TIMEOUT) def incr(self, delta: int = 1): @@ -34,6 +78,8 @@ class ItemCounter: def decr(self, delta: int = 1): """Decrement counter by delta.""" + if self._minimum is not None and self.value() == self._minimum: + return try: cache.decr(self._cache_key, delta) except ValueError: @@ -42,3 +88,18 @@ class ItemCounter: def value(self) -> Optional[int]: """Return current value or None if not yet initialized.""" return cache.get(self._cache_key) + + +def get_redis_client_or_stub(): + """Return AA's default cache client or a stub if Redis is not available.""" + redis = get_redis_client() + try: + if not redis.ping(): + raise RuntimeError() + except (AttributeError, RedisError, RuntimeError): + logger.exception( + "Failed to establish a connection with Redis. " + "This EventSeries object is disabled.", + ) + return _RedisStub() + return redis diff --git a/allianceauth/authentication/task_statistics/tests/test_event_series.py b/allianceauth/authentication/task_statistics/tests/test_event_series.py index 70804cf4..17889180 100644 --- a/allianceauth/authentication/task_statistics/tests/test_event_series.py +++ b/allianceauth/authentication/task_statistics/tests/test_event_series.py @@ -1,48 +1,19 @@ import datetime as dt -from unittest.mock import patch from pytz import utc -from redis import RedisError from django.test import TestCase from django.utils.timezone import now from allianceauth.authentication.task_statistics.event_series import ( EventSeries, - _RedisStub, ) +from allianceauth.authentication.task_statistics.helpers import _RedisStub MODULE_PATH = "allianceauth.authentication.task_statistics.event_series" class TestEventSeries(TestCase): - def test_should_abort_without_redis_client(self): - # when - with patch(MODULE_PATH + ".get_redis_client") as mock: - mock.return_value = None - events = EventSeries("dummy") - # then - self.assertTrue(events._redis, _RedisStub) - self.assertTrue(events.is_disabled) - - def test_should_disable_itself_if_redis_not_available_1(self): - # when - with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client: - mock_get_master_client.return_value.ping.side_effect = RedisError - events = EventSeries("dummy") - # then - self.assertIsInstance(events._redis, _RedisStub) - self.assertTrue(events.is_disabled) - - def test_should_disable_itself_if_redis_not_available_2(self): - # when - with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client: - mock_get_master_client.return_value.ping.return_value = False - events = EventSeries("dummy") - # then - self.assertIsInstance(events._redis, _RedisStub) - self.assertTrue(events.is_disabled) - def test_should_add_event(self): # given events = EventSeries("dummy") @@ -166,3 +137,15 @@ class TestEventSeries(TestCase): results = events.all() # then self.assertEqual(len(results), 2) + + def test_should_not_report_as_disabled_when_initialized_normally(self): + # given + events = EventSeries("dummy") + # when/then + self.assertFalse(events.is_disabled) + + def test_should_report_as_disabled_when_initialized_with_redis_stub(self): + # given + events = EventSeries("dummy", redis=_RedisStub()) + # when/then + self.assertTrue(events.is_disabled) diff --git a/allianceauth/authentication/task_statistics/tests/test_item_counter.py b/allianceauth/authentication/task_statistics/tests/test_helpers.py similarity index 58% rename from allianceauth/authentication/task_statistics/tests/test_item_counter.py rename to allianceauth/authentication/task_statistics/tests/test_helpers.py index da6f49c1..c4d4f56b 100644 --- a/allianceauth/authentication/task_statistics/tests/test_item_counter.py +++ b/allianceauth/authentication/task_statistics/tests/test_helpers.py @@ -1,6 +1,13 @@ from unittest import TestCase +from unittest.mock import patch -from allianceauth.authentication.task_statistics.helpers import ItemCounter +from redis import RedisError + +from allianceauth.authentication.task_statistics.helpers import ( + ItemCounter, _RedisStub, get_redis_client_or_stub, +) + +MODULE_PATH = "allianceauth.authentication.task_statistics.helpers" COUNTER_NAME = "test-counter" @@ -72,3 +79,37 @@ class TestItemCounter(TestCase): counter.decr(1) # then self.assertEqual(counter.value(), -1) + + def test_can_not_decrement_counter_below_minimum(self): + # given + counter = ItemCounter(COUNTER_NAME, minimum=0) + counter.reset(0) + # when + counter.decr(1) + # then + self.assertEqual(counter.value(), 0) + + def test_can_not_reset_counter_below_minimum(self): + # given + counter = ItemCounter(COUNTER_NAME, minimum=0) + # when/then + with self.assertRaises(ValueError): + counter.reset(-1) + + +class TestGetRedisClient(TestCase): + def test_should_return_mock_if_redis_not_available_1(self): + # when + with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client: + mock_get_master_client.return_value.ping.side_effect = RedisError + result = get_redis_client_or_stub() + # then + self.assertIsInstance(result, _RedisStub) + + def test_should_return_mock_if_redis_not_available_2(self): + # when + with patch(MODULE_PATH + ".get_redis_client") as mock_get_master_client: + mock_get_master_client.return_value.ping.return_value = False + result = get_redis_client_or_stub() + # then + self.assertIsInstance(result, _RedisStub)