Skip to content

Commit c08b79c

Browse files
committed
fix: Avoid reverse in apps.ready() since it is not async safe
1 parent 53689ee commit c08b79c

File tree

2 files changed

+117
-16
lines changed

2 files changed

+117
-16
lines changed

djangocms_form_builder/apps.py

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from django.apps import AppConfig
44
from django.conf import settings
5-
from django.urls import NoReverseMatch, clear_url_caches, include, path, reverse
5+
from django.urls import clear_url_caches, include, path
66
from django.utils.translation import gettext_lazy as _
77

88

@@ -13,18 +13,29 @@ class FormsConfig(AppConfig):
1313

1414
def ready(self):
1515
"""Install the URLs"""
16-
try:
17-
reverse("form_builder:ajaxview", args=(1,))
18-
except NoReverseMatch: # Not installed yet
19-
urlconf_module = import_module(settings.ROOT_URLCONF)
20-
urlconf_module.urlpatterns = [
21-
path(
22-
"@form-builder/",
23-
include(
24-
"djangocms_form_builder.urls",
25-
namespace="form_builder",
26-
),
27-
)
28-
] + urlconf_module.urlpatterns
29-
clear_url_caches()
30-
reverse("form_builder:ajaxview", args=(1,))
16+
urlconf_module = import_module(settings.ROOT_URLCONF)
17+
18+
# Idempotency guard
19+
for pattern in urlconf_module.urlpatterns:
20+
if hasattr(pattern, "urlconf_name"):
21+
# Check if this pattern includes the form builder urls
22+
urlconf_name = pattern.urlconf_name
23+
if urlconf_name == "djangocms_form_builder.urls" or (
24+
hasattr(pattern, "urlconf_module")
25+
and getattr(pattern.urlconf_module, "__name__", None)
26+
== "djangocms_form_builder.urls"
27+
):
28+
return
29+
30+
urlconf_module.urlpatterns = [
31+
path(
32+
"@form-builder/",
33+
include(
34+
"djangocms_form_builder.urls",
35+
namespace="form_builder",
36+
),
37+
),
38+
*urlconf_module.urlpatterns,
39+
]
40+
41+
clear_url_caches()

tests/test_apps.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
from unittest.mock import MagicMock, patch
2+
3+
from django.test import TestCase
4+
5+
6+
class AppsIdempotencyGuardTests(TestCase):
7+
"""Test that the FormsConfig.ready() method properly handles idempotency."""
8+
9+
@patch("djangocms_form_builder.apps.clear_url_caches")
10+
@patch("djangocms_form_builder.apps.settings")
11+
@patch("djangocms_form_builder.apps.import_module")
12+
def test_urls_not_registered_twice_with_same_module(
13+
self, mock_import, mock_settings, mock_clear_cache
14+
):
15+
"""Test that URLs are not registered twice when the same module is already included."""
16+
# Create a mock urlconf module
17+
mock_urlconf = MagicMock()
18+
19+
# Create a mock pattern that already has the form builder urls
20+
mock_pattern = MagicMock()
21+
mock_pattern.urlconf_name = "djangocms_form_builder.urls"
22+
mock_pattern.urlconf_module = None # Ensure this doesn't match the second check
23+
24+
mock_urlconf.urlpatterns = [mock_pattern]
25+
mock_import.return_value = mock_urlconf
26+
mock_settings.ROOT_URLCONF = "test_urlconf"
27+
28+
from djangocms_form_builder.apps import FormsConfig
29+
30+
config = FormsConfig.__new__(FormsConfig)
31+
config.ready()
32+
33+
# Verify that urlpatterns was not modified
34+
self.assertEqual(len(mock_urlconf.urlpatterns), 1)
35+
36+
@patch("djangocms_form_builder.apps.clear_url_caches")
37+
@patch("djangocms_form_builder.apps.settings")
38+
@patch("djangocms_form_builder.apps.import_module")
39+
def test_urls_not_registered_twice_with_module_attribute(
40+
self, mock_import, mock_settings, mock_clear_cache
41+
):
42+
"""Test that URLs are not registered twice when urlconf_module is already set."""
43+
# Create a mock urlconf module
44+
mock_urlconf = MagicMock()
45+
46+
# Create a mock pattern with urlconf_module attribute
47+
mock_pattern = MagicMock()
48+
mock_pattern.urlconf_name = "some.other.path"
49+
mock_pattern.urlconf_module = MagicMock()
50+
mock_pattern.urlconf_module.__name__ = "djangocms_form_builder.urls"
51+
52+
mock_urlconf.urlpatterns = [mock_pattern]
53+
original_patterns = mock_urlconf.urlpatterns.copy()
54+
mock_import.return_value = mock_urlconf
55+
mock_settings.ROOT_URLCONF = "test_urlconf"
56+
57+
from djangocms_form_builder.apps import FormsConfig
58+
59+
config = FormsConfig.__new__(FormsConfig)
60+
config.ready()
61+
62+
# Verify that urlpatterns was not modified
63+
self.assertEqual(len(mock_urlconf.urlpatterns), 1)
64+
self.assertEqual(mock_urlconf.urlpatterns, original_patterns)
65+
66+
@patch("djangocms_form_builder.apps.clear_url_caches")
67+
@patch("djangocms_form_builder.apps.settings")
68+
@patch("djangocms_form_builder.apps.import_module")
69+
def test_urls_registered_when_not_already_present(
70+
self, mock_import, mock_settings, mock_clear_cache
71+
):
72+
"""Test that URLs are registered when not already present."""
73+
# Create a mock urlconf module
74+
mock_urlconf = MagicMock()
75+
76+
# Create a mock pattern that doesn't have form builder urls
77+
mock_pattern = MagicMock()
78+
mock_pattern.urlconf_name = "some.other.urls"
79+
80+
mock_urlconf.urlpatterns = [mock_pattern]
81+
mock_import.return_value = mock_urlconf
82+
mock_settings.ROOT_URLCONF = "test_urlconf"
83+
84+
from djangocms_form_builder.apps import FormsConfig
85+
86+
config = FormsConfig.__new__(FormsConfig)
87+
config.ready()
88+
89+
# Verify that urlpatterns was modified (new pattern prepended)
90+
self.assertEqual(len(mock_urlconf.urlpatterns), 2)

0 commit comments

Comments
 (0)