Split up remove_expired_reponses() into remove() and reset_expiration() methods, with more granular arguments

5 files changed, 193 insertions, 156 deletions

diff --git a/tests/conftest.py b/tests/conftest.py
index aa17c0e..edf9f28 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -14,7 +14,7 @@ from functools import wraps
 from importlib import import_module
 from logging import basicConfig, getLogger
 from pathlib import Path
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 from uuid import uuid4
 
 import pytest
@@ -268,3 +268,7 @@ def skip_missing_deps(module_name: str) -> pytest.Mark:
     return pytest.mark.skipif(
         not is_installed(module_name), reason=f'{module_name} is not installed'
     )
+
+
+# Some tests must disable url normalization to retain the custom `http+mock://` protocol
+patch_normalize_url = patch('requests_cache.cache_keys.normalize_url', side_effect=lambda x, y: x)
diff --git a/tests/integration/base_cache_test.py b/tests/integration/base_cache_test.py
index 8282452..6f63197 100644
--- a/tests/integration/base_cache_test.py
+++ b/tests/integration/base_cache_test.py
@@ -285,7 +285,7 @@ class BaseCacheTest:
         for i in range(5):
             assert session.post(httpbin('post'), files={'file1': BytesIO(b'10' * 1024)}).from_cache
 
-    def test_remove_expired_responses(self):
+    def test_remove_expired(self):
         session = self.init_session(expire_after=1)
 
         # Populate the cache with several responses that should expire immediately
@@ -298,13 +298,21 @@ class BaseCacheTest:
         session.get(httpbin('get'), expire_after=-1)
         session.get(httpbin('redirect/3'), expire_after=-1)
         assert len(session.cache.redirects.keys()) == 4
-        session.cache.remove_expired_responses()
+        session.cache.remove(expired=True)
 
         assert len(session.cache.responses.keys()) == 2
         assert len(session.cache.redirects.keys()) == 3
         assert not session.cache.has_url(httpbin('redirect/1'))
         assert not any([session.cache.has_url(httpbin(f)) for f in HTTPBIN_FORMATS])
 
+    def test_bulk_delete__noop(self):
+        """Just make sure bulk_delete doesn't do anything unexpected if no keys are provided"""
+        session = self.init_session()
+        for i in range(100):
+            session.cache.responses[f'key_{i}'] = f'value_{i}'
+        session.cache.bulk_delete([])
+        assert len(session.cache.responses) == 100
+
     @pytest.mark.parametrize('method', HTTPBIN_METHODS)
     def test_filter_request_headers(self, method):
         url = httpbin(method.lower())
diff --git a/tests/unit/test_base_cache.py b/tests/unit/test_base_cache.py
index a39b238..6e68ea5 100644
--- a/tests/unit/test_base_cache.py
+++ b/tests/unit/test_base_cache.py
@@ -1,14 +1,24 @@
 """BaseCache tests that use mocked responses only"""
 from datetime import datetime, timedelta
+from logging import getLogger
+from pickle import PickleError
+from time import sleep
 from unittest.mock import patch
 
 import pytest
 
-from requests_cache import CachedResponse
 from requests_cache.backends import BaseCache, SQLiteDict
-from tests.conftest import MOCKED_URL, MOCKED_URL_HTTPS, MOCKED_URL_JSON, MOCKED_URL_REDIRECT
+from requests_cache.models import CachedRequest, CachedResponse
+from tests.conftest import (
+    MOCKED_URL,
+    MOCKED_URL_HTTPS,
+    MOCKED_URL_JSON,
+    MOCKED_URL_REDIRECT,
+    patch_normalize_url,
+)
 
 YESTERDAY = datetime.utcnow() - timedelta(days=1)
+logger = getLogger(__name__)
 
 
 class TimeBomb:
@@ -40,17 +50,6 @@ def test_keys(mock_session):
     assert set(mock_session.cache.keys()) == all_keys
 
 
-def test_update(mock_session):
-    src_cache = BaseCache()
-    for i in range(20):
-        src_cache.responses[f'key_{i}'] = f'value_{i}'
-        src_cache.redirects[f'key_{i}'] = f'value_{i}'
-
-    mock_session.cache.update(src_cache)
-    assert len(mock_session.cache.responses) == 20
-    assert len(mock_session.cache.redirects) == 20
-
-
 def test_values(mock_session):
     for url in [MOCKED_URL, MOCKED_URL_JSON, MOCKED_URL_HTTPS]:
         mock_session.get(url)
@@ -60,23 +59,23 @@ def test_values(mock_session):
     assert all([isinstance(response, CachedResponse) for response in responses])
 
 
-@pytest.mark.parametrize('check_expiry, expected_count', [(True, 1), (False, 2)])
-def test_values__with_invalid_responses(check_expiry, expected_count, mock_session):
+@pytest.mark.parametrize('include_expired, expected_count', [(False, 1), (True, 2)])
+def test_values__with_invalid_responses(include_expired, expected_count, mock_session):
     """values() should always exclude invalid responses, and optionally exclude expired responses"""
     responses = [mock_session.get(url) for url in [MOCKED_URL, MOCKED_URL_JSON, MOCKED_URL_HTTPS]]
     responses[1] = AttributeError
     responses[2] = CachedResponse(expires=YESTERDAY, url='test')
 
     with patch.object(SQLiteDict, '__getitem__', side_effect=responses):
-        values = mock_session.cache.values(check_expiry=check_expiry)
+        values = mock_session.cache.values(include_expired=include_expired)
         assert len(list(values)) == expected_count
 
     # The invalid response should be skipped, but remain in the cache for now
     assert len(mock_session.cache.responses.keys()) == 3
 
 
-@pytest.mark.parametrize('check_expiry, expected_count', [(True, 2), (False, 3)])
-def test_response_count(check_expiry, expected_count, mock_session):
+@pytest.mark.parametrize('include_expired, expected_count', [(False, 2), (True, 3)])
+def test_response_count(include_expired, expected_count, mock_session):
     """response_count() should always exclude invalid responses, and optionally exclude expired
     and invalid responses"""
     mock_session.get(MOCKED_URL)
@@ -84,7 +83,144 @@ def test_response_count(check_expiry, expected_count, mock_session):
 
     mock_session.cache.responses['expired_response'] = CachedResponse(expires=YESTERDAY)
     mock_session.cache.responses['invalid_response'] = TimeBomb()
-    assert mock_session.cache.response_count(check_expiry=check_expiry) == expected_count
+    assert mock_session.cache.response_count(include_expired=include_expired) == expected_count
+
+
+@patch_normalize_url
+def test_remove__expired(mock_normalize_url, mock_session):
+    """Test BaseCache.remove_expired_responses()"""
+    unexpired_url = f'{MOCKED_URL}?x=1'
+    mock_session.mock_adapter.register_uri(
+        'GET', unexpired_url, status_code=200, text='mock response'
+    )
+    mock_session.settings.expire_after = 1
+    mock_session.get(MOCKED_URL)
+    mock_session.get(MOCKED_URL_JSON)
+    sleep(1)
+    mock_session.get(unexpired_url)
+
+    # At this point we should have 1 unexpired response and 2 expired responses
+    assert len(mock_session.cache.responses) == 3
+
+    # Use the generic BaseCache implementation, not the SQLite-specific one
+    BaseCache.remove(mock_session.cache, expired=True)
+    assert len(mock_session.cache.responses) == 1
+    cached_response = list(mock_session.cache.responses.values())[0]
+    assert cached_response.url == unexpired_url
+
+    # Now the last response should be expired as well
+    sleep(1)
+    BaseCache.remove(mock_session.cache, expired=True)
+    assert len(mock_session.cache.responses) == 0
+
+
+def test_remove__error(mock_session):
+    # Start with two cached responses, one of which will raise an error
+    response_1 = mock_session.get(MOCKED_URL)
+    response_2 = mock_session.get(MOCKED_URL_JSON)
+
+    def error_on_key(key):
+        if key == response_2.cache_key:
+            raise PickleError
+        return CachedResponse.from_response(response_1)
+
+    # Use the generic BaseCache implementation, not the SQLite-specific one
+    with patch.object(SQLiteDict, '__getitem__', side_effect=error_on_key):
+        BaseCache.remove(mock_session.cache, expired=True)
+
+    assert len(mock_session.cache.responses) == 1
+    assert mock_session.get(MOCKED_URL).from_cache is True
+    assert mock_session.get(MOCKED_URL_JSON).from_cache is False
+
+
+def test_remove__expired__per_request(mock_session):
+    # Cache 3 responses with different expiration times
+    second_url = f'{MOCKED_URL}/endpoint_2'
+    third_url = f'{MOCKED_URL}/endpoint_3'
+    mock_session.mock_adapter.register_uri('GET', second_url, status_code=200)
+    mock_session.mock_adapter.register_uri('GET', third_url, status_code=200)
+    mock_session.get(MOCKED_URL)
+    mock_session.get(second_url, expire_after=2)
+    mock_session.get(third_url, expire_after=4)
+
+    # All 3 responses should still be cached
+    mock_session.cache.remove(expired=True)
+    for response in mock_session.cache.responses.values():
+        logger.info(f'Expires in {response.expires_delta} seconds')
+    assert len(mock_session.cache.responses) == 3
+
+    # One should be expired after 2s, and another should be expired after 4s
+    sleep(2)
+    mock_session.cache.remove(expired=True)
+    assert len(mock_session.cache.responses) == 2
+    sleep(2)
+    mock_session.cache.remove(expired=True)
+    assert len(mock_session.cache.responses) == 1
+
+
+def test_remove__older_than(mock_session):
+    # Cache 4 responses with different creation times
+    response_0 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_0'))
+    mock_session.cache.save_response(response_0)
+    response_1 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_1'))
+    response_1.created_at -= timedelta(seconds=1)
+    mock_session.cache.save_response(response_1)
+    response_2 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_2'))
+    response_2.created_at -= timedelta(seconds=2)
+    mock_session.cache.save_response(response_2)
+    response_3 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_3'))
+    response_3.created_at -= timedelta(seconds=3)
+    mock_session.cache.save_response(response_3)
+
+    # Incrementally remove responses older than 3, 2, and 1 seconds
+    assert len(mock_session.cache.responses) == 4
+    mock_session.cache.remove(older_than=timedelta(seconds=3))
+    assert len(mock_session.cache.responses) == 3
+    mock_session.cache.remove(older_than=timedelta(seconds=2))
+    assert len(mock_session.cache.responses) == 2
+    mock_session.cache.remove(older_than=timedelta(seconds=1))
+    assert len(mock_session.cache.responses) == 1
+
+    # Remove the last response after it's 1 second old
+    sleep(1)
+    mock_session.cache.remove(older_than=timedelta(seconds=1))
+    assert len(mock_session.cache.responses) == 0
+
+
+def test_remove_expired_responses(mock_session):
+    """Test for backwards-compatibility"""
+    with patch.object(mock_session.cache, 'remove') as mock_remove, patch.object(
+        mock_session.cache, 'reset_expiration'
+    ) as mock_reset:
+        mock_session.cache.remove_expired_responses(expire_after=1)
+        mock_remove.assert_called_once_with(expired=True, invalid=True)
+        mock_reset.assert_called_once_with(1)
+
+        mock_session.cache.remove_expired_responses()
+        assert mock_remove.call_count == 2 and mock_reset.call_count == 1
+
+
+def test_reset_expiration__extend_expiration(mock_session):
+    # Start with an expired response
+    mock_session.settings.expire_after = datetime.utcnow() - timedelta(seconds=0.01)
+    mock_session.get(MOCKED_URL)
+
+    # Set expiration in the future
+    mock_session.cache.reset_expiration(datetime.utcnow() + timedelta(seconds=1))
+    assert len(mock_session.cache.responses) == 1
+    response = mock_session.get(MOCKED_URL)
+    assert response.is_expired is False and response.from_cache is True
+
+
+def test_reset_expiration__shorten_expiration(mock_session):
+    # Start with a non-expired response
+    mock_session.settings.expire_after = datetime.utcnow() + timedelta(seconds=1)
+    mock_session.get(MOCKED_URL)
+
+    # Set expiration in the past
+    mock_session.cache.reset_expiration(datetime.utcnow() - timedelta(seconds=0.01))
+    response = mock_session.get(MOCKED_URL)
+    assert response.is_expired is False and response.from_cache is False
 
 
 def test_clear(mock_session):
@@ -147,7 +283,18 @@ def test_delete_urls(mock_session):
         assert not mock_session.cache.has_url(MOCKED_URL_REDIRECT)
 
 
-def test_save_response_manual(mock_session):
+def test_save_response__manual(mock_session):
     response = mock_session.get(MOCKED_URL)
     mock_session.cache.clear()
     mock_session.cache.save_response(response)
+
+
+def test_update(mock_session):
+    src_cache = BaseCache()
+    for i in range(20):
+        src_cache.responses[f'key_{i}'] = f'value_{i}'
+        src_cache.redirects[f'key_{i}'] = f'value_{i}'
+
+    mock_session.cache.update(src_cache)
+    assert len(mock_session.cache.responses) == 20
+    assert len(mock_session.cache.redirects) == 20
diff --git a/tests/unit/test_patcher.py b/tests/unit/test_patcher.py
index 23904d1..d656b19 100644
--- a/tests/unit/test_patcher.py
+++ b/tests/unit/test_patcher.py
@@ -81,15 +81,15 @@ def test_is_installed():
     assert requests_cache.is_installed() is False
 
 
-@patch.object(BaseCache, 'remove_expired_responses')
-def test_remove_expired_responses(remove_expired_responses):
+@patch.object(BaseCache, 'remove')
+def test_remove_expired_responses(mock_remove):
     requests_cache.install_cache(backend='memory', expire_after=360)
     requests_cache.remove_expired_responses()
-    assert remove_expired_responses.called is True
+    assert mock_remove.called is True
     requests_cache.uninstall_cache()
 
 
-@patch.object(BaseCache, 'remove_expired_responses')
-def test_remove_expired_responses__cache_not_installed(remove_expired_responses):
+@patch.object(BaseCache, 'remove')
+def test_remove_expired_responses__cache_not_installed(mock_remove):
     requests_cache.remove_expired_responses()
-    assert remove_expired_responses.called is False
+    assert mock_remove.called is False
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index 3a927af..2354ea3 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -18,7 +18,6 @@ from requests_cache import ALL_METHODS, CachedSession
 from requests_cache._utils import get_placeholder_class
 from requests_cache.backends import BACKEND_CLASSES, BaseCache, SQLiteDict
 from requests_cache.backends.base import DESERIALIZE_ERRORS
-from requests_cache.models import CachedRequest, CachedResponse
 from requests_cache.policy.expiration import DO_NOT_CACHE, EXPIRE_IMMEDIATELY, NEVER_EXPIRE
 from tests.conftest import (
     MOCKED_URL,
@@ -29,11 +28,9 @@ from tests.conftest import (
     MOCKED_URL_JSON,
     MOCKED_URL_REDIRECT,
     MOCKED_URL_REDIRECT_TARGET,
+    patch_normalize_url,
 )
 
-# Some tests must disable url normalization to retain the custom `http+mock//` protocol
-patch_normalize_url = patch('requests_cache.cache_keys.normalize_url', side_effect=lambda x, y: x)
-
 logger = getLogger(__name__)
 
 # Basic initialization
@@ -633,129 +630,10 @@ def test_url_allowlist(mock_session):
     assert not mock_session.cache.has_url(MOCKED_URL)
 
 
-@patch_normalize_url
-def test_remove_expired_responses(mock_normalize_url, mock_session):
-    """Test BaseCache.remove_expired_responses()"""
-    unexpired_url = f'{MOCKED_URL}?x=1'
-    mock_session.mock_adapter.register_uri(
-        'GET', unexpired_url, status_code=200, text='mock response'
-    )
-    mock_session.settings.expire_after = 1
-    mock_session.get(MOCKED_URL)
-    mock_session.get(MOCKED_URL_JSON)
-    sleep(1)
-    mock_session.get(unexpired_url)
-
-    # At this point we should have 1 unexpired response and 2 expired responses
-    assert len(mock_session.cache.responses) == 3
-    BaseCache.remove_expired_responses(mock_session.cache)
-    assert len(mock_session.cache.responses) == 1
-    cached_response = list(mock_session.cache.responses.values())[0]
-    assert cached_response.url == unexpired_url
-
-    # Now the last response should be expired as well
-    sleep(1)
-    BaseCache.remove_expired_responses(mock_session.cache)
-    assert len(mock_session.cache.responses) == 0
-
-
-def test_remove_expired_responses__error(mock_session):
-    # Start with two cached responses, one of which will raise an error
-    response_1 = mock_session.get(MOCKED_URL)
-    response_2 = mock_session.get(MOCKED_URL_JSON)
-
-    def error_on_key(key):
-        if key == response_2.cache_key:
-            raise PickleError
-        return CachedResponse.from_response(response_1)
-
-    # Test the generic BaseCache implementation, not the SQLite-specific one
-    with patch.object(SQLiteDict, '__getitem__', side_effect=error_on_key):
-        BaseCache.remove_expired_responses(mock_session.cache)
-
-    assert len(mock_session.cache.responses) == 1
-    assert mock_session.get(MOCKED_URL).from_cache is True
-    assert mock_session.get(MOCKED_URL_JSON).from_cache is False
-
-
-def test_remove_expired_responses__extend_expiration(mock_session):
-    # Start with an expired response
-    mock_session.settings.expire_after = datetime.utcnow() - timedelta(seconds=0.01)
-    mock_session.get(MOCKED_URL)
-
-    # Set expiration in the future
-    BaseCache.remove_expired_responses(
-        mock_session.cache, expire_after=datetime.utcnow() + timedelta(seconds=1)
-    )
-    assert len(mock_session.cache.responses) == 1
-    response = mock_session.get(MOCKED_URL)
-    assert response.is_expired is False and response.from_cache is True
-
-
-def test_remove_expired_responses__shorten_expiration(mock_session):
-    # Start with a non-expired response
-    mock_session.settings.expire_after = datetime.utcnow() + timedelta(seconds=1)
-    mock_session.get(MOCKED_URL)
-
-    # Set expiration in the past
-    mock_session.remove_expired_responses(expire_after=datetime.utcnow() - timedelta(seconds=0.01))
-    assert len(mock_session.cache.responses) == 0
-    response = mock_session.get(MOCKED_URL)
-    assert response.is_expired is False and response.from_cache is False
-
-
-def test_remove_expired_responses__per_request(mock_session):
-    # Cache 3 responses with different expiration times
-    second_url = f'{MOCKED_URL}/endpoint_2'
-    third_url = f'{MOCKED_URL}/endpoint_3'
-    mock_session.mock_adapter.register_uri('GET', second_url, status_code=200)
-    mock_session.mock_adapter.register_uri('GET', third_url, status_code=200)
-    mock_session.get(MOCKED_URL)
-    mock_session.get(second_url, expire_after=2)
-    mock_session.get(third_url, expire_after=4)
-
-    # All 3 responses should still be cached
-    mock_session.remove_expired_responses()
-    for response in mock_session.cache.responses.values():
-        logger.info(f'Expires in {response.expires_delta} seconds')
-    assert len(mock_session.cache.responses) == 3
-
-    # One should be expired after 2s, and another should be expired after 4s
-    sleep(2)
-    mock_session.remove_expired_responses()
-    assert len(mock_session.cache.responses) == 2
-    sleep(2)
-    mock_session.remove_expired_responses()
-    assert len(mock_session.cache.responses) == 1
-
-
-def test_remove_expired_responses__older_than(mock_session):
-    # Cache 4 responses with different creation times
-    response_0 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_0'))
-    mock_session.cache.save_response(response_0)
-    response_1 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_1'))
-    response_1.created_at -= timedelta(seconds=1)
-    mock_session.cache.save_response(response_1)
-    response_2 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_2'))
-    response_2.created_at -= timedelta(seconds=2)
-    mock_session.cache.save_response(response_2)
-    response_3 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_3'))
-    response_3.created_at -= timedelta(seconds=3)
-    mock_session.cache.save_response(response_3)
-
-    # Incrementally remove responses older than 3, 2, and 1 seconds
-    assert len(mock_session.cache.responses) == 4
-    mock_session.remove_expired_responses(older_than=timedelta(seconds=3))
-    assert len(mock_session.cache.responses) == 3
-    mock_session.remove_expired_responses(older_than=timedelta(seconds=2))
-    assert len(mock_session.cache.responses) == 2
-    mock_session.remove_expired_responses(older_than=timedelta(seconds=1))
-    assert len(mock_session.cache.responses) == 1
-
-    # Remove the last response after it's 1 second old
-    sleep(1)
-    mock_session.remove_expired_responses(older_than=timedelta(seconds=1))
-    assert len(mock_session.cache.responses) == 0
+def test_remove_expired_responses(mock_session):
+    with patch.object(mock_session.cache, 'remove') as mock_remove:
+        mock_session.remove_expired_responses()
+        mock_remove.assert_called_once_with(expired=True, invalid=True)
 
 
 def test_stale_while_revalidate(mock_session):