Repository: karpetrosyan/hishel

Issue:
Request extensions are not passed through cache revalidation
I notice that cached request extensions are not passed through to the revalidation request.

For example, my original request contained `{'timeout': {'connect': 5.0, 'read': 5.0, 'write': 5.0, 'pool': 5.0}}` (which are, I believe, set by httpx according to arguments I did set), and also a few custom extensions that an intermediate layer will use for processing (but this is a detail specific to my use, let's ignore that point for now). The request triggering revalidation contains the same extensions.

I wonder if not passing through original request's extensions or the new-request-that-triggers-the-revalidation's extensions to the revalidation request is a conscious choice for a good reason or just an omission.

One of my elements of thinking is that if for whatever reason I set the timeout to 5 secs for the request, there is no good reason (I believe) that the revalidation request gets whatever default timeout it finds.

In my case (specific), I also pass some metadata through extensions that I expect to use in an intermediate transport layer that I added inbetween hishel's AsyncCacheTransport and httpx' AsyncTransport, to be able to manipulate either the request or the response when it goes through cache. Getting the metadata from extensions is kind of important, otherwise I miss a lot of identification / tracing informations there.

Thanks for your thoughts on this (of course, as before, if this is something that you think we do want in hishel, I can work on implementation and tests).

Hints from issue comments:
@karpetrosyan: Thanks for raising these questions. It is something I thought needed some tidying up. I think passing the extensions to the revalidation request would be a user experience improvement, though we may encounter some challenges here, for example:

- which extensions should be stored in the cache?
- If we decide to store all kinds of extensions, we need to somehow serialize/deserialize them

Though I fully support using the original request extensions for the revalidation request, that makes sense.@hartym: There are a few different things here, I'll write a few thoughts :

**Which extensions should be stored in the cache ?** It looks like as of today, extensions stored in the cache based on allowance lists (KNOWN_REQUEST_EXTENSIONS and KNOWN_RESPONSE_EXTENSIONS) used by the serializers. It would be quite easy to allow serializers to override this on a per-instance basis, keeping the current default as a reasonable one.

**Using the original request extensions for the revalidation request** : do we agree that the extensions we should use for the revalidation request are the one from the request that triggered the revalidation, not the one from cache ? Let's take an example : 

```
GET /foo (timeout 5sec) -> miss -> HTTP 200 -> store
...
GET /foo (timeout 10sec) -> stale, revalidate -> HTTP 200 -> store
```

Do we agree that the revalidation request should use the 10seconds timeout ?

I can work on implementing both points, I'll work on them separately though as they are unrelated. Let me know if you're ok.@karpetrosyan: > Do we agree that the revalidation request should use the 10seconds timeout ?

I think yes, it's more predictable.@hartym: PR attached (see comment within).

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index afa53fc..b4ded9a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,8 @@
 ## development
 
 - Fix cache update on revalidation response with content (rfc9111 section 4.3.3) (#239)
+- Fix request extensions that were not passed into revalidation request for transport-based implementation (but were
+  passed for the pool-based impl) (#247).
 
 ## 0.0.29 (23th June, 2024)
 
diff --git a/hishel/_async/_transports.py b/hishel/_async/_transports.py
index 4c67e2e..4955645 100644
--- a/hishel/_async/_transports.py
+++ b/hishel/_async/_transports.py
@@ -156,6 +156,7 @@ class AsyncCacheTransport(httpx.AsyncBaseTransport):
                     url=normalized_url(res.url),
                     headers=res.headers,
                     stream=AsyncCacheStream(res.stream),
+                    extensions=res.extensions,
                 )
                 try:
                     revalidation_response = await self._transport.handle_async_request(revalidation_request)
diff --git a/hishel/_sync/_transports.py b/hishel/_sync/_transports.py
index 753f07b..bbf1ac0 100644
--- a/hishel/_sync/_transports.py
+++ b/hishel/_sync/_transports.py
@@ -156,6 +156,7 @@ class CacheTransport(httpx.BaseTransport):
                     url=normalized_url(res.url),
                     headers=res.headers,
                     stream=CacheStream(res.stream),
+                    extensions=res.extensions,
                 )
                 try:
                     revalidation_response = self._transport.handle_request(revalidation_request)


Test patch:
diff --git a/tests/_async/test_pool.py b/tests/_async/test_pool.py
index 659579b..69ec13c 100644
--- a/tests/_async/test_pool.py
+++ b/tests/_async/test_pool.py
@@ -344,3 +344,58 @@ async def test_revalidation_with_new_content():
             stored_response, stored_request, stored_metadata = stored
             assert extract_header_values_decoded(stored_response.headers, b"Date") == ["Mon, 25 Aug 2015 12:00:11 GMT"]
             assert stored_response.content == b"Eat at Joe's."
+
+
+@pytest.mark.anyio
+async def test_poool_revalidation_forward_extensions():
+    class MockedClock(BaseClock):
+        current = 1440504000  # Mon, 25 Aug 2015 12:00:00 GMT
+
+        def now(self) -> int:
+            return self.current
+
+    class MockedConnectionPoolWithExtensionsMemory(hishel.MockAsyncConnectionPool):
+        async def handle_async_request(self, request: httpcore.Request) -> httpcore.Response:
+            self.last_request_extensions = request.extensions
+            return await super().handle_async_request(request)
+
+    clock = MockedClock()
+    controller = hishel.Controller(clock=clock)
+
+    async with MockedConnectionPoolWithExtensionsMemory() as pool:
+        pool.add_responses(
+            [
+                httpcore.Response(
+                    200,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:00 GMT"),
+                    ],
+                ),
+                httpcore.Response(
+                    304,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:01 GMT"),
+                    ],
+                ),
+            ]
+        )
+        async with hishel.AsyncCacheConnectionPool(
+            pool=pool, controller=controller, storage=hishel.AsyncInMemoryStorage()
+        ) as cache_pool:
+            # first request with extensions
+            await cache_pool.handle_async_request(
+                httpcore.Request("GET", "https://www.example.com", extensions={"foo": "bar"})
+            )
+            assert pool.last_request_extensions["foo"] == "bar"
+
+            # cache expires
+            clock.current += 1
+
+            # second request with extensions that should be passed to revalidation request
+            response = await cache_pool.handle_async_request(
+                httpcore.Request("GET", "https://www.example.com", extensions={"foo": "baz"})
+            )
+            assert response.extensions["revalidated"] is True
+            assert pool.last_request_extensions["foo"] == "baz"
diff --git a/tests/_async/test_transport.py b/tests/_async/test_transport.py
index a2c6f14..9ceffb0 100644
--- a/tests/_async/test_transport.py
+++ b/tests/_async/test_transport.py
@@ -373,3 +373,58 @@ async def test_revalidation_with_new_content():
             stored_response, stored_request, stored_metadata = stored
             assert extract_header_values_decoded(stored_response.headers, b"Date") == ["Mon, 25 Aug 2015 12:00:11 GMT"]
             assert stored_response.content == b"Eat at Joe's."
+
+
+@pytest.mark.anyio
+async def test_transport_revalidation_forward_extensions():
+    class MockedClock(BaseClock):
+        current = 1440504000  # Mon, 25 Aug 2015 12:00:00 GMT
+
+        def now(self) -> int:
+            return self.current
+
+    class MockedTransportWithExtensionsMemory(hishel.MockAsyncTransport):
+        async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
+            self.last_request_extensions = request.extensions
+            return await super().handle_async_request(request)
+
+    clock = MockedClock()
+    controller = hishel.Controller(clock=clock)
+
+    async with MockedTransportWithExtensionsMemory() as transport:
+        transport.add_responses(
+            [
+                httpx.Response(
+                    200,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:00 GMT"),
+                    ],
+                ),
+                httpx.Response(
+                    304,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:01 GMT"),
+                    ],
+                ),
+            ]
+        )
+        async with hishel.AsyncCacheTransport(
+            transport=transport, controller=controller, storage=hishel.AsyncInMemoryStorage()
+        ) as cache_transport:
+            # first request with extensions
+            await cache_transport.handle_async_request(
+                httpx.Request("GET", "https://www.example.com", extensions={"foo": "bar"})
+            )
+            assert transport.last_request_extensions["foo"] == "bar"
+
+            # cache expires
+            clock.current += 1
+
+            # second request with extensions that should be passed to revalidation request
+            response = await cache_transport.handle_async_request(
+                httpx.Request("GET", "https://www.example.com", extensions={"foo": "baz"})
+            )
+            assert response.extensions["revalidated"] is True
+            assert transport.last_request_extensions["foo"] == "baz"
diff --git a/tests/_sync/test_pool.py b/tests/_sync/test_pool.py
index ec57638..c741739 100644
--- a/tests/_sync/test_pool.py
+++ b/tests/_sync/test_pool.py
@@ -344,3 +344,58 @@ def test_revalidation_with_new_content():
             stored_response, stored_request, stored_metadata = stored
             assert extract_header_values_decoded(stored_response.headers, b"Date") == ["Mon, 25 Aug 2015 12:00:11 GMT"]
             assert stored_response.content == b"Eat at Joe's."
+
+
+
+def test_poool_revalidation_forward_extensions():
+    class MockedClock(BaseClock):
+        current = 1440504000  # Mon, 25 Aug 2015 12:00:00 GMT
+
+        def now(self) -> int:
+            return self.current
+
+    class MockedConnectionPoolWithExtensionsMemory(hishel.MockConnectionPool):
+        def handle_request(self, request: httpcore.Request) -> httpcore.Response:
+            self.last_request_extensions = request.extensions
+            return super().handle_request(request)
+
+    clock = MockedClock()
+    controller = hishel.Controller(clock=clock)
+
+    with MockedConnectionPoolWithExtensionsMemory() as pool:
+        pool.add_responses(
+            [
+                httpcore.Response(
+                    200,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:00 GMT"),
+                    ],
+                ),
+                httpcore.Response(
+                    304,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:01 GMT"),
+                    ],
+                ),
+            ]
+        )
+        with hishel.CacheConnectionPool(
+            pool=pool, controller=controller, storage=hishel.InMemoryStorage()
+        ) as cache_pool:
+            # first request with extensions
+            cache_pool.handle_request(
+                httpcore.Request("GET", "https://www.example.com", extensions={"foo": "bar"})
+            )
+            assert pool.last_request_extensions["foo"] == "bar"
+
+            # cache expires
+            clock.current += 1
+
+            # second request with extensions that should be passed to revalidation request
+            response = cache_pool.handle_request(
+                httpcore.Request("GET", "https://www.example.com", extensions={"foo": "baz"})
+            )
+            assert response.extensions["revalidated"] is True
+            assert pool.last_request_extensions["foo"] == "baz"
diff --git a/tests/_sync/test_transport.py b/tests/_sync/test_transport.py
index 688e63b..ba20c0a 100644
--- a/tests/_sync/test_transport.py
+++ b/tests/_sync/test_transport.py
@@ -373,3 +373,58 @@ def test_revalidation_with_new_content():
             stored_response, stored_request, stored_metadata = stored
             assert extract_header_values_decoded(stored_response.headers, b"Date") == ["Mon, 25 Aug 2015 12:00:11 GMT"]
             assert stored_response.content == b"Eat at Joe's."
+
+
+
+def test_transport_revalidation_forward_extensions():
+    class MockedClock(BaseClock):
+        current = 1440504000  # Mon, 25 Aug 2015 12:00:00 GMT
+
+        def now(self) -> int:
+            return self.current
+
+    class MockedTransportWithExtensionsMemory(hishel.MockTransport):
+        def handle_request(self, request: httpx.Request) -> httpx.Response:
+            self.last_request_extensions = request.extensions
+            return super().handle_request(request)
+
+    clock = MockedClock()
+    controller = hishel.Controller(clock=clock)
+
+    with MockedTransportWithExtensionsMemory() as transport:
+        transport.add_responses(
+            [
+                httpx.Response(
+                    200,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:00 GMT"),
+                    ],
+                ),
+                httpx.Response(
+                    304,
+                    headers=[
+                        (b"Cache-Control", b"max-age=1"),
+                        (b"Date", b"Mon, 25 Aug 2015 12:00:01 GMT"),
+                    ],
+                ),
+            ]
+        )
+        with hishel.CacheTransport(
+            transport=transport, controller=controller, storage=hishel.InMemoryStorage()
+        ) as cache_transport:
+            # first request with extensions
+            cache_transport.handle_request(
+                httpx.Request("GET", "https://www.example.com", extensions={"foo": "bar"})
+            )
+            assert transport.last_request_extensions["foo"] == "bar"
+
+            # cache expires
+            clock.current += 1
+
+            # second request with extensions that should be passed to revalidation request
+            response = cache_transport.handle_request(
+                httpx.Request("GET", "https://www.example.com", extensions={"foo": "baz"})
+            )
+            assert response.extensions["revalidated"] is True
+            assert transport.last_request_extensions["foo"] == "baz"