From 203101c06c61a80d1023f1d21302eceae5f66e81 Mon Sep 17 00:00:00 2001 From: Russell Ballestrini Date: Sat, 6 May 2017 20:15:40 -0400 Subject: Update url.py | Refactor parse_url_overrides Refactor parse_url_overrides: * pop values with default if key is missing * change conditionals to test for truth * prevent throwing an exception if passing keyword with default value * test if anchor starts with '#' before blindly adding it --- pyramid/url.py | 42 +++++++++++++++--------------------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/pyramid/url.py b/pyramid/url.py index 9634f61da..7b88c385c 100644 --- a/pyramid/url.py +++ b/pyramid/url.py @@ -34,40 +34,28 @@ ANCHOR_SAFE = QUERY_SAFE def parse_url_overrides(kw): """Parse special arguments passed when generating urls. - The supplied dictionary is mutated, popping arguments as necessary. - Returns a 6-tuple of the format ``(app_url, scheme, host, port, - qs, anchor)``. + The supplied dictionary is mutated when we pop arguments. + Returns a 6-tuple of the format: + + ``(app_url, scheme, host, port, qs, anchor)``. """ - anchor = '' - qs = '' - app_url = None - host = None - scheme = None - port = None - - if '_query' in kw: - query = kw.pop('_query') + app_url = kw.pop('_app_url', None) + scheme = kw.pop('_scheme', None) + host = kw.pop('_host', None) + port = kw.pop('_port', None) + qs = query = kw.pop('_query', '') + anchor = kw.pop('_anchor', '') + + if query: if isinstance(query, string_types): qs = '?' + url_quote(query, QUERY_SAFE) elif query: qs = '?' + urlencode(query, doseq=True) - if '_anchor' in kw: - anchor = kw.pop('_anchor') + if anchor: anchor = url_quote(anchor, ANCHOR_SAFE) - anchor = '#' + anchor - - if '_app_url' in kw: - app_url = kw.pop('_app_url') - - if '_host' in kw: - host = kw.pop('_host') - - if '_scheme' in kw: - scheme = kw.pop('_scheme') - - if '_port' in kw: - port = kw.pop('_port') + if anchor.startswith('#') == False: + anchor = '#' + anchor return app_url, scheme, host, port, qs, anchor -- cgit v1.2.3 From 707e22afe530367645a25044a8ea4a1b8d803cd4 Mon Sep 17 00:00:00 2001 From: russellballestrini Date: Sat, 6 May 2017 20:37:06 -0400 Subject: make adjustments to make tests pass. modified: pyramid/url.py --- pyramid/url.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyramid/url.py b/pyramid/url.py index 7b88c385c..7371acaa3 100644 --- a/pyramid/url.py +++ b/pyramid/url.py @@ -43,13 +43,14 @@ def parse_url_overrides(kw): scheme = kw.pop('_scheme', None) host = kw.pop('_host', None) port = kw.pop('_port', None) - qs = query = kw.pop('_query', '') + query = kw.pop('_query', '') anchor = kw.pop('_anchor', '') + qs = '' if query: if isinstance(query, string_types): qs = '?' + url_quote(query, QUERY_SAFE) - elif query: + else: qs = '?' + urlencode(query, doseq=True) if anchor: -- cgit v1.2.3 From 666e8a72aad017633c9c2ca8c2fe0dcfe54a2b29 Mon Sep 17 00:00:00 2001 From: russellballestrini Date: Sat, 6 May 2017 20:39:27 -0400 Subject: modified: CONTRIBUTORS.txt --- CONTRIBUTORS.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index ca1f56f51..b8fb081ec 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -298,3 +298,5 @@ Contributors - Kirill Kuzminykh, 2017/03/01 - Jeremy(Ching-Rui) Chen, 2017/04/19 + +- Russell Ballestrini, 2017/05/06 -- cgit v1.2.3 From 03e2626a6b33584e2b4e1c7c446b4f311c0fb350 Mon Sep 17 00:00:00 2001 From: russellballestrini Date: Sat, 6 May 2017 21:29:30 -0400 Subject: pep8 fix modified: pyramid/url.py --- pyramid/url.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyramid/url.py b/pyramid/url.py index 7371acaa3..cbfbfc553 100644 --- a/pyramid/url.py +++ b/pyramid/url.py @@ -55,7 +55,7 @@ def parse_url_overrides(kw): if anchor: anchor = url_quote(anchor, ANCHOR_SAFE) - if anchor.startswith('#') == False: + if not anchor.startswith('#'): anchor = '#' + anchor return app_url, scheme, host, port, qs, anchor -- cgit v1.2.3 From 4983421128e2e0fc92c771510f7b3af57de6d855 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 18 Jun 2017 01:31:45 -0500 Subject: configure resource_url to use the same logic --- docs/api/url.rst | 2 +- pyramid/config/views.py | 3 +- pyramid/tests/test_config/test_views.py | 1 + pyramid/url.py | 86 +++++++++------------------------ 4 files changed, 27 insertions(+), 65 deletions(-) diff --git a/docs/api/url.rst b/docs/api/url.rst index 131d85806..8aaabc352 100644 --- a/docs/api/url.rst +++ b/docs/api/url.rst @@ -5,7 +5,7 @@ .. automodule:: pyramid.url - .. autofunction:: pyramid.url.resource_url(context, request, *elements, query=None, anchor=None) + .. autofunction:: resource_url .. autofunction:: route_url diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 48c4e3437..e5ebc8e07 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -1950,8 +1950,7 @@ class StaticURLInfo(object): kw['subpath'] = subpath return request.route_url(route_name, **kw) else: - app_url, scheme, host, port, qs, anchor = \ - parse_url_overrides(kw) + app_url, qs, anchor = parse_url_overrides(request, kw) parsed = url_parse(url) if not parsed.scheme: url = urlparse.urlunparse(parsed._replace( diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 0816d9958..860254385 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -3411,6 +3411,7 @@ class DummyRequest: subpath = () matchdict = None request_iface = IRequest + application_url = 'http://example.com/foo' def __init__(self, environ=None): if environ is None: diff --git a/pyramid/url.py b/pyramid/url.py index cbfbfc553..c21d49c33 100644 --- a/pyramid/url.py +++ b/pyramid/url.py @@ -31,13 +31,13 @@ from pyramid.traversal import ( QUERY_SAFE = "/?:@!$&'()*+,;=" # RFC 3986 ANCHOR_SAFE = QUERY_SAFE -def parse_url_overrides(kw): +def parse_url_overrides(request, kw): """Parse special arguments passed when generating urls. The supplied dictionary is mutated when we pop arguments. - Returns a 6-tuple of the format: + Returns a 3-tuple of the format: - ``(app_url, scheme, host, port, qs, anchor)``. + ``(app_url, qs, anchor)``. """ app_url = kw.pop('_app_url', None) scheme = kw.pop('_scheme', None) @@ -46,6 +46,12 @@ def parse_url_overrides(kw): query = kw.pop('_query', '') anchor = kw.pop('_anchor', '') + if app_url is None: + if (scheme is not None or host is not None or port is not None): + app_url = request._partial_application_url(scheme, host, port) + else: + app_url = request.application_url + qs = '' if query: if isinstance(query, string_types): @@ -54,11 +60,9 @@ def parse_url_overrides(kw): qs = '?' + urlencode(query, doseq=True) if anchor: - anchor = url_quote(anchor, ANCHOR_SAFE) - if not anchor.startswith('#'): - anchor = '#' + anchor + anchor = '#' + url_quote(anchor, ANCHOR_SAFE) - return app_url, scheme, host, port, qs, anchor + return app_url, qs, anchor class URLMethodsMixin(object): """ Request methods mixin for BaseRequest having to do with URL @@ -255,13 +259,7 @@ class URLMethodsMixin(object): if route.pregenerator is not None: elements, kw = route.pregenerator(self, elements, kw) - app_url, scheme, host, port, qs, anchor = parse_url_overrides(kw) - - if app_url is None: - if (scheme is not None or host is not None or port is not None): - app_url = self._partial_application_url(scheme, host, port) - else: - app_url = self.application_url + app_url, qs, anchor = parse_url_overrides(self, kw) path = route.generate(kw) # raises KeyError if generate fails @@ -522,13 +520,15 @@ class URLMethodsMixin(object): virtual_path = getattr(url_adapter, 'virtual_path', None) - app_url = None - scheme = None - host = None - port = None + urlkw = {} + for name in ( + 'app_url', 'scheme', 'host', 'port', 'query', 'anchor' + ): + val = kw.get(name, None) + if val is not None: + urlkw['_' + name] = val if 'route_name' in kw: - newkw = {} route_name = kw['route_name'] remainder = getattr(url_adapter, 'virtual_path_tuple', None) if remainder is None: @@ -536,39 +536,16 @@ class URLMethodsMixin(object): # virtual_path_tuple remainder = tuple(url_adapter.virtual_path.split('/')) remainder_name = kw.get('route_remainder_name', 'traverse') - newkw[remainder_name] = remainder - - for name in ( - 'app_url', 'scheme', 'host', 'port', 'query', 'anchor' - ): - val = kw.get(name, None) - if val is not None: - newkw['_' + name] = val - + urlkw[remainder_name] = remainder + if 'route_kw' in kw: route_kw = kw.get('route_kw') if route_kw is not None: - newkw.update(route_kw) - - return self.route_url(route_name, *elements, **newkw) - - if 'app_url' in kw: - app_url = kw['app_url'] + urlkw.update(route_kw) - if 'scheme' in kw: - scheme = kw['scheme'] + return self.route_url(route_name, *elements, **urlkw) - if 'host' in kw: - host = kw['host'] - - if 'port' in kw: - port = kw['port'] - - if app_url is None: - if scheme or host or port: - app_url = self._partial_application_url(scheme, host, port) - else: - app_url = self.application_url + app_url, qs, anchor = parse_url_overrides(self, urlkw) resource_url = None local_url = getattr(resource, '__resource_url__', None) @@ -589,21 +566,6 @@ class URLMethodsMixin(object): # __resource_url__ function returned None resource_url = app_url + virtual_path - qs = '' - anchor = '' - - if 'query' in kw: - query = kw['query'] - if isinstance(query, string_types): - qs = '?' + url_quote(query, QUERY_SAFE) - elif query: - qs = '?' + urlencode(query, doseq=True) - - if 'anchor' in kw: - anchor = kw['anchor'] - anchor = url_quote(anchor, ANCHOR_SAFE) - anchor = '#' + anchor - if elements: suffix = _join_elements(elements) else: -- cgit v1.2.3 From effe0e6c5adf64ac99f54082121373f84be4611b Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 18 Jun 2017 02:12:51 -0500 Subject: document changes and add tests --- pyramid/tests/test_url.py | 32 ++++++++++++++++++++++++ pyramid/url.py | 63 ++++++++++++++++++++++++++++------------------- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/pyramid/tests/test_url.py b/pyramid/tests/test_url.py index ddf28e0b0..af2e5405c 100644 --- a/pyramid/tests/test_url.py +++ b/pyramid/tests/test_url.py @@ -115,6 +115,14 @@ class TestURLMethodsMixin(unittest.TestCase): self.assertEqual(result, 'http://example.com:5432/context/a') + def test_resource_url_with_query_None(self): + request = self._makeOne() + self._registerResourceURL(request.registry) + context = DummyContext() + result = request.resource_url(context, 'a', query=None) + self.assertEqual(result, + 'http://example.com:5432/context/a') + def test_resource_url_anchor_is_after_root_when_no_elements(self): request = self._makeOne() self._registerResourceURL(request.registry) @@ -157,6 +165,13 @@ class TestURLMethodsMixin(unittest.TestCase): self.assertEqual(result, 'http://example.com:5432/context/#%20/%23?&+') + def test_resource_url_anchor_is_None(self): + request = self._makeOne() + self._registerResourceURL(request.registry) + context = DummyContext() + result = request.resource_url(context, anchor=None) + self.assertEqual(result, 'http://example.com:5432/context/') + def test_resource_url_no_IResourceURL_registered(self): # falls back to ResourceURL root = DummyContext() @@ -421,6 +436,14 @@ class TestURLMethodsMixin(unittest.TestCase): self.assertEqual(result, 'http://example.com:5432/1/2/3?a=1#foo') + def test_route_url_with_query_None(self): + from pyramid.interfaces import IRoutesMapper + request = self._makeOne() + mapper = DummyRoutesMapper(route=DummyRoute('/1/2/3')) + request.registry.registerUtility(mapper, IRoutesMapper) + result = request.route_url('flub', a=1, b=2, c=3, _query=None) + self.assertEqual(result, 'http://example.com:5432/1/2/3') + def test_route_url_with_anchor_binary(self): from pyramid.interfaces import IRoutesMapper request = self._makeOne() @@ -442,6 +465,15 @@ class TestURLMethodsMixin(unittest.TestCase): self.assertEqual(result, 'http://example.com:5432/1/2/3#La%20Pe%C3%B1a') + def test_route_url_with_anchor_None(self): + from pyramid.interfaces import IRoutesMapper + request = self._makeOne() + mapper = DummyRoutesMapper(route=DummyRoute('/1/2/3')) + request.registry.registerUtility(mapper, IRoutesMapper) + result = request.route_url('flub', _anchor=None) + + self.assertEqual(result, 'http://example.com:5432/1/2/3') + def test_route_url_with_query(self): from pyramid.interfaces import IRoutesMapper request = self._makeOne() diff --git a/pyramid/url.py b/pyramid/url.py index c21d49c33..2e964dc7e 100644 --- a/pyramid/url.py +++ b/pyramid/url.py @@ -32,12 +32,14 @@ QUERY_SAFE = "/?:@!$&'()*+,;=" # RFC 3986 ANCHOR_SAFE = QUERY_SAFE def parse_url_overrides(request, kw): - """Parse special arguments passed when generating urls. + """ + Parse special arguments passed when generating urls. The supplied dictionary is mutated when we pop arguments. Returns a 3-tuple of the format: ``(app_url, qs, anchor)``. + """ app_url = kw.pop('_app_url', None) scheme = kw.pop('_scheme', None) @@ -59,10 +61,11 @@ def parse_url_overrides(request, kw): else: qs = '?' + urlencode(query, doseq=True) + frag = '' if anchor: - anchor = '#' + url_quote(anchor, ANCHOR_SAFE) + frag = '#' + url_quote(anchor, ANCHOR_SAFE) - return app_url, qs, anchor + return app_url, qs, frag class URLMethodsMixin(object): """ Request methods mixin for BaseRequest having to do with URL @@ -78,6 +81,7 @@ class URLMethodsMixin(object): passed, the ``port`` value is assumed to ``443``. Likewise, if ``scheme`` is passed as ``http`` and ``port`` is not passed, the ``port`` value is assumed to be ``80``. + """ e = self.environ if scheme is None: @@ -184,10 +188,6 @@ class URLMethodsMixin(object): as values, and a k=v pair will be placed into the query string for each value. - .. versionchanged:: 1.5 - Allow the ``_query`` option to be a string to enable alternative - encodings. - If a keyword argument ``_anchor`` is present, its string representation will be quoted per :rfc:`3986#section-3.5` and used as a named anchor in the generated URL @@ -201,10 +201,6 @@ class URLMethodsMixin(object): ``_anchor`` is passed as a Unicode object, it will be converted to UTF-8 before being appended to the URL. - .. versionchanged:: 1.5 - The ``_anchor`` option will be escaped instead of using - its raw string representation. - If both ``_anchor`` and ``_query`` are specified, the anchor element will always follow the query element, e.g. ``http://example.com?foo=1#bar``. @@ -245,6 +241,18 @@ class URLMethodsMixin(object): If the route object which matches the ``route_name`` argument has a :term:`pregenerator`, the ``*elements`` and ``**kw`` arguments passed to this function might be augmented or changed. + + .. versionchanged:: 1.5 + Allow the ``_query`` option to be a string to enable alternative + encodings. + + The ``_anchor`` option will be escaped instead of using + its raw string representation. + + .. versionchanged:: 1.9 + If ``_query`` or ``_anchor`` are falsey (such as ``None`` or an + empty string) they will not be included in the generated url. + """ try: reg = self.registry @@ -298,13 +306,13 @@ class URLMethodsMixin(object): implemented in terms of :meth:`pyramid.request.Request.route_url` in just this way. As a result, any ``_app_url`` passed within the ``**kw`` values to ``route_path`` will be ignored. + """ kw['_app_url'] = self.script_name return self.route_url(route_name, *elements, **kw) def resource_url(self, resource, *elements, **kw): """ - Generate a string representing the absolute URL of the :term:`resource` object based on the ``wsgi.url_scheme``, ``HTTP_HOST`` or ``SERVER_NAME`` in the request, plus any @@ -370,10 +378,6 @@ class URLMethodsMixin(object): as values, and a k=v pair will be placed into the query string for each value. - .. versionchanged:: 1.5 - Allow the ``query`` option to be a string to enable alternative - encodings. - If a keyword argument ``anchor`` is present, its string representation will be used as a named anchor in the generated URL (e.g. if ``anchor`` is passed as ``foo`` and the resource URL is @@ -386,10 +390,6 @@ class URLMethodsMixin(object): ``anchor`` is passed as a Unicode object, it will be converted to UTF-8 before being appended to the URL. - .. versionchanged:: 1.5 - The ``anchor`` option will be escaped instead of using - its raw string representation. - If both ``anchor`` and ``query`` are specified, the anchor element will always follow the query element, e.g. ``http://example.com?foo=1#bar``. @@ -418,9 +418,6 @@ class URLMethodsMixin(object): pass ``app_url=''``. Passing ``app_url=''`` when the resource path is ``/baz/bar`` will return ``/baz/bar``. - .. versionadded:: 1.3 - ``app_url`` - If ``app_url`` is passed and any of ``scheme``, ``port``, or ``host`` are also passed, ``app_url`` will take precedence and the values passed for ``scheme``, ``host``, and/or ``port`` will be ignored. @@ -432,9 +429,6 @@ class URLMethodsMixin(object): .. seealso:: See also :ref:`overriding_resource_url_generation`. - - .. versionadded:: 1.5 - ``route_name``, ``route_kw``, and ``route_remainder_name`` If ``route_name`` is passed, this function will delegate its URL production to the ``route_url`` function. Calling @@ -508,6 +502,23 @@ class URLMethodsMixin(object): For backwards compatibility purposes, this method is also aliased as the ``model_url`` method of request. + + .. versionchanged:: 1.3 + Added the ``app_url`` keyword argument. + + .. versionchanged:: 1.5 + Allow the ``query`` option to be a string to enable alternative + encodings. + + The ``anchor`` option will be escaped instead of using + its raw string representation. + + Added the ``route_name``, ``route_kw``, and + ``route_remainder_name`` keyword arguments. + + .. versionchanged:: 1.9 + If ``query`` or ``anchor`` are falsey (such as ``None`` or an + empty string) they will not be included in the generated url. """ try: reg = self.registry -- cgit v1.2.3