[Product-Developers] Re: [Plone-developers] plone.app.layout ViewletBase and portal_url
wichert at wiggy.net
Sat Mar 8 14:49:37 UTC 2008
Previously Martin Aspeli wrote:
> Wichert Akkerman wrote:
> > Previously Martin Aspeli wrote:
> >> Wichert Akkerman wrote:
> >>> I'm debugging some code that broke in a very unexpected way:
> >>> (Pdb) p getToolByName(self, 'portal_url')
> >>> 'http://localhost:8080/Plone'
> >>> That should return a tool, not a string. This turned out to be caused by
> >>> plone.app.layout.viewlets.common.ViewletBase.update which sets
> >>> self.portal_url to the portal URL. This looks harmless, but due to
> >>> acquisition this will break patterns like this:
> >>> class MyViewlet(ViewletBase):
> >>> def update(self):
> >>> super(MyViewlet, self).update()
> >>> self.mt=getToolByName(self.context, "portal_membership")
> >>> self.somemethod()
> >>> def somemethod(self):
> >>> return self.mt.ToolMethod()
> >>> the problem here is that in somemethod() self.mt gets acquisition wrapped in
> >>> the viewlet, so when ToolMethod does getToolByName(self, "portal_url") it
> >>> finds the portal_url variable ViewletBase.update set on the viewlet instead
> >>> of the tool.
> >> Ouch!
> >> This is precisely the reason that I *always* do (and advocate):
> >> context = aq_inner(self.context)
> >> at the top of any method in a view or viewlet, and then use the
> >> 'context' variable throughout the method instead of self.context.
> > That would still have broken: self.mt will give you the membership tool
> > wrapped in the view, so a getToolByName call inside the membership tool
> > would get the wrong result.
> That's true. I also never assign instance variables in update() like
> that, I tend to use memoized methods if necessary.
> >>> I would like to rename that variable in ViewletBase to prevent this problem
> >>> but I do not have a good overview of how many people are relying on that
> >>> specific variable to be set. If you are relying on this please let me know
> >>> ASAP. If I here no objection I intend to rename the variable for the
> >>> upcoming Plone 3.1 alpha release.
> >> I'm not sure we can do this. It's used in templates (view/portal_url)
> >> that are likely to have been customised in several places. :-/
> > Hmm. Perhaps we can add a zope.deprecate wrapped portal_url in
> > ViewletBase to warn people?
> Perhaps, yeah.
> On the other hand, why is it a problem if you do this in your code?
> def foo(self):
> context = aq_inner(self.context)
> portal_url = getToolByName(context, 'portal_url')
getToolByName always looks at the inner acquisition chain so you do not
need aq_inner here. (You may need it for a later section of the code.)
> That is, don't try to acquire it anywhere, just look it up and use it
> off the aq-inner'd context? If the template does view/portal_url then
> that's fine - it gets the string.
That still would not have solved my specific issue: the getToolByName
call to get portal_url was outside my control but an implementation
detail of other code I called. Due to acquisitions nature of inserting
itself everywhere that magically broke. You can of course add lots of
aq_inners everywhere and pray things work, but I don't feel that is
the right approach.
Wichert Akkerman <wichert at wiggy.net> It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.
More information about the Product-Developers