From 441d74221480889019c02f33e56912be751b6dba Mon Sep 17 00:00:00 2001 From: Iacopo Spalletti Date: Sat, 14 Nov 2020 16:05:07 +0100 Subject: [PATCH] Switch request handling to thread locals (#125) * Switch request handling to thread locals * Add test for deprecation warning 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/example_app/models.py | 2 +- tests/test_asgi.py | 70 +++++++++++++++++++++++++++++++++++++ tests/test_mixin.py | 34 +++++++++++++----- tox.ini | 7 ++++ 9 files changed, 141 insertions(+), 23 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/example_app/models.py b/tests/example_app/models.py index 2fe5507..ec5c64a 100644 --- a/tests/example_app/models.py +++ b/tests/example_app/models.py @@ -38,7 +38,7 @@ class Post(ModelMeta, models.Model): _metadata = { "title": "title", - "og_title": "og title", + "og_title": "og_title", "twitter_title": "twitter title", "schemaorg_title": "schemaorg title", "description": "get_description", diff --git a/tests/test_asgi.py b/tests/test_asgi.py new file mode 100644 index 0000000..c3ea722 --- /dev/null +++ b/tests/test_asgi.py @@ -0,0 +1,70 @@ +from datetime import timedelta + +import django +import pytest +from django.utils.text import slugify +from django.utils.timezone import now + +from tests.example_app.models import Post + +try: + from asgiref.sync import sync_to_async + from django.test import AsyncRequestFactory +except ImportError: + # stub to avoid decorator failures + def sync_to_async(__): + return + + pytestmark = pytest.mark.skip("asgiref not installed, skipping async tests") + +minversion = pytest.mark.skipif(django.VERSION < (3, 1), reason="at least Django 3.1 required") + + +@sync_to_async +def get_post(title): + post, __ = Post.objects.get_or_create( + title=title, + og_title="og {title}".format(title=title), + twitter_title="twitter {title}".format(title=title), + schemaorg_title="schemaorg {title}".format(title=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", + ) + print(post.og_title) + 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) + + +@minversion +@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" + assert meta.og_title == "og first post" + + +@minversion +@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" + assert meta.og_title == "og first post" diff --git a/tests/test_mixin.py b/tests/test_mixin.py index e5198ce..3b080b9 100644 --- a/tests/test_mixin.py +++ b/tests/test_mixin.py @@ -1,3 +1,4 @@ +import warnings from datetime import timedelta from app_helper.base_test import BaseTestCase @@ -14,9 +15,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 +27,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): @@ -156,6 +158,20 @@ def test_as_meta_with_request(self): settings.FB_PAGES = "" settings.FB_APPID = "" + @override_settings(META_SITE_PROTOCOL="http") + def test_as_meta_get_request_deprecation(self): + request = self.get_request(None, "en", path="/title/", secure=True) + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.post.get_meta(request) + assert len(w) == 0 + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.post.get_request() + assert len(w) == 1 + assert issubclass(w[-1].category, PendingDeprecationWarning) + def test_templatetag(self): self.post.as_meta() response = self.client.get("/title/") diff --git a/tox.ini b/tox.ini index 786acb5..2085823 100644 --- a/tox.ini +++ b/tox.ini @@ -21,6 +21,7 @@ deps = -r{toxinidir}/requirements-test.txt passenv = COMMAND + PYTEST_* [testenv:pep8] commands = @@ -150,3 +151,9 @@ ignore = *.mo ignore-bad-ideas = *.mo + +[pytest] +DJANGO_SETTINGS_MODULE = cms_helper +python_files = test_*.py +traceback = short +addopts = --reuse-db