diff --git a/flickr_api/__init__.py b/flickr_api/__init__.py index 7cb7288..ebff92b 100644 --- a/flickr_api/__init__.py +++ b/flickr_api/__init__.py @@ -35,8 +35,11 @@ from .auth import set_auth_handler as set_auth_handler from .method_call import disable_cache as disable_cache from .method_call import enable_cache as enable_cache +from .method_call import get_rate_limit as get_rate_limit +from .method_call import get_rate_limit_status as get_rate_limit_status from .method_call import get_retry_config as get_retry_config from .method_call import get_timeout as get_timeout +from .method_call import set_rate_limit as set_rate_limit from .method_call import set_retry_config as set_retry_config from .method_call import set_timeout as set_timeout from .keys import set_keys as set_keys diff --git a/flickr_api/method_call.py b/flickr_api/method_call.py index 5f91254..ee224cb 100644 --- a/flickr_api/method_call.py +++ b/flickr_api/method_call.py @@ -36,6 +36,10 @@ RETRY_BASE_DELAY: float = 1.0 # Base delay in seconds for exponential backoff RETRY_MAX_DELAY: float = 60.0 # Maximum delay between retries +# Proactive rate limiting configuration +_RATE_LIMIT_REQUESTS_PER_HOUR: float | None = None +_RATE_LIMIT_LAST_REQUEST: float | None = None + def enable_cache(cache_object: Any | None = None) -> None: """enable caching @@ -109,6 +113,85 @@ def get_retry_config() -> dict[str, Any]: } +def set_rate_limit(requests_per_hour: float | None) -> None: + """Enable or disable proactive rate limiting. + + Parameters: + ----------- + requests_per_hour: float | None + Maximum requests per hour. Set to None to disable rate limiting. + Flickr's documented limit is 3600 requests per hour. + + Raises: + ------- + ValueError: If requests_per_hour is not positive (zero or negative). + """ + if requests_per_hour is not None and requests_per_hour <= 0: + raise ValueError("requests_per_hour must be positive") + global _RATE_LIMIT_REQUESTS_PER_HOUR + _RATE_LIMIT_REQUESTS_PER_HOUR = requests_per_hour + + +def get_rate_limit() -> dict[str, float | None]: + """Get current rate limit configuration. + + Returns: + -------- + dict with key: requests_per_hour (float | None) + """ + return {"requests_per_hour": _RATE_LIMIT_REQUESTS_PER_HOUR} + + +def get_rate_limit_status() -> dict[str, Any]: + """Get detailed rate limit status. + + Returns: + -------- + dict with keys: + - enabled: bool - Whether rate limiting is active + - requests_per_hour: float | None - Configured limit + - interval_seconds: float - Minimum time between requests (0.0 if disabled) + - last_request_time: float | None - Timestamp of last request + """ + enabled = _RATE_LIMIT_REQUESTS_PER_HOUR is not None + interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR if enabled else 0.0 + return { + "enabled": enabled, + "requests_per_hour": _RATE_LIMIT_REQUESTS_PER_HOUR, + "interval_seconds": interval, + "last_request_time": _RATE_LIMIT_LAST_REQUEST, + } + + +def _maybe_wait_for_rate_limit() -> None: + """Wait if necessary to respect rate limit. + + This function should be called before making a request. It will: + 1. Do nothing if rate limiting is disabled + 2. Do nothing if this is the first request + 3. Sleep for the remaining interval time if needed + 4. Update the last request timestamp + """ + global _RATE_LIMIT_LAST_REQUEST + + if _RATE_LIMIT_REQUESTS_PER_HOUR is None: + return + + current_time = time.time() + + if _RATE_LIMIT_LAST_REQUEST is not None: + interval = 3600.0 / _RATE_LIMIT_REQUESTS_PER_HOUR + elapsed = current_time - _RATE_LIMIT_LAST_REQUEST + remaining = interval - elapsed + + if remaining > 0: + logger.debug("Rate limiting: sleeping for %.2f seconds", remaining) + time.sleep(remaining) + current_time = time.time() + + _RATE_LIMIT_LAST_REQUEST = current_time + + def _calculate_retry_delay(attempt: int, retry_after: float | None) -> float: """Calculate delay before next retry. @@ -181,6 +264,8 @@ def _make_request_with_retry( ------- FlickrRateLimitError: If rate limit exceeded and max retries exhausted """ + _maybe_wait_for_rate_limit() + last_error: FlickrRateLimitError | None = None for attempt in range(MAX_RETRIES + 1): diff --git a/test/test_rate_limit_throttle.py b/test/test_rate_limit_throttle.py new file mode 100644 index 0000000..06f8b7d --- /dev/null +++ b/test/test_rate_limit_throttle.py @@ -0,0 +1,261 @@ +"""Tests for proactive rate limiting (throttling) in method_call module.""" + +import unittest +from unittest.mock import patch + +from flickr_api import method_call + + +class TestRateLimitDisabledByDefault(unittest.TestCase): + """Test that rate limiting is disabled by default.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_rate_limit_disabled_by_default(self): + """Rate limiting should be disabled by default.""" + result = method_call.get_rate_limit() + self.assertIsNone(result["requests_per_hour"]) + + +class TestSetAndGetRateLimit(unittest.TestCase): + """Test setting and getting rate limit configuration.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_set_and_get_rate_limit(self): + """Can set and get rate limit value.""" + method_call.set_rate_limit(3600.0) + result = method_call.get_rate_limit() + self.assertEqual(3600.0, result["requests_per_hour"]) + + def test_disable_rate_limit(self): + """Can disable rate limiting by setting to None.""" + method_call.set_rate_limit(3600.0) + method_call.set_rate_limit(None) + result = method_call.get_rate_limit() + self.assertIsNone(result["requests_per_hour"]) + + def test_reject_zero_rate_limit(self): + """Zero rate limit should raise ValueError.""" + with self.assertRaises(ValueError) as ctx: + method_call.set_rate_limit(0.0) + self.assertEqual(str(ctx.exception), "requests_per_hour must be positive") + + def test_reject_negative_rate_limit(self): + """Negative rate limit should raise ValueError.""" + with self.assertRaises(ValueError) as ctx: + method_call.set_rate_limit(-100.0) + self.assertEqual(str(ctx.exception), "requests_per_hour must be positive") + + +class TestIntervalCalculation(unittest.TestCase): + """Test interval calculation for rate limiting.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + + def test_interval_calculation_3600_per_hour(self): + """3600 requests/hour = 1.0 second interval.""" + method_call.set_rate_limit(3600.0) + status = method_call.get_rate_limit_status() + self.assertEqual(1.0, status["interval_seconds"]) + + def test_interval_calculation_1800_per_hour(self): + """1800 requests/hour = 2.0 second interval.""" + method_call.set_rate_limit(1800.0) + status = method_call.get_rate_limit_status() + self.assertEqual(2.0, status["interval_seconds"]) + + def test_interval_calculation_7200_per_hour(self): + """7200 requests/hour = 0.5 second interval.""" + method_call.set_rate_limit(7200.0) + status = method_call.get_rate_limit_status() + self.assertEqual(0.5, status["interval_seconds"]) + + +class TestGetRateLimitStatus(unittest.TestCase): + """Test get_rate_limit_status function.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def test_get_rate_limit_status_disabled(self): + """Status shows disabled state correctly.""" + status = method_call.get_rate_limit_status() + self.assertFalse(status["enabled"]) + self.assertIsNone(status["requests_per_hour"]) + self.assertEqual(0.0, status["interval_seconds"]) + self.assertIsNone(status["last_request_time"]) + + def test_get_rate_limit_status_enabled(self): + """Status shows enabled state correctly.""" + method_call.set_rate_limit(3600.0) + status = method_call.get_rate_limit_status() + self.assertTrue(status["enabled"]) + self.assertEqual(3600.0, status["requests_per_hour"]) + self.assertEqual(1.0, status["interval_seconds"]) + self.assertIsNone(status["last_request_time"]) + + def test_get_rate_limit_status_with_last_request(self): + """Status includes last request time when set.""" + method_call.set_rate_limit(3600.0) + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 + status = method_call.get_rate_limit_status() + self.assertEqual(1000.0, status["last_request_time"]) + + +class TestNoSleepWhenDisabled(unittest.TestCase): + """Test that no sleep occurs when rate limiting is disabled.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_no_sleep_when_disabled(self, mock_time, mock_sleep): + """No sleep when rate limiting is disabled.""" + method_call._maybe_wait_for_rate_limit() + mock_sleep.assert_not_called() + + +class TestSleepsWhenIntervalNotElapsed(unittest.TestCase): + """Test that sleep occurs when interval hasn't elapsed.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.5) + def test_sleeps_when_interval_not_elapsed(self, mock_time, mock_sleep): + """Sleep for remaining time when interval hasn't elapsed.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 # 0.5 seconds ago + + method_call._maybe_wait_for_rate_limit() + + # Should sleep for 0.5 seconds (1.0 - 0.5) + mock_sleep.assert_called_once_with(0.5) + + +class TestNoSleepWhenIntervalElapsed(unittest.TestCase): + """Test that no sleep occurs when interval has elapsed.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1002.0) + def test_no_sleep_when_interval_elapsed(self, mock_time, mock_sleep): + """No sleep when interval has already elapsed.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.0 # 2.0 seconds ago + + method_call._maybe_wait_for_rate_limit() + + mock_sleep.assert_not_called() + + +class TestFirstRequestNoSleep(unittest.TestCase): + """Test that first request doesn't sleep.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_first_request_no_sleep(self, mock_time, mock_sleep): + """First request (no last_request_time) doesn't sleep.""" + method_call.set_rate_limit(3600.0) # Rate limiting enabled + # _RATE_LIMIT_LAST_REQUEST is None (first request) + + method_call._maybe_wait_for_rate_limit() + + mock_sleep.assert_not_called() + + +class TestUpdatesLastRequestTime(unittest.TestCase): + """Test that last request time is updated after waiting.""" + + def setUp(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + def tearDown(self): + """Reset rate limit state.""" + method_call.set_rate_limit(None) + method_call._RATE_LIMIT_LAST_REQUEST = None + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1000.0) + def test_updates_last_request_time(self, mock_time, mock_sleep): + """Last request time is updated after _maybe_wait_for_rate_limit.""" + method_call.set_rate_limit(3600.0) + + method_call._maybe_wait_for_rate_limit() + + self.assertEqual(1000.0, method_call._RATE_LIMIT_LAST_REQUEST) + + @patch.object(method_call.time, "sleep") + @patch.object(method_call.time, "time", return_value=1001.0) + def test_updates_last_request_time_after_sleep(self, mock_time, mock_sleep): + """Last request time is updated to current time after sleeping.""" + method_call.set_rate_limit(3600.0) # 1 second interval + method_call._RATE_LIMIT_LAST_REQUEST = 1000.5 # Would need to wait + + method_call._maybe_wait_for_rate_limit() + + # Should update to the current time after potential sleep + self.assertEqual(1001.0, method_call._RATE_LIMIT_LAST_REQUEST)