Merge pull request #375 from JWCook/conditional-requests

Enable conditional requests by default

4 files changed, 15 insertions, 22 deletions

diff --git a/HISTORY.md b/HISTORY.md
index 77d902b..94b13ea 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -4,7 +4,7 @@
 [See all issues and PRs for 0.8](https://github.com/reclosedev/requests-cache/milestone/3?closed=1)
 
 **Conditional requests:**
-* Add support for the following request + response headers to enable conditional requests:
+* Add support for conditional requests using the following request + response headers:
   * `ETag` + `If-None-Match` headers
   * `Last-Modified` + `If-Modified-Since` headers
   * `304 Not Modified` responses
diff --git a/requests_cache/cache_control.py b/requests_cache/cache_control.py
index 18ff919..b3d1703 100644
--- a/requests_cache/cache_control.py
+++ b/requests_cache/cache_control.py
@@ -54,10 +54,10 @@ class CacheActions:
     5. Per-session expiration
     """
 
-    add_request_headers: Dict = field(factory=dict)
     cache_control: bool = field(default=False)
     cache_key: str = field(default=None)
     expire_after: ExpirationTime = field(default=None)
+    request_headers: Dict[str, str] = field(factory=dict)
     skip_read: bool = field(default=False)
     skip_write: bool = field(default=False)
 
@@ -121,17 +121,18 @@ class CacheActions:
         """Convert the user/header-provided expiration value to a datetime"""
         return get_expiration_datetime(self.expire_after)
 
-    # TODO: Behavior if no other expiration method was specified (expire_after=-1)?
     def update_from_cached_response(self, response: CachedResponse):
         """Used after fetching a cached response, but before potentially sending a new request.
         Check for relevant cache headers on a cached response, and set corresponding request headers.
         """
-        if not self.cache_control or not response or not response.is_expired:
+        if not response or not response.is_expired:
             return
 
-        self.add_request_headers['If-None-Match'] = response.headers.get('ETag')
-        self.add_request_headers['If-Modified-Since'] = response.headers.get('Last-Modified')
-        self.add_request_headers = {k: v for k, v in self.add_request_headers.items() if v}
+        if response.headers.get('ETag'):
+            self.request_headers['If-None-Match'] = response.headers['ETag']
+        if response.headers.get('Last-Modified'):
+            self.request_headers['If-Modified-Since'] = response.headers['Last-Modified']
+        self.request_headers = {k: v for k, v in self.request_headers.items() if v}
 
     def update_from_response(self, response: Response):
         """Used after receiving a new response but before saving it to the cache.
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 2c841f0..9590cad 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -173,7 +173,7 @@ class CacheMixin(MIXIN_BASE):
         If applicable, also add request headers to check if the remote resource has been modified.
         If we get a 304 Not Modified response, return the expired cache item.
         """
-        request.headers.update(actions.add_request_headers)
+        request.headers.update(actions.request_headers)
         response = super().send(request, **kwargs)
         actions.update_from_response(response)
 
diff --git a/tests/unit/test_cache_control.py b/tests/unit/test_cache_control.py
index 4606dc6..083d03d 100644
--- a/tests/unit/test_cache_control.py
+++ b/tests/unit/test_cache_control.py
@@ -138,37 +138,29 @@ def test_init_from_settings(url, request_expire_after, expected_expiration):
     ],
 )
 def test_update_from_cached_response(response_headers, expected_request_headers):
-    """Test with Cache-Control response headers"""
+    """Test that conditional request headers are added if the cached response is expired"""
     actions = CacheActions.from_request(
         cache_key='key',
         request=MagicMock(url='https://img.site.com/base/img.jpg'),
-        cache_control=True,
     )
     cached_response = CachedResponse(headers=response_headers, expires=datetime.now() - timedelta(1))
+
     actions.update_from_cached_response(cached_response)
-    assert actions.add_request_headers == expected_request_headers
+    assert actions.request_headers == expected_request_headers
 
 
 def test_update_from_cached_response__ignored():
-    """Test with Cache-Control response headers"""
-    # Do nothing if cache-control=Fase
+    """Test that conditional request headers are NOT applied if the cached response is not expired"""
     actions = CacheActions.from_request(
         cache_key='key',
         request=MagicMock(url='https://img.site.com/base/img.jpg'),
-        cache_control=False,
     )
     cached_response = CachedResponse(
-        headers={'ETag': ETAG, 'Last-Modified': LAST_MODIFIED},
-        expires=datetime.now() - timedelta(1),
+        headers={'ETag': ETAG, 'Last-Modified': LAST_MODIFIED}, expires=None
     )
-    actions.update_from_cached_response(cached_response)
-    assert actions.add_request_headers == {}
 
-    # Do nothing if the response is not expired
-    actions.cache_control = True
-    cached_response.expires = None
     actions.update_from_cached_response(cached_response)
-    assert actions.add_request_headers == {}
+    assert actions.request_headers == {}
 
 
 @pytest.mark.parametrize(