Close database connections (if applicable) on CachedSession.__aexit__

HISTORY.md

@@ -1,5 +1,8 @@
 # History
 
+## 0.8.1 (2023-01-04)
+* For SQLite backend, close database connection on `ClientSession` context exit
+
 ## 0.8.0 (2022-12-29)
 * Lazily initialize and reuse SQLite connection objects
 * Fix `AttributeError` when using a response cached with an older version of `attrs`

aiohttp_client_cache/__init__.py

@@ -1,4 +1,4 @@
-__version__ = '0.8.0'
+__version__ = '0.8.1'
 
 # flake8: noqa: F401, F403
 try:

aiohttp_client_cache/backends/base.py

@@ -244,6 +244,8 @@ async def get_urls(self) -> AsyncIterable[str]:
 
     async def close(self):
         """Close any active connections, if applicable"""
+        await self.responses.close()
+        await self.redirects.close()
 
 
 # TODO: Support yarl.URL like aiohttp does?
@@ -303,6 +305,9 @@ async def contains(self, key: str) -> bool:
     async def clear(self):
         """Delete all items from the cache"""
 
+    async def close(self):
+        """Close any active connections, if applicable"""
+
     @abstractmethod
     async def delete(self, key: str):
         """Delete an item from the cache. Does not raise an error if the item is missing."""

aiohttp_client_cache/backends/sqlite.py

@@ -45,9 +45,6 @@ def __init__(
         )
         self.redirects = SQLiteCache(cache_name, 'redirects', use_temp=use_temp, **kwargs)
 
-    async def close(self):
-        await self.responses.close()
-
 
 class SQLiteCache(BaseCache):
     """An async interface for caching objects in a SQLite database.

aiohttp_client_cache/session.py

@@ -62,6 +62,11 @@ async def _request(
                 await self.cache.save_response(new_response, actions.key, actions.expires)
             return set_response_defaults(new_response)
 
+    async def close(self):
+        """Close both aiohttp connector and any backend connection(s) on contextmanager exit"""
+        await super().close()
+        await self.cache.close()
+
     @asynccontextmanager
     async def disabled(self):
         """Temporarily disable the cache

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "aiohttp-client-cache"
-version = "0.8.0"
+version = "0.8.1"
 description = "Persistent cache for aiohttp requests"
 authors = ["Jordan Cook"]
 license = "MIT License"

test/integration/base_backend_test.py

@@ -41,7 +41,6 @@ async def init_session(self, clear=True, **kwargs) -> AsyncIterator[CachedSessio
 
         async with CachedSession(cache=cache, **self.init_kwargs, **kwargs) as session:
             yield session
-            await session.cache.close()
 
     @pytest.mark.parametrize('method', HTTPBIN_METHODS)
     @pytest.mark.parametrize('field', ['params', 'data', 'json'])

test/integration/base_storage_test.py

@@ -1,5 +1,6 @@
+from contextlib import asynccontextmanager
 from datetime import datetime
-from typing import Dict, Type
+from typing import Any, AsyncIterator, Dict, Type
 
 import pytest
 
@@ -17,82 +18,84 @@ class BaseStorageTest:
     init_kwargs: Dict = {}
     picklable: bool = False
     storage_class: Type[BaseCache] = None  # type: ignore
-    test_data = picklable_test_data
+    test_data: Dict[str, Any] = picklable_test_data
 
-    async def init_cache(self, index=0, **kwargs):
-        self.test_data = picklable_test_data if self.picklable else str_test_data
+    @asynccontextmanager
+    async def init_cache(self, index=0, **kwargs) -> AsyncIterator[BaseCache]:
+        self.test_data = picklable_test_data if self.picklable else str_test_data  # type: ignore
         cache = self.storage_class(CACHE_NAME, f'table_{index}', **self.init_kwargs, **kwargs)
         await cache.clear()
-        return cache
+        yield cache
+        await cache.close()
 
     async def test_write_read(self):
-        cache = await self.init_cache()
-        # Test write() and contains()
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
-            assert await cache.contains(k) is True
+        async with self.init_cache() as cache:
+            # Test write() and contains()
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
+                assert await cache.contains(k) is True
 
-        # Test read()
-        for k, v in self.test_data.items():
-            assert await cache.read(k) == v
+            # Test read()
+            for k, v in self.test_data.items():
+                assert await cache.read(k) == v
 
     async def test_missing_key(self):
-        cache = await self.init_cache()
-        assert await cache.contains('nonexistent_key') is False
-        assert await cache.read('nonexistent_key') is None
+        async with self.init_cache() as cache:
+            assert await cache.contains('nonexistent_key') is False
+            assert await cache.read('nonexistent_key') is None
 
     async def test_delete(self):
-        cache = await self.init_cache()
-        await cache.write('do_not_delete', 'value')
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
+        async with self.init_cache() as cache:
+            await cache.write('do_not_delete', 'value')
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
 
-        for k in self.test_data.keys():
-            await cache.delete(k)
-            assert await cache.contains(k) is False
+            for k in self.test_data.keys():
+                await cache.delete(k)
+                assert await cache.contains(k) is False
 
-        assert await cache.read('do_not_delete') == 'value'
+            assert await cache.read('do_not_delete') == 'value'
 
     async def test_bulk_delete(self):
-        cache = await self.init_cache()
-        await cache.write('do_not_delete', 'value')
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
+        async with self.init_cache() as cache:
+            await cache.write('do_not_delete', 'value')
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
 
-        await cache.bulk_delete(self.test_data.keys())
+            await cache.bulk_delete(self.test_data.keys())
 
-        for k in self.test_data.keys():
-            assert await cache.contains(k) is False
+            for k in self.test_data.keys():
+                assert await cache.contains(k) is False
 
     async def test_bulk_delete_ignores_nonexistent_keys(self):
-        cache = await self.init_cache()
-        await cache.bulk_delete(self.test_data.keys())
+        async with self.init_cache() as cache:
+            await cache.bulk_delete(self.test_data.keys())
 
     async def test_keys_values(self):
-        cache = await self.init_cache()
-        assert [k async for k in cache.keys()] == []
-        assert [v async for v in cache.values()] == []
+        async with self.init_cache() as cache:
+            assert [k async for k in cache.keys()] == []
+            assert [v async for v in cache.values()] == []
 
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
 
-        assert {k async for k in cache.keys()} == set(self.test_data.keys())
-        assert {v async for v in cache.values()} == set(self.test_data.values())
+            assert {k async for k in cache.keys()} == set(self.test_data.keys())
+            assert {v async for v in cache.values()} == set(self.test_data.values())
 
     async def test_size(self):
-        cache = await self.init_cache()
-        assert await cache.size() == 0
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
+        async with self.init_cache() as cache:
+            assert await cache.size() == 0
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
 
         assert await cache.size() == len(self.test_data)
 
     async def test_clear(self):
-        cache = await self.init_cache()
-        for k, v in self.test_data.items():
-            await cache.write(k, v)
-
-        await cache.clear()
-        assert await cache.size() == 0
-        assert {k async for k in cache.keys()} == set()
-        assert {v async for v in cache.values()} == set()
+        async with self.init_cache() as cache:
+            for k, v in self.test_data.items():
+                await cache.write(k, v)
+
+            await cache.clear()
+            assert await cache.size() == 0
+            assert {k async for k in cache.keys()} == set()
+            assert {v async for v in cache.values()} == set()

test/integration/test_filesystem.py

@@ -1,9 +1,12 @@
+from contextlib import asynccontextmanager
 from os.path import isfile
 from shutil import rmtree
 from tempfile import gettempdir
+from typing import AsyncIterator
 
 import pytest
 
+from aiohttp_client_cache.backends.base import BaseCache
 from aiohttp_client_cache.backends.filesystem import FileBackend, FileCache
 from test.conftest import CACHE_NAME
 from test.integration import BaseBackendTest, BaseStorageTest
@@ -13,10 +16,12 @@ class TestFileCache(BaseStorageTest):
     storage_class = FileCache
     picklable = True
 
-    async def init_cache(self, index=0, **kwargs):
+    @asynccontextmanager
+    async def init_cache(self, index=0, **kwargs) -> AsyncIterator[BaseCache]:
         cache = self.storage_class(f'{CACHE_NAME}_{index}', use_temp=True, **kwargs)
         await cache.clear()
-        return cache
+        yield cache
+        await cache.close()
 
     @classmethod
     def teardown_class(cls):
@@ -29,13 +34,13 @@ async def test_use_temp(self):
         assert temp_path.startswith(gettempdir())
 
     async def test_paths(self):
-        cache = await self.init_cache()
-        for i in range(10):
-            await cache.write(f'key_{i}', f'value_{i}')
+        async with self.init_cache() as cache:
+            for i in range(10):
+                await cache.write(f'key_{i}', f'value_{i}')
 
-        assert len([p async for p in cache.paths()]) == 10
-        async for path in cache.paths():
-            assert isfile(path)
+            assert len([p async for p in cache.paths()]) == 10
+            async for path in cache.paths():
+                assert isfile(path)
 
     # TODO
     async def test_write_error(self):

test/integration/test_mongodb.py

@@ -28,11 +28,11 @@ class TestMongoDBCache(BaseStorageTest):
     async def test_values_many(self):
         # If some entries are missing the "data" field for some reason, they
         # should not be returned with the results.
-        cache = await self.init_cache()
-        await cache.collection.insert_many({"data": f'value_{i}'} for i in range(10))
-        await cache.collection.insert_many({"not_data": f'value_{i}'} for i in range(10))
-        actual_results = [v async for v in cache.values()]
-        assert actual_results == [f'value_{i}' for i in range(10)]
+        async with self.init_cache() as cache:
+            await cache.collection.insert_many({"data": f'value_{i}'} for i in range(10))
+            await cache.collection.insert_many({"not_data": f'value_{i}'} for i in range(10))
+            actual_results = [v async for v in cache.values()]
+            assert actual_results == [f'value_{i}' for i in range(10)]
 
 
 class TestMongoDBPickleCache(TestMongoDBCache):

test/integration/test_sqlite.py

@@ -30,19 +30,19 @@ def test_use_temp(self):
         assert temp_path.startswith(gettempdir())
 
     async def test_bulk_commit(self):
-        cache = await self.init_cache()
-        async with cache.bulk_commit():
-            pass
+        async with self.init_cache() as cache:
+            async with cache.bulk_commit():
+                pass
 
-        n_items = 1000
-        async with cache.bulk_commit():
-            for i in range(n_items):
-                await cache.write(f'key_{i}', f'value_{i}')
+            n_items = 1000
+            async with cache.bulk_commit():
+                for i in range(n_items):
+                    await cache.write(f'key_{i}', f'value_{i}')
 
-        keys = {k async for k in cache.keys()}
-        values = {v async for v in cache.values()}
-        assert keys == {f'key_{i}' for i in range(n_items)}
-        assert values == {f'value_{i}' for i in range(n_items)}
+            keys = {k async for k in cache.keys()}
+            values = {v async for v in cache.values()}
+            assert keys == {f'key_{i}' for i in range(n_items)}
+            assert values == {f'value_{i}' for i in range(n_items)}
 
     @skip_37
     @patch('aiohttp_client_cache.backends.sqlite.aiosqlite')
@@ -52,33 +52,33 @@ async def test_concurrent_bulk_commit(self, mock_sqlite):
 
         mock_connection = AsyncMock()
         mock_sqlite.connect = AsyncMock(return_value=mock_connection)
-        cache = await self.init_cache()
 
-        async def bulk_commit_items(n_items):
-            async with cache.bulk_commit():
-                for i in range(n_items):
-                    await cache.write(f'key_{n_items}_{i}', f'value_{i}')
+        async with self.init_cache() as cache:
+
+            async def bulk_commit_items(n_items):
+                async with cache.bulk_commit():
+                    for i in range(n_items):
+                        await cache.write(f'key_{n_items}_{i}', f'value_{i}')
 
         assert mock_connection.commit.call_count == 1
         tasks = [asyncio.create_task(bulk_commit_items(n)) for n in [10, 100, 1000, 10000]]
         await asyncio.gather(*tasks)
         assert mock_connection.commit.call_count == 5
 
     async def test_fast_save(self):
-        cache_1 = await self.init_cache(index=1, fast_save=True)
-        cache_2 = await self.init_cache(index=2, fast_save=True)
-
-        n = 1000
-        for i in range(n):
-            await cache_1.write(i, i)
-            await cache_2.write(i, i)
-
-        keys_1 = {k async for k in cache_1.keys()}
-        keys_2 = {k async for k in cache_2.keys()}
-        values_1 = {v async for v in cache_1.values()}
-        values_2 = {v async for v in cache_2.values()}
-        assert keys_1 == keys_2 == set(range(n))
-        assert values_1 == values_2 == set(range(n))
+        async with self.init_cache(index=1, fast_save=True) as cache_1, self.init_cache(
+            index=2, fast_save=True
+        ) as cache_2:
+            for i in range(1000):
+                await cache_1.write(i, i)
+                await cache_2.write(i, i)
+
+            keys_1 = {k async for k in cache_1.keys()}
+            keys_2 = {k async for k in cache_2.keys()}
+            values_1 = {v async for v in cache_1.values()}
+            values_2 = {v async for v in cache_2.values()}
+            assert keys_1 == keys_2 == set(range(1000))
+            assert values_1 == values_2 == set(range(1000))
 
     @skip_37
     @patch('aiohttp_client_cache.backends.sqlite.aiosqlite')
@@ -87,16 +87,16 @@ async def test_connection_kwargs(self, mock_sqlite):
         from unittest.mock import AsyncMock
 
         mock_sqlite.connect = AsyncMock()
-        cache = await self.init_cache(timeout=0.5, invalid_kwarg='???')
-        mock_sqlite.connect.assert_called_with(cache.filename, timeout=0.5)
+        async with self.init_cache(timeout=0.5, invalid_kwarg='???') as cache:
+            mock_sqlite.connect.assert_called_with(cache.filename, timeout=0.5)
 
     async def test_close(self):
-        cache = await self.init_cache()
-        async with cache.get_connection():
-            pass
-        await cache.close()
-        await cache.close()  # Closing again should be a no-op
-        assert cache._connection is None
+        async with self.init_cache() as cache:
+            async with cache.get_connection():
+                pass
+            await cache.close()
+            await cache.close()  # Closing again should be a no-op
+            assert cache._connection is None
 
     # TODO: Tests for unimplemented features
     # async def test_chunked_bulk_delete(self):