[Zope-dev] zope.traversing's ILocation behavior
Hi there, in zope.traversing.browser.absoluteurl there was a change a while back that breaks my code, now that I'm upgrading to it. The code in the absolute URL generation code used to be this: container = getattr(context, '__parent__', None) This simply gets the __parent__ of the context so that it can ask for its absolute URL. If no container can be found the answer will be None and the system will raise a TypeError, complaining about insufficient context. The code was then changed to this: context = ILocation(context, context) container = getattr(context, '__parent__', None) While this code looks initially confusing, what it does is try to adapt context to ILocation, but if it fails, falls back to 'context' to look up parent, preserving backwards compatibility but adding the feature that people can register custom ILocation adapters for models. But then, in dependency work by Christian Theune, the code got changed to this: context = ILocation(context) container = getattr(context, '__parent__', None) In addition, in zope.location an adapter got added for everything (Interface) that creates a LocationProxy. This means that the ILocation adaptation *always* succeeds, and might return a location proxy with __parent__ and __name__ set to None. This rather confused me initially, as this is one of those magic transparent proxies (except for __parent__ and __name__!). 'context' has a __parent__ *until* the point where ILocation() is called on it, and then the proxied context suddenly loses it __parent__! This broke the behavior of code that relied on __parent__ being checked first. I realize that this was depending implicit behavior, as models don't *really* provide ILocation. But on the other hand working code is broken in a really obscure way. The fix in my code was to make my model provide ILocation itself, so that it would adapt itself and therefore avoid the proxy. I think that might be correct but at the same time is really obscure; in my code the __parent__ is provided by a completely different subsystem (traject). Models that didn't provide ILocation used to work, and now they fail. Do we really want to require everybody to start providing ILocation in all their models? I propose the following adjustment: try: container = context.__parent__ except AttributeError: container = ILocation(context).__parent__ So, try to find the __parent__ the old way first, without adapter lookup. If that fails because the attribute is missing, look up the adapter. That might get one the proxy again, but at least not when a __parent__ is actually available. Still, this deserves some commenting for future people trying to debug what's going on. I've verified that at least in my application this unbreaks the code. I'll also try to add a test that demonstrates the expected behaviors so that this doesn't get broken again. Opinions? Regards, Martijn P.S. I want a publisher with location support but without proxy magic. :) ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
On Thu, Jul 8, 2010 at 5:04 PM, Martijn Faassen wrote: > The fix in my code was to make my model provide ILocation itself, so > that it would adapt itself and therefore avoid the proxy. I think that > might be correct but at the same time is really obscure; in my code the > __parent__ is provided by a completely different subsystem (traject). > Models that didn't provide ILocation used to work, and now they fail. Do > we really want to require everybody to start providing ILocation in all > their models? > > I propose the following adjustment: > > try: > container = context.__parent__ > except AttributeError: > container = ILocation(context).__parent__ > > So, try to find the __parent__ the old way first, without adapter > lookup. If that fails because the attribute is missing, look up the > adapter. That might get one the proxy again, but at least not when a > __parent__ is actually available. Still, this deserves some commenting > for future people trying to debug what's going on. > > I've verified that at least in my application this unbreaks the code. > I'll also try to add a test that demonstrates the expected behaviors so > that this doesn't get broken again. > > Opinions? I think this sounds reasonable. In Zope 2 Acquisition wrappers provide __parent__ pointers, but they don't implement ILocation either. I think the presence of the __parent__ pointer is the established "protocol", independent of the ILocation interface. Hanno P.S. Acquisition wrappers don't have interfaces of their own, but defer all lookups to the wrapped object, so letting them provide ILocation without changing the model classes isn't easily possible. ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
On Jul 8, 2010, at 11:04 AM, Martijn Faassen wrote: > > I propose the following adjustment: > > try: > container = context.__parent__ > except AttributeError: > container = ILocation(context).__parent__ +1 Gary ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
Hi all
> Betreff: Re: [Zope-dev] zope.traversing's ILocation behavior
>
>
> On Jul 8, 2010, at 11:04 AM, Martijn Faassen wrote:
> >
> > I propose the following adjustment:
> >
> > try:
> > container = context.__parent__
> > except AttributeError:
> > container = ILocation(context).__parent__
I'm fine with that too. But this will skip the __conform__
lookup, right?
See: zope/interface/interfaces.py line: 167
class InterfaceBasePy(object):
"""Base class that wants to be replaced with a C base :)
"""
def __call__(self, obj, alternate=_marker):
"""Adapt an object to the interface
"""
conform = getattr(obj, '__conform__', None)
if conform is not None:
adapter = self._call_conform(conform)
if adapter is not None:
return adapter
adapter = self.__adapt__(obj)
if adapter is not None:
return adapter
elif alternate is not _marker:
return alternate
else:
raise TypeError("Could not adapt", obj, self)
But anyway, also the initial implementation which we
used a long time whould skip such __conform__ call.
In my point of view is the usage of the absoluteURL
method an explicit pypass of the ILocation adaption
and an improved speedup lookup based on the directly
__parent__ attribute access. Probably we should add
a comment about that in the absoluteURL method.
Regards
Roger Ineichen
___
Zope-Dev maillist - Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
https://mail.zope.org/mailman/listinfo/zope-announce
https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
Hey, [zope.traversing absolute URL behavior] I've applied the fix to zope.traversing and added a test as well. It's released as zope.traversing. I've also updated the ZTK to use this version. (initially I used checkversions to update a few other packages too, namely zope.dublincore, zope.index, zope.pagetemplate and zope.testing, but this *did* result in zopeapp breakages, so I reverted that quickly. Someone needs to look into this to see what can be upgraded and what causes the break later) Regards, Martijn ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
On Fri, Jul 9, 2010 at 2:03 PM, Martijn Faassen wrote: > [zope.traversing absolute URL behavior] > > I've applied the fix to zope.traversing and added a test as well. It's > released as zope.traversing. I've also updated the ZTK to use this version. Thanks! > (initially I used checkversions to update a few other packages too, > namely zope.dublincore, zope.index, zope.pagetemplate and zope.testing, > but this *did* result in zopeapp breakages, so I reverted that quickly. > Someone needs to look into this to see what can be upgraded and what > causes the break later) I think zope.dublincore causes the breakage in some zope.app packages. zope.index and zope.pagetemplate should be upgraded. zope.testing 3.9.5 will throw deprecation warnings for not using zope.testrunner. The release team decided to keep the current zope.testing / zc.testrunner recipe for the ZTK 1.0 release, so we stick to zope.testing 3.9.4 for the time being. Hanno ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] zope.traversing's ILocation behavior
On Fri, Jul 9, 2010 at 9:08 AM, Hanno Schlichting wrote: > I think zope.dublincore causes the breakage in some zope.app packages. This one's a sticking point for a large application here as well (one where we're trying to move to the ZTK as the base). I think I'm sticking to zope.dublincore 3.6.0 on that for now. -Fred -- Fred L. Drake, Jr. "A storm broke loose in my mind." --Albert Einstein ___ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
