Backwards Incompatibilities

---------------------------

- The ``repoze.bfg.traversal.model_path`` API now returns a tuple
  instead of a string.  Previously it returned a string representing
  the model path, with each segment name in the path joined together
  via ``/`` characters, e.g. ``/foo/bar``.  Now it returns a tuple,
  where each segment is an element in the tuple e.g. ``('', 'foo',
  'bar')`` (the leading empty element indicates that this path is
  absolute).  This change was (as discussed on the repoze-dev
  maillist) necessary to accomodate model objects which themselves
  have names that contain the ``/`` character.  See the API
  documentation for ``repoze.bfg.traversal.model_path`` for more
  information.

- The ``repoze.bfg.traversal.find_model`` API no longer implicitly
  converts unicode path representations into a UTF-8 string.  Callers
  should either use path tuples or use the guidelines about passing a
  string ``path`` argument described in its API documentation.

Features
--------

- The ``find_model`` API now accepts "path tuples" (see the above note
  regarding ``model_path``) as well as string path representations as
  a ``path`` argument.

3 files changed, 206 insertions, 89 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index d938ec00e..c240b4736 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,9 +1,33 @@
 Next release
 ============
 
+Backwards Incompatibilities
+---------------------------
+
+- The ``repoze.bfg.traversal.model_path`` API now returns a tuple
+  instead of a string.  Previously it returned a string representing
+  the model path, with each segment name in the path joined together
+  via ``/`` characters, e.g. ``/foo/bar``.  Now it returns a tuple,
+  where each segment is an element in the tuple e.g. ``('', 'foo',
+  'bar')`` (the leading empty element indicates that this path is
+  absolute).  This change was (as discussed on the repoze-dev
+  maillist) necessary to accomodate model objects which themselves
+  have names that contain the ``/`` character.  See the API
+  documentation for ``repoze.bfg.traversal.model_path`` for more
+  information.
+
+- The ``repoze.bfg.traversal.find_model`` API no longer implicitly
+  converts unicode path representations into a UTF-8 string.  Callers
+  should either use path tuples or use the guidelines about passing a
+  string ``path`` argument described in its API documentation.
+
 Features
 --------
 
+- The ``find_model`` API now accepts "path tuples" (see the above note
+  regarding ``model_path``) as well as string path representations as
+  a ``path`` argument.
+
 - Add ` `renderer`` argument (defaulting to None) to
   ``repoze.bfg.testing.registerDummyRenderer``.  This makes it
   possible, for instance, to register a custom renderer that raises an
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 4782ab264..1486e5b91 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -279,63 +279,119 @@ class FindModelTests(unittest.TestCase):
         from zope.interface import Interface
         gsm.registerAdapter(traverser, (Interface,), ITraverserFactory)
 
-    def test_relative_found(self):
-        dummy = DummyContext()
+    def test_list(self):
+        model = DummyContext()
+        traverser = make_traverser(model, '', [])
+        self._registerTraverserFactory(traverser)
+        result = self._callFUT(model, [''])
+        self.assertEqual(result, model)
+        self.assertEqual(model.environ['PATH_INFO'], '/')
+
+    def test_generator(self):
+        model = DummyContext()
+        traverser = make_traverser(model, '', [])
+        self._registerTraverserFactory(traverser)
+        def foo():
+            yield ''
+        result = self._callFUT(model, foo())
+        self.assertEqual(result, model)
+        self.assertEqual(model.environ['PATH_INFO'], '/')
+
+    def test_self_string_found(self):
+        model = DummyContext()
+        traverser = make_traverser(model, '', [])
+        self._registerTraverserFactory(traverser)
+        result = self._callFUT(model, '')
+        self.assertEqual(result, model)
+        self.assertEqual(model.environ['PATH_INFO'], '')
+
+    def test_self_tuple_found(self):
+        model = DummyContext()
+        traverser = make_traverser(model, '', [])
+        self._registerTraverserFactory(traverser)
+        result = self._callFUT(model, ())
+        self.assertEqual(result, model)
+        self.assertEqual(model.environ['PATH_INFO'], '')
+
+    def test_relative_string_found(self):
+        model = DummyContext()
         baz = DummyContext()
         traverser = make_traverser(baz, '', [])
         self._registerTraverserFactory(traverser)
-        result = self._callFUT(dummy, 'baz')
+        result = self._callFUT(model, 'baz')
         self.assertEqual(result, baz)
+        self.assertEqual(model.environ['PATH_INFO'], 'baz')
 
-    def test_relative_notfound(self):
-        dummy = DummyContext()
+    def test_relative_tuple_found(self):
+        model = DummyContext()
         baz = DummyContext()
-        traverser = make_traverser(baz, 'bar', [])
+        traverser = make_traverser(baz, '', [])
         self._registerTraverserFactory(traverser)
-        self.assertRaises(KeyError, self._callFUT, dummy, 'baz')
+        result = self._callFUT(model, ('baz',))
+        self.assertEqual(result, baz)
+        self.assertEqual(model.environ['PATH_INFO'], 'baz')
 
-    def test_absolute_found(self):
-        dummy = DummyContext()
+    def test_relative_string_notfound(self):
+        model = DummyContext()
         baz = DummyContext()
-        baz.__parent__ = dummy
-        baz.__name__ = 'baz'
-        dummy.__parent__ = None
-        dummy.__name__ = None
-        traverser = make_traverser(dummy, '', [])
+        traverser = make_traverser(baz, 'bar', [])
         self._registerTraverserFactory(traverser)
-        result = self._callFUT(baz, '/')
-        self.assertEqual(result, dummy)
-        self.assertEqual(dummy.wascontext, True)
+        self.assertRaises(KeyError, self._callFUT, model, 'baz')
+        self.assertEqual(model.environ['PATH_INFO'], 'baz')
 
-    def test_absolute_notfound(self):
-        dummy = DummyContext()
+    def test_relative_tuple_notfound(self):
+        model = DummyContext()
         baz = DummyContext()
-        baz.__parent__ = dummy
-        baz.__name__ = 'baz'
-        dummy.__parent__ = None
-        dummy.__name__ = None
-        traverser = make_traverser(dummy, 'fuz', [])
+        traverser = make_traverser(baz, 'bar', [])
         self._registerTraverserFactory(traverser)
-        self.assertRaises(KeyError, self._callFUT, baz, '/')
-        self.assertEqual(dummy.wascontext, True)
+        self.assertRaises(KeyError, self._callFUT, model, ('baz',))
+        self.assertEqual(model.environ['PATH_INFO'], 'baz')
 
-    def test_unicode_pathinfo_converted_to_utf8(self):
-        la = unicode('LaPe\xc3\xb1a', 'utf-8')
+    def test_absolute_string_found(self):
+        root = DummyContext()
+        model = DummyContext()
+        model.__parent__ = root
+        model.__name__ = 'baz'
+        traverser = make_traverser(root, '', [])
+        self._registerTraverserFactory(traverser)
+        result = self._callFUT(model, '/')
+        self.assertEqual(result, root)
+        self.assertEqual(root.wascontext, True)
+        self.assertEqual(root.environ['PATH_INFO'], '/')
 
-        dummy = DummyContext()
-        dummy.__parent__ = None
-        dummy.__name__ = None
-        baz = DummyContext()
-        baz.__parent__ = dummy
-        baz.__name__ = la
+    def test_absolute_tuple_found(self):
+        root = DummyContext()
+        model = DummyContext()
+        model.__parent__ = root
+        model.__name__ = 'baz'
+        traverser = make_traverser(root, '', [])
+        self._registerTraverserFactory(traverser)
+        result = self._callFUT(model, ('',))
+        self.assertEqual(result, root)
+        self.assertEqual(root.wascontext, True)
+        self.assertEqual(root.environ['PATH_INFO'], '/')
 
-        traverser = make_traverser(baz, '', [])
+    def test_absolute_string_notfound(self):
+        root = DummyContext()
+        model = DummyContext()
+        model.__parent__ = root
+        model.__name__ = 'baz'
+        traverser = make_traverser(root, 'fuz', [])
         self._registerTraverserFactory(traverser)
-        path = '/' + la
-        result = self._callFUT(baz, path)
-        self.assertEqual(result, baz)
-        self.assertEqual(dummy.wascontext, True)
-        self.assertEqual(dummy.environ['PATH_INFO'], path.encode('utf-8'))
+        self.assertRaises(KeyError, self._callFUT, model, '/')
+        self.assertEqual(root.wascontext, True)
+        self.assertEqual(root.environ['PATH_INFO'], '/')
+
+    def test_absolute_tuple_notfound(self):
+        root = DummyContext()
+        model = DummyContext()
+        model.__parent__ = root
+        model.__name__ = 'baz'
+        traverser = make_traverser(root, 'fuz', [])
+        self._registerTraverserFactory(traverser)
+        self.assertRaises(KeyError, self._callFUT, model, ('',))
+        self.assertEqual(root.wascontext, True)
+        self.assertEqual(root.environ['PATH_INFO'], '/')
 
 class ModelPathTests(unittest.TestCase):
     def _callFUT(self, model, *elements):
@@ -356,7 +412,8 @@ class ModelPathTests(unittest.TestCase):
         baz.__parent__ = bar
         baz.__name__ = 'baz'
         result = self._callFUT(baz, 'this/theotherthing', 'that')
-        self.assertEqual(result, '/foo /bar/baz/this/theotherthing/that')
+        self.assertEqual(result, ('','foo ', 'bar', 'baz', 'this/theotherthing',
+                                  'that'))
 
     def test_root_default(self):
         root = DummyContext()
@@ -364,7 +421,7 @@ class ModelPathTests(unittest.TestCase):
         root.__name__ = None
         request = DummyRequest()
         result = self._callFUT(root)
-        self.assertEqual(result, '/')
+        self.assertEqual(result, ('',))
         
     def test_nonroot_default(self):
         root = DummyContext()
@@ -375,7 +432,7 @@ class ModelPathTests(unittest.TestCase):
         other.__name__ = 'other'
         request = DummyRequest()
         result = self._callFUT(other)
-        self.assertEqual(result, '/other')
+        self.assertEqual(result, ('', 'other'))
 
 class TraversalContextURLTests(unittest.TestCase):
     def _makeOne(self, context, url):
@@ -534,6 +591,8 @@ def make_traverser(*args):
     return DummyTraverser
         
 class DummyContext(object):
+    __parent__ = None
+    __name__ = None
     def __init__(self, next=None):
         self.next = next
         
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index f43935ff4..ee565f85e 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -54,72 +54,106 @@ def find_root(model):
     return model
 
 def find_model(model, path):
-    """ Given a model object and a string representing a path
-    reference (a set of names delimited by forward-slashes, such as
+    """ Given a model object and a tuple representing a path (such as
     the return value of ``model_path``), return an context in this
-    application's model graph at the specified path.  If the path
-    starts with a slash, the path is considered absolute and the graph
-    traversal will start at the root object.  If the path does not
-    start with a slash, the path is considered relative and graph
-    traversal will begin at the model object supplied to the function.
-    In either case, if the path cannot be resolved, a KeyError will be
-    thrown. Note that the model passed in should be
-    :term:`location`-aware."""
+    application's model graph at the specified path.  The model passed
+    in *must* be :term:`location`-aware.  If the first element in the
+    path tuple is the empty string (for example ``('', 'a', 'b',
+    'c')``, the path is considered absolute and the graph traversal
+    will start at the graph root object.  If the first element in the
+    path tuple is not the empty string (for example ``('a', 'b',
+    'c')``), the path is considered relative and graph traversal will
+    begin at the model object supplied to the function.  No
+    URL-quoting or UTF-8-encoding of individual path segments within
+    the tuple is required (each segment may be any string or unicode
+    object representing a model name).  If an empty sequence is passed
+    as ``path``, the ``model`` passed in itself will be returned.  If
+    the path cannot be resolved, a KeyError will be raised.
+
+    .. note:: It is also permissible to pass a string to this function
+    as the ``path``, as long as each Unicode path segment is encoded
+    as UTF-8 and as long as each path segment is escaped via Python's
+    ``urllib.quote``.  For example, ``/path/to%20the/La%20Pe%C3%B1a``
+    (absolute) or ``to%20the/La%20Pe%C3%B1a`` (relative).
+    ``find_model`` will consider a string path absolute if it starts
+    with the ``/`` character; it will consider the path relative to
+    the ``model`` passed in if it does not start with the ``/``
+    character.  If an empty string is passed as ``path``, the
+    ``model`` passed in will be returned.
+
+    .. note:: This function is the logical inverse of ``model_path``;
+    it can resolve any path tuple generated by ``model_path``.
+    """
+
+    if hasattr(path, '__iter__'): # it's a tuple or some other iterable
+        # the traverser factory expects PATH_INFO to be a string, not
+        # unicode and it expects path segments to be utf-8 and
+        # urlencoded (it's the same traverser which accepts PATH_INFO
+        # from user agents; user agents always send strings).
+        path = [_urlsegment(name) for name in path]
+        if path:
+            path = '/'.join(path) or '/'
+        else:
+            path = ''
 
     if path.startswith('/'):
         model = find_root(model)
 
-    if path.__class__ is unicode:
-        # the traverser factory expects PATH_INFO to be a string,
-        # not unicode (it's the same traverser which accepts PATH_INFO
-        # from user agents; user agents always send strings).
-        path = path.encode('utf-8')
-
-    ob, name, path = ITraverserFactory(model)({'PATH_INFO':path})
+    ob, name, _ = ITraverserFactory(model)({'PATH_INFO':path})
     if name:
         raise KeyError('%r has no subelement %s' % (ob, name))
     return ob
 
 def find_interface(model, interface):
     """
-    Return the first object found which provides the interface
-    ``interface`` in the parent chain of ``model`` or ``None`` if no
+    Return the first object found in the parent chain of ``model``
+    which provides the interface ``interface``.  Return ``None`` if no
     object providing ``interface`` can be found in the parent chain.
-    The ``model`` passed in should be :term:`location`-aware.
+    The ``model`` passed in *must* be :term:`location`-aware.
     """
     for location in lineage(model):
         if interface.providedBy(location):
             return location
 
 def model_path(model, *elements):
-    """ Return a string representing the absolute path of the model
-    object based on its position in the model graph, e.g ``/foo/bar``.
-    This function is the inverse of ``find_model``: it can be used to
-    generate paths that can later be resolved via ``find_model``.  Any
-    positional arguments passed in as ``elements`` will be joined by
-    slashes and appended to the generated path.  The ``model`` passed
-    in must be :term:`location`-aware.
-
-    Note that the way this function treats path segments is not
-    equivalent to how the ``repoze.bfg.url.model_url`` API treats path
-    segments: individual segments that make up the path are not quoted
-    in any way.  Thus, to ensure that this function generates paths
-    that can later be resolved to models via ``find_model``, you
-    should ensure that your model __name__ attributes do not contain
-    any forward slashes.
-
-    The path returned may be a unicode object if any model name in the
-    model graph is a unicode object; otherwise it will be a string.
+    """ Return a tuple representing the absolute physical path of the
+    model object based on its position in the model graph, e.g ``('',
+    'foo', 'bar')``.  Any positional arguments passed in as
+    ``elements`` will be appended as elements in the tuple
+    representing the the model path.  For instance, if the model's
+    path is ``('', 'foo', 'bar')`` and elements equals ``('a', 'b')``,
+    the returned tuple will be ``('foo', 'bar', 'a', b')``.  The
+    ``model`` passed in *must* be :term:`location`-aware.  The first
+    element of this tuple will always be the empty string (a leading
+    empty string element in a path tuple represents that the path is
+    absolute).  This function is the logical inverse of
+    ``find_model``: it can be used to generate path references that
+    can later be resolved via ``find_model``.
+
+    .. note:: Each segment in the path tuple returned will equal the
+    ``__name__`` attribute of the model it represents within the
+    graph.  Each of these segments *should* be a unicode or string
+    object (as per the contract of :term:`location-awareness`).
+    However, no conversion or safety checking of model names is
+    performed.  For instance, if one of the models in your graph has a
+    ``__name__`` which (by error) is a dictionary, that dictionary
+    will be placed in the path tuple; no warning or error will be
+    given.  A single exception to this rule exists: the :term:`root`
+    model may have a ``__name__`` attribute of any value; the value of
+    this attribute will always be ignored (and effectively replaced
+    with ``''``) when the path is generated.
     """
-    rpath = []
+    path = []
     for location in lineage(model):
-        if location.__name__:
-            rpath.append(location.__name__)
-    path = '/' + '/'.join(reversed(rpath))
-    if elements: # never have a trailing slash
-        suffix = '/'.join(elements)
-        path = '/'.join([path, suffix])
-    return path
+        path.append(location.__name__)
+
+    # replace root __name__ with the empty string
+    path.pop() 
+    path.append('')
+
+    path.reverse()
+    path.extend(elements)
+    return tuple(path)
 
 def virtual_root(model, request):
     """