From d90d427517cf81cb30a126716f3ccd8cf8e1f03b Mon Sep 17 00:00:00 2001 From: Iacopo Spalletti Date: Sat, 14 Nov 2020 11:49:40 +0100 Subject: [PATCH] Switch request handling to thread locals This is safer in general, and especially in ASGI environment. If ASGI (which ships its own version) is not available, we use threading local Move to pytest runner to run async tests (though not on ASGI) --- changes/115.bugfix | 1 + cms_helper.py | 1 + meta/models.py | 22 ++++++++---------- meta/utils.py | 23 ++++++++++++++++++ requirements-test.txt | 4 ++++ tests/test_asgi.py | 54 +++++++++++++++++++++++++++++++++++++++++++ tests/test_mixin.py | 19 +++++++-------- tox.ini | 6 +++++ 8 files changed, 108 insertions(+), 22 deletions(-) create mode 100644 changes/115.bugfix create mode 100644 meta/utils.py create mode 100644 tests/test_asgi.py diff --git a/changes/115.bugfix b/changes/115.bugfix new file mode 100644 index 0000000..85b38a4 --- /dev/null +++ b/changes/115.bugfix @@ -0,0 +1 @@ +Switch request handling to thread locals diff --git a/cms_helper.py b/cms_helper.py index b3b903a..18fb13d 100755 --- a/cms_helper.py +++ b/cms_helper.py @@ -11,6 +11,7 @@ META_USE_TWITTER_PROPERTIES=True, META_USE_SCHEMAORG_PROPERTIES=True, FILE_UPLOAD_TEMP_DIR=mkdtemp(), + TEST_RUNNER="app_helper.pytest_runner.PytestTestRunner", ) try: diff --git a/meta/models.py b/meta/models.py index 174df7f..fffc167 100644 --- a/meta/models.py +++ b/meta/models.py @@ -1,9 +1,10 @@ -import contextlib +import warnings from copy import copy from django.conf import settings as dj_settings from . import settings +from .utils import get_request, set_request NEED_REQUEST_OBJECT_ERR_MSG = """ Meta models needs request objects when initializing if sites framework is not used. @@ -62,7 +63,7 @@ def _retrieve_data(self, request, metadata): """ Build the data according to the metadata configuration """ - with self._set_request(request): + with set_request(request): for field, value in metadata.items(): if value: data = self._get_meta_value(field, value) @@ -109,20 +110,15 @@ def as_meta(self, request=None): setattr(meta, field, generaldesc) return meta - @contextlib.contextmanager - def _set_request(self, request): - """ - Context processor that sets the request on the current instance - """ - self._request = request - yield - delattr(self, "_request") - def get_request(self): """ Retrieve request from current instance """ - return getattr(self, "_request", None) + warnings.warn( + "use meta.utils.get_request function, ModelMeta.get_request will be removed in version 3.0", + PendingDeprecationWarning, + ) + return get_request() def get_author(self): """ @@ -186,7 +182,7 @@ def build_absolute_uri(self, url): """ Return the full url for the provided url argument """ - request = self.get_request() + request = get_request() if request: return request.build_absolute_uri(url) diff --git a/meta/utils.py b/meta/utils.py new file mode 100644 index 0000000..131dd4d --- /dev/null +++ b/meta/utils.py @@ -0,0 +1,23 @@ +import contextlib + +try: + from asgiref.local import Local +except ImportError: + from threading import local as Local # noqa: N812 +_thread_locals = Local() + + +@contextlib.contextmanager +def set_request(request): + """ + Context processor that sets the request on the current instance + """ + _thread_locals._request = request + yield + + +def get_request(): + """ + Retrieve request from current instance + """ + return getattr(_thread_locals, "_request", None) diff --git a/requirements-test.txt b/requirements-test.txt index 1d51513..b572d0b 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -4,3 +4,7 @@ coveralls>=2.0 mock>=1.0.1 pillow django-app-helper>=2.0.1 + +pytest +pytest-django +pytest-asyncio diff --git a/tests/test_asgi.py b/tests/test_asgi.py new file mode 100644 index 0000000..5f52b8a --- /dev/null +++ b/tests/test_asgi.py @@ -0,0 +1,54 @@ +from datetime import timedelta + +import pytest +from asgiref.sync import sync_to_async +from django.test import AsyncRequestFactory +from django.utils.text import slugify +from django.utils.timezone import now + +from tests.example_app.models import Post + + +@sync_to_async +def get_post(title): + post, __ = Post.objects.get_or_create( + title=title, + og_title=f"og {title}", + twitter_title="twitter {title}", + schemaorg_title="schemaorg {title}", + slug=slugify(title), + abstract="post abstract", + meta_description="post meta", + meta_keywords="post keyword1,post keyword 2", + date_published_end=now() + timedelta(days=2), + text="post text", + image_url="/path/to/image", + ) + return post + + +@sync_to_async +def delete_post(post): + post.delete() + + +@sync_to_async +def get_meta(post, request=None): + return post.as_meta(request) + + +@pytest.mark.asyncio +@pytest.mark.django_db +async def test_mixin_on_asgi(): + post = await get_post("first post") + meta = await get_meta(post) + assert meta.title == "first post" + + +@pytest.mark.asyncio +@pytest.mark.django_db +async def test_mixin_on_asgi_request(): + request = AsyncRequestFactory().get("/") + post = await get_post("first post") + meta = await get_meta(post, request) + assert meta.title == "first post" diff --git a/tests/test_mixin.py b/tests/test_mixin.py index e5198ce..418ed62 100644 --- a/tests/test_mixin.py +++ b/tests/test_mixin.py @@ -14,9 +14,10 @@ class TestMeta(BaseTestCase): post = None - def setUp(self): - super().setUp() - self.post = Post.objects.create( + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.post, __ = Post.objects.get_or_create( title="a title", og_title="og title", twitter_title="twitter title", @@ -25,16 +26,16 @@ def setUp(self): abstract="post abstract", meta_description="post meta", meta_keywords="post keyword1,post keyword 2", - author=self.user, + author=cls.user, date_published_end=timezone.now() + timedelta(days=2), text="post text", image_url="/path/to/image", ) - self.post.main_image = self.create_django_image_object() - self.post.save() - self.image_url = self.post.main_image.url - self.image_width = self.post.main_image.width - self.image_height = self.post.main_image.height + cls.post.main_image, __ = cls.create_django_image() + cls.post.save() + cls.image_url = cls.post.main_image.url + cls.image_width = cls.post.main_image.width + cls.image_height = cls.post.main_image.height @override_settings(META_SITE_PROTOCOL="http") def test_as_meta(self): diff --git a/tox.ini b/tox.ini index 786acb5..ea8230c 100644 --- a/tox.ini +++ b/tox.ini @@ -150,3 +150,9 @@ ignore = *.mo ignore-bad-ideas = *.mo + +[pytest] +DJANGO_SETTINGS_MODULE = cms_helper +python_files = test_*.py +traceback = short +addopts = --reuse-db