From 14d5070cb6c786f9cb3562a39828b1ce13b68753 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sat, 25 Jun 2022 19:02:03 -0700 Subject: [PATCH 1/8] Fix Django ORM usage within components --- src/django_idom/layout.py | 18 ++++++++++++++++++ src/django_idom/websocket/consumer.py | 5 +++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 src/django_idom/layout.py diff --git a/src/django_idom/layout.py b/src/django_idom/layout.py new file mode 100644 index 00000000..a4fbcdac --- /dev/null +++ b/src/django_idom/layout.py @@ -0,0 +1,18 @@ +import os + +from idom.core.layout import Layout, LayoutUpdate + + +class DjangoLayout(Layout): + """Fixes Django ORM usage within components. + These issues are caused by async/sync mixed context limitations in the ORM. + Without this, `SynchronousOnlyOperation` exceptions occur when using the ORM in IDOM components. + This may be fixed in a future version, such as Django 5.0.""" + + def _create_layout_update(self, old_state) -> LayoutUpdate: + """Create a layout update, but set ALLOW ASYNC UNSAFE flags prior. + This allows the Django ORM to be used within components.""" + os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" + layout_update = super()._create_layout_update(old_state) + os.environ.pop("DJANGO_ALLOW_ASYNC_UNSAFE") + return layout_update diff --git a/src/django_idom/websocket/consumer.py b/src/django_idom/websocket/consumer.py index aa988afa..9a54472e 100644 --- a/src/django_idom/websocket/consumer.py +++ b/src/django_idom/websocket/consumer.py @@ -8,11 +8,12 @@ from channels.auth import login from channels.db import database_sync_to_async as convert_to_async from channels.generic.websocket import AsyncJsonWebsocketConsumer -from idom.core.layout import Layout, LayoutEvent +from idom.core.layout import LayoutEvent from idom.core.serve import serve_json_patch from django_idom.config import IDOM_REGISTERED_COMPONENTS from django_idom.hooks import IdomWebsocket, WebsocketContext +from django_idom.layout import DjangoLayout _logger = logging.getLogger(__name__) @@ -73,7 +74,7 @@ async def _run_dispatch_loop(self): self._idom_recv_queue = recv_queue = asyncio.Queue() try: await serve_json_patch( - Layout(WebsocketContext(component_instance, value=socket)), + DjangoLayout(WebsocketContext(component_instance, value=socket)), self.send_json, recv_queue.get, ) From d8c6825d2468e91de6cdbd7ad06c96b3efb6222b Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sat, 25 Jun 2022 19:11:59 -0700 Subject: [PATCH 2/8] pin selenium version --- requirements/test-env.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/test-env.txt b/requirements/test-env.txt index 6f2e151e..61ee65ee 100644 --- a/requirements/test-env.txt +++ b/requirements/test-env.txt @@ -1,3 +1,3 @@ django -selenium +selenium <= 4.2.0 twisted From 5e65234dcc19ee5e2ba1a69018ff1d7016a70dc4 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sat, 25 Jun 2022 20:10:15 -0700 Subject: [PATCH 3/8] add tests --- tests/test_app/components.py | 18 +++++++++++++++ tests/test_app/migrations/0001_initial.py | 28 +++++++++++++++++++++++ tests/test_app/models.py | 8 +++++++ tests/test_app/templates/base.html | 1 + tests/test_app/tests/test_components.py | 4 ++++ 5 files changed, 59 insertions(+) create mode 100644 tests/test_app/migrations/0001_initial.py create mode 100644 tests/test_app/models.py diff --git a/tests/test_app/components.py b/tests/test_app/components.py index 2efd878c..34736271 100644 --- a/tests/test_app/components.py +++ b/tests/test_app/components.py @@ -1,3 +1,5 @@ +from uuid import uuid4 + import idom import django_idom @@ -71,3 +73,19 @@ def UseLocation(): f"UseLocation: {location}", idom.html.hr(), ) + + +@idom.component +def OrmInComponent(): + from .models import NamedThingy + + NamedThingy.objects.all().delete() + NamedThingy(name=f"foo-{uuid4()}").save() + model = NamedThingy.objects.all() + success = bool(model) + + return idom.html.div( + {"id": "orm-in-component", "data-success": success}, + f"OrmInComponent: {model}", + idom.html.hr(), + ) diff --git a/tests/test_app/migrations/0001_initial.py b/tests/test_app/migrations/0001_initial.py new file mode 100644 index 00000000..94c016db --- /dev/null +++ b/tests/test_app/migrations/0001_initial.py @@ -0,0 +1,28 @@ +# Generated by Django 4.0.5 on 2022-06-26 02:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="NamedThingy", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=255)), + ], + ), + ] diff --git a/tests/test_app/models.py b/tests/test_app/models.py new file mode 100644 index 00000000..3d3981c0 --- /dev/null +++ b/tests/test_app/models.py @@ -0,0 +1,8 @@ +from django.db import models + + +class NamedThingy(models.Model): + def __str__(self): + return self.name + + name = models.CharField(max_length=255) diff --git a/tests/test_app/templates/base.html b/tests/test_app/templates/base.html index 0c52a8cc..d98676d5 100644 --- a/tests/test_app/templates/base.html +++ b/tests/test_app/templates/base.html @@ -19,6 +19,7 @@

IDOM Test Page

{% component "test_app.components.UseWebsocket" %}
{% component "test_app.components.UseScope" %}
{% component "test_app.components.UseLocation" %}
+
{% component "test_app.components.OrmInComponent" %}
diff --git a/tests/test_app/tests/test_components.py b/tests/test_app/tests/test_components.py index fbfda829..9e09b815 100644 --- a/tests/test_app/tests/test_components.py +++ b/tests/test_app/tests/test_components.py @@ -59,6 +59,10 @@ def test_use_location(self): element = self.driver.find_element_by_id("use-location") self.assertEqual(element.get_attribute("data-success"), "true") + def test_orm_in_component(self): + element = self.driver.find_element_by_id("orm-in-component") + self.assertEqual(element.get_attribute("data-success"), "true") + def make_driver(page_load_timeout, implicit_wait_timeout): options = webdriver.ChromeOptions() From d19fb1c03ef6495d5853b56d19dfbfc6b7269fef Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sat, 25 Jun 2022 20:26:14 -0700 Subject: [PATCH 4/8] negation test --- tests/test_app/templates/base.html | 3 ++- tests/test_app/templatetags/__init__.py | 0 tests/test_app/templatetags/idom_tests.py | 11 +++++++++++ tests/test_app/tests/test_components.py | 4 ++++ 4 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/test_app/templatetags/__init__.py create mode 100644 tests/test_app/templatetags/idom_tests.py diff --git a/tests/test_app/templates/base.html b/tests/test_app/templates/base.html index d98676d5..9e73e49a 100644 --- a/tests/test_app/templates/base.html +++ b/tests/test_app/templates/base.html @@ -1,4 +1,4 @@ -{% load static %} {% load idom %} +{% load static %} {% load idom %} {% load idom_tests %} @@ -20,6 +20,7 @@

IDOM Test Page

{% component "test_app.components.UseScope" %}
{% component "test_app.components.UseLocation" %}
{% component "test_app.components.OrmInComponent" %}
+
DJANGO_ALLOW_ASYNC_UNSAFE: {% check_async_unsafe %}
diff --git a/tests/test_app/templatetags/__init__.py b/tests/test_app/templatetags/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_app/templatetags/idom_tests.py b/tests/test_app/templatetags/idom_tests.py new file mode 100644 index 00000000..a93b92b2 --- /dev/null +++ b/tests/test_app/templatetags/idom_tests.py @@ -0,0 +1,11 @@ +import os + +from django import template + + +register = template.Library() + + +@register.simple_tag +def check_async_unsafe(): + return bool(os.environ.get("DJANGO_ALLOW_ASYNC_UNSAFE", None)) diff --git a/tests/test_app/tests/test_components.py b/tests/test_app/tests/test_components.py index 9e09b815..ea9fba9f 100644 --- a/tests/test_app/tests/test_components.py +++ b/tests/test_app/tests/test_components.py @@ -63,6 +63,10 @@ def test_orm_in_component(self): element = self.driver.find_element_by_id("orm-in-component") self.assertEqual(element.get_attribute("data-success"), "true") + # Make sure ASYNC_UNSAFE value was reset after component render + element = self.driver.find_element_by_id("allow-async-unsafe") + self.assertEqual(element.get_attribute("data-value"), "False") + def make_driver(page_load_timeout, implicit_wait_timeout): options = webdriver.ChromeOptions() From 8b51988bf337ed77502bf587d09c3a9d8f31c6ce Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sat, 25 Jun 2022 22:12:20 -0700 Subject: [PATCH 5/8] Reset async unsafe flag to the previous value --- src/django_idom/layout.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/django_idom/layout.py b/src/django_idom/layout.py index a4fbcdac..8c26898a 100644 --- a/src/django_idom/layout.py +++ b/src/django_idom/layout.py @@ -12,7 +12,16 @@ class DjangoLayout(Layout): def _create_layout_update(self, old_state) -> LayoutUpdate: """Create a layout update, but set ALLOW ASYNC UNSAFE flags prior. This allows the Django ORM to be used within components.""" + async_unsafe_prev = os.environ.get("IDOM_ALLOW_ASYNC_UNSAFE", None) + + # Set ALLOW ASYNC UNSAFE to True os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" layout_update = super()._create_layout_update(old_state) - os.environ.pop("DJANGO_ALLOW_ASYNC_UNSAFE") + + # Reset async unsafe flag to the previous value + if async_unsafe_prev is not None: + os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = async_unsafe_prev + else: + os.environ.pop("DJANGO_ALLOW_ASYNC_UNSAFE") + return layout_update From 7a07adc3be2a7bc5b49faa97e7b09bb460d3d000 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 28 Jun 2022 21:39:56 -0700 Subject: [PATCH 6/8] fix typo --- src/django_idom/layout.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/django_idom/layout.py b/src/django_idom/layout.py index 8c26898a..2cbc7e37 100644 --- a/src/django_idom/layout.py +++ b/src/django_idom/layout.py @@ -12,7 +12,7 @@ class DjangoLayout(Layout): def _create_layout_update(self, old_state) -> LayoutUpdate: """Create a layout update, but set ALLOW ASYNC UNSAFE flags prior. This allows the Django ORM to be used within components.""" - async_unsafe_prev = os.environ.get("IDOM_ALLOW_ASYNC_UNSAFE", None) + async_unsafe_prev = os.environ.get("DJANGO_ALLOW_ASYNC_UNSAFE", None) # Set ALLOW ASYNC UNSAFE to True os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" From 4b69416321f0356ab3049347b04feca7d67ab3f1 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Thu, 30 Jun 2022 06:32:52 -0700 Subject: [PATCH 7/8] add to changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a696e82..3cbed77e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,9 @@ Types of changes are to be listed in this order ## [Unreleased] -- Nothing (yet) +### Fixed + +- Django ORM can now be directly called within components without causing a `SynchronousOnlyOperation` exception. ## [1.0.0] - 2022-05-22 From 7cfcfb87ae8da4a33a501bb70d053b2c7497d082 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Sun, 3 Jul 2022 02:04:28 -0700 Subject: [PATCH 8/8] use snake_cakse --- tests/test_app/components.py | 4 ++-- tests/test_app/templates/base.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_app/components.py b/tests/test_app/components.py index 93978f4d..361f2593 100644 --- a/tests/test_app/components.py +++ b/tests/test_app/components.py @@ -100,7 +100,7 @@ def django_js(): @idom.component -def OrmInComponent(): +def orm_in_component(): from .models import NamedThingy NamedThingy.objects.all().delete() @@ -110,6 +110,6 @@ def OrmInComponent(): return idom.html.div( {"id": "orm-in-component", "data-success": success}, - f"OrmInComponent: {model}", + f"orm_in_component: {model}", idom.html.hr(), ) diff --git a/tests/test_app/templates/base.html b/tests/test_app/templates/base.html index 637d1b0a..b67fc634 100644 --- a/tests/test_app/templates/base.html +++ b/tests/test_app/templates/base.html @@ -21,7 +21,7 @@

IDOM Test Page

{% component "test_app.components.use_location" %}
{% component "test_app.components.django_css" %}
{% component "test_app.components.django_js" %}
-
{% component "test_app.components.OrmInComponent" %}
+
{% component "test_app.components.orm_in_component" %}
DJANGO_ALLOW_ASYNC_UNSAFE: {% check_async_unsafe %}