Also skip cache read for requests excluded by allowable_methods

6 files changed, 81 insertions, 32 deletions

diff --git a/HISTORY.md b/HISTORY.md
index 797aa78..a9aea84 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -26,6 +26,7 @@
   * `is_expired`
 * Populate `cache_key` and `expires` for new (non-cached) responses, if it was written to the cache
 * Add return type hints for all `CachedSession` request methods (`get()`, `post()`, etc.)
+* Always skip both cache read and write for requests excluded by `allowable_methods` (previously only skipped write)
 
 **Dependencies:**
 * Replace `appdirs` with `platformdirs`
diff --git a/requests_cache/cache_control.py b/requests_cache/cache_control.py
index f32d368..c02289c 100644
--- a/requests_cache/cache_control.py
+++ b/requests_cache/cache_control.py
@@ -107,6 +107,7 @@ class CacheActions:
             or force_refresh
             or settings.disabled
             or expire_after == DO_NOT_CACHE
+            or str(request.method) not in settings.allowable_methods
         )
 
         actions = cls(
diff --git a/tests/conftest.py b/tests/conftest.py
index 5ece398..7de9b6d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -63,6 +63,7 @@ MOCKED_URL_JSON = 'http+mock://requests-cache.com/json'
 MOCKED_URL_REDIRECT = 'http+mock://requests-cache.com/redirect'
 MOCKED_URL_REDIRECT_TARGET = 'http+mock://requests-cache.com/redirect_target'
 MOCKED_URL_404 = 'http+mock://requests-cache.com/nonexistent'
+MOCKED_URL_500 = 'http+mock://requests-cache.com/answer?q=this-statement-is-false'
 MOCK_PROTOCOLS = ['mock://', 'http+mock://', 'https+mock://']
 
 PROJECT_DIR = abspath(dirname(dirname(__file__)))
@@ -212,11 +213,8 @@ def get_mock_adapter() -> Adapter:
         text='mock redirected response',
         status_code=200,
     )
-    adapter.register_uri(
-        ANY_METHOD,
-        MOCKED_URL_404,
-        status_code=404,
-    )
+    adapter.register_uri(ANY_METHOD, MOCKED_URL_404, status_code=404)
+    adapter.register_uri(ANY_METHOD, MOCKED_URL_500, status_code=500)
     return adapter
 
 
diff --git a/tests/unit/test_cache_control.py b/tests/unit/test_cache_control.py
index 43f75a9..5bfa2ad 100644
--- a/tests/unit/test_cache_control.py
+++ b/tests/unit/test_cache_control.py
@@ -2,12 +2,12 @@ from datetime import datetime, timedelta
 from unittest.mock import MagicMock, patch
 
 import pytest
-from requests import PreparedRequest
+from requests import PreparedRequest, Request
 
 from requests_cache.cache_control import EXPIRE_IMMEDIATELY, CacheActions
 from requests_cache.models import CachedResponse
 from requests_cache.settings import CacheSettings
-from tests.conftest import ETAG, HTTPDATE_STR, LAST_MODIFIED, get_mock_response
+from tests.conftest import ETAG, HTTPDATE_STR, LAST_MODIFIED, MOCKED_URL, get_mock_response
 
 IGNORED_DIRECTIVES = [
     'no-transform',
@@ -61,7 +61,8 @@ def test_init(
 def test_init_from_headers(headers, expected_expiration):
     """Test with Cache-Control request headers"""
     settings = CacheSettings(cache_control=True)
-    actions = CacheActions.from_request('key', MagicMock(headers=headers), settings)
+    request = Request(method='GET', url=MOCKED_URL, headers=headers).prepare()
+    actions = CacheActions.from_request('key', request, settings)
 
     assert actions.cache_key == 'key'
     if expected_expiration != EXPIRE_IMMEDIATELY:
@@ -73,9 +74,8 @@ def test_init_from_headers(headers, expected_expiration):
 def test_init_from_headers__no_store():
     """Test with Cache-Control request headers"""
     settings = CacheSettings(cache_control=True)
-    actions = CacheActions.from_request(
-        'key', MagicMock(headers={'Cache-Control': 'no-store'}), settings
-    )
+    request = Request(method='GET', url=MOCKED_URL, headers={'Cache-Control': 'no-store'}).prepare()
+    actions = CacheActions.from_request('key', request, settings)
 
     assert actions.skip_read is True
     assert actions.skip_write is True
@@ -84,19 +84,19 @@ def test_init_from_headers__no_store():
 @pytest.mark.parametrize(
     'url, request_expire_after, expected_expiration',
     [
-        ('img.site_1.com', None, timedelta(hours=12)),
-        ('img.site_1.com', 60, 60),
-        ('http://img.site.com/base/', None, 1),
+        ('https://img.site_1.com', None, timedelta(hours=12)),
+        ('https://img.site_1.com', 60, 60),
+        ('https://img.site.com/base/', None, 1),
         ('https://img.site.com/base/img.jpg', None, 1),
-        ('site_2.com/resource_1', None, timedelta(hours=20)),
-        ('http://site_2.com/resource_1/index.html', None, timedelta(hours=20)),
+        ('http://site_2.com/resource_1', None, timedelta(hours=20)),
+        ('ftp://site_2.com/resource_1/index.html', None, timedelta(hours=20)),
         ('http://site_2.com/resource_2/', None, timedelta(days=7)),
         ('http://site_2.com/static/', None, -1),
         ('http://site_2.com/static/img.jpg', None, -1),
-        ('site_2.com', None, 1),
-        ('site_2.com', 60, 60),
-        ('some_other_site.com', None, 1),
-        ('some_other_site.com', 60, 60),
+        ('http://site_2.com', None, 1),
+        ('http://site_2.com', 60, 60),
+        ('https://some_other_site.com', None, 1),
+        ('https://some_other_site.com', 60, 60),
     ],
 )
 def test_init_from_settings(url, request_expire_after, expected_expiration):
@@ -110,11 +110,11 @@ def test_init_from_settings(url, request_expire_after, expected_expiration):
             'site_2.com/static': -1,
         },
     )
-    request = MagicMock(url=url)
+    request = Request(method='GET', url=url)
     if request_expire_after:
         request.headers = {'Cache-Control': f'max-age={request_expire_after}'}
 
-    actions = CacheActions.from_request('key', request, settings)
+    actions = CacheActions.from_request('key', request.prepare(), settings)
     assert actions.expire_after == expected_expiration
 
 
@@ -134,7 +134,7 @@ def test_init_from_settings_and_headers(
     headers, expire_after, expected_expiration, expected_skip_read
 ):
     """Test behavior with both cache settings and request headers."""
-    request = get_mock_response(headers=headers)
+    request = Request(method='GET', url=MOCKED_URL, headers=headers)
     settings = CacheSettings(expire_after=expire_after)
     actions = CacheActions.from_request('key', request, settings)
 
@@ -266,9 +266,10 @@ def test_update_from_response__revalidate(mock_datetime, cache_headers, validato
 @pytest.mark.parametrize('directive', IGNORED_DIRECTIVES)
 def test_ignored_headers(directive):
     """Ensure that currently unimplemented Cache-Control headers do not affect behavior"""
-    request = PreparedRequest()
-    request.url = 'https://img.site.com/base/img.jpg'
-    request.headers = {'Cache-Control': 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_patcher.py b/tests/unit/test_patcher.py
index 1004dd8..6043c86 100644
--- a/tests/unit/test_patcher.py
+++ b/tests/unit/test_patcher.py
@@ -73,6 +73,14 @@ def test_enabled(cached_request, original_request, tempfile_path):
     assert original_request.call_count == 0
 
 
+def test_is_installed():
+    assert requests_cache.is_installed() is False
+    requests_cache.install_cache(name=CACHE_NAME, use_temp=True)
+    assert requests_cache.is_installed() is True
+    requests_cache.uninstall_cache()
+    assert requests_cache.is_installed() is False
+
+
 @patch.object(BaseCache, 'remove_expired_responses')
 def test_remove_expired_responses(remove_expired_responses, tempfile_path):
     requests_cache.install_cache(tempfile_path, expire_after=360)
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index f5392e5..4fa1221 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -22,6 +22,7 @@ from requests_cache.expiration import DO_NOT_CACHE, EXPIRE_IMMEDIATELY
 from tests.conftest import (
     MOCKED_URL,
     MOCKED_URL_404,
+    MOCKED_URL_500,
     MOCKED_URL_ETAG,
     MOCKED_URL_HTTPS,
     MOCKED_URL_JSON,
@@ -53,11 +54,11 @@ def test_init_cache_path_expansion():
     assert session.cache.cache_dir == Path("~").expanduser()
 
 
-@patch.dict(BACKEND_CLASSES, {'mongo': get_placeholder_class()})
+@patch.dict(BACKEND_CLASSES, {'mongodb': get_placeholder_class()})
 def test_init_missing_backend_dependency():
     """Test that the correct error is thrown when a user does not have a dependency installed"""
     with pytest.raises(ImportError):
-        CachedSession(backend='mongo')
+        CachedSession(backend='mongodb')
 
 
 def test_repr(mock_session):
@@ -413,13 +414,46 @@ def test_unpickle_errors(mock_session):
 # -----------------------------------------------------
 
 
+def test_allowable_codes(mock_session):
+    mock_session.settings.allowable_codes = (200, 404)
+
+    # This request should be cached
+    mock_session.get(MOCKED_URL_404)
+    assert mock_session.cache.has_url(MOCKED_URL_404)
+    assert mock_session.get(MOCKED_URL_404).from_cache is True
+
+    # This request should be filtered out on both read and write
+    mock_session.get(MOCKED_URL_500)
+    assert not mock_session.cache.has_url(MOCKED_URL_500)
+    assert mock_session.get(MOCKED_URL_500).from_cache is False
+
+
+def test_allowable_methods(mock_session):
+    mock_session.settings.allowable_methods = ['GET', 'OPTIONS']
+
+    # This request should be cached
+    mock_session.options(MOCKED_URL)
+    assert mock_session.cache.has_url(MOCKED_URL, method='OPTIONS')
+    assert mock_session.options(MOCKED_URL).from_cache is True
+
+    # This request should be filtered out on both read and write
+    mock_session.put(MOCKED_URL)
+    assert not mock_session.cache.has_url(MOCKED_URL, method='PUT')
+    assert mock_session.put(MOCKED_URL).from_cache is False
+
+
 def test_filter_fn(mock_session):
     mock_session.settings.filter_fn = lambda r: r.request.url != MOCKED_URL_JSON
-    mock_session.get(MOCKED_URL)
-    mock_session.get(MOCKED_URL_JSON)
 
+    # This request should be cached
+    mock_session.get(MOCKED_URL)
     assert mock_session.cache.has_url(MOCKED_URL)
+    assert mock_session.get(MOCKED_URL).from_cache is True
+
+    # This request should be filtered out on both read and write
+    mock_session.get(MOCKED_URL_JSON)
     assert not mock_session.cache.has_url(MOCKED_URL_JSON)
+    assert mock_session.get(MOCKED_URL_JSON).from_cache is False
 
 
 def test_filter_fn__retroactive(mock_session):
@@ -427,7 +461,6 @@ def test_filter_fn__retroactive(mock_session):
     mock_session.get(MOCKED_URL_JSON)
     mock_session.settings.filter_fn = lambda r: r.request.url != MOCKED_URL_JSON
     mock_session.get(MOCKED_URL_JSON)
-
     assert not mock_session.cache.has_url(MOCKED_URL_JSON)
 
 
@@ -458,6 +491,12 @@ def test_hooks(mock_session):
         assert state[hook] == 5
 
 
+def test_expire_after_alias(mock_session):
+    """CachedSession has an `expire_after` property for backwards-compatibility"""
+    mock_session.expire_after = 60
+    assert mock_session.expire_after == mock_session.settings.expire_after == 60
+
+
 def test_do_not_cache(mock_session):
     """DO_NOT_CACHE should bypass the cache on both read and write"""
     mock_session.get(MOCKED_URL)
@@ -519,12 +558,13 @@ def test_url_allowlist(mock_session):
     """If the default is 0, only URLs matching patterns in urls_expire_after should be cached"""
     mock_session.settings.urls_expire_after = {
         MOCKED_URL_JSON: 60,
-        '*': 0,
+        '*': DO_NOT_CACHE,
     }
     mock_session.get(MOCKED_URL_JSON)
     assert mock_session.get(MOCKED_URL_JSON).from_cache is True
     mock_session.get(MOCKED_URL)
     assert mock_session.get(MOCKED_URL).from_cache is False
+    assert not mock_session.cache.has_url(MOCKED_URL)
 
 
 def test_remove_expired_responses(mock_session):