From ed2e2494d3789b74ab5fb7334770212b51b90640 Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 5 Apr 2023 14:48:58 -0700 Subject: [PATCH 1/7] :sparkles: Add caching logic to static content --- config/settings.py | 8 ++++++++ core/views.py | 24 ++++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/config/settings.py b/config/settings.py index c46debcd..244e7251 100755 --- a/config/settings.py +++ b/config/settings.py @@ -263,8 +263,16 @@ CACHES = { "BACKEND": "django_redis.cache.RedisCache", "LOCATION": f"redis://{REDIS_HOST}:6379", }, + "static_content": { + "BACKEND": "django_redis.cache.RedisCache", + "LOCATION": f"redis://{REDIS_HOST}:6379", + "TIMEOUT": env( + "STATIC_CACHE_TIMEOUT", default=86400 + ), # Cache timeout in seconds: 1 day + }, } + HAYSTACK_CONNECTIONS = { "default": { "ENGINE": "haystack.backends.simple_backend.SimpleEngine", diff --git a/core/views.py b/core/views.py index 38213395..d911d424 100644 --- a/core/views.py +++ b/core/views.py @@ -2,6 +2,7 @@ import os.path import re from django.conf import settings +from django.core.cache import caches from django.http import Http404, HttpResponse from django.template.response import TemplateResponse from django.views.generic import TemplateView @@ -78,11 +79,26 @@ class StaticContentTemplateView(TemplateView): """ Verifies the file and returns the frontmatter and content """ - result = get_content_from_s3(key=kwargs.get("content_path")) - if not result: - raise Http404("Page not found") + content_path = kwargs.get("content_path") - content, content_type = result + # Get the static content cache + static_content_cache = caches["static_content"] + + # Check if the content is in the cache + cache_key = f"static_content_{content_path}" + cached_result = static_content_cache.get(cache_key) + + if cached_result: + content, content_type = cached_result + else: + # Fetch content from S3 if not in cache + result = get_content_from_s3(key=content_path) + if not result: + raise Http404("Page not found") + content, content_type = result + + # Store the result in cache + static_content_cache.set(cache_key, (content, content_type)) response = HttpResponse(content, content_type=content_type) return response From 1a42c133532f4a82b3a9c80e61acab3c1750dee3 Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 5 Apr 2023 14:52:46 -0700 Subject: [PATCH 2/7] :construction: :umbrella: Add a placeholder test for caching Having trouble with the mocks --- core/tests/test_views.py | 81 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/core/tests/test_views.py b/core/tests/test_views.py index e21a5b17..0a3f7618 100644 --- a/core/tests/test_views.py +++ b/core/tests/test_views.py @@ -1,10 +1,89 @@ import pytest +import responses -from django.test import RequestFactory +from django.core.cache import caches +from django.test import RequestFactory, TestCase +from django.test.utils import override_settings from django.urls import reverse from core.views import StaticContentTemplateView +from unittest.mock import patch +from django.core.cache import caches + + +@pytest.mark.skip(reason="Having trouble with mocking") +# Override cache settings to use local memory cache for testing +@override_settings( + CACHES={ + "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}, + "machina_attachments": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache" + }, + "static_content": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "TIMEOUT": 86400, + }, + } +) +class TestStaticContentTemplateView(TestCase): + def setUp(self): + self.factory = RequestFactory() + self.content_path = "/some/content/path" + + @pytest.mark.django_db + @responses.activate + @patch("core.boostrenderer.get_content_from_s3") + def test_cache_behavior(self, mock_get_content_from_s3): + # Set up the mocked API call + mock_content = "mocked content" + mock_content_type = "text/plain" + mock_get_content_from_s3.return_value = (mock_content, mock_content_type) + + # Mock the API request using the `responses` library + responses.add( + responses.GET, + "s3://static.boost.org/site/develop/some/content/path/", + body=mock_content, + status=200, + content_type=mock_content_type, + ) + + # Clear the cache before testing + cache = caches["static_content"] + cache.clear() + + # Cache miss scenario + response = self.call_view(self.content_path) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content.decode(), mock_content) + self.assertEqual(response["Content-Type"], mock_content_type) + mock_get_content_from_s3.assert_called_once_with(key=self.content_path) + + # # Cache hit scenario + # mock_get_content_from_s3.reset_mock() + # response = self.call_view(self.content_path) + # self.assertEqual(response.status_code, 200) + # self.assertEqual(response.content.decode(), mock_content) + # self.assertEqual(response['Content-Type'], mock_content_type) + # mock_get_content_from_s3.assert_not_called() + + # # Cache expiration scenario + # cache.set(self.content_path, (mock_content, mock_content_type), 1) # Set a 1-second cache timeout + # time.sleep(2) # Wait for the cache to expire + # mock_get_content_from_s3.reset_mock() + # response = self.call_view(self.content_path) + # self.assertEqual(response.status_code, 200) + # self.assertEqual(response.content.decode(), mock_content) + # self.assertEqual(response['Content-Type'], mock_content_type) + # mock_get_content_from_s3.assert_called_once_with(key=self.content_path) + + def call_view(self, content_path): + request = self.factory.get(content_path) + view = StaticContentTemplateView.as_view() + response = view(request, content_path=content_path) + return response + static_content_test_cases = [ "/site/develop/rst.css", From 2d987242c4e59528f3e368b461af73d0266e1040 Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 19 Apr 2023 11:21:21 -0700 Subject: [PATCH 3/7] :umbrella: Fix cache tests --- core/tests/test_views.py | 108 ++++++++++++++++++++------------------- core/views.py | 2 +- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/core/tests/test_views.py b/core/tests/test_views.py index 0a3f7618..d9129f73 100644 --- a/core/tests/test_views.py +++ b/core/tests/test_views.py @@ -1,5 +1,6 @@ import pytest import responses +import time from django.core.cache import caches from django.test import RequestFactory, TestCase @@ -12,8 +13,27 @@ from unittest.mock import patch from django.core.cache import caches -@pytest.mark.skip(reason="Having trouble with mocking") -# Override cache settings to use local memory cache for testing +@pytest.fixture +def request_factory(): + """Returns a RequestFactory instance.""" + return RequestFactory() + + +@pytest.fixture +def content_path(): + """Returns a sample content path.""" + return "/some/content/path" + + +def call_view(request_factory, content_path): + """Calls the view with the given request_factory and content path.""" + request = request_factory.get(content_path) + view = StaticContentTemplateView.as_view() + response = view(request, content_path=content_path) + return response + + +@pytest.mark.django_db @override_settings( CACHES={ "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}, @@ -26,63 +46,45 @@ from django.core.cache import caches }, } ) -class TestStaticContentTemplateView(TestCase): - def setUp(self): - self.factory = RequestFactory() - self.content_path = "/some/content/path" +def test_cache_behavior(request_factory, content_path): + """Tests the cache behavior of the StaticContentTemplateView view.""" + # Set up the mocked API call + mock_content = "mocked content" + mock_content_type = "text/plain" - @pytest.mark.django_db - @responses.activate - @patch("core.boostrenderer.get_content_from_s3") - def test_cache_behavior(self, mock_get_content_from_s3): - # Set up the mocked API call - mock_content = "mocked content" - mock_content_type = "text/plain" + # Clear the cache before testing + cache = caches["static_content"] + cache.clear() + + with patch("core.views.get_content_from_s3") as mock_get_content_from_s3: mock_get_content_from_s3.return_value = (mock_content, mock_content_type) - # Mock the API request using the `responses` library - responses.add( - responses.GET, - "s3://static.boost.org/site/develop/some/content/path/", - body=mock_content, - status=200, - content_type=mock_content_type, - ) - - # Clear the cache before testing - cache = caches["static_content"] - cache.clear() - # Cache miss scenario - response = self.call_view(self.content_path) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.content.decode(), mock_content) - self.assertEqual(response["Content-Type"], mock_content_type) - mock_get_content_from_s3.assert_called_once_with(key=self.content_path) + response = call_view(request_factory, content_path) + assert response.status_code == 200 + assert response.content.decode() == mock_content + assert response["Content-Type"] == mock_content_type + mock_get_content_from_s3.assert_called_once_with(key=content_path) - # # Cache hit scenario - # mock_get_content_from_s3.reset_mock() - # response = self.call_view(self.content_path) - # self.assertEqual(response.status_code, 200) - # self.assertEqual(response.content.decode(), mock_content) - # self.assertEqual(response['Content-Type'], mock_content_type) - # mock_get_content_from_s3.assert_not_called() + # Cache hit scenario + mock_get_content_from_s3.reset_mock() + response = call_view(request_factory, content_path) + assert response.status_code == 200 + assert response.content.decode() == mock_content + assert response["Content-Type"] == mock_content_type + mock_get_content_from_s3.assert_not_called() - # # Cache expiration scenario - # cache.set(self.content_path, (mock_content, mock_content_type), 1) # Set a 1-second cache timeout - # time.sleep(2) # Wait for the cache to expire - # mock_get_content_from_s3.reset_mock() - # response = self.call_view(self.content_path) - # self.assertEqual(response.status_code, 200) - # self.assertEqual(response.content.decode(), mock_content) - # self.assertEqual(response['Content-Type'], mock_content_type) - # mock_get_content_from_s3.assert_called_once_with(key=self.content_path) - - def call_view(self, content_path): - request = self.factory.get(content_path) - view = StaticContentTemplateView.as_view() - response = view(request, content_path=content_path) - return response + # Cache expiration scenario + cache.set( + "static_content_" + content_path, (mock_content, mock_content_type), 1 + ) # Set a 1-second cache timeout + time.sleep(2) # Wait for the cache to expire + mock_get_content_from_s3.reset_mock() + response = call_view(request_factory, content_path) + assert response.status_code == 200 + assert response.content.decode() == mock_content + assert response["Content-Type"] == mock_content_type + mock_get_content_from_s3.assert_called_once_with(key=content_path) static_content_test_cases = [ diff --git a/core/views.py b/core/views.py index 479bdc62..2d178e44 100644 --- a/core/views.py +++ b/core/views.py @@ -129,4 +129,4 @@ class StaticContentTemplateView(View): key=kwargs.get("content_path"), status_code=response.status_code, ) - return response \ No newline at end of file + return response From 3fe65e7051c80f99dd74d304499dccf7f9ce69b9 Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 19 Apr 2023 11:24:04 -0700 Subject: [PATCH 4/7] :shirt: Fix imports --- core/tests/test_views.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/tests/test_views.py b/core/tests/test_views.py index d9129f73..8a0a1726 100644 --- a/core/tests/test_views.py +++ b/core/tests/test_views.py @@ -1,17 +1,14 @@ import pytest -import responses import time +from unittest.mock import patch from django.core.cache import caches -from django.test import RequestFactory, TestCase +from django.test import RequestFactory from django.test.utils import override_settings from django.urls import reverse from core.views import StaticContentTemplateView -from unittest.mock import patch -from django.core.cache import caches - @pytest.fixture def request_factory(): From 4f99628ea78f835fc28f21d1377161c1bd5630b4 Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 19 Apr 2023 11:30:10 -0700 Subject: [PATCH 5/7] :umbrella: Fix the test that's failing in CI --- ak/tests/test_default_pages.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/ak/tests/test_default_pages.py b/ak/tests/test_default_pages.py index 98722d5a..62da00b2 100644 --- a/ak/tests/test_default_pages.py +++ b/ak/tests/test_default_pages.py @@ -1,6 +1,8 @@ import pytest import random +from django.test.utils import override_settings + def test_homepage(db, tp): """Ensure we can hit the homepage""" @@ -27,8 +29,28 @@ def test_403_page(db, tp): tp.response_403(response) +@override_settings( + CACHES={ + "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}, + "machina_attachments": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache" + }, + "static_content": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "TIMEOUT": 86400, + }, + } +) def test_404_page(db, tp): - """Test a 404 error page""" + """ + Test a 404 error page + + This test is a bit more complicated than the others because the + this/should/not/exist URL will hit StaticContentTemplateView first + to see if there is static content to serve, and cache it if so. To avoid + errors in CI, we need to make sure that the cache is cleared before + running this test. + """ rando = random.randint(1000, 20000) url = f"/this/should/not/exist/{rando}/" From eee4f39bea2d6cb34808bcd9031bc86d7d099e3d Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 19 Apr 2023 11:32:26 -0700 Subject: [PATCH 6/7] :shirt: Linter --- ak/tests/test_default_pages.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ak/tests/test_default_pages.py b/ak/tests/test_default_pages.py index 62da00b2..6433a4ec 100644 --- a/ak/tests/test_default_pages.py +++ b/ak/tests/test_default_pages.py @@ -44,10 +44,10 @@ def test_403_page(db, tp): def test_404_page(db, tp): """ Test a 404 error page - - This test is a bit more complicated than the others because the + + This test is a bit more complicated than the others because the this/should/not/exist URL will hit StaticContentTemplateView first - to see if there is static content to serve, and cache it if so. To avoid + to see if there is static content to serve, and cache it if so. To avoid errors in CI, we need to make sure that the cache is cleared before running this test. """ From eb23b08e11ab8ab666da718b6edf9ff18ce4aada Mon Sep 17 00:00:00 2001 From: Lacey Williams Henschel Date: Wed, 3 May 2023 14:11:17 -0700 Subject: [PATCH 7/7] :snowflake: Make separate redis db for static content --- config/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/settings.py b/config/settings.py index 8ac3c91b..16a60432 100755 --- a/config/settings.py +++ b/config/settings.py @@ -269,7 +269,7 @@ CACHES = { }, "static_content": { "BACKEND": "django_redis.cache.RedisCache", - "LOCATION": f"redis://{REDIS_HOST}:6379", + "LOCATION": f"redis://{REDIS_HOST}:6379/2", "TIMEOUT": env( "STATIC_CACHE_TIMEOUT", default=86400 ), # Cache timeout in seconds: 1 day