Make use of index with SQLiteCache.filter(expired=False)

4 files changed, 41 insertions, 30 deletions

diff --git a/requests_cache/backends/base.py b/requests_cache/backends/base.py
index 825cd55..efbd214 100644
--- a/requests_cache/backends/base.py
+++ b/requests_cache/backends/base.py
@@ -292,8 +292,11 @@ class BaseCache:
             DeprecationWarning,
         )
         yield from self.redirects.keys()
-        for response in self.filter(expired=not check_expiry):
-            yield response.cache_key
+        if not check_expiry:
+            yield from self.responses.keys()
+        else:
+            for response in self.filter(expired=False):
+                yield response.cache_key
 
     def response_count(self, check_expiry: bool = False) -> int:
         warn(
diff --git a/requests_cache/backends/sqlite.py b/requests_cache/backends/sqlite.py
index 907e027..f936a77 100644
--- a/requests_cache/backends/sqlite.py
+++ b/requests_cache/backends/sqlite.py
@@ -19,6 +19,7 @@ from platformdirs import user_cache_dir
 
 from requests_cache.backends.base import VT
 from requests_cache.models.response import CachedResponse
+from requests_cache.policy import ExpirationTime
 
 from .._utils import chunkify, get_valid_kwargs
 from . import BaseCache, BaseStorage
@@ -56,8 +57,8 @@ class SQLiteCache(BaseCache):
         return self.responses.db_path
 
     def clear(self):
-        """Clear the cache. If this fails due to a corrupted cache or other I/O error, this will
-        attempt to delete the cache file and re-initialize.
+        """Delete all items from the cache. If this fails due to a corrupted cache or other I/O
+        error, this will  attempt to delete the cache file and re-initialize.
         """
         try:
             super().clear()
@@ -68,13 +69,13 @@ class SQLiteCache(BaseCache):
             self.responses.init_db()
             self.redirects.init_db()
 
+    # A more efficient SQLite implementation of :py:meth:`BaseCache.delete`
     def delete(
         self,
         *keys: str,
         expired: bool = False,
         **kwargs,
     ):
-        """A more efficient SQLite implementation of :py:meth:`BaseCache.delete`"""
         if keys:
             self.responses.bulk_delete(keys)
         if expired:
@@ -118,19 +119,23 @@ class SQLiteCache(BaseCache):
         """
         return self.responses.count(expired=expired)
 
-    def filter(  # type: ignore
-        self, valid: bool = True, expired: bool = True, **kwargs
+    # A more efficient implementation of :py:meth:`BaseCache.filter` to make use of indexes
+    def filter(
+        self,
+        valid: bool = True,
+        expired: bool = True,
+        invalid: bool = False,
+        older_than: ExpirationTime = None,
     ) -> Iterator[CachedResponse]:
-        """A more efficient implementation of :py:meth:`BaseCache.filter`, in the case where we want
-        to get **only** expired responses
-        """
-        if expired and not valid and not kwargs:
-            return self.responses.sorted(expired=True)
+        if valid and not invalid:
+            return self.responses.sorted(expired=expired)
         else:
-            return super().filter(valid, expired, **kwargs)
+            return super().filter(
+                valid=valid, expired=expired, invalid=invalid, older_than=older_than
+            )
 
+    # A more efficient implementation of :py:meth:`BaseCache.recreate_keys
     def recreate_keys(self):
-        """A more efficient implementation of :py:meth:`BaseCache.recreate_keys`"""
         with self.responses.bulk_commit():
             super().recreate_keys()
 
diff --git a/tests/integration/test_sqlite.py b/tests/integration/test_sqlite.py
index 9aeaa3f..06b17cb 100644
--- a/tests/integration/test_sqlite.py
+++ b/tests/integration/test_sqlite.py
@@ -308,11 +308,15 @@ class TestSQLiteCache(BaseCacheTest):
         assert session.cache.count(expired=False) == 1
 
     @patch.object(SQLiteDict, 'sorted')
-    def test_filter__expired_only(self, mock_sorted):
-        """Filtering by expired only should use a more efficient SQL query"""
+    def test_filter__expired(self, mock_sorted):
+        """Filtering by expired should use a more efficient SQL query"""
         session = self.init_session()
-        session.cache.filter(valid=False, expired=True)
-        mock_sorted.assert_called_once_with(expired=True)
+
+        session.cache.filter()
+        mock_sorted.assert_called_with(expired=True)
+
+        session.cache.filter(expired=False)
+        mock_sorted.assert_called_with(expired=False)
 
     def test_sorted(self):
         """Test wrapper method for SQLiteDict.sorted(), with all arguments combined"""
diff --git a/tests/unit/test_base_cache.py b/tests/unit/test_base_cache.py
index 23b4e4f..b57ed09 100644
--- a/tests/unit/test_base_cache.py
+++ b/tests/unit/test_base_cache.py
@@ -9,7 +9,7 @@ from unittest.mock import patch
 import pytest
 from requests import Request
 
-from requests_cache.backends import BaseCache, SQLiteDict
+from requests_cache.backends import BaseCache, SQLiteCache, SQLiteDict
 from requests_cache.cache_keys import create_key
 from requests_cache.models import CachedRequest, CachedResponse
 from requests_cache.session import CachedSession
@@ -232,7 +232,7 @@ def test_recreate_keys__same_key_fn(mock_session):
 
 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.settings.expire_after = datetime.utcnow() - timedelta(seconds=1)
     mock_session.get(MOCKED_URL)
 
     # Set expiration in the future
@@ -248,7 +248,7 @@ def test_reset_expiration__shorten_expiration(mock_session):
     mock_session.get(MOCKED_URL)
 
     # Set expiration in the past
-    mock_session.cache.reset_expiration(datetime.utcnow() - timedelta(seconds=0.01))
+    mock_session.cache.reset_expiration(datetime.utcnow() - timedelta(seconds=1))
     response = mock_session.get(MOCKED_URL)
     assert response.is_expired is False and response.from_cache is False
 
@@ -289,8 +289,8 @@ def test_urls(mock_normalize_url, mock_session):
 
 def test_urls__error(mock_session):
     responses = [mock_session.get(url) for url in [MOCKED_URL, MOCKED_URL_JSON, MOCKED_URL_HTTPS]]
-    responses[2] = AttributeError
-    with patch.object(SQLiteDict, '__getitem__', side_effect=responses):
+    responses[2] = None
+    with patch.object(SQLiteDict, 'deserialize', side_effect=responses):
         expected_urls = [MOCKED_URL_JSON, MOCKED_URL]
         assert mock_session.cache.urls() == expected_urls
 
@@ -358,6 +358,7 @@ def test_keys(mock_session):
         response_keys = set(mock_session.cache.responses.keys())
         redirect_keys = set(mock_session.cache.redirects.keys())
         assert set(mock_session.cache.keys()) == response_keys | redirect_keys
+        assert len(list(mock_session.cache.keys(check_expiry=True))) == 5
 
 
 def test_remove_expired_responses(mock_session):
@@ -397,16 +398,14 @@ 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):
-    """values() should always exclude invalid responses, and optionally exclude expired responses"""
+def test_values__with_invalid_responses(mock_session):
     responses = [mock_session.get(url) for url in [MOCKED_URL, MOCKED_URL_JSON, MOCKED_URL_HTTPS]]
-    responses[1] = AttributeError
+    responses[1] = None
     responses[2] = CachedResponse(expires=YESTERDAY, url='test')
 
-    with ignore_deprecation(), patch.object(SQLiteDict, '__getitem__', side_effect=responses):
-        values = mock_session.cache.values(check_expiry=check_expiry)
-        assert len(list(values)) == expected_count
+    with ignore_deprecation(), patch.object(SQLiteCache, 'filter', side_effect=responses):
+        values = mock_session.cache.values(check_expiry=True)
+        assert len(list(values)) == 1
 
     # The invalid response should be skipped, but remain in the cache for now
     assert len(mock_session.cache.responses.keys()) == 3