diff options
| -rw-r--r-- | CHANGES.txt | 37 | ||||
| -rw-r--r-- | repoze/bfg/tests/test_traversal.py | 16 | ||||
| -rw-r--r-- | repoze/bfg/traversal.py | 81 | ||||
| -rw-r--r-- | repoze/bfg/url.py | 18 |
4 files changed, 111 insertions, 41 deletions
diff --git a/CHANGES.txt b/CHANGES.txt index beaf2e81a..deab41700 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,40 @@ +Next Release + +Bug Fixes +--------- + +- Empty location names in model paths when generating a URL using + ``repoze.bfg.model_url`` based on a model obtained via traversal are + no longer ignored in the generated URL. This means that if a + non-root model object has a ``__name__`` of ``''``, the URL will + reflect it (e.g. ``model_url`` will generate ``http://foo/bar//baz`` + if an object with the ``__name__`` of ``''`` is a child of bar and + the parent of baz). URLs generated with empty path segments are, + however, still irresolveable by the model graph traverser on request + ingress (the traverser strips empty path segment names). + +Features +-------- + +- Microspeedups of ``repoze.bfg.traversal.model_path``, + ``repoze.bfg.traversal.model_path_tuple``, + ``repoze.bfg.traversal.quote_path_segment``, and + ``repoze.bfg.url.urlencode``. + +Documentation +------------- + +- Add a note to the ``repoze.bfg.traversal.quote_path_segment`` API + docs about caching of computed values. + +Implementation Changes +---------------------- + +- Simplification of + ``repoze.bfg.traversal.TraversalContextURL.__call__`` (it now uses + ``repoze.bfg.traversal.model_path`` instead of rolling its own + path-generation). + 0.6.8 (2009-02-05) ================== diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py index ebac680d4..c911ebfed 100644 --- a/repoze/bfg/tests/test_traversal.py +++ b/repoze/bfg/tests/test_traversal.py @@ -612,6 +612,22 @@ class TraversalContextURLTests(unittest.TestCase): self.assertEqual(context_url.virtual_root(), traversed_to) self.assertEqual(context.environ['PATH_INFO'], '/one') + def test_empty_names_not_ignored(self): + bar = DummyContext() + empty = DummyContext(bar) + root = DummyContext(empty) + root.__parent__ = None + root.__name__ = None + empty.__parent__ = root + empty.__name__ = '' + bar.__parent__ = empty + bar.__name__ = 'bar' + request = DummyRequest() + context_url = self._makeOne(bar, request) + result = context_url() + self.assertEqual(result, 'http://example.com:5432//bar/') + + class TestVirtualRoot(unittest.TestCase): def setUp(self): cleanUp() diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py index e330a14eb..02b6589ac 100644 --- a/repoze/bfg/traversal.py +++ b/repoze/bfg/traversal.py @@ -108,7 +108,7 @@ def find_model(model, path): else: path = '' - if path.startswith('/'): + if path and path[0] == '/': model = find_root(model) ob, name, _ = ITraverserFactory(model)({'PATH_INFO':path}) @@ -165,10 +165,11 @@ def model_path(model, *elements): effectively replaced with a leading ``/``) when the path is generated. """ - path = [ quote_path_segment(name) for name in - model_path_tuple(model, *elements) ] - path = '/'.join(path) or '/' - return path + path_list = _model_path_list(model, *elements) + if path_list: + path_list = [ quote_path_segment(name) for name in path_list ] + return '/' + '/'.join(path_list) + return '/' def model_path_tuple(model, *elements): """ @@ -204,18 +205,20 @@ def model_path_tuple(model, *elements): always be ignored (and effectively replaced with ``''``) when the path is generated. """ - path = [] - for location in lineage(model): - path.append(location.__name__) + path = _model_path_list(model, *elements) + if path: + return ('',) + tuple(path) + return ('',) - # replace root __name__ with the empty string - path.pop() - path.append('') +def _model_path_list(model, *elements): + """ Implementation detail shared by model_path and model_path_tuple """ + lpath = reversed(list(lineage(model))[:-1]) + path = [ location.__name__ for location in lpath ] - path.reverse() - path.extend(elements) - return tuple(path) + if elements: + path = path + list(elements) + return path def virtual_root(model, request): """ @@ -329,14 +332,26 @@ def quote_path_segment(segment): URL-quoted using Python's ``urllib.quote``. If the segment passed in is not a string or unicode object, an error will be raised. The return value of ``quote_path_segment`` is always a string, - never Unicode.""" + never Unicode. + + .. note:: The return value for each segment passed to this + function is cached in a module-scope dictionary for + speed: the cached version is returned when possible + rather than recomputing the quoted version. No cache + emptying is ever done for the lifetime of an + application, however. If you pass arbitrary + user-supplied strings to this function (as opposed to + some bounded set of values from a 'working set' known to + your application), it may become a memory leak. + """ # The bit of this code that deals with ``_segment_cache`` is an # optimization: we cache all the computation of URL path segments # in this module-scope dictionary with the original string (or # unicode value) as the key, so we can look it up later without # needing to reencode or re-url-quote it - result = _segment_cache.get(segment) - if result is None: + try: + return _segment_cache[segment] + except KeyError: if segment.__class__ is unicode: # isinstance slighly slower (~15%) result = _url_quote(segment.encode('utf-8')) else: @@ -344,7 +359,7 @@ def quote_path_segment(segment): # we don't need a lock to mutate _segment_cache, as the below # will generate exactly one Python bytecode (STORE_SUBSCR) _segment_cache[segment] = result - return result + return result _marker = object() @@ -417,26 +432,22 @@ class TraversalContextURL(object): """ Generate a URL based on the lineage of a model obtained via traversal. If any model in the context lineage has a unicode name, it will be converted to a UTF-8 string before - being attached to the URL. When composing the path based on - the model lineage, empty names in the model graph are ignored. - If a ``HTTP_X_VHM_ROOT`` key is present in the WSGI - environment, its value will be treated as a 'virtual root - path': the path of the URL generated by this will be - left-stripped of this virtual root path value. + being attached to the URL. If a ``HTTP_X_VHM_ROOT`` key is + present in the WSGI environment, its value will be treated as + a 'virtual root path': the path of the URL generated by this + will be left-stripped of this virtual root path value. """ - rpath = [] - for location in lineage(self.context): - name = location.__name__ - if name: - rpath.append(quote_path_segment(name)) - if rpath: - path = '/' + '/'.join(reversed(rpath)) + '/' - else: - path = '/' + path = model_path(self.context) + if path != '/': + path = path + '/' request = self.request + # if the path starts with the virtual root path, trim it out - vroot_path = request.environ.get(self.vroot_varname) - if vroot_path is not None: + try: + vroot_path = request.environ[self.vroot_varname] + except KeyError: + pass + else: if path.startswith(vroot_path): path = path[len(vroot_path):] diff --git a/repoze/bfg/url.py b/repoze/bfg/url.py index 0c6031c26..802dfca89 100644 --- a/repoze/bfg/url.py +++ b/repoze/bfg/url.py @@ -89,26 +89,32 @@ def urlencode(query, doseq=False): with an ``.items()`` method that returns a sequence of two-tuples representing key/value pairs. ``doseq`` controls what happens when a sequence is presented as one of the values. See the Python - stdlib documentation for more information. + stdlib documentation for ``urllib.urlencode`` for more + information. """ if hasattr(query, 'items'): - # dictionary + # presumed to be a dictionary query = query.items() - # presumed to be a sequence of two-tuples + newquery = [] for k, v in query: + if k.__class__ is unicode: k = k.encode('utf-8') - if isinstance(v, (tuple, list)): + try: + v.__iter__ + except AttributeError: + if v.__class__ is unicode: + v = v.encode('utf-8') + else: L = [] for x in v: if x.__class__ is unicode: x = x.encode('utf-8') L.append(x) v = L - elif v.__class__ is unicode: - v = v.encode('utf-8') + newquery.append((k, v)) return urllib.urlencode(newquery, doseq=doseq) |