More code cleanup and comments

8 files changed, 168 insertions, 184 deletions

diff --git a/docs/user_guide/headers.md b/docs/user_guide/headers.md
index 1dccde6..1d4cbcb 100644
--- a/docs/user_guide/headers.md
+++ b/docs/user_guide/headers.md
@@ -62,5 +62,5 @@ The following headers are currently supported:
 - `Cache-Control: no-store` Skip writing to the cache
 - `Cache-Control: immutable`: Cache the response with no expiration
 - `Expires`: Used as an absolute expiration datetime
-- `ETag`: Used for revalidation; update and return stale cache data if the remote content has not changed (`304 Not Modified` response)
-- `Last-Modified`: Used for revalidation; update and return stale cache data if the remote content has not changed (`304 Not Modified` response)
+- `ETag`: Validator used for conditional requests
+- `Last-Modified`: Validator used for conditional requests
diff --git a/requests_cache/_utils.py b/requests_cache/_utils.py
index b0067cd..edbf4c6 100644
--- a/requests_cache/_utils.py
+++ b/requests_cache/_utils.py
@@ -34,9 +34,9 @@ def get_placeholder_class(original_exception: Exception = None):
     """Create a placeholder type for a class that does not have dependencies installed.
     This allows delaying ImportErrors until init time, rather than at import time.
     """
-    msg = 'Dependencies are not installed for this feature'
 
     def _log_error():
+        msg = 'Dependencies are not installed for this feature'
         logger.error(msg)
         raise original_exception or ImportError(msg)
 
diff --git a/requests_cache/backends/__init__.py b/requests_cache/backends/__init__.py
index 3a30c57..9ae56ac 100644
--- a/requests_cache/backends/__init__.py
+++ b/requests_cache/backends/__init__.py
@@ -4,27 +4,11 @@ from logging import getLogger
 from typing import Callable, Dict, Iterable, Optional, Type, Union
 
 from .._utils import get_placeholder_class, get_valid_kwargs
-from ..models.settings import CacheSettings
 from .base import BaseCache, BaseStorage, DictStorage
 
 # Backend-specific keyword arguments equivalent to 'cache_name'
 CACHE_NAME_KWARGS = ['db_path', 'db_name', 'namespace', 'table_name']
 
-# All backend-specific keyword arguments
-BACKEND_KWARGS = CACHE_NAME_KWARGS + [
-    'connection',
-    'endpoint_url',
-    'fast_save',
-    'ignored_parameters',
-    'match_headers',
-    'name',
-    'read_capacity_units',
-    'region_name',
-    'salt',
-    'secret_key',
-    'write_capacity_units',
-]
-
 BackendSpecifier = Union[str, BaseCache, Type[BaseCache]]
 logger = getLogger(__name__)
 
@@ -72,18 +56,14 @@ BACKEND_CLASSES = {
 }
 
 
-# TODO: Make this less of a mess
-# TODO: Backend instance: Handle both settings already on instance, and settings passed via kwargs
-def init_backend(
-    cache_name: str, backend: Optional[BackendSpecifier], settings: CacheSettings = None, **kwargs
-) -> BaseCache:
+def init_backend(cache_name: str, backend: BackendSpecifier = None, **kwargs) -> BaseCache:
     """Initialize a backend from a name, class, or instance"""
     logger.debug(f'Initializing backend: {backend} {cache_name}')
-    settings = settings or CacheSettings(**kwargs)
 
     # The 'cache_name' arg has a different purpose depending on the backend. If an equivalent
-    # backend-specific keyword arg is specified, handle that here to avoid conflicts. A consistent
-    # positional-only or keyword-only arg would be better, but probably not worth a breaking change.
+    # backend-specific keyword arg is specified, handle that here to avoid conflicts with the
+    # 'cache_name' positional-or-keyword arg. In hindsight, a consistent positional-only or
+    # keyword-only arg would have been better, but probably not worth a breaking change.
     cache_name_kwargs = [kwargs.pop(k) for k in CACHE_NAME_KWARGS if k in kwargs]
     cache_name = cache_name or cache_name_kwargs[0]
 
@@ -91,17 +71,17 @@ def init_backend(
     if isinstance(backend, BaseCache):
         if cache_name:
             backend.cache_name = cache_name
-        backend.settings = settings
+        backend.settings.update(**kwargs)
         return backend
+    # TODO: Initializing a backend by class instead of instance seems superfluous. Deprecate this?
     elif isinstance(backend, type):
-        return backend(cache_name, settings=settings, **kwargs)
+        return backend(cache_name, **kwargs)
     elif not backend:
         sqlite_supported = issubclass(BACKEND_CLASSES['sqlite'], BaseCache)
         backend = 'sqlite' if sqlite_supported else 'memory'
 
+    # Get backend class by name
     backend = str(backend).lower()
     if backend not in BACKEND_CLASSES:
         raise ValueError(f'Invalid backend: {backend}. Choose from: {BACKEND_CLASSES.keys()}')
-
-    cls = BACKEND_CLASSES[backend]
-    return cls(cache_name, settings=settings, **kwargs)
+    return BACKEND_CLASSES[backend](cache_name, **kwargs)
diff --git a/requests_cache/cache_control.py b/requests_cache/cache_control.py
index 9e7fc65..6fa6152 100644
--- a/requests_cache/cache_control.py
+++ b/requests_cache/cache_control.py
@@ -14,7 +14,7 @@ from __future__ import annotations
 
 from datetime import datetime
 from logging import getLogger
-from typing import TYPE_CHECKING, Dict, MutableMapping, Optional, Tuple, Union
+from typing import Dict, MutableMapping, Optional, Tuple, Union
 
 from attr import define, field
 from requests import PreparedRequest, Response
@@ -28,14 +28,14 @@ from .expiration import (
     get_expiration_datetime,
     get_url_expiration,
 )
+from .models import CachedResponse, CacheSettings, RequestSettings
 
 __all__ = ['CacheActions']
-if TYPE_CHECKING:
-    from .models import CachedResponse, CacheSettings, RequestSettings
 
+# Temporary header only used to support the 'refresh' option in CachedSession.request()
+REFRESH_TEMP_HEADER = 'requests-cache-refresh'
 
 CacheDirective = Union[None, int, bool]
-
 logger = getLogger(__name__)
 
 
@@ -45,42 +45,52 @@ class CacheActions:
 
     * See :ref:`precedence` for behavior if multiple sources provide an expiration
     * See :ref:`headers` for more details about header behavior
+
+    Args:
+        cache_key: The cache key created based on the initial request
+        error_504: Indicates the request cannot be fulfilled based on cache settings
+        expire_after: User or header-provided expiration value
+        send_request: Send a new request
+        resend_request: Send a new request to refresh a stale cache item
+        skip_read: Skip reading from the cache
+        skip_write: Skip writing to the cache
+        _settings: Merged session-level and request-level cache settings
+        _validation_headers: Headers to send with conditional requests
+
     """
 
+    # Outputs
     cache_key: str = field(default=None)
+    error_504: bool = field(default=False)
     expire_after: ExpirationTime = field(default=None)
-    request_directives: Dict[str, CacheDirective] = field(factory=dict)
     resend_request: bool = field(default=False)
-    revalidate: bool = field(default=False)
-    error_504: bool = field(default=False)
     send_request: bool = field(default=False)
-    settings: CacheSettings = field(default=None)
     skip_read: bool = field(default=False)
     skip_write: bool = field(default=False)
-    validation_headers: Dict[str, str] = field(factory=dict)
+
+    # Inputs/internal attributes
+    _settings: CacheSettings = field(default=None)
+    _validation_headers: Dict[str, str] = field(factory=dict)
 
     @classmethod
     def from_request(
         cls,
         cache_key: str,
         request: PreparedRequest,
-        settings: 'RequestSettings',
+        settings: CacheSettings,
         **kwargs,
     ):
-        """Initialize from request info and cache settings.
-
-        Notes:
-
-        * If ``cache_control=True``, ``expire_after`` will be handled in
-          :py:meth:`update_from_response()` since it may be overridden by response headers.
-        * The ``requests-cache-refresh`` temporary header is used solely to support the ``refresh``
-          option in :py:meth:`CachedSession.request`; see notes there on interactions between
-          ``request()`` and ``send()``.
-        """
+        """Initialize from request info and cache settings"""
         request.headers = request.headers or CaseInsensitiveDict()
         directives = get_cache_directives(request.headers)
         logger.debug(f'Cache directives from request headers: {directives}')
 
+        # Merge session settings, request settings
+        settings = RequestSettings(settings, **kwargs)
+        settings.only_if_cached = settings.only_if_cached or 'only-if-cached' in directives
+        settings.refresh = settings.refresh or bool(request.headers.pop(REFRESH_TEMP_HEADER, False))
+        settings.revalidate = settings.revalidate or 'no-cache' in directives
+
         # Check expiration values in order of precedence
         expire_after = coalesce(
             directives.get('max-age'),
@@ -89,25 +99,21 @@ class CacheActions:
             settings.expire_after,
         )
 
-        # Check conditions for cache read and write based on args and request headers
-        refresh_temp_header = request.headers.pop('requests-cache-refresh', False)
-        check_expiration = directives.get('max-age') if settings.cache_control else expire_after
-        skip_write = check_expiration == DO_NOT_CACHE or 'no-store' in directives
-
-        # These behaviors may be set by either request headers or keyword arguments
-        settings.only_if_cached = settings.only_if_cached or 'only-if-cached' in directives
-        revalidate = settings.revalidate or 'no-cache' in directives
+        # Check conditions for reading from the cache
         skip_read = any(
-            [settings.refresh, skip_write, bool(refresh_temp_header), settings.disabled]
+            [
+                settings.refresh,
+                settings.disabled,
+                'no-store' in directives,
+                try_int(expire_after) == DO_NOT_CACHE,
+            ]
         )
 
         return cls(
             cache_key=cache_key,
             expire_after=expire_after,
-            request_directives=directives,
-            revalidate=revalidate,
             skip_read=skip_read,
-            skip_write=skip_write,
+            skip_write='no-store' in directives,
             settings=settings,
         )
 
@@ -124,11 +130,11 @@ class CacheActions:
         """
         # Determine if we need to send a new request or respond with an error
         is_expired = getattr(cached_response, 'is_expired', False)
-        if self.settings.only_if_cached and (cached_response is None or is_expired):
+        if self._settings.only_if_cached and (cached_response is None or is_expired):
             self.error_504 = True
         elif cached_response is None:
             self.send_request = True
-        elif is_expired and not (self.settings.only_if_cached and self.settings.stale_if_error):
+        elif is_expired and not (self._settings.only_if_cached and self._settings.stale_if_error):
             self.resend_request = True
 
         if cached_response is not None:
@@ -137,66 +143,69 @@ class CacheActions:
     def _update_validation_headers(self, response: CachedResponse):
         # Revalidation may be triggered by either stale response or request/cached response headers
         directives = get_cache_directives(response.headers)
-        self.revalidate = _has_validator(response.headers) and any(
+        revalidate = _has_validator(response.headers) and any(
             [
                 response.is_expired,
-                self.revalidate,
+                self._settings.revalidate,
                 'no-cache' in directives,
                 'must-revalidate' in directives and directives.get('max-age') == 0,
             ]
         )
 
         # Add the appropriate validation headers, if needed
-        if self.revalidate:
+        if revalidate:
             self.send_request = True
             if response.headers.get('ETag'):
-                self.validation_headers['If-None-Match'] = response.headers['ETag']
+                self._validation_headers['If-None-Match'] = response.headers['ETag']
             if response.headers.get('Last-Modified'):
-                self.validation_headers['If-Modified-Since'] = response.headers['Last-Modified']
+                self._validation_headers['If-Modified-Since'] = response.headers['Last-Modified']
 
     def update_from_response(self, response: Response):
-        """Update expiration + actions based on headers from a new response.
+        """Update expiration + actions based on headers and other details from a new response.
 
         Used after receiving a new response, but before saving it to the cache.
         """
-        if not response or not self.settings.cache_control:
-            return
+        if self._settings.cache_control:
+            self._update_from_response_headers(response)
 
-        directives = get_cache_directives(response.headers)
-        logger.debug(f'Cache directives from response headers: {directives}')
-
-        # Check headers for expiration, validators, and other cache directives
-        if directives.get('immutable'):
-            self.expire_after = NEVER_EXPIRE
-        else:
-            self.expire_after = coalesce(
-                directives.get('max-age'), directives.get('expires'), self.expire_after
-            )
-        no_store = 'no-store' in directives or 'no-store' in self.request_directives
-
-        # If expiration is 0 and there's a validator, save it to the cache and revalidate on use
-        # Otherwise, skip writing to the cache if specified by expiration or other headers
+        # If "expired" but there's a validator, save it to the cache and revalidate on use
         expire_immediately = try_int(self.expire_after) == DO_NOT_CACHE
-        self.skip_write = (expire_immediately or no_store) and not _has_validator(response.headers)
+        has_validator = _has_validator(response.headers)
+        self.skip_write = self.skip_write or (expire_immediately and not has_validator)
 
         # Apply filter callback, if any
-        filtered_out = self.settings.filter_fn is not None and not self.settings.filter_fn(response)
+        callback = self._settings.filter_fn
+        filtered_out = callback is not None and not callback(response)
 
         # Apply and log remaining checks needed to determine if the response should be cached
         cache_criteria = {
-            'disabled cache': self.settings.disabled,
-            'disabled method': str(response.request.method) not in self.settings.allowable_methods,
-            'disabled status': response.status_code not in self.settings.allowable_codes,
+            'disabled cache': self._settings.disabled,
+            'disabled method': str(response.request.method) not in self._settings.allowable_methods,
+            'disabled status': response.status_code not in self._settings.allowable_codes,
             'disabled by filter': filtered_out,
             'disabled by headers or expiration params': self.skip_write,
         }
         logger.debug(f'Pre-cache checks for response from {response.url}: {cache_criteria}')
         self.skip_write = any(cache_criteria.values())
 
+    def _update_from_response_headers(self, response: Response):
+        """Check response headers for expiration and other cache directives"""
+        directives = get_cache_directives(response.headers)
+        logger.debug(f'Cache directives from response headers: {directives}')
+
+        if directives.get('immutable'):
+            self.expire_after = NEVER_EXPIRE
+        else:
+            self.expire_after = coalesce(
+                directives.get('max-age'),
+                directives.get('expires'),
+                self.expire_after,
+            )
+        self.skip_write = self.skip_write or 'no-store' in directives
+
     def update_request(self, request: PreparedRequest) -> PreparedRequest:
         """Apply validation headers (if any) before sending a request"""
-        # if self.revalidate:
-        request.headers.update(self.validation_headers)
+        request.headers.update(self._validation_headers)
         return request
 
     def update_revalidated_response(
@@ -233,14 +242,14 @@ def get_cache_directives(headers: MutableMapping) -> Dict[str, CacheDirective]:
     kv_directives = {}
     if headers.get('Cache-Control'):
         cache_directives = headers['Cache-Control'].split(',')
-        kv_directives = dict([split_kv_directive(value) for value in cache_directives])
+        kv_directives = dict([_split_kv_directive(value) for value in cache_directives])
 
     if 'Expires' in headers:
         kv_directives['expires'] = headers['Expires']
     return kv_directives
 
 
-def split_kv_directive(header_value: str) -> Tuple[str, CacheDirective]:
+def _split_kv_directive(header_value: str) -> Tuple[str, CacheDirective]:
     """Split a cache directive into a ``(key, int)`` pair, if possible; otherwise just
     ``(key, True)``.
     """
diff --git a/requests_cache/models/settings.py b/requests_cache/models/settings.py
index 4b674a6..ddb51f1 100644
--- a/requests_cache/models/settings.py
+++ b/requests_cache/models/settings.py
@@ -13,15 +13,6 @@ FilterCallback = Callable[['AnyResponse'], bool]
 KeyCallback = Callable[..., str]
 
 
-# TODO: RequestSettings subclass?
-#   This would work by merging settings for an individual request on top of session settings
-#   This would also allow for *any* session setting to be passed as a per-request setting
-#   ...But only for send(), not request()
-# TODO: HeaderSettings class?
-#   This would encapsulate settings from Cache-Control directives and other cache headers
-#   Downside: that logic would then live in this moduel instead of in cache_control (which may be a better fit)
-
-
 @define(init=False)
 class CacheSettings:
     """Settings that affect caching behavior, used by :py:class:`.CachedSession` and
@@ -55,29 +46,38 @@ class CacheSettings:
     stale_if_error: bool = field(default=False)
     urls_expire_after: Dict[str, ExpirationTime] = field(factory=dict)
 
-    def __init__(self, **kwargs):
-        # Backwards-compatibility for old argument names
-        if 'old_data_on_error' in kwargs:
-            kwargs['stale_if_error'] = kwargs.pop('old_data_on_error')
-        if 'include_get_headers' in kwargs:
-            kwargs['match_headers'] = kwargs.pop('include_get_headers')
+    # Additional settings that may be set for an individual request; not used at session level
+    refresh: bool = field(default=False)
+    revalidate: bool = field(default=False)
+    request_expire_after: ExpirationTime = field(default=None)
 
-        # Ignore invalid kwargs for easier initialization from mixed **kwargs
+    def __init__(self, **kwargs):
+        """Ignore invalid kwargs for easier initialization from mixed ``**kwargs``"""
+        kwargs = self._rename_kwargs(kwargs)
         kwargs = get_valid_kwargs(self.__attrs_init__, kwargs)
         self.__attrs_init__(**kwargs)
 
+    def update(self, **kwargs):
+        """Update settings with new values"""
+        for k, v in self._rename_kwargs(kwargs).items():
+            if hasattr(self, k):
+                setattr(self, k, v)
+
+    @staticmethod
+    def _rename_kwargs(kwargs):
+        """Handle some deprecated argument names"""
+        kwargs.setdefault('stale_if_error', kwargs.pop('old_data_on_error', False))
+        kwargs.setdefault('match_headers', kwargs.pop('include_get_headers', False))
+        return kwargs
+
 
 @define(init=False)
 class RequestSettings(CacheSettings):
-    """Cache settings that may be set for an individual request"""
-
-    refresh: bool = field(default=False)
-    revalidate: bool = field(default=False)
-    request_expire_after: ExpirationTime = field(default=None)
+    """Cache settings that may be set for an individual request. Starts with session-level cache
+    settings and adds/overrides with request-level settings"""
 
     def __init__(self, session_settings: CacheSettings = None, **kwargs):
-        # Start with session-level cache settings and add/override with request-level settings
         session_kwargs = asdict(session_settings) if session_settings else {}
+        # request-level expiration needs to be stored separately
         kwargs['request_expire_after'] = kwargs.pop('expire_after', None)
-        kwargs = {**session_kwargs, **kwargs}
-        super().__init__(**kwargs)
+        super().__init__(**{**session_kwargs, **kwargs})
diff --git a/requests_cache/serializers/__init__.py b/requests_cache/serializers/__init__.py
index 08ac11c..1e6e6c0 100644
--- a/requests_cache/serializers/__init__.py
+++ b/requests_cache/serializers/__init__.py
@@ -36,7 +36,7 @@ SERIALIZERS = {
 
 
 def init_serializer(serializer=None, **kwargs):
-    """Initialize a serializer from a name, class, or instance"""
+    """Initialize a serializer from a name or instance"""
     serializer = serializer or 'pickle'
     if isinstance(serializer, str):
         serializer = SERIALIZERS[serializer]
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 8341b97..c10dbda 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -25,15 +25,9 @@ from urllib3 import filepost
 
 from ._utils import get_valid_kwargs
 from .backends import BackendSpecifier, init_backend
-from .cache_control import CacheActions, append_directive
+from .cache_control import REFRESH_TEMP_HEADER, CacheActions, append_directive
 from .expiration import ExpirationTime, get_expiration_seconds
-from .models import (
-    AnyResponse,
-    CachedResponse,
-    CacheSettings,
-    RequestSettings,
-    set_response_defaults,
-)
+from .models import AnyResponse, CachedResponse, CacheSettings, set_response_defaults
 
 __all__ = ['ALL_METHODS', 'CachedSession', 'CacheMixin']
 ALL_METHODS = ['GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'PATCH', 'DELETE']
@@ -48,8 +42,6 @@ else:
 
 # TODO: Better docs for __init__
 # TODO: Better function signatures (due to passing around **kwargs instead of explicit keyword args)
-
-
 class CacheMixin(MIXIN_BASE):
     """Mixin class that extends :py:class:`requests.Session` with caching features.
     See :py:class:`.CachedSession` for usage details.
@@ -84,7 +76,7 @@ class CacheMixin(MIXIN_BASE):
         (get, post, etc.). This is not used by :py:class:`~requests.PreparedRequest` objects, which
         are handled by :py:meth:`send()`.
 
-        See :py:meth:`requests.Session.request` for parameters. Additional parameters:
+        See :py:meth:`requests.Session.request` for base parameters. Additional parameters:
 
         Args:
             expire_after: Expiration time to set only for this request; see details below.
@@ -97,16 +89,6 @@ class CacheMixin(MIXIN_BASE):
 
         Returns:
             Either a new or cached response
-
-        **Order of operations:** For reference, a request will pass through the following methods:
-
-        1. :py:func:`requests.get`/:py:meth:`requests.Session.get` or other method-specific functions (optional)
-        2. :py:meth:`.CachedSession.request`
-        3. :py:meth:`requests.Session.request`
-        4. :py:meth:`.CachedSession.send`
-        5. :py:meth:`.BaseCache.get_response`
-        6. :py:meth:`requests.Session.send` (if not previously cached)
-        7. :py:meth:`.BaseCache.save_response` (if not previously cached)
         """
         # Set extra options as headers to be handled in send(), since we can't pass args directly
         headers = headers or {}
@@ -117,22 +99,29 @@ class CacheMixin(MIXIN_BASE):
         if revalidate:
             headers = append_directive(headers, 'no-cache')
         if refresh:
-            headers['requests-cache-refresh'] = 'true'
+            headers[REFRESH_TEMP_HEADER] = 'true'
         kwargs['headers'] = headers
 
         with patch_form_boundary(**kwargs):
             return super().request(method, url, *args, **kwargs)
 
     def send(self, request: PreparedRequest, **kwargs) -> AnyResponse:
-        """Send a prepared request, with caching. See :py:meth:`.request` for notes on behavior, and
-        see :py:meth:`requests.Session.send` for parameters.
+        """Send a prepared request, with caching. See :py:meth:`requests.Session.send` for base
+        parameters, and see :py:meth:`.request` for extra parameters.
+
+        **Order of operations:** For reference, a request will pass through the following methods:
+
+        1. :py:func:`requests.get`/:py:meth:`requests.Session.get` or other method-specific functions (optional)
+        2. :py:meth:`.CachedSession.request`
+        3. :py:meth:`requests.Session.request`
+        4. :py:meth:`.CachedSession.send`
+        5. :py:meth:`.BaseCache.get_response`
+        6. :py:meth:`requests.Session.send` (if not previously cached)
+        7. :py:meth:`.BaseCache.save_response` (if not previously cached)
         """
         # Determine which actions to take based on settings and request info
         actions = CacheActions.from_request(
-            self.cache.create_key(request, **kwargs),
-            request,
-            RequestSettings(self.settings, **kwargs),
-            **kwargs,
+            self.cache.create_key(request, **kwargs), request, self.settings, **kwargs
         )
 
         # Attempt to fetch a cached response
diff --git a/tests/unit/test_cache_control.py b/tests/unit/test_cache_control.py
index ac9342a..64c6745 100644
--- a/tests/unit/test_cache_control.py
+++ b/tests/unit/test_cache_control.py
@@ -60,7 +60,6 @@ def test_init(
         ({'Cache-Control': 'max-age=60'}, 60),
         ({'Cache-Control': 'public, max-age=60'}, 60),
         ({'Cache-Control': 'max-age=0'}, DO_NOT_CACHE),
-        ({'Cache-Control': 'no-store'}, DO_NOT_CACHE),
     ],
 )
 def test_init_from_headers(headers, expected_expiration):
@@ -71,13 +70,23 @@ def test_init_from_headers(headers, expected_expiration):
     assert actions.cache_key == 'key'
     if expected_expiration == DO_NOT_CACHE:
         assert actions.skip_read is True
-        assert actions.skip_write is True
     else:
         assert actions.expire_after == expected_expiration
         assert actions.skip_read is False
         assert actions.skip_write is False
 
 
+def test_init_from_headers__no_store():
+    """Test with Cache-Control request headers"""
+    settings = RequestSettings(cache_control=True)
+    actions = CacheActions.from_request(
+        'key', MagicMock(headers={'Cache-Control': 'no-store'}), settings
+    )
+
+    assert actions.skip_read is True
+    assert actions.skip_write is True
+
+
 @pytest.mark.parametrize(
     'url, request_expire_after, expected_expiration',
     [
@@ -116,27 +125,23 @@ def test_init_from_settings(url, request_expire_after, expected_expiration):
 
 
 @pytest.mark.parametrize(
-    'cache_control, headers, expire_after, expected_expiration, expected_skip_read',
+    'headers, expire_after, expected_expiration, expected_skip_read',
     [
-        (False, {'Cache-Control': 'max-age=60'}, 1, 60, False),
-        (False, {}, 1, 1, False),
-        (False, {}, 0, 0, True),
-        (True, {'Cache-Control': 'max-age=60'}, 1, 60, False),
-        (True, {'Cache-Control': 'max-age=0'}, 1, 0, True),
-        (True, {'Cache-Control': 'no-store'}, 1, 1, True),
-        (True, {'Cache-Control': 'no-cache'}, 1, 1, False),
-        (True, {}, 1, 1, False),
-        (True, {}, 0, 0, False),
+        ({'Cache-Control': 'max-age=60'}, 1, 60, False),
+        ({}, 1, 1, False),
+        ({}, 0, 0, True),
+        ({'Cache-Control': 'max-age=60'}, 1, 60, False),
+        ({'Cache-Control': 'max-age=0'}, 1, 0, True),
+        ({'Cache-Control': 'no-store'}, 1, 1, True),
+        ({'Cache-Control': 'no-cache'}, 1, 1, False),
     ],
 )
 def test_init_from_settings_and_headers(
-    cache_control, headers, expire_after, expected_expiration, expected_skip_read
+    headers, expire_after, expected_expiration, expected_skip_read
 ):
-    """Test behavior with both cache settings and request headers. The only variation in behavior
-    with cache_control=True is that expire_after=0 should *not* cause the cache read to be skipped.
-    """
+    """Test behavior with both cache settings and request headers."""
     request = get_mock_response(headers=headers)
-    settings = CacheSettings(cache_control=cache_control, expire_after=expire_after)
+    settings = CacheSettings(expire_after=expire_after)
     actions = CacheActions.from_request('key', request, RequestSettings(settings))
 
     assert actions.expire_after == expected_expiration
@@ -167,8 +172,7 @@ def test_update_from_cached_response(response_headers, expected_validation_heade
     )
 
     actions.update_from_cached_response(cached_response)
-    assert actions.validation_headers == expected_validation_headers
-    assert actions.revalidate is bool(expected_validation_headers)
+    assert actions._validation_headers == expected_validation_headers
 
 
 @pytest.mark.parametrize(
@@ -190,13 +194,12 @@ def test_update_from_cached_response__revalidate_headers(request_headers, respon
     cached_response = CachedResponse(headers={'ETag': ETAG, **response_headers}, expires=None)
 
     actions.update_from_cached_response(cached_response)
-    assert actions.revalidate is True
-    assert actions.validation_headers == {'If-None-Match': ETAG}
+    assert actions._validation_headers == {'If-None-Match': ETAG}
 
 
 def test_update_from_cached_response__ignored():
     """Conditional request headers should NOT be added if the cached response is not expired and
-    revalidation is not requested by headers"""
+    revalidation is otherwise not requested"""
     actions = CacheActions.from_request(
         'key',
         MagicMock(url='https://img.site.com/base/img.jpg', headers={}),
@@ -207,19 +210,18 @@ def test_update_from_cached_response__ignored():
     )
 
     actions.update_from_cached_response(cached_response)
-    assert actions.validation_headers == {}
-    assert actions.revalidate is False
+    assert actions._validation_headers == {}
 
 
 @pytest.mark.parametrize(
     'headers, expected_expiration',
     [
         ({}, None),
-        ({'Cache-Control': 'no-cache'}, None),  # Only valid for request headers
+        ({'Cache-Control': 'no-cache'}, None),  # Forces revalidation, but no effect on expiration
+        ({'Cache-Control': 'max-age=0'}, 0),
         ({'Cache-Control': 'max-age=60'}, 60),
         ({'Cache-Control': 'public, max-age=60'}, 60),
-        ({'Cache-Control': 'max-age=0'}, DO_NOT_CACHE),
-        ({'Cache-Control': 'no-store'}, DO_NOT_CACHE),
+        ({'Cache-Control': 'max-age=0'}, 0),
         ({'Cache-Control': 'immutable'}, -1),
         ({'Cache-Control': 'immutable, max-age=60'}, -1),  # Immutable should take precedence
         ({'Expires': HTTPDATE_STR}, HTTPDATE_STR),
@@ -234,12 +236,17 @@ def test_update_from_response(headers, expected_expiration):
     )
     actions.update_from_response(get_mock_response(headers=headers))
 
-    if expected_expiration == DO_NOT_CACHE:
-        assert not actions.expire_after  # May be either 0 or None
-        assert actions.skip_write is True
-    else:
-        assert actions.expire_after == expected_expiration
-        assert actions.skip_write is False
+    assert actions.expire_after == expected_expiration
+    assert actions.skip_write is (expected_expiration == DO_NOT_CACHE)
+
+
+def test_update_from_response__no_store():
+    url = 'https://img.site.com/base/img.jpg'
+    actions = CacheActions.from_request(
+        'key', MagicMock(url=url), RequestSettings(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():
@@ -279,6 +286,5 @@ def test_ignored_headers(directive):
     actions = CacheActions.from_request('key', request, RequestSettings(settings))
 
     assert actions.expire_after == 1
-    assert actions.revalidate is False
     assert actions.skip_read is False
     assert actions.skip_write is False