Make ModelGraphTraverser assume that all traversed objects provide ILocation. Make WrappingModelGraphTraverser assume that *no* traversed objects provide ILocation. This makes it unnecessary to explain why the root object in a WrappingModelGraphTraverser setup needs to supply the ILocation interface. Now it doesn't.

4 files changed, 128 insertions, 100 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index f345ed6a0..5070ee707 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -4,25 +4,24 @@ Next release
 Backwards Incompatibilities
 ---------------------------
 
-- In all previous releases, to support "toy" applications, which use,
-  for instance, a dictionary as the root object as opposed to an
-  object that supplies the ILocation interface (a ``__parent__``
-  attribute and a ``__name__`` attribute), :mod:`repoze.bfg` provided
-  some "help" during traversal.  This traversal feature wrapped
-  subobjects that did not implement ``ILocation`` in proxies which
-  automatically provided them with a ``__name__`` and ``__parent__``
-  attribute based on the name being traversed.  This feature has now
-  been disabled in the default configuration for purposes of speed.
-  In order to satisfy the new default, your root object and all
-  published subobjects should supply a ``__parent__`` attribute (which
-  is a reference to its direct traversal parent) and a ``__name__``
-  attribute.
+- In all previous releases, by default, if the root object supplied
+  the``repoze.bfg.interfaces.ILocation`` interface, but the children
+  returned via its ``__getitem__`` did not return objects that
+  implemented the same interface, :mod:`repoze.bfg` provided some
+  implicit help during traversal.  This traversal feature wrapped
+  subobjects from the root that did not implement ``ILocation`` in
+  proxies which automatically provided them with a ``__name__`` and
+  ``__parent__`` attribute based on the name being traversed and the
+  previous object traversed.  This feature has now been disabled in
+  the default configuration for purposes of speed and
+  understandability.
 
   In order to re-enable the ILocation wrapper behavior for older
   applications which cannot be changed, register the
   ``WrappingModelGraphTraverser`` as the traversal policy, rather than
-  the standard ``ModelGraphTraverser``.  E.g., your application will
-  need to have the following in its ``configure.zcml``::
+  the default ``ModelGraphTraverser``. To use this feature, your
+  application will need to have the following in its
+  ``configure.zcml``::
 
     <adapter
         factory="repoze.bfg.traversal.WrappingModelGraphTraverser"
@@ -30,6 +29,16 @@ Backwards Incompatibilities
         for="*"
         />
 
+   When this ITraverserFactory is used, no object in the graph (even
+   the root object) must supply a ``__name__`` or ``__parent__``
+   attribute.  Even if subobjects returned from the root *do*
+   implement the ILocation interface, these will still be wrapped in
+   proxies that override the object's "real" ``__parent__`` and
+   ``__name__`` attributes.
+
+   See also changes to the "Models" chapter of the documentation (in
+   the "Location-Aware Model Instances") section.
+
 Features
 --------
 
diff --git a/docs/narr/models.rst b/docs/narr/models.rst
index e9eddf39b..624a29002 100644
--- a/docs/narr/models.rst
+++ b/docs/narr/models.rst
@@ -92,29 +92,49 @@ instance nodes in the graph:
 Location-Aware Model Instances
 ------------------------------
 
-For :mod:`repoze.bfg` security and convenience URL-generation
-functions to work properly against a model instance graph, all nodes
-in the graph should have two attributes:: ``__parent__`` and
-``__name__``.  The ``__parent__`` attribute should be a reference to
-the node's parent model instance in the graph.  The ``__name__``
-attribute should be the name that a node's parent refers to the node
-by via ``__getitem__``.
-
-If you choose not to manage the ``__name__`` and ``__parent__``
-attributes of your models "by hand", :mod:`repoze.bfg`` is willing to
-help you do this.  If your "root" node claims it implements the
-interface ``repoze.bfg.interfaces.ILocation``, you don't need to
-manage these attributes by hand.  During :term:`traversal`, if the
-root node says it implements the ``ILocation`` :term:`interface`,
-:mod:`repoze.bfg` will wrap each child in a ``LocationProxy`` which
-will dynamically assign a ``__name__`` and a ``__parent__`` to it,
-recursively.
+In order for :mod:`repoze.bfg` location, security, URL-generation, and
+traversal functions (such as the functions exposed in
+:ref:`location_module`, :ref:`traversal_module`, and :ref:`url_module`
+as well as certain functions in :ref:`security_module` ) to work
+properly against a instances in a model graph, all nodes in the graph
+must be "location-aware".  This means they must have two attributes:
+``__parent__`` and ``__name__``.  The ``__parent__`` attribute should
+be a reference to the node's parent model instance in the graph.  The
+``__name__`` attribute should be the name that a node's parent refers
+to the node via ``__getitem__``.  The ``__parent__`` of the root
+object should be ``None`` and its ``__name__`` should be the empty
+string.  For instance:
+
+.. code-block:: python
+
+   class MyRootObject(object):
+       __name__ = ''
+       __parent__ = None
+
+A node returned from the root item's ``__getitem__`` method should
+have a ``__parent__`` attribute that is a reference to the root
+object, and its ``__name__`` attribute should match the name by which
+it is are reachable via the root object's ``__getitem__``.  *That*
+object's ``__getitem__`` should return objects that have a
+``__parent__`` attribute that points at that object, and
+``__getitem__``-returned objects should have a ``__name__`` attribute
+that matches the name by which they are retrieved via ``__getitem__``,
+and so on.
 
 .. note::
-   In order to use this feature, you must register the
-   ``WrappingModelGraphTraverser`` as the traversal policy, rather
-   than the standard ``ModelGraphTraverser``.  E.g., your application
-   will need to have the following in its ``configure.zcml``::
+
+  If you'd rather not manage the ``__name__`` and ``__parent__``
+  attributes of your models "by hand", :mod:`repoze.bfg`` can help you
+  do this.
+
+  In order to use this helper feature, you must first register the
+  ``WrappingModelGraphTraverser`` as the traversal policy, rather than
+  the default ``ModelGraphTraverser``. To register the
+  ``WrappingModelGraphTraverser`` as the traversal policy, your
+  application will need to have the following in its
+  ``configure.zcml`` file:
+
+  .. code-block:: xml
 
     <adapter
         factory="repoze.bfg.traversal.WrappingModelGraphTraverser"
@@ -122,24 +142,13 @@ recursively.
         for="*"
     />
 
-
-If you choose to make use of the location-based dynamic assignment of
-``__parent__`` and ``__name__``, the root node must have a
-``__parent__`` that is ``None``, a ``__name__`` with any value, and it
-must provide the ``repoze.bfg.interfaces.ILocation`` interface.  The
-easiest way to do this is to claim that the class representing the
-root node ``implements(ILocation)``:
-
-.. code-block:: python
-   :linenos:
-
-   from repoze.bfg.interfaces import ILocation
-   from zope.interface import implements
-
-   class MyRootObject(object):
-       implements(ILocation)
-       __parent__ = None
-       __name__ = ''
+  If this statement is made in ZCML, you don't need to manage the
+  ``__parent__`` and ``__name__`` attributes on graph objects "by
+  hand".  Instead, as necessary, during traversal :mod:`repoze.bfg`
+  will wrap each object in a ``LocationProxy`` which will dynamically
+  assign a ``__name__`` and a ``__parent__`` to the traversed object
+  (based on the last traversed object and the name supplied to
+  ``__getitem__``).
 
 :mod:`repoze.bfg` API Functions That Act Against Models
 -------------------------------------------------------
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 3354dcce1..942f84684 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -195,17 +195,12 @@ class WrappingModelGraphTraverserTests(ModelGraphTraverserTests):
         from repoze.bfg.traversal import WrappingModelGraphTraverser
         return WrappingModelGraphTraverser
 
-    def test_call_with_ILocation_root_proxies(self):
+    def test_call_proxies(self):
         baz = DummyContext()
         bar = DummyContext(baz)
         foo = DummyContext(bar)
         root = DummyContext(foo)
-        from zope.interface import directlyProvides
-        from repoze.bfg.interfaces import ILocation
         from zope.proxy import isProxy
-        directlyProvides(root, ILocation)
-        root.__name__ = None
-        root.__parent__ = None
         policy = self._makeOne(root)
         environ = self._getEnviron(PATH_INFO='/foo/bar/baz')
         ctx, name, subpath, traversed, vroot, vroot_path = policy(environ)
@@ -221,47 +216,13 @@ class WrappingModelGraphTraverserTests(ModelGraphTraverserTests):
         self.failUnless(isProxy(ctx.__parent__.__parent__))
         self.assertEqual(ctx.__parent__.__parent__.__name__, 'foo')
         self.assertEqual(ctx.__parent__.__parent__.__parent__, root)
-        self.failIf(isProxy(ctx.__parent__.__parent__.__parent__))
+        self.failUnless(isProxy(ctx.__parent__.__parent__.__parent__))
         self.assertEqual(ctx.__parent__.__parent__.__parent__.__name__, None)
         self.assertEqual(ctx.__parent__.__parent__.__parent__.__parent__, None)
         self.assertEqual(traversed, [u'foo', u'bar', u'baz'])
         self.assertEqual(vroot, root)
         self.assertEqual(vroot_path, [])
 
-    def test_call_with_ILocation_root_proxies_til_next_ILocation(self):
-        # This is a test of an insane setup; it tests the case where
-        # intermediate objects (foo and bar) do not implement
-        # ILocation, and so are returned as proxies to the traverser,
-        # but when we reach the "baz" object, it *does* implement
-        # ILocation, and its parent should be the *real* "bar" object
-        # rather than the proxied bar.
-        from zope.interface import directlyProvides
-        from repoze.bfg.interfaces import ILocation
-        baz = DummyContext()
-        directlyProvides(baz, ILocation)
-        baz.__name__ = 'baz'
-        bar = DummyContext(baz)
-        baz.__parent__ = bar
-        foo = DummyContext(bar)
-        root = DummyContext(foo)
-        from zope.proxy import isProxy
-        directlyProvides(root, ILocation)
-        root.__name__ = None
-        root.__parent__ = None
-        policy = self._makeOne(root)
-        environ = self._getEnviron(PATH_INFO='/foo/bar/baz')
-        ctx, name, subpath, traversed, vroot, vroot_path = policy(environ)
-        self.assertEqual(name, '')
-        self.assertEqual(subpath, [])
-        self.assertEqual(ctx, baz)
-        self.failIf(isProxy(ctx))
-        self.assertEqual(ctx.__name__, 'baz')
-        self.assertEqual(ctx.__parent__, bar)
-        self.failIf(isProxy(ctx.__parent__))
-        self.assertEqual(traversed, [u'foo', u'bar', u'baz'])
-        self.assertEqual(vroot, root)
-        self.assertEqual(vroot_path, [])
-
 class FindInterfaceTests(unittest.TestCase):
     def _callFUT(self, context, iface):
         from repoze.bfg.traversal import find_interface
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index 6c2e6aa9a..ddfd7183d 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -380,9 +380,11 @@ def quote_path_segment(segment):
 _marker = object()
 
 class ModelGraphTraverser(object):
+    """ A model graph traverser that should be used (for speed) when
+    every object in the graph supplies a ``__name__`` and
+    ``__parent__`` attribute (ie. every object 'provides ILocation') ."""
     classProvides(ITraverserFactory)
     implements(ITraverser)
-    SUPPLY_LOCATION_PROXIES = False
     def __init__(self, root):
         self.root = root
 
@@ -407,7 +409,6 @@ class ModelGraphTraverser(object):
 
         ob = vroot = self.root
         name = ''
-        locatable = self.SUPPLY_LOCATION_PROXIES and ILocation.providedBy(ob)
 
         i = 1
 
@@ -422,8 +423,6 @@ class ModelGraphTraverser(object):
                 next = getitem(segment)
             except KeyError:
                 return ob, segment, path[i:], traversed, vroot, vroot_path
-            if locatable and (not ILocation.providedBy(next)):
-                next = LocationProxy(next, ob, segment)
             if vroot_idx == i-1:
                 vroot = ob
             traversed.append(segment)
@@ -433,7 +432,57 @@ class ModelGraphTraverser(object):
         return ob, '', [], traversed, vroot, vroot_path
 
 class WrappingModelGraphTraverser(ModelGraphTraverser):
-    SUPPLY_LOCATION_PROXIES = True
+    """ A model graph traverser that should be used (for convenience)
+    when no object in the graph supplies either a ``__name__`` or a
+    ``__parent__`` attribute (ie. no object 'provides ILocation') ."""
+    classProvides(ITraverserFactory)
+    implements(ITraverser)
+    def __init__(self, root):
+        self.root = root
+
+    def __call__(self, environ, _marker=_marker):
+        try:
+            path = environ['PATH_INFO']
+        except KeyError:
+            path = '/'
+        try:
+            vroot_path_string = environ[VH_ROOT_KEY]
+        except KeyError:
+            vroot_path = []
+            vroot_idx = 0
+        else:
+            vroot_path = list(traversal_path(vroot_path_string))
+            vroot_idx = len(vroot_path)
+            path = vroot_path_string + path
+
+        path = list(traversal_path(path))
+
+        traversed = []
+
+        ob = vroot = LocationProxy(self.root)
+        name = ''
+
+        i = 1
+
+        for segment in path:
+            if segment[:2] =='@@':
+                return ob, segment[2:], path[i:], traversed, vroot, vroot_path
+            try:
+                getitem = ob.__getitem__
+            except AttributeError:
+                return ob, segment, path[i:], traversed, vroot, vroot_path
+            try:
+                next = getitem(segment)
+            except KeyError:
+                return ob, segment, path[i:], traversed, vroot, vroot_path
+            next = LocationProxy(next, ob, segment)
+            if vroot_idx == i-1:
+                vroot = ob
+            traversed.append(segment)
+            ob = next
+            i += 1
+
+        return ob, '', [], traversed, vroot, vroot_path
 
 class TraversalContextURL(object):
     """ The IContextURL adapter used to generate URLs for a context