[Zope-dev] ZPublisher doesn't commit transaction if target not callable

[Phillip J. Eby]

This isn't necessarily a Zope bug, in the sense that it affects anything in
Zope proper at the present time, to my knowledge.  However, it could be
under certain circumstances, and it does affect ZPublisher-based code I'm
currently working on.

ZPublisher.Publish begins a transaction right before determining whether
the to-be-published object is callable.  If in its determination the object
is *not* callable, it falls through to a return *without committing the
transaction*.  This makes two rather questionable assumptions: first, that
the 'transaction' object can handle unbalanced begin/commit operations, and
second, that the response.setBody() call (and therefore the object's
asHTML() method) has no side-effects.  The first may hold true for ZODB,
and the second may hold true for DC-written objects, but both are probably
bad assumptions for ZPublisher as a whole.

Another, similar assumption present is that the object traversal process is
side-effect free and need not be part of the overall transaction.  If I
have a situation where an object does (for example) logging of certain
sub-object accesses to a relational DB from a __bobo_traverse__ method, it
will not get wrapped in a transaction, and will not be rolled back if an
error occurs later.  I currently work around this by using a
__bobo_before__ that ensures a transaction is started, and having my
transaction object ignore ZPublisher's .begin() call.  Unfortunately, there
is no similar workaround for the asHTML issue, since at the time of a
__bobo_after__ call, there is no way to tell whether the transaction
should've been committed or aborted.

May I suggest changing the relevant code (return response.setBody(object)) to:

t=response.setBody(object)
if transaction: transaction.commit()
return t

This would at least fix the mismatched begin/commits.  Ideally, I'd like to
see the transaction begun at the very beginning of the publishing
traversal, but as I said, I have an existing workaround for that.  Thanks.

  _____
   _/__)
___/