Implement Cache-Control: stale-while-revalidate

3 files changed, 176 insertions, 77 deletions

diff --git a/tests/conftest.py b/tests/conftest.py
index b242946..aa17c0e 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -19,6 +19,7 @@ from uuid import uuid4
 
 import pytest
 import requests
+from requests import Request
 from requests_mock import ANY as ANY_METHOD
 from requests_mock import Adapter
 from rich.logging import RichHandler
@@ -61,7 +62,7 @@ HTTPBIN_FORMATS = [
 ]
 HTTPDATE_STR = 'Fri, 16 APR 2021 21:13:00 GMT'
 HTTPDATE_DATETIME = datetime(2021, 4, 16, 21, 13)
-EXPIRED_DT = datetime.now() - timedelta(1)
+EXPIRED_DT = datetime.utcnow() - timedelta(1)
 ETAG = '"644b5b0155e6404a9cc4bd9d8b1ae730"'
 LAST_MODIFIED = 'Thu, 05 Jul 2012 15:31:30 GMT'
 
@@ -221,7 +222,7 @@ def get_mock_response(
         url=url,
         status_code=status_code,
         headers=headers,
-        request=MagicMock(method=method, url=url, headers=request_headers),
+        request=Request(method=method, url=url, headers=request_headers),
     )
 
 
diff --git a/tests/unit/policy/test_actions.py b/tests/unit/policy/test_actions.py
index 5c63f31..317ecc8 100644
--- a/tests/unit/policy/test_actions.py
+++ b/tests/unit/policy/test_actions.py
@@ -1,5 +1,5 @@
 from datetime import datetime, timedelta
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import pytest
 from requests import PreparedRequest, Request
@@ -16,6 +16,8 @@ IGNORED_DIRECTIVES = [
     'public',
     's-maxage=<seconds>',
 ]
+BASIC_REQUEST = Request(method='GET', url='https://site.com/img.jpg', headers={})
+EXPIRED_RESPONSE = CachedResponse(expires=datetime.utcnow() - timedelta(1))
 
 
 @pytest.mark.parametrize(
@@ -142,6 +144,19 @@ def test_init_from_settings_and_headers(
     assert actions.skip_read == expected_skip_read
 
 
+def test_update_from_cached_response__new_request():
+    actions = CacheActions.from_request('key', BASIC_REQUEST)
+    actions.update_from_cached_response(None)
+    assert actions.send_request is True
+
+
+def test_update_from_cached_response__resend_request():
+    actions = CacheActions.from_request('key', BASIC_REQUEST)
+
+    actions.update_from_cached_response(EXPIRED_RESPONSE)
+    assert actions.resend_request is True
+
+
 @pytest.mark.parametrize(
     'response_headers, expected_validation_headers',
     [
@@ -154,16 +169,15 @@ def test_init_from_settings_and_headers(
         ),
     ],
 )
-def test_update_from_cached_response(response_headers, expected_validation_headers):
+def test_update_from_cached_response__revalidate(response_headers, expected_validation_headers):
     """Conditional request headers should be added if the cached response is expired"""
-    actions = CacheActions.from_request(
-        'key', MagicMock(url='https://img.site.com/base/img.jpg', headers={})
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST)
     cached_response = CachedResponse(
-        headers=response_headers, expires=datetime.now() - timedelta(1)
+        headers=response_headers, expires=datetime.utcnow() - timedelta(1)
     )
 
     actions.update_from_cached_response(cached_response)
+    assert actions.send_request is bool(expected_validation_headers)
     assert actions._validation_headers == expected_validation_headers
 
 
@@ -174,24 +188,24 @@ def test_update_from_cached_response(response_headers, expected_validation_heade
         ({}, {'Cache-Control': 'max-age=0,must-revalidate'}),
     ],
 )
-def test_update_from_cached_response__revalidate_headers(request_headers, response_headers):
-    """Conditional request headers should be added if requested by headers (even if the response
-    is not expired)"""
+def test_update_from_cached_response__refresh(request_headers, response_headers):
+    """Conditional request headers should be added if requested by response headers, even if the
+    response is not expired
+    """
     actions = CacheActions.from_request(
-        'key', MagicMock(url='https://img.site.com/base/img.jpg', headers=request_headers)
+        'key', Request(url='https://img.site.com/base/img.jpg', headers=request_headers)
     )
     cached_response = CachedResponse(headers={'ETag': ETAG, **response_headers}, expires=None)
 
     actions.update_from_cached_response(cached_response)
+    assert actions.send_request is True
     assert actions._validation_headers == {'If-None-Match': ETAG}
 
 
-def test_update_from_cached_response__ignored():
+def test_update_from_cached_response__no_revalidation():
     """Conditional request headers should NOT be added if the cached response is not expired and
     revalidation is otherwise not requested"""
-    actions = CacheActions.from_request(
-        'key', MagicMock(url='https://img.site.com/base/img.jpg', headers={})
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST)
     cached_response = CachedResponse(
         headers={'ETag': ETAG, 'Last-Modified': LAST_MODIFIED}, expires=None
     )
@@ -200,6 +214,27 @@ def test_update_from_cached_response__ignored():
     assert actions._validation_headers == {}
 
 
+def test_update_from_cached_response__504():
+    settings = CacheSettings(only_if_cached=True)
+    actions = CacheActions.from_request('key', BASIC_REQUEST, settings=settings)
+    actions.update_from_cached_response(EXPIRED_RESPONSE)
+    assert actions.error_504 is True
+
+
+def test_update_from_cached_response__stale_if_error():
+    settings = CacheSettings(only_if_cached=True, stale_if_error=True)
+    actions = CacheActions.from_request('key', BASIC_REQUEST, settings=settings)
+    actions.update_from_cached_response(EXPIRED_RESPONSE)
+    assert actions.error_504 is False and actions.resend_request is False
+
+
+def test_update_from_cached_response__stale_while_revalidate():
+    settings = CacheSettings(only_if_cached=True, stale_while_revalidate=True)
+    actions = CacheActions.from_request('key', BASIC_REQUEST, settings=settings)
+    actions.update_from_cached_response(EXPIRED_RESPONSE)
+    assert actions.resend_async is True
+
+
 @pytest.mark.parametrize('max_stale, usable', [(5, False), (15, True)])
 def test_is_usable__max_stale(max_stale, usable):
     """For a response that expired 10 seconds ago, it may be either accepted or rejected based on
@@ -210,9 +245,7 @@ def test_is_usable__max_stale(max_stale, usable):
         headers={'Cache-Control': f'max-stale={max_stale}'},
     )
     actions = CacheActions.from_request('key', request)
-    cached_response = CachedResponse(
-        expires=datetime.utcnow() - timedelta(seconds=10),
-    )
+    cached_response = CachedResponse(expires=datetime.utcnow() - timedelta(seconds=10))
     assert actions.is_usable(cached_response) is usable
 
 
@@ -226,9 +259,7 @@ def test_is_usable__min_fresh(min_fresh, usable):
         headers={'Cache-Control': f'min-fresh={min_fresh}'},
     )
     actions = CacheActions.from_request('key', request)
-    cached_response = CachedResponse(
-        expires=datetime.utcnow() + timedelta(seconds=10),
-    )
+    cached_response = CachedResponse(expires=datetime.utcnow() + timedelta(seconds=10))
     assert actions.is_usable(cached_response) is usable
 
 
@@ -249,13 +280,31 @@ def test_is_usable__stale_if_error(stale_if_error, error, usable):
         headers={'Cache-Control': f'stale-if-error={stale_if_error}'},
     )
     actions = CacheActions.from_request('key', request)
-    cached_response = CachedResponse(
-        expires=datetime.utcnow() - timedelta(seconds=10),
-    )
+    cached_response = CachedResponse(expires=datetime.utcnow() - timedelta(seconds=10))
     assert actions.is_usable(cached_response, error=error) is usable
 
 
 @pytest.mark.parametrize(
+    'stale_while_revalidate, usable',
+    [
+        (5, False),
+        (15, True),
+    ],
+)
+def test_is_usable__stale_while_revalidate(stale_while_revalidate, usable):
+    """For a response that expired 10 seconds ago, if an error occured while refreshing, it may be
+    either accepted or rejected based on stale-while-revalidate
+    """
+    request = Request(
+        url='https://img.site.com/base/img.jpg',
+        headers={'Cache-Control': f'stale-while-revalidate={stale_while_revalidate}'},
+    )
+    actions = CacheActions.from_request('key', request)
+    cached_response = CachedResponse(expires=datetime.utcnow() - timedelta(seconds=10))
+    assert actions.is_usable(cached_response=cached_response) is usable
+
+
+@pytest.mark.parametrize(
     'headers, expected_expiration',
     [
         ({}, None),
@@ -272,10 +321,7 @@ def test_is_usable__stale_if_error(stale_if_error, error, usable):
 )
 def test_update_from_response(headers, expected_expiration):
     """Test with Cache-Control response headers"""
-    url = 'https://img.site.com/base/img.jpg'
-    actions = CacheActions.from_request(
-        'key', MagicMock(url=url), CacheSettings(cache_control=True)
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST, CacheSettings(cache_control=True))
     actions.update_from_response(get_mock_response(headers=headers))
 
     assert actions.expire_after == expected_expiration
@@ -283,19 +329,13 @@ def test_update_from_response(headers, expected_expiration):
 
 
 def test_update_from_response__no_store():
-    url = 'https://img.site.com/base/img.jpg'
-    actions = CacheActions.from_request(
-        'key', MagicMock(url=url), CacheSettings(cache_control=True)
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST, CacheSettings(cache_control=True))
     actions.update_from_response(get_mock_response(headers={'Cache-Control': 'no-store'}))
     assert actions.skip_write is True
 
 
 def test_update_from_response__ignored():
-    url = 'https://img.site.com/base/img.jpg'
-    actions = CacheActions.from_request(
-        'key', MagicMock(url=url), CacheSettings(cache_control=False)
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST, CacheSettings(cache_control=False))
     actions.update_from_response(get_mock_response(headers={'Cache-Control': 'max-age=5'}))
     assert actions.expire_after is None
 
@@ -307,10 +347,7 @@ def test_update_from_response__revalidate(mock_datetime, cache_headers, validato
     """If expiration is 0 and there's a validator, the response should be cached, but with immediate
     expiration
     """
-    url = 'https://img.site.com/base/img.jpg'
-    actions = CacheActions.from_request(
-        'key', MagicMock(url=url), CacheSettings(cache_control=True)
-    )
+    actions = CacheActions.from_request('key', BASIC_REQUEST, CacheSettings(cache_control=True))
     response = get_mock_response(headers={**cache_headers, **validator_headers})
     actions.update_from_response(response)
 
@@ -324,7 +361,6 @@ def test_ignored_headers(directive):
     request = Request(
         method='GET', url='https://img.site.com/base/img.jpg', headers={'Cache-Control': directive}
     ).prepare()
-
     settings = CacheSettings(expire_after=1, cache_control=True)
     actions = CacheActions.from_request('key', request, settings)
 
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index 7d22631..3a927af 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -1,11 +1,11 @@
 """CachedSession tests that use mocked responses only"""
 import json
-import time
 from collections import UserDict, defaultdict
 from datetime import datetime, timedelta
 from logging import getLogger
 from pathlib import Path
 from pickle import PickleError
+from time import sleep, time
 from unittest.mock import patch
 from urllib.parse import urlencode
 
@@ -343,7 +343,7 @@ def test_expired_request_error(mock_session):
     mock_session.settings.stale_if_error = False
     mock_session.settings.expire_after = 1
     mock_session.get(MOCKED_URL)
-    time.sleep(1)
+    sleep(1)
 
     with patch.object(mock_session.cache, 'save_response', side_effect=ValueError):
         with pytest.raises(ValueError):
@@ -357,7 +357,7 @@ def test_stale_if_error__exception(mock_session):
 
     assert mock_session.get(MOCKED_URL).from_cache is False
     assert mock_session.get(MOCKED_URL).from_cache is True
-    time.sleep(1)
+    sleep(1)
     with patch.object(mock_session.cache, 'save_response', side_effect=RequestException):
         response = mock_session.get(MOCKED_URL)
         assert response.from_cache is True and response.is_expired is True
@@ -371,7 +371,7 @@ def test_stale_if_error__error_code(mock_session):
 
     assert mock_session.get(MOCKED_URL_404).from_cache is False
 
-    time.sleep(1)
+    sleep(1)
     response = mock_session.get(MOCKED_URL_404)
     assert response.from_cache is True and response.is_expired is True
 
@@ -471,6 +471,26 @@ def test_allowable_methods(mock_session):
     assert mock_session.delete(MOCKED_URL).from_cache is False
 
 
+def test_always_revalidate(mock_session):
+    """The session always_revalidate option should send a conditional request, if possible"""
+    mock_session.settings.expire_after = 60
+    response_1 = mock_session.get(MOCKED_URL_ETAG)
+    response_2 = mock_session.get(MOCKED_URL_ETAG)
+    mock_session.mock_adapter.register_uri('GET', MOCKED_URL_ETAG, status_code=304)
+
+    mock_session.settings.always_revalidate = True
+    response_3 = mock_session.get(MOCKED_URL_ETAG)
+    response_4 = mock_session.get(MOCKED_URL_ETAG)
+
+    assert response_1.from_cache is False
+    assert response_2.from_cache is True
+    assert response_3.from_cache is True and response_3.revalidated is True
+    assert response_4.from_cache is True and response_4.revalidated is True
+
+    # Expect expiration to get reset after revalidation
+    assert response_2.expires < response_4.expires
+
+
 def test_default_ignored_parameters(mock_session):
     """Common auth params and headers (for OAuth2, etc.) should be ignored by default"""
     mock_session.get(
@@ -590,7 +610,7 @@ def test_304_not_modified(
 ):
     url = f'{MOCKED_URL}/endpoint_2'
     if cache_expired:
-        mock_session.settings.expire_after = datetime.now() - timedelta(1)
+        mock_session.settings.expire_after = datetime.utcnow() - timedelta(1)
     if cache_hit:
         mock_session.mock_adapter.register_uri('GET', url, status_code=200)
         mock_session.get(url)
@@ -623,7 +643,7 @@ def test_remove_expired_responses(mock_normalize_url, mock_session):
     mock_session.settings.expire_after = 1
     mock_session.get(MOCKED_URL)
     mock_session.get(MOCKED_URL_JSON)
-    time.sleep(1)
+    sleep(1)
     mock_session.get(unexpired_url)
 
     # At this point we should have 1 unexpired response and 2 expired responses
@@ -634,7 +654,7 @@ def test_remove_expired_responses(mock_normalize_url, mock_session):
     assert cached_response.url == unexpired_url
 
     # Now the last response should be expired as well
-    time.sleep(1)
+    sleep(1)
     BaseCache.remove_expired_responses(mock_session.cache)
     assert len(mock_session.cache.responses) == 0
 
@@ -701,15 +721,14 @@ def test_remove_expired_responses__per_request(mock_session):
     assert len(mock_session.cache.responses) == 3
 
     # One should be expired after 2s, and another should be expired after 4s
-    time.sleep(2)
+    sleep(2)
     mock_session.remove_expired_responses()
     assert len(mock_session.cache.responses) == 2
-    time.sleep(2)
+    sleep(2)
     mock_session.remove_expired_responses()
     assert len(mock_session.cache.responses) == 1
 
 
-# @patch_normalize_url
 def test_remove_expired_responses__older_than(mock_session):
     # Cache 4 responses with different creation times
     response_0 = CachedResponse(request=CachedRequest(method='GET', url='https://test.com/test_0'))
@@ -734,11 +753,74 @@ def test_remove_expired_responses__older_than(mock_session):
     assert len(mock_session.cache.responses) == 1
 
     # Remove the last response after it's 1 second old
-    time.sleep(1)
+    sleep(1)
     mock_session.remove_expired_responses(older_than=timedelta(seconds=1))
     assert len(mock_session.cache.responses) == 0
 
 
+def test_stale_while_revalidate(mock_session):
+    # Start with expired responses
+    mocked_url_2 = f'{MOCKED_URL_ETAG}?k=v'
+    mock_session.settings.stale_while_revalidate = True
+    mock_session.get(MOCKED_URL_ETAG, expire_after=timedelta(seconds=-2))
+    mock_session.get(mocked_url_2, expire_after=timedelta(seconds=-2))
+    assert mock_session.cache.has_url(MOCKED_URL_ETAG)
+
+    # First, let's just make sure the correct method is called
+    mock_session.mock_adapter.register_uri('GET', MOCKED_URL_ETAG, status_code=304)
+    with patch.object(CachedSession, '_resend_async') as mock_send:
+        response = mock_session.get(MOCKED_URL_ETAG)
+        mock_send.assert_called_once()
+
+    def slow_request(*args, **kwargs):
+        sleep(0.1)
+        return mock_session._send_and_cache(*args, **kwargs)
+
+    # Next, test that the revalidation request is non-blocking
+    start = time()
+    with patch.object(CachedSession, '_send_and_cache', side_effect=slow_request) as mock_send:
+        response = mock_session.get(mocked_url_2, expire_after=60)
+        assert response.from_cache is True and response.is_expired is True
+        assert time() - start < 0.1
+        sleep(0.1)
+        mock_send.assert_called()
+
+    # Finally, check that the cached response has been refreshed
+    sleep(0.2)  # Background thread may be a bit slow on CI runner
+    response = mock_session.get(mocked_url_2)
+    assert response.from_cache is True and response.is_expired is False
+
+
+def test_stale_while_revalidate__time(mock_session):
+    """stale_while_revalidate should also accept a time value (max acceptable staleness)"""
+    mocked_url_2 = f'{MOCKED_URL_ETAG}?k=v'
+    mock_session.settings.stale_while_revalidate = timedelta(seconds=3)
+    mock_session.get(MOCKED_URL_ETAG, expire_after=timedelta(seconds=-2))
+    response = mock_session.get(mocked_url_2, expire_after=timedelta(seconds=-4))
+
+    # stale_while_revalidate should apply to this response (expired 2 seconds ago)
+    response = mock_session.get(MOCKED_URL_ETAG)
+    assert response.from_cache is True and response.is_expired is True
+
+    # but not this response (expired 4 seconds ago)
+    response = mock_session.get(mocked_url_2)
+    assert response.from_cache is False and response.is_expired is False
+
+
+def test_stale_while_revalidate__refresh(mock_session):
+    """stale_while_revalidate should also apply to normal refresh requests"""
+    mock_session.settings.stale_while_revalidate = True
+    mock_session.get(MOCKED_URL, expire_after=1)
+    sleep(1)  # An expired response without a validator won't be cached, so need to sleep
+
+    response = mock_session.get(MOCKED_URL)
+    assert response.from_cache is True and response.is_expired is True
+
+    sleep(0.2)
+    response = mock_session.get(MOCKED_URL)
+    assert response.from_cache is True and response.is_expired is False
+
+
 # Additional request() and send() options
 # -----------------------------------------------------
 
@@ -750,7 +832,7 @@ def test_request_expire_after__enable_expiration(mock_session):
     assert response.from_cache is False
     assert mock_session.get(MOCKED_URL).from_cache is True
 
-    time.sleep(1)
+    sleep(1)
     response = mock_session.get(MOCKED_URL)
     assert response.from_cache is False
 
@@ -772,7 +854,7 @@ def test_request_expire_after__prepared_request(mock_session):
     assert response.from_cache is False
     assert mock_session.send(request).from_cache is True
 
-    time.sleep(1)
+    sleep(1)
     response = mock_session.get(MOCKED_URL)
     assert response.from_cache is False
 
@@ -796,7 +878,7 @@ def test_request_only_if_cached__uncached(mock_session):
 def test_request_only_if_cached__expired(mock_session):
     """By default, only_if_cached will not return an expired response"""
     mock_session.get(MOCKED_URL, expire_after=1)
-    time.sleep(1)
+    sleep(1)
 
     response = mock_session.get(MOCKED_URL, only_if_cached=True)
     assert response.status_code == 504
@@ -805,7 +887,7 @@ def test_request_only_if_cached__expired(mock_session):
 def test_request_only_if_cached__stale_if_error__expired(mock_session):
     """only_if_cached *will* return an expired response if stale_if_error is also set"""
     mock_session.get(MOCKED_URL, expire_after=1)
-    time.sleep(1)
+    sleep(1)
 
     mock_session.settings.stale_if_error = True
     response = mock_session.get(MOCKED_URL, only_if_cached=True)
@@ -841,26 +923,6 @@ def test_request_refresh(mock_session):
     assert response_2.expires < response_4.expires
 
 
-def test_request_always_revalidate(mock_session):
-    """The session always_revalidate option should send a conditional request, if possible"""
-    mock_session.settings.expire_after = 60
-    response_1 = mock_session.get(MOCKED_URL_ETAG)
-    response_2 = mock_session.get(MOCKED_URL_ETAG)
-    mock_session.mock_adapter.register_uri('GET', MOCKED_URL_ETAG, status_code=304)
-
-    mock_session.settings.always_revalidate = True
-    response_3 = mock_session.get(MOCKED_URL_ETAG)
-    response_4 = mock_session.get(MOCKED_URL_ETAG)
-
-    assert response_1.from_cache is False
-    assert response_2.from_cache is True
-    assert response_3.from_cache is True and response_3.revalidated is True
-    assert response_4.from_cache is True and response_4.revalidated is True
-
-    # Expect expiration to get reset after revalidation
-    assert response_2.expires < response_4.expires
-
-
 def test_request_refresh__no_validator(mock_session):
     """The refresh option should result in a new (unconditional) request if the cached response has
     no validator