- ``repoze.bfg.traversal.model_url`` now always appends a slash to

    all generated URLs unless further elements are passed in as the
    third and following arguments.  Rationale: views often use
    ``model_url`` without the third-and-following arguments in order
    to generate a URL for a model in order to point at the default
    view of a model.  The URL that points to the default view of the
    *root* model is technically ``http://mysite/`` as opposed to
    ``http://mysite`` (browsers happen to ask for '/' implicitly in
    the GET request).  Because URLs are never automatically generated
    for anything *except* models by ``model_url``, and because the
    root model is not really special, we continue this pattern.  The
    impact of this change is minimal (at most you will have too many
    slashes in your URL, which BFG deals with gracefully anyway).

Prep for 0.4.8.

5 files changed, 112 insertions, 71 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 01f6d3676..f3db37337 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,21 @@
+0.4.8 (11/12/2008)
+
+  Backwards Incompatibilities
+
+  - ``repoze.bfg.traversal.model_url`` now always appends a slash to
+    all generated URLs unless further elements are passed in as the
+    third and following arguments.  Rationale: views often use
+    ``model_url`` without the third-and-following arguments in order
+    to generate a URL for a model in order to point at the default
+    view of a model.  The URL that points to the default view of the
+    *root* model is technically ``http://mysite/`` as opposed to
+    ``http://mysite`` (browsers happen to ask for '/' implicitly in
+    the GET request).  Because URLs are never automatically generated
+    for anything *except* models by ``model_url``, and because the
+    root model is not really special, we continue this pattern.  The
+    impact of this change is minimal (at most you will have too many
+    slashes in your URL, which BFG deals with gracefully anyway).
+
 0.4.7 (11/11/2008)
 
   Features
diff --git a/docs/conf.py b/docs/conf.py
index fc8cc1378..d494ce20a 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -51,9 +51,9 @@ copyright = '2008, Agendaless Consulting'
 # other places throughout the built documents.
 #
 # The short X.Y version.
-version = '0.4.7'
+version = '0.4.8'
 # The full version, including alpha/beta/rc tags.
-release = '0.4.7'
+release = '0.4.8'
 
 # There are two options for replacing |today|: either, you set today to some
 # non-false value, then it is used:
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index be75ed646..90e64dc97 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -3,30 +3,25 @@ import unittest
 from zope.component.testing import PlacelessSetup
 
 class SplitPathTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, path):
         from repoze.bfg.traversal import split_path
-        return split_path
+        return split_path(path)
         
     def test_cleanPath_path_startswith_endswith(self):
-        f = self._getFUT()
-        self.assertEqual(f('/foo/'), ['foo'])
+        self.assertEqual(self._callFUT('/foo/'), ['foo'])
 
     def test_cleanPath_empty_elements(self):
-        f = self._getFUT()
-        self.assertEqual(f('foo///'), ['foo'])
+        self.assertEqual(self._callFUT('foo///'), ['foo'])
 
     def test_cleanPath_onedot(self):
-        f = self._getFUT()
-        self.assertEqual(f('foo/./bar'), ['foo', 'bar'])
+        self.assertEqual(self._callFUT('foo/./bar'), ['foo', 'bar'])
 
     def test_cleanPath_twodots(self):
-        f = self._getFUT()
-        self.assertEqual(f('foo/../bar'), ['bar'])
+        self.assertEqual(self._callFUT('foo/../bar'), ['bar'])
 
     def test_cleanPath_element_urllquoted(self):
-        f = self._getFUT()
-        self.assertEqual(f('/foo/space%20thing/bar'), ['foo', 'space thing',
-                                                       'bar'])
+        self.assertEqual(self._callFUT('/foo/space%20thing/bar'),
+                         ['foo', 'space thing', 'bar'])
 
 class ModelGraphTraverserTests(unittest.TestCase, PlacelessSetup):
     def setUp(self):
@@ -135,9 +130,9 @@ class ModelGraphTraverserTests(unittest.TestCase, PlacelessSetup):
         self.assertEqual(ctx.__parent__.__parent__.__parent__.__parent__, None)
 
 class FindInterfaceTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, context, iface):
         from repoze.bfg.traversal import find_interface
-        return find_interface
+        return find_interface(context, iface)
 
     def test_it(self):
         baz = DummyContext()
@@ -158,16 +153,15 @@ class FindInterfaceTests(unittest.TestCase):
         class IFoo(Interface):
             pass
         directlyProvides(root, IFoo)
-        finder = self._getFUT()
-        result = finder(baz, IFoo)
+        result = self._callFUT(baz, IFoo)
         self.assertEqual(result.__name__, 'root')
 
 class ModelURLTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, model, request, *elements):
         from repoze.bfg.traversal import model_url
-        return model_url
+        return model_url(model, request, *elements)
 
-    def test_it(self):
+    def test_extra_args(self):
         baz = DummyContext()
         bar = DummyContext(baz)
         foo = DummyContext(bar)
@@ -181,28 +175,58 @@ class ModelURLTests(unittest.TestCase):
         baz.__parent__ = bar
         baz.__name__ = 'baz'
         request = DummyRequest()
-        model_url = self._getFUT()
-        request = DummyRequest()
-        result = model_url(baz, request, 'this/theotherthing', 'that')
+        result = self._callFUT(baz, request, 'this/theotherthing', 'that')
 
         self.assertEqual(
             result,
             'http://example.com:5432/foo%20/bar/baz/this/theotherthing/that')
 
-    def test_root(self):
+    def test_root_default_app_url_endswith_slash(self):
         root = DummyContext()
         root.__parent__ = None
         root.__name__ = None
-        model_url = self._getFUT()
         request = DummyRequest()
-        result = model_url(root, request)
+        request.application_url = 'http://example.com:5432/'
+        result = self._callFUT(root, request)
+        self.assertEqual(result, 'http://example.com:5432/')
+
+    def test_root_default_app_url_endswith_nonslash(self):
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        request = DummyRequest()
+        request.application_url = 'http://example.com:5432'
+        result = self._callFUT(root, request)
         self.assertEqual(result, 'http://example.com:5432/')
-        
+
+    def test_nonroot_default_app_url_endswith_slash(self):
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        other = DummyContext()
+        other.__parent__ = root
+        other.__name__ = 'nonroot object'
+        request = DummyRequest()
+        request.application_url = 'http://example.com:5432/'
+        result = self._callFUT(other, request)
+        self.assertEqual(result, 'http://example.com:5432/nonroot%20object/')
+
+    def test_nonroot_default_app_url_endswith_nonslash(self):
+        root = DummyContext()
+        root.__parent__ = None
+        root.__name__ = None
+        other = DummyContext()
+        other.__parent__ = root
+        other.__name__ = 'nonroot object'
+        request = DummyRequest()
+        request.application_url = 'http://example.com:5432'
+        result = self._callFUT(other, request)
+        self.assertEqual(result, 'http://example.com:5432/nonroot%20object/')
 
 class FindRootTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, context):
         from repoze.bfg.traversal import find_root
-        return find_root
+        return find_root(context)
 
     def test_it(self):
         dummy = DummyContext()
@@ -211,14 +235,13 @@ class FindRootTests(unittest.TestCase):
         baz.__name__ = 'baz'
         dummy.__parent__ = None
         dummy.__name__ = None
-        find = self._getFUT()
-        result = find(baz)
+        result = self._callFUT(baz)
         self.assertEqual(result, dummy)
 
 class FindModelTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, context, name):
         from repoze.bfg.traversal import find_model
-        return find_model
+        return find_model(context, name)
 
     def _registerTraverserFactory(self, traverser):
         import zope.component
@@ -230,19 +253,17 @@ class FindModelTests(unittest.TestCase):
     def test_relative_found(self):
         dummy = DummyContext()
         baz = DummyContext()
-        find = self._getFUT()
         traverser = make_traverser(baz, '', [])
         self._registerTraverserFactory(traverser)
-        result = find(dummy, 'baz')
+        result = self._callFUT(dummy, 'baz')
         self.assertEqual(result, baz)
 
     def test_relative_notfound(self):
         dummy = DummyContext()
         baz = DummyContext()
-        find = self._getFUT()
         traverser = make_traverser(baz, 'bar', [])
         self._registerTraverserFactory(traverser)
-        self.assertRaises(KeyError, find, dummy, 'baz')
+        self.assertRaises(KeyError, self._callFUT, dummy, 'baz')
 
     def test_absolute_found(self):
         dummy = DummyContext()
@@ -251,10 +272,9 @@ class FindModelTests(unittest.TestCase):
         baz.__name__ = 'baz'
         dummy.__parent__ = None
         dummy.__name__ = None
-        find = self._getFUT()
         traverser = make_traverser(dummy, '', [])
         self._registerTraverserFactory(traverser)
-        result = find(baz, '/')
+        result = self._callFUT(baz, '/')
         self.assertEqual(result, dummy)
         self.assertEqual(dummy.wascontext, True)
 
@@ -265,16 +285,15 @@ class FindModelTests(unittest.TestCase):
         baz.__name__ = 'baz'
         dummy.__parent__ = None
         dummy.__name__ = None
-        find = self._getFUT()
         traverser = make_traverser(dummy, 'fuz', [])
         self._registerTraverserFactory(traverser)
-        self.assertRaises(KeyError, find, baz, '/')
+        self.assertRaises(KeyError, self._callFUT, baz, '/')
         self.assertEqual(dummy.wascontext, True)
 
 class ModelPathTests(unittest.TestCase):
-    def _getFUT(self):
+    def _callFUT(self, model, *elements):
         from repoze.bfg.traversal import model_path
-        return model_path
+        return model_path(model, *elements)
 
     def test_it(self):
         baz = DummyContext()
@@ -289,19 +308,27 @@ class ModelPathTests(unittest.TestCase):
         bar.__name__ = 'bar'
         baz.__parent__ = bar
         baz.__name__ = 'baz'
-        model_path = self._getFUT()
-        result = model_path(baz, 'this/theotherthing', 'that')
+        result = self._callFUT(baz, 'this/theotherthing', 'that')
         self.assertEqual(result, '/foo /bar/baz/this/theotherthing/that')
 
-    def test_root(self):
+    def test_root_default(self):
         root = DummyContext()
         root.__parent__ = None
         root.__name__ = None
-        model_path = self._getFUT()
         request = DummyRequest()
-        result = model_path(root)
+        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')
 
 def make_traverser(*args):
     class DummyTraverser(object):
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index 29677fe99..8c2df6a41 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -103,30 +103,25 @@ def find_interface(model, interface):
         if interface.providedBy(location):
             return location
 
-def _pjoin(path):
-    path = list(path)
-    path.insert(0, '')
-    result = '/'.join(path)
-    if not result:
-        result = '/'
-    return result
-
 def model_url(model, request, *elements):
     """ Return the absolute URL of the model object based on the
     ``wsgi.url_scheme``, ``HTTP_HOST`` or ``SERVER_NAME`` in the
-    request, plus any ``SCRIPT_NAME``.  Any positional arguments
-    passed in as ``elements`` will be joined by slashes and appended
-    to the generated URL.  The passed in elements are *not*
-    URL-quoted.  The ``model`` passed in must be
-    :term:`location`-aware."""
+    request, plus any ``SCRIPT_NAME``.  The model URL will end with a
+    trailing slash.  Any positional arguments passed in as
+    ``elements`` will be joined by slashes and appended to the model
+    URL.  The passed in elements are *not* URL-quoted.  The ``model``
+    passed in must be :term:`location`-aware."""
     rpath = []
     for location in lineage(model):
         if location.__name__:
             rpath.append(urllib.quote(location.__name__))
-    path = list(reversed(rpath))
-    path.extend(elements)
-    path = _pjoin(path)
-    return urlparse.urljoin(request.application_url, path)
+    prefix = '/'.join(reversed(rpath))
+    suffix = '/'.join(elements)
+    path = '/'.join([prefix, suffix]) # always have trailing slash
+    app_url = request.application_url
+    if not app_url.endswith('/'):
+        app_url = app_url+'/'
+    return urlparse.urljoin(app_url, path)
 
 def model_path(model, *elements):
     """ Return a string representing the absolute path of the model
@@ -138,8 +133,9 @@ def model_path(model, *elements):
     for location in lineage(model):
         if location.__name__:
             rpath.append(location.__name__)
-    path = list(reversed(rpath))
-    path.extend(elements)
-    path = _pjoin(path)
+    path = '/' + '/'.join(reversed(rpath))
+    if elements: # never have a trailing slash
+        suffix = '/'.join(elements)
+        path = '/'.join([path, suffix])
     return path
 
diff --git a/setup.py b/setup.py
index 66d957d8b..4d574aeb5 100644
--- a/setup.py
+++ b/setup.py
@@ -12,7 +12,7 @@
 #
 ##############################################################################
 
-__version__ = '0.4.7'
+__version__ = '0.4.8'
 
 import os