From bd0bc75c8f49c637c20f9674de907f3e97dbd2ec Mon Sep 17 00:00:00 2001 From: Alessio <148966056+alessio-locatelli@users.noreply.github.com> Date: Thu, 7 Nov 2024 21:16:39 +0200 Subject: [PATCH] Fix `CachedResposnse.read()` and `ClientResponse.read()` consistency (#279) * fix: make 'read()' consistent with 'ClientResponse.read()' * test: extend test_streaming_requests --- HISTORY.md | 1 + aiohttp_client_cache/response.py | 6 ++-- test/integration/base_backend_test.py | 42 +++++++++++++++++---------- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index f514e896..aa7b9850 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,7 @@ ## (unreleased) +- Fixed `CachedResponse.read()` to be consistent with `ClientResponse.read()` by allowing to call `read()` multiple times. (#289) - Now a warning is raised when a cache backend is accessed after disconnecting (after exiting the `CachedSession` context manager). (#241) ## 0.12.4 (2024-10-30) diff --git a/aiohttp_client_cache/response.py b/aiohttp_client_cache/response.py index 0338a52a..6c8128f0 100644 --- a/aiohttp_client_cache/response.py +++ b/aiohttp_client_cache/response.py @@ -69,7 +69,7 @@ class CachedResponse(HeadersMixin): status: int = attr.ib() url: URL = attr.ib(converter=URL) version: str = attr.ib() - _body: Any = attr.ib(default=b'') + _body: Any = attr.ib(default=None) _content: StreamReader | None = attr.ib(default=None) _links: LinkItems = attr.ib(factory=list) cookies: SimpleCookie = attr.ib(factory=SimpleCookie) @@ -215,7 +215,9 @@ def raise_for_status(self): async def read(self) -> bytes: """Read response payload.""" - return await self.content.read() + # We are ready to return the body because `read()` + # was called on a `CachedResponse` creation. + return self._body def reset(self): """Reset the stream reader to re-read a streamed response""" diff --git a/test/integration/base_backend_test.py b/test/integration/base_backend_test.py index 111fd055..d48f88a2 100644 --- a/test/integration/base_backend_test.py +++ b/test/integration/base_backend_test.py @@ -5,6 +5,7 @@ import asyncio import pickle from contextlib import asynccontextmanager + from test.conftest import ( ALL_METHODS, CACHE_NAME, @@ -174,28 +175,37 @@ async def test_include_headers(self): async def test_streaming_requests(self): """Test that streaming requests work both for the original and cached responses""" async with self.init_session() as session: - for _ in range(2): - response = cast(CachedResponse, await session.get(httpbin('stream-bytes/64'))) + for i in range(2): + any_response = await session.get(httpbin('stream-bytes/64')) + + # Can read multiple times. + assert {len(await any_response.read()) for _ in range(3)} == {64} + + if i == 0: + continue + + response = cast(CachedResponse, any_response) + lines = [line async for line in response.content] assert len(b''.join(lines)) == 64 - # Test some additional methods on the cached response - response.reset() - chunks = [c async for (c, _) in response.content.iter_chunks()] - assert len(b''.join(chunks)) == 64 - response.reset() + # Test some additional methods on the cached response + response.reset() + chunks = [c async for (c, _) in response.content.iter_chunks()] + assert len(b''.join(chunks)) == 64 + response.reset() - chunks = [c async for c in response.content.iter_chunked(2)] - assert len(b''.join(chunks)) == 64 - response.reset() + chunks = [c async for c in response.content.iter_chunked(2)] + assert len(b''.join(chunks)) == 64 + response.reset() - chunks = [c async for c in response.content.iter_any()] - assert len(b''.join(chunks)) == 64 - response.reset() + chunks = [c async for c in response.content.iter_any()] + assert len(b''.join(chunks)) == 64 + response.reset() - # readany() should return empty bytes after being consumed - assert len(await response.content.readany()) == 64 - assert await response.content.readany() == b'' + # readany() should return empty bytes after being consumed + assert len(await response.content.readany()) == 64 + assert await response.content.readany() == b'' async def test_streaming_request__ignored(self): """If a streaming request is filtered out (expire_after=0), its body should be readable as usual"""