From dbab77392727456df89d3e91bd97899d919b6654 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Thu, 5 Feb 2009 08:27:08 +0000 Subject: 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. --- CHANGES.txt | 24 ++++++ repoze/bfg/tests/test_traversal.py | 145 ++++++++++++++++++++++++++----------- repoze/bfg/traversal.py | 126 ++++++++++++++++++++------------ 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): """ -- cgit v1.2.3