- Fix ModelGraphTraverser; don't try to change the ``__name__`` or

    ``__parent__`` of an object that claims it implements ILocation
    during traversal even if the ``__name__`` or ``__parent__`` of the
    object traversed does not match the name used in the traversal
    step or the or the traversal parent .  Rationale: it was insane to
    do so. This bug was only found due to a misconfiguration in an
    application that mistakenly had intermediate persistent
    non-ILocation objects; traversal was causing a persistent write on
    every request under this setup.

  - ``repoze.bfg.location.locate`` now unconditionally sets
    ``__name__`` and ``__parent__`` on objects which provide ILocation
    (it previously only set them conditionally if they didn't match
    attributes already present on the object via equality).

Prep for 0.5.0.

3 files changed, 48 insertions, 20 deletions

diff --git a/repoze/bfg/location.py b/repoze/bfg/location.py
index 11f08faf9..ed7d2ad1f 100644
--- a/repoze/bfg/location.py
+++ b/repoze/bfg/location.py
@@ -39,10 +39,10 @@ def inside(model1, model2):
 def locate(model, parent, name=None):
     """
     If ``model`` explicitly provides the
-    ``repoze.bfg.interfaces.ILocation`` interface, locate ``model``
-    directly set ``model`` 's ``__parent__`` attribute to the
-    ``parent`` object (also a model), and its ``__name__`` to the
-    supplied ``name`` argument.
+    ``repoze.bfg.interfaces.ILocation`` interface, directly set
+    ``model`` 's ``__parent__`` attribute to the ``parent`` object
+    (also a model), and its ``__name__`` to the supplied ``name``
+    argument, and return the model.
 
     If ``model`` does *not* explicitly provide the
     ``repoze.bfg.interfaces.ILocation`` interface, return a
@@ -53,8 +53,8 @@ def locate(model, parent, name=None):
     instances.
     """
     if ILocation.providedBy(model):
-        if parent is not model.__parent__ or name != model.__name__:
-            _locate(model, parent, name)
+        model.__parent__ = parent
+        model.__name__ = name
         return model
     return LocationProxy(model, parent, name)
 
@@ -83,10 +83,6 @@ def lineage(model):
         yield model
         model = getattr(model, '__parent__', None)
 
-def _locate(model, parent, name=None):
-    model.__parent__ = parent
-    model.__name__ = name
-
 class ClassAndInstanceDescr(object):
 
     def __init__(self, *args):
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 90e64dc97..2c02d7d63 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -101,34 +101,67 @@ class ModelGraphTraverserTests(unittest.TestCase, PlacelessSetup):
         self.assertEqual(name, 'foo')
         self.assertEqual(subpath, [])
 
-    def test_call_with_ILocation_root(self):
+    def test_call_with_ILocation_root_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
-        # give bar a direct parent and name to mix things up a bit
-        bar.__name__ = 'bar'
-        bar.__parent__ = foo
         policy = self._makeOne(root)
         environ = self._getEnviron(PATH_INFO='/foo/bar/baz')
         ctx, name, subpath = policy(environ)
-        self.assertEqual(ctx, baz)
         self.assertEqual(name, '')
         self.assertEqual(subpath, [])
+        self.assertEqual(ctx, baz)
+        self.failUnless(isProxy(ctx))
         self.assertEqual(ctx.__name__, 'baz')
         self.assertEqual(ctx.__parent__, bar)
+        self.failUnless(isProxy(ctx.__parent__))
         self.assertEqual(ctx.__parent__.__name__, 'bar')
         self.assertEqual(ctx.__parent__.__parent__, foo)
+        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.assertEqual(ctx.__parent__.__parent__.__parent__.__name__, None)
         self.assertEqual(ctx.__parent__.__parent__.__parent__.__parent__, None)
 
+    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 = 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__))
+
 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 8c2df6a41..f6a415b85 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -3,7 +3,7 @@ import urlparse
    
 from zope.interface import classProvides
 from zope.interface import implements
-from repoze.bfg.location import locate
+from repoze.bfg.location import LocationProxy
 from repoze.bfg.location import lineage
 
 from repoze.bfg.interfaces import ILocation
@@ -48,9 +48,8 @@ class ModelGraphTraverser(object):
     def __call__(self, environ):
         path = environ.get('PATH_INFO', '/')
         path = split_path(path)
-        root = self.root
-
         ob = self.root
+
         name = ''
 
         while path:
@@ -59,8 +58,8 @@ class ModelGraphTraverser(object):
             if next is _marker:
                 name = segment
                 break
-            if self.locatable:
-                next = locate(next, ob, segment)
+            if (self.locatable) and (not ILocation.providedBy(next)):
+                next = LocationProxy(next, ob, segment)
             ob = next
 
         return ob, name, path