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

@@ -0,0 +1 @@
+Switch request handling to thread locals

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:

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)
 

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)

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

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",

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"

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/")

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