Skip to content

Commit 90efae2

Browse files
bckohanbckohan
committed
make sure defaults can be used when no placeholders are registered - involves removal of PlaceholderNotFound exception closes #8 
1 parent 61b9d5d commit 90efae2

File tree

11 files changed

+325
-112
lines changed

11 files changed

+325
-112
lines changed

‎README.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ So you can now fetch paths like this:
306306
URLGenerationFailed Exceptions & Placeholders
307307
---------------------------------------------
308308
309-
If you encounter a ``URLGenerationFailed`` exception, resulting from a ``PlaceholderNotFound`` exception, not to worry. You simply need to register a placeholder for the argument in question. A placeholder is just a string or object that can be coerced to a string that matches the regular expression for the argument:
309+
If you encounter a ``URLGenerationFailed`` exception, not to worry. You most likely need to register a placeholder for the argument in question. A placeholder is just a string or object that can be coerced to a string that matches the regular expression for the argument:
310310
311311
.. code:: python
312312

‎doc/source/reference.rst‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ exceptions
6464

6565
.. automodule:: render_static.exceptions
6666

67-
.. autoclass:: PlaceholderNotFound
6867
.. autoclass:: URLGenerationFailed
6968

7069

‎doc/source/tldr.rst‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,9 @@ So you can now fetch paths like this::
196196

197197
.. warning::
198198

199-
If you get an exception when you run render_static that originated from a PlaceholderNotFound
200-
exception, you need to register some :ref:`placeholders` before calling :ref:`urls_to_js`.
199+
If you get an exception when you run render_static that originated from a URLGenerationFailed
200+
exception, you mostly likely need to register some :ref:`placeholders` before calling
201+
:ref:`urls_to_js`.
201202

202203
.. note::
203204
The JavaScript URL resolution is guaranteed to produce the same paths as Django's reversal

‎render_static/exceptions.py‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,7 @@
22
Define exceptions thrown by `django-render-static`
33
"""
44

5-
__all__ = ['PlaceholderNotFound', 'URLGenerationFailed']
6-
7-
8-
class PlaceholderNotFound(Exception):
9-
"""
10-
Thrown by `urls_to_js` when a reversible URL requires placeholders in order ot be reversed
11-
but none are registered.
12-
"""
5+
__all__ = ['URLGenerationFailed']
136

147

158
class URLGenerationFailed(Exception):

‎render_static/placeholders.py‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
from typing import Any, Dict, Iterable, List, Optional, Type
2323

2424
from django.urls import converters
25-
from render_static.exceptions import PlaceholderNotFound
2625

2726
__all__ = [
2827
'register_converter_placeholder',
@@ -32,6 +31,10 @@
3231
'resolve_unnamed_placeholders'
3332
]
3433

34+
# DO NOT EXPAND THIS LIST LIGHTLY - for URLs with large numbers of arguments, say 5 where all these
35+
# defaults are tried that would be an O(n^5) operation, where n is the number of defaults.
36+
# URLs with large numbers of arguments should be rare - but there is a complexity check that will
37+
# throw an exception when a maximum number of tries is reached
3538
ALWAYS_TRY_THESE = [0, 'a', 1, 'A', 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa']
3639

3740
converter_placeholders: Dict[Type, List] = {
@@ -139,17 +142,6 @@ def resolve_placeholders(
139142
if app_name:
140143
placeholders.extend(app_variable_placeholders.get(app_name, {}).get(var_name, []))
141144
placeholders.extend(variable_placeholders.get(var_name, []))
142-
143-
if not placeholders:
144-
lookup = {
145-
'parameter': var_name,
146-
'converter': str(converter),
147-
'app_name': app_name
148-
}
149-
raise PlaceholderNotFound(
150-
f'No placeholders are registered for any of the lookup parameters: {lookup}'
151-
)
152-
153145
return placeholders + [always for always in ALWAYS_TRY_THESE if always not in placeholders]
154146

155147

@@ -170,16 +162,6 @@ def resolve_unnamed_placeholders(
170162
if app_name:
171163
placeholders.extend(app_unnamed_placeholders.get(app_name, {}).get(url_name, []))
172164
placeholders.extend(unnamed_placeholders.get(url_name, []))
173-
174-
if not placeholders:
175-
lookup = {
176-
'url_name': url_name,
177-
'app_name': app_name
178-
}
179-
raise PlaceholderNotFound(
180-
f'No unnamed placeholders are registered for any of the lookup parameters: {lookup}'
181-
)
182-
183165
return placeholders + [always for always in ALWAYS_TRY_THESE if always not in placeholders]
184166

185167

‎render_static/tests/app3/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
default_app_config = 'render_static.tests.app1.apps.App1Config'
1+
default_app_config = 'render_static.tests.app3.apps.App3Config'

‎render_static/tests/settings.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
INSTALLED_APPS = (
4545
'render_static.tests.app1',
4646
'render_static.tests.app2',
47+
'render_static.tests.app3',
4748
'render_static',
4849
'django.contrib.auth',
4950
'django.contrib.contenttypes',

‎render_static/tests/tests.py‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,31 @@ def test_full_url_dump_class_es6(self):
980980
self.test_full_url_dump(es5=False)
981981
self.class_mode = None
982982

983+
@override_settings(STATIC_TEMPLATES={
984+
'ENGINES': [{
985+
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
986+
'OPTIONS': {
987+
'loaders': [
988+
('render_static.loaders.StaticLocMemLoader', {
989+
'urls.js': 'var urls = {\n'
990+
'{% urls_to_js include=include %}'
991+
'\n};'
992+
})
993+
],
994+
'builtins': ['render_static.templatetags.render_static']
995+
},
996+
}],
997+
'context': {
998+
'include': ['admin']
999+
}
1000+
})
1001+
def test_admin_urls(self):
1002+
"""
1003+
Admin urls should work out-of-box - just check that it doesnt raise
1004+
"""
1005+
call_command('render_static', 'urls.js')
1006+
self.assertTrue(True)
1007+
9831008
@override_settings(STATIC_TEMPLATES={
9841009
'ENGINES': [{
9851010
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
@@ -1512,6 +1537,162 @@ def tearDown(self):
15121537
pass
15131538

15141539

1540+
@override_settings(ROOT_URLCONF='render_static.tests.urls2')
1541+
class UnregisteredURLTest(URLJavascriptMixin, BaseTestCase):
1542+
1543+
def setUp(self):
1544+
self.clear_placeholder_registries()
1545+
1546+
@override_settings(STATIC_TEMPLATES={
1547+
'ENGINES': [{
1548+
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
1549+
'OPTIONS': {
1550+
'loaders': [
1551+
('render_static.loaders.StaticLocMemLoader', {
1552+
'urls.js': ('{% urls_to_js '
1553+
'visitor="render_static.ClassURLWriter" '
1554+
'include=include '
1555+
'%}')
1556+
})
1557+
],
1558+
'builtins': ['render_static.templatetags.render_static']
1559+
},
1560+
}],
1561+
'templates': {'urls.js': {'context': {'include': ['default']}}}
1562+
})
1563+
def test_no_default_registered(self):
1564+
"""
1565+
Tests: https://github.com/bckohan/django-render-static/issues/8
1566+
:return:
1567+
"""
1568+
self.es6_mode = True
1569+
self.url_js = None
1570+
self.class_mode = ClassURLWriter.class_name_
1571+
1572+
call_command('render_static', 'urls.js')
1573+
self.compare('default', kwargs={'def': 'named'})
1574+
self.compare('default', args=['unnamed'])
1575+
1576+
@override_settings(STATIC_TEMPLATES={
1577+
'context': {'include': ['default']},
1578+
'ENGINES': [{
1579+
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
1580+
'OPTIONS': {
1581+
'loaders': [
1582+
('render_static.loaders.StaticLocMemLoader', {
1583+
'urls.js': ('{% urls_to_js '
1584+
'visitor="render_static.ClassURLWriter" '
1585+
'include=include '
1586+
'%}')
1587+
})
1588+
],
1589+
'builtins': ['render_static.templatetags.render_static']
1590+
},
1591+
}],
1592+
'templates': {'urls.js': {'context': {'include': ['special']}}}
1593+
})
1594+
def test_named_unnamed_conflation1(self):
1595+
"""
1596+
https://github.com/bckohan/django-render-static/issues/9
1597+
"""
1598+
self.es6_mode = True
1599+
self.url_js = None
1600+
self.class_mode = ClassURLWriter.class_name_
1601+
1602+
print(reverse('special', kwargs={'choice': 'first'}))
1603+
print(reverse('special', args=['first']))
1604+
1605+
self.assertRaises(CommandError, lambda: call_command('render_static', 'urls.js'))
1606+
1607+
placeholders.register_variable_placeholder('choice', 'first')
1608+
self.assertRaises(CommandError, lambda: call_command('render_static', 'urls.js'))
1609+
placeholders.register_unnamed_placeholders('special', ['first'])
1610+
1611+
call_command('render_static', 'urls.js')
1612+
self.compare('special', {'choice': 'first'})
1613+
self.compare('special', ['first'])
1614+
1615+
1616+
@override_settings(
1617+
ROOT_URLCONF='render_static.tests.urls2',
1618+
STATIC_TEMPLATES={
1619+
'context': {'include': ['default']},
1620+
'ENGINES': [{
1621+
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
1622+
'OPTIONS': {
1623+
'loaders': [
1624+
('render_static.loaders.StaticLocMemLoader', {
1625+
'urls.js': ('{% urls_to_js '
1626+
'visitor="render_static.ClassURLWriter" '
1627+
'include=include '
1628+
'%}')
1629+
})
1630+
],
1631+
'builtins': ['render_static.templatetags.render_static']
1632+
},
1633+
}],
1634+
'templates': {'urls.js': {'context': {'include': ['special']}}}
1635+
}
1636+
)
1637+
def test_named_unnamed_conflation2(self):
1638+
"""
1639+
https://github.com/bckohan/django-render-static/issues/9
1640+
"""
1641+
self.es6_mode = True
1642+
self.url_js = None
1643+
self.class_mode = ClassURLWriter.class_name_
1644+
1645+
print(reverse('special', kwargs={'choice': 'first'}))
1646+
print(reverse('special', args=['first']))
1647+
1648+
self.assertRaises(CommandError, lambda: call_command('render_static', 'urls.js'))
1649+
1650+
placeholders.register_variable_placeholder('choice', 'first')
1651+
self.assertRaises(CommandError, lambda: call_command('render_static', 'urls.js'))
1652+
placeholders.register_unnamed_placeholders('special', ['first'])
1653+
1654+
call_command('render_static', 'urls.js')
1655+
self.compare('special', {'choice': 'first'})
1656+
self.compare('special', ['first'])
1657+
1658+
1659+
"""
1660+
@override_settings(STATIC_TEMPLATES={
1661+
'context': {'include': ['default']},
1662+
'ENGINES': [{
1663+
'BACKEND': 'render_static.backends.StaticDjangoTemplates',
1664+
'OPTIONS': {
1665+
'loaders': [
1666+
('render_static.loaders.StaticLocMemLoader', {
1667+
'urls.js': ('{% urls_to_js '
1668+
'visitor="render_static.ClassURLWriter" '
1669+
'include=include '
1670+
'%}')
1671+
})
1672+
],
1673+
'builtins': ['render_static.templatetags.render_static']
1674+
},
1675+
}],
1676+
'templates': {'urls.js': {'context': {'include': ['special']}}}
1677+
})
1678+
def test_complexity_boundary(self):
1679+
https://github.com/bckohan/django-render-static/issues/10
1680+
For URLs with lots of unregistered arguments, the reversal attempts may produce an explosion
1681+
of complexity. If there are
1682+
:return:
1683+
self.es6_mode = True
1684+
self.url_js = None
1685+
1686+
self.assertRaises(CommandError, call_command('render_static', 'urls.js'))
1687+
1688+
self.compare('default')
1689+
"""
1690+
1691+
# uncomment to not delete generated js
1692+
def tearDown(self):
1693+
pass
1694+
1695+
15151696
class URLSToJavascriptOffNominalTest(URLJavascriptMixin, BaseTestCase):
15161697

15171698
def setUp(self):

‎render_static/tests/urls2.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
from django.urls import re_path, path
2+
from django.urls.converters import register_converter
3+
from .views import TestView
4+
5+
6+
class CustomConverter:
7+
regex = '[6]{3}'
8+
9+
def to_python(self, value):
10+
return int(value)
11+
12+
def to_url(self, value):
13+
return str(value)
14+
15+
16+
register_converter(CustomConverter, 'ctm')
17+
18+
19+
urlpatterns = [
20+
re_path(r'^default/(?P<def>\w+)$', TestView.as_view(), name='default'),
21+
re_path(r'^default2/([\w]+)$', TestView.as_view(), name='default'),
22+
re_path(r'^special2/((?:first)|(?:second))$', TestView.as_view(), name='special'),
23+
path(
24+
'<ctm:one>/<ctm:two>/<ctm:three>/<ctm:four>/<ctm:five>/<ctm:six>/<ctm:seven>/<ctm:eight>',
25+
TestView.as_view(),
26+
name='complex'
27+
),
28+
re_path(r'^special1/(?P<choice>(:?first)|(:?second))$', TestView.as_view(), name='special'),
29+
re_path(r'^special1/(?P<choice>(:?first)|(:?second))/(?P<choice1>(:?first)|(:?second))$',
30+
TestView.as_view(), name='special'),
31+
]

‎render_static/tests/urls3.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
from django.urls import re_path, path
2+
from django.urls.converters import register_converter
3+
from .views import TestView
4+
5+
6+
class CustomConverter:
7+
regex = '[6]{3}'
8+
9+
def to_python(self, value):
10+
return int(value)
11+
12+
def to_url(self, value):
13+
return str(value)
14+
15+
16+
register_converter(CustomConverter, 'ctm')
17+
18+
19+
urlpatterns = [
20+
re_path(r'^special2/((?:first)|(?:second))$', TestView.as_view(), name='special'),
21+
re_path(r'^special1/(?P<choice>(:?first)|(:?second))$', TestView.as_view(), name='special'),
22+
re_path(
23+
r'^special1/(?P<choice>(:?first)|(:?second))/(?P<choice1>(:?first)|(:?second))$',
24+
TestView.as_view(),
25+
name='special'
26+
),
27+
]

0 commit comments

Comments
 (0)