From eb6afe916a2c7456d4398e2f0df04ebacccd7f30 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 12 Nov 2008 20:15:24 +0000 Subject: - ``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. --- CHANGES.txt | 18 ++++++ docs/conf.py | 4 +- repoze/bfg/tests/test_traversal.py | 123 ++++++++++++++++++++++--------------- repoze/bfg/traversal.py | 36 +++++------ setup.py | 2 +- 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 -- cgit v1.2.3