txGenshi calls athena internal functions during rendering

Bug #502823 reported by Cory Dodt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
txGenshi
Fix Released
High
Cory Dodt

Bug Description

getObjectData avoids calling rendPageAttrs but will call athena LivePage attrs. Avoid doing so.

Related branches

Revision history for this message
Cory Dodt (corydodt) wrote :

Yipes, really ugly one here. I'd like a code review on this please, when done.

There's a bit of a wtf for me in the original code:

rendPageAttrs = [x[0] for x in getmembers(rend.Page)]

def getObjectData(obj):
...
        if callable(member):
            val = member()
...
    return data

Wow, really? We call every callable member of the class during rendering, regardless of whether the template refers to it? To fix it for athena I had to do this, for starters:

class MyLivePage(athena.LivePage):
    """
    this evil hack is needed to work around a bug in getmembers+z.i
    __provides__

    Using zope.interface, you use __provides__ to discuss interfaces provided
    by a class. However, see bug mentioned here: http://old.nabble.com/Problem-with-zope.interface-leaking-attributes-td25824576.html

    This causes getmembers to blow up on LivePage. So this class exists so
    that there is a class with the same members as LivePage, but which does
    *not* have a broken __provides__ attribute, and now we can (ta-DA) use
    getmembers on the subclass.
    """
    __provides__ = None

livePageAttrs = [x[0] for x in getmembers(MyLivePage)]

Then I'll also have to hide @renderer and @expose methods. But what about just.. application methods? They could do something with nasty side effects. Am I missing something here about how completely evil this isn't?

Revision history for this message
Cory Dodt (corydodt) wrote :

In-progress. Still need to make sure this works with @expose and @renderer methods to be congruent with what the current code does, but see discussion question above. Can we perhaps use a gentle wrapper that lazily calls the method when it's actually accessed by the template? (Ditto data members - any of which might be a descriptor with side effects, too.)

Changed in txgenshi:
status: New → In Progress
assignee: nobody → Cory Dodt (corydodt)
Revision history for this message
Duncan McGreggor (oubiwann) wrote :

Sorry to be anal about this, but could you do the same thing with this branch as for the other branches? Split out this work into it's own branch?

Thanks man!

Revision history for this message
Cory Dodt (corydodt) wrote :

done,

Revision history for this message
Cory Dodt (corydodt) wrote :

After reading through some of the Genshi doc, it doesn't look like there's any expectation that callable methods of the class will be called, since templates seem to always be invoked using kwargs to the Template.generate method. I had assumed you were doing something to be compatible with the Genshi philosophy or some specific high-level API. But I guess this ability to implicitly call callables is a txGenshi invention?

I vote that callables not be called; and that everything else be accessed through a proxy so that data attributes/properties are only dereferenced *if* used in the template.

Cory Dodt (corydodt)
Changed in txgenshi:
importance: Undecided → High
Cory Dodt (corydodt)
Changed in txgenshi:
status: In Progress → Fix Committed
Revision history for this message
Cory Dodt (corydodt) wrote :

Merged this change in. Using DataProxy and TxGenshiContext to replace getObjectData allows everything to be lazy-evaluated, at the moment that the template requires the reference. This also allows us to drop any special handling for Nevow/Athena attributes; it's fine that they are now available for use by the template, since they won't get accessed unless you specifically ask for them in the template.

Changed in txgenshi:
status: Fix Committed → Fix Released
Revision history for this message
Cory Dodt (corydodt) wrote :

Also, I kept special handling for callables: they will still get called when the template asks for them as a variable, e.g.

class FooPage(rend.Page):
    docFactory = genshifile('foo.xhtml')
    def date(self):
        return str(datetime.datetime())

(... in the template ...)

<div>$date</div>

will still produce <div>(current timestamp)</div>. Again, this is possible because lazy evaluation means never having to call things and get side effects.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.