Revert my decision to make ``model_path`` return a tuple; it

now still returns a string; albeit a quoted one.  An additional API 
(model_path_tuple) now also exists which can be used to get a model 
path as a tuple.

- The ``repoze.bfg.traversal.model_path`` API now returns a *quoted*
  string rather than a string represented by series of unquoted
  elements joined via ``/`` characters.  Previously it returned a
  string or unicode object representing the model path, with each
  segment name in the path joined together via ``/`` characters,
  e.g. ``/foo /bar``.  Now it returns a string, where each segment is
  a UTF-8 encoded and URL-quoted element e.g. ``/foo%20/bar``.  This
  change was (as discussed briefly on the repoze-dev maillist)
  necessary to accomodate model objects which themselves have
  ``__name__`` attributes that contain the ``/`` character.

  For people that have no models that have high-order Unicode
  ``__name__`` attributes or ``__name__`` attributes with values that
  require URL-quoting with in their model graphs, this won't cause any
  issue.  However, if you have code that currently expects
  ``model_path`` to return an unquoted string, or you have an existing
  application with data generated via the old method, and you're too
  lazy to change anything, you may wish replace the BFG-imported
  ``model_path`` in your code with this function (this is the code of
  the "old" ``model_path`` implementation)::

        from repoze.bfg.location import lineage

        def i_am_too_lazy_to_move_to_the_new_model_path(model, *elements):
            rpath = []
            for location in lineage(model):
                if location.__name__:
                    rpath.append(location.__name__)
            path = '/' + '/'.join(reversed(rpath))
            if elements:
                suffix = '/'.join(elements)
                path = '/'.join([path, suffix])
            return path

- The ``repoze.bfg.traversal.find_model`` API no longer implicitly
  converts unicode representations of a full path passed to it as a
  Unicode object into a UTF-8 string.  Callers should either use
  prequoted path strings returned by
  ``repoze.bfg.traversal.model_path``, or tuple values returned by the
  result of ``repoze.bfg.traversal.model_path_tuple`` or they should
  use the guidelines about passing a string ``path`` argument
  described in the ``find_model`` API documentation.

- Each argument contained in ``elements`` passed to
  ``repoze.bfg.traversal.model_path`` will now have any ``/``
  characters contained within quoted to ``%2F`` in the returned
  string.  Previously, ``/`` characters in elements were left unquoted
  (a bug).

- A ``repoze.bfg.traversal.model_path_tuple`` API was added.  This API
  is an alternative to ``model_path`` (which returns a string);
  ``model_path_tuple`` returns a model path as a tuple (much like
  Zope's ``getPhysicalPath``).

- A ``repoze.bfg.traversal.quote_path_segment`` API was added.  This
  API will quote an individual path segment (string or unicode
  object).  See the ``repoze.bfg.traversal`` API documentation for
  more information.

2 files changed, 56 insertions, 1 deletions

diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 1486e5b91..ebac680d4 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -412,6 +412,46 @@ class ModelPathTests(unittest.TestCase):
         baz.__parent__ = bar
         baz.__name__ = 'baz'
         result = self._callFUT(baz, 'this/theotherthing', 'that')
+        self.assertEqual(result, '/foo%20/bar/baz/this%2Ftheotherthing/that')
+
+    def test_root_default(self):
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        request = DummyRequest()
+        result = self._callFUT(root)
+        self.assertEqual(result, '/')
+        
+    def test_nonroot_default(self):
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        other = DummyContext()
+        other.__parent__ = root
+        other.__name__ = 'other'
+        request = DummyRequest()
+        result = self._callFUT(other)
+        self.assertEqual(result, '/other')
+
+class ModelPathTupleTests(unittest.TestCase):
+    def _callFUT(self, model, *elements):
+        from repoze.bfg.traversal import model_path_tuple
+        return model_path_tuple(model, *elements)
+
+    def test_it(self):
+        baz = DummyContext()
+        bar = DummyContext(baz)
+        foo = DummyContext(bar)
+        root = DummyContext(foo)
+        root.__parent__ = None
+        root.__name__ = None
+        foo.__parent__ = root
+        foo.__name__ = 'foo '
+        bar.__parent__ = foo
+        bar.__name__ = 'bar'
+        baz.__parent__ = bar
+        baz.__name__ = 'baz'
+        result = self._callFUT(baz, 'this/theotherthing', 'that')
         self.assertEqual(result, ('','foo ', 'bar', 'baz', 'this/theotherthing',
                                   'that'))
 
@@ -434,6 +474,21 @@ class ModelPathTests(unittest.TestCase):
         result = self._callFUT(other)
         self.assertEqual(result, ('', 'other'))
 
+class QuotePathSegmentTests(unittest.TestCase):
+    def _callFUT(self, s):
+        from repoze.bfg.traversal import quote_path_segment
+        return quote_path_segment(s)
+
+    def test_unicode(self):
+        la = unicode('/La Pe\xc3\xb1a', 'utf-8')
+        result = self._callFUT(la)
+        self.assertEqual(result, '%2FLa%20Pe%C3%B1a')
+
+    def test_string(self):
+        s = '/ hello!'
+        result = self._callFUT(s)
+        self.assertEqual(result, '%2F%20hello%21')
+
 class TraversalContextURLTests(unittest.TestCase):
     def _makeOne(self, context, url):
         return self._getTargetClass()(context, url)
diff --git a/repoze/bfg/tests/test_url.py b/repoze/bfg/tests/test_url.py
index dee86c05f..b9733f802 100644
--- a/repoze/bfg/tests/test_url.py
+++ b/repoze/bfg/tests/test_url.py
@@ -40,7 +40,7 @@ class ModelURLTests(unittest.TestCase):
         result = self._callFUT(context, request, 'this/theotherthing', 'that')
         self.assertEqual(
             result,
-            'http://example.com/context/this/theotherthing/that')
+            'http://example.com/context/this%2Ftheotherthing/that')
 
     def test_unicode_in_element_names(self):
         self._registerContextURL()