Merge pull request #465 from meggiman/fix_conditinal_request_expiration

Fix conditinal request expiration

3 files changed, 53 insertions, 4 deletions

diff --git a/requests_cache/models/response.py b/requests_cache/models/response.py
index 974d878..ec65034 100755
--- a/requests_cache/models/response.py
+++ b/requests_cache/models/response.py
@@ -2,6 +2,7 @@ from datetime import datetime, timedelta, timezone
 from logging import getLogger
 from typing import TYPE_CHECKING, List, Optional, Tuple, Union
 
+import attr
 from attr import define, field
 from requests import PreparedRequest, Response
 from requests.cookies import RequestsCookieJar
@@ -46,9 +47,13 @@ class CachedResponse(Response):
             self.raw.headers = HTTPHeaderDict(self.headers)
 
     @classmethod
-    def from_response(cls, original_response: Response, **kwargs):
-        """Create a CachedResponse based on an original response object"""
-        obj = cls(**kwargs)
+    def from_response(
+        cls, original_response: Union[Response, 'CachedResponse'], expires: datetime = None, **kwargs
+    ):
+        """Create a CachedResponse based on an original Response or another CachedResponse object"""
+        if isinstance(original_response, CachedResponse):
+            return attr.evolve(original_response, expires=expires)
+        obj = cls(expires=expires, **kwargs)
 
         # Copy basic attributes
         for k in Response.__attrs__:
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 9b2531d..f45b7c5 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -180,7 +180,18 @@ class CacheMixin(MIXIN_BASE):
         if self._is_cacheable(response, actions):
             self.cache.save_response(response, actions.cache_key, actions.expires)
         elif cached_response and response.status_code == 304:
-            logger.debug(f'Response for URL {request.url} has not been modified; using cached response')
+            logger.debug(
+                f'Response for URL {request.url} has not been modified; using cached response and updating expiration date.'
+            )
+            # Update the cache expiration date and the headers (see RFC7234 4.3.4, p.18). Since we performed validation,
+            # the cache entry may be marked as fresh again.
+            cached_response.headers.update(response.headers)
+            # Since it is a 304 response we have to update cache control once again with combination of
+            # cached_response's and 304 response's Cache-Control directives.
+            response.headers = cached_response.headers
+            actions.update_from_response(response)
+            cached_response.expires = actions.expires
+            self.cache.save_response(cached_response, actions.cache_key, cached_response.expires)
             return cached_response
         else:
             logger.debug(f'Skipping cache write for URL: {request.url}')
diff --git a/tests/integration/base_cache_test.py b/tests/integration/base_cache_test.py
index 9b84a14..e35af90 100644
--- a/tests/integration/base_cache_test.py
+++ b/tests/integration/base_cache_test.py
@@ -220,6 +220,39 @@ class BaseCacheTest:
         assert response.from_cache is True
         assert response.is_expired is True
 
+    @pytest.mark.parametrize('validator_headers', [{'ETag': ETAG}, {'Last-Modified': LAST_MODIFIED}])
+    @pytest.mark.parametrize('cache_headers', [{'Cache-Control': 'max-age=0'}])
+    def test_conditional_request_refreshenes_expire_date(self, cache_headers, validator_headers):
+        """Test that revalidation attempt with 304 responses causes stale entry to become fresh again considering
+        Cache-Control header of the 304 response."""
+        url = httpbin('response-headers')
+        first_response_headers = {**cache_headers, **validator_headers}
+        session = self.init_session(cache_control=True)
+
+        # This endpoint returns request params as response headers
+        session.get(url, params=first_response_headers)
+
+        # Add different Response Header to mocked return value of the session.send() function.
+        updated_response_headers = {**first_response_headers, 'Cache-Control': 'max-age=60'}
+        with patch.object(
+            Session, 'send', return_value=MagicMock(status_code=304, headers=updated_response_headers)
+        ):
+            response = session.get(url, params=first_response_headers)
+        assert response.from_cache is True
+        assert response.is_expired is False
+
+        # Make sure an immediate subsequent request will be served from the cache for another max-age==60 secondss
+        try:
+            with patch.object(Session, 'send', side_effect=AssertionError):
+                response = session.get(url, params=first_response_headers)
+        except AssertionError:
+            assert False, (
+                "Session tried to perform re-validation although cached response should have been "
+                "refreshened."
+            )
+        assert response.from_cache is True
+        assert response.is_expired is False
+
     @pytest.mark.parametrize('stream', [True, False])
     def test_response_decode(self, stream):
         """Test that gzip-compressed raw responses (including streamed responses) can be manually