first cut at hybrid url generation; still needs tests for resource_url logic

8 files changed, 190 insertions, 11 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 93349abe6..657c50009 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -232,6 +232,16 @@ Backwards Incompatibilities
   respectively using the machinery described in the "Internationalization"
   chapter of the documentation.
 
+- If you send an ``X-Vhm-Root`` header with a value that ends with a slash (or
+  any number of slashes), the trailing slash(es) will be removed before a URL
+  is generated when you use use ``request.resource_url`` or
+  ``request.resource_path``.  Previously the virtual root path would not have
+  trailing slashes stripped, which would influence URL generation.
+
+- The ``pyramid.interfaces.IResourceURL`` interface has now grown two new
+  attributes: ``virtual_path_tuple`` and ``physical_path_tuple``.  These should
+  be the tuple form of the resource's path (physical and virtual).
+
 1.4 (2012-12-18)
 ================
 
diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py
index 2a14df7c7..b31edd0e3 100644
--- a/pyramid/interfaces.py
+++ b/pyramid/interfaces.py
@@ -692,6 +692,16 @@ class IRoute(Interface):
     pregenerator = Attribute('This attribute should either be ``None`` or '
                              'a callable object implementing the '
                              '``IRoutePregenerator`` interface')
+    remainder_name = Attribute(
+        'The name of any stararg remainder that is present at the end of '
+        'the pattern. For example, if the pattern is ``/foo*bar``, the '
+        '``remainder_name`` will be ``bar``; if the pattern is ` '
+        '`/foo*traverse``, the ``remainder_name`` will be ``traverse``. '
+        'If the route does not have a stararg remainder name in its pattern, '
+        'the value of ``remainder_name`` will be ``None``.  This attribute '
+        'is new as of Pyramid 1.5.'
+        )
+        
     def match(path):
         """
         If the ``path`` passed to this function can be matched by the
@@ -738,8 +748,18 @@ class IRoutesMapper(Interface):
         matched.  Static routes will not be considered for matching.  """
 
 class IResourceURL(Interface):
-    virtual_path = Attribute('The virtual url path of the resource.')
-    physical_path = Attribute('The physical url path of the resource.')
+    virtual_path = Attribute(
+        'The virtual url path of the resource as a string.'
+        )
+    physical_path = Attribute(
+        'The physical url path of the resource as a string.'
+        )
+    virtual_path_tuple = Attribute(
+        'The virtual url path of the resource as a tuple.  (New in 1.5)'
+        )
+    physical_path = Attribute(
+        'The physical url path of the resource as a tuple. (New in 1.5)'
+        )
 
 class IContextURL(IResourceURL):
     """ An adapter which deals with URLs related to a context.
diff --git a/pyramid/tests/test_request.py b/pyramid/tests/test_request.py
index 565c6377e..4f1c23e7b 100644
--- a/pyramid/tests/test_request.py
+++ b/pyramid/tests/test_request.py
@@ -594,6 +594,7 @@ class DummyRoutesMapper:
 
 class DummyRoute:
     pregenerator = None
+    remainder_name = None
     def __init__(self, result='/1/2/3'):
         self.result = result
 
diff --git a/pyramid/tests/test_traversal.py b/pyramid/tests/test_traversal.py
index ba0be7e06..ff5937811 100644
--- a/pyramid/tests/test_traversal.py
+++ b/pyramid/tests/test_traversal.py
@@ -1063,7 +1063,28 @@ class ResourceURLTests(unittest.TestCase):
         context_url = self._makeOne(two, request)
         self.assertEqual(context_url.physical_path, '/one/two/')
         self.assertEqual(context_url.virtual_path, '/two/')
-
+        self.assertEqual(context_url.physical_path_tuple, ('', 'one', 'two',''))
+        self.assertEqual(context_url.virtual_path_tuple, ('', 'two', ''))
+        
+    def test_IResourceURL_attributes_vroot_ends_with_slash(self):
+        from pyramid.interfaces import VH_ROOT_KEY
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        one = DummyContext()
+        one.__parent__ = root
+        one.__name__ = 'one'
+        two = DummyContext()
+        two.__parent__ = one
+        two.__name__ = 'two'
+        environ = {VH_ROOT_KEY:'/one/'}
+        request = DummyRequest(environ)
+        context_url = self._makeOne(two, request)
+        self.assertEqual(context_url.physical_path, '/one/two/')
+        self.assertEqual(context_url.virtual_path, '/two/')
+        self.assertEqual(context_url.physical_path_tuple, ('', 'one', 'two',''))
+        self.assertEqual(context_url.virtual_path_tuple, ('', 'two', ''))
+        
     def test_IResourceURL_attributes_no_vroot(self):
         root = DummyContext()
         root.__parent__ = None
@@ -1079,7 +1100,9 @@ class ResourceURLTests(unittest.TestCase):
         context_url = self._makeOne(two, request)
         self.assertEqual(context_url.physical_path, '/one/two/')
         self.assertEqual(context_url.virtual_path, '/one/two/')
-        
+        self.assertEqual(context_url.physical_path_tuple, ('', 'one', 'two',''))
+        self.assertEqual(context_url.virtual_path_tuple, ('', 'one', 'two', ''))
+
 class TestVirtualRoot(unittest.TestCase):
     def setUp(self):
         cleanUp()
diff --git a/pyramid/tests/test_url.py b/pyramid/tests/test_url.py
index 6f1ee3bf0..8a55ab328 100644
--- a/pyramid/tests/test_url.py
+++ b/pyramid/tests/test_url.py
@@ -441,6 +441,31 @@ class TestURLMethodsMixin(unittest.TestCase):
         self.assertEqual(result,
                          'http://example2.com/1/2/3/element1?q=1#anchor')
 
+    def test_route_url_with_remainder(self):
+        from pyramid.interfaces import IRoutesMapper
+        request = self._makeOne()
+        route = DummyRoute('/1/2/3/')
+        route.remainder_name = 'fred'
+        mapper = DummyRoutesMapper(route=route)
+        request.registry.registerUtility(mapper, IRoutesMapper)
+        result = request.route_url('flub', _remainder='abc')
+        self.assertEqual(result,
+                         'http://example.com:5432/1/2/3/')
+        self.assertEqual(route.kw['fred'], 'abc')
+        self.assertFalse('_remainder' in route.kw)
+
+    def test_route_url_with_remainder_name_already_in_kw(self):
+        from pyramid.interfaces import IRoutesMapper
+        request = self._makeOne()
+        route = DummyRoute('/1/2/3/')
+        route.remainder_name = 'fred'
+        mapper = DummyRoutesMapper(route=route)
+        request.registry.registerUtility(mapper, IRoutesMapper)
+        self.assertRaises(
+            ValueError,
+            request.route_url, 'flub', _remainder='abc', fred='foo'
+            )
+
     def test_route_url_integration_with_real_request(self):
         # to try to replicate https://github.com/Pylons/pyramid/issues/213
         from pyramid.interfaces import IRoutesMapper
@@ -503,7 +528,8 @@ class TestURLMethodsMixin(unittest.TestCase):
         from pyramid.interfaces import IRoutesMapper
         from webob.multidict import GetDict
         request = self._makeOne()
-        request.GET = GetDict([('q', '123'), ('b', '2'), ('b', '2'), ('q', '456')], {})
+        request.GET = GetDict(
+            [('q', '123'), ('b', '2'), ('b', '2'), ('q', '456')], {})
         route = DummyRoute('/1/2/3')
         mapper = DummyRoutesMapper(route=route)
         request.matched_route = route
@@ -1113,6 +1139,7 @@ class DummyRoutesMapper:
 class DummyRoute:
     pregenerator = None
     name = 'route'
+    remainder_name = None
     def __init__(self, result='/1/2/3'):
         self.result = result
 
diff --git a/pyramid/traversal.py b/pyramid/traversal.py
index 469e77454..341ed2d75 100644
--- a/pyramid/traversal.py
+++ b/pyramid/traversal.py
@@ -733,11 +733,15 @@ class ResourceURL(object):
     vroot_varname = VH_ROOT_KEY
 
     def __init__(self, resource, request):
-        physical_path = resource_path(resource)
-        if physical_path != '/':
+        physical_path_tuple = resource_path_tuple(resource)
+        physical_path = _join_path_tuple(physical_path_tuple)
+
+        if physical_path_tuple != ('',):
+            physical_path_tuple = physical_path_tuple + ('',)
             physical_path = physical_path + '/'
 
         virtual_path = physical_path
+        virtual_path_tuple = physical_path_tuple
 
         environ = request.environ
         vroot_path = environ.get(self.vroot_varname)
@@ -745,11 +749,17 @@ class ResourceURL(object):
         # if the physical path starts with the virtual root path, trim it out
         # of the virtual path
         if vroot_path is not None:
-            if physical_path.startswith(vroot_path):
+            vroot_path = vroot_path.rstrip('/')
+            if vroot_path and physical_path.startswith(vroot_path):
+                vroot_path_tuple = tuple(vroot_path.split('/'))
+                numels = len(vroot_path_tuple)
+                virtual_path_tuple = ('',) + physical_path_tuple[numels:]
                 virtual_path = physical_path[len(vroot_path):]
 
         self.virtual_path = virtual_path    # IResourceURL attr
         self.physical_path = physical_path  # IResourceURL attr
+        self.virtual_path_tuple = virtual_path_tuple # IResourceURL attr (1.5)
+        self.physical_path_tuple = physical_path_tuple # IResourceURL attr (1.5)
 
         # bw compat for IContextURL methods
         self.resource = resource
diff --git a/pyramid/url.py b/pyramid/url.py
index 3d95d7cc9..c2009e773 100644
--- a/pyramid/url.py
+++ b/pyramid/url.py
@@ -192,6 +192,15 @@ class URLMethodsMixin(object):
         are passed, ``_app_url`` takes precedence and any values passed for
         ``_scheme``, ``_host``, and ``_port`` will be ignored.
 
+        If a ``_remainder`` keyword argument is supplied, it will be used to
+        replace *any* ``*remainder`` stararg at the end of the route pattern.
+        For example, if the route pattern is ``/foo/*traverse``, and you pass
+        ``_remainder=('a', 'b', 'c')``, it is entirely equivalent to passing
+        ``traverse=('a', 'b', 'c')``, and in either case the generated path
+        will be ``/foo/a/b/c``.  It is an error to pass both ``*remainder`` and
+        the explicit value for a remainder name; a :exc:`ValueError` will be
+        raised.  This feature was added in Pyramid 1.5.
+
         This function raises a :exc:`KeyError` if the URL cannot be
         generated due to missing replacement names.  Extra replacement
         names are ignored.
@@ -213,6 +222,7 @@ class URLMethodsMixin(object):
         if route.pregenerator is not None:
             elements, kw = route.pregenerator(self, elements, kw)
 
+        remainder_name = route.remainder_name
         anchor = ''
         qs = ''
         app_url = None
@@ -248,6 +258,16 @@ class URLMethodsMixin(object):
             else:
                 app_url = self.application_url
 
+        remainder = kw.pop('_remainder', None)
+
+        if remainder and remainder_name:
+            if remainder_name in kw:
+                raise ValueError(
+                    'Cannot pass both "%s" and "_remainder", '
+                    'these conflict for this route' % remainder_name
+                    )
+            kw[remainder_name] = remainder
+
         path = route.generate(kw) # raises KeyError if generate fails
 
         if elements:
@@ -400,9 +420,48 @@ class URLMethodsMixin(object):
         are also passed, ``app_url`` will take precedence and the values
         passed for ``scheme``, ``host``, and/or ``port`` will be ignored.
 
+        If ``route_name`` is passed, this function will delegate its URL
+        production to the ``route_url`` function.  Calling
+        ``resource_url(someresource, 'element1', 'element2', query={'a':1},
+        route_name='blogentry')`` is roughly equivalent to doing::
+
+           remainder_path = request.resource_path(someobject)
+           url = request.route_url(
+                     'blogentry',
+                     'element1',
+                     'element2',
+                     _query={'a':'1'},
+                     _remainder=remainder_path,
+                     )
+
+        It is only sensible to pass ``route_name`` if the route being named has
+        a ``*remainder`` stararg value such as ``*traverse``.  The remainder
+        will be ignored in the output otherwise.
+
+        If ``route_name`` is passed, it is also permissible to pass
+        ``route_kw``, which will passed as additional keyword arguments to
+        ``route_url``.  Saying ``resource_url(someresource, 'element1',
+        'element2', route_name='blogentry', route_kw={'id':'4'},
+        _query={'a':'1'})`` is equivalent to::
+
+           remainder_path = request.resource_path_tuple(someobject)
+           kw = {'id':'4', '_query':{'a':'1'}, '_remainder':remainder_path}
+           url = request.route_url(
+                     'blogentry',
+                     'element1',
+                     'element2',
+                     **kw,
+                     )
+
+        If route_kw is passed, but route_name is not passed, a
+        :exc:`ValueError` will be raised.
+        
+        The ``route_name`` and ``route_kw`` arguments were added in Pyramid
+        1.5.
+
         If the ``resource`` passed in has a ``__resource_url__`` method, it
-        will be used to generate the URL (scheme, host, port, path) that for
-        the base resource which is operated upon by this function.  See also
+        will be used to generate the URL (scheme, host, port, path) for the
+        base resource which is operated upon by this function.  See also
         :ref:`overriding_resource_url_generation`.
 
         .. note::
@@ -458,6 +517,28 @@ class URLMethodsMixin(object):
             host = None
             port = None
 
+            if 'route_name' in kw:
+                newkw = {}
+                route_name = kw['route_name']
+                remainder = getattr(resource_url, 'virtual_path_tuple', None)
+                if remainder is None:
+                    # older user-supplied IResourceURL adapter without 1.5
+                    # virtual_path_tuple
+                    remainder = tuple(resource_url.virtual_path.split('/'))
+                newkw['_remainder'] = remainder
+
+                for name in ('app_url', 'scheme', 'host', 'port'):
+                    val = kw.get(name, None)
+                    if val is not None:
+                        newkw['_' + name] = val
+                    
+                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']
 
diff --git a/pyramid/urldispatch.py b/pyramid/urldispatch.py
index 8090f07f2..621b6d939 100644
--- a/pyramid/urldispatch.py
+++ b/pyramid/urldispatch.py
@@ -33,6 +33,7 @@ class Route(object):
         self.pattern = pattern
         self.path = pattern # indefinite b/w compat, not in interface
         self.match, self.generate = _compile_route(pattern)
+        self.remainder_name = get_remainder_name(pattern)
         self.name = name
         self.factory = factory
         self.predicates = predicates
@@ -91,7 +92,7 @@ class RoutesMapper(object):
 
 # stolen from bobo and modified
 old_route_re = re.compile(r'(\:[_a-zA-Z]\w*)')
-star_at_end = re.compile(r'\*\w*$')
+star_at_end = re.compile(r'(\*\w*)$')
 
 # The tortuous nature of the regex named ``route_re`` below is due to the
 # fact that we need to support at least one level of "inner" squigglies
@@ -233,3 +234,9 @@ def _compile_route(route):
         return result
 
     return matcher, generator
+
+def get_remainder_name(pattern):
+    match = star_at_end.search(pattern)
+    if match:
+        return match.groups()[0]
+