ZPublisher doesn't commit transaction if target not callable
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. _____ _/__) ___/
"Phillip J. Eby" wrote:
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.
The points you make above are entirely valid. In fact, I paid attention the first time you brought them up. ;) I took this into account when ZPublisher was reorganized some time ago (back in February). While 1.10 doesn't reflect this change, 1.11 did and 2.0 does. Jim -- Jim Fulton mailto:jim@digicool.com Python Powered! Technical Director (888) 344-4332 http://www.python.org Digital Creations http://www.digicool.com http://www.zope.org Under US Code Title 47, Sec.227(b)(1)(C), Sec.227(a)(2)(B) This email address may not be added to any commercial mail list with out my permission. Violation of my privacy with advertising or SPAM will result in a suit for a MINIMUM of $500 damages/incident, $1500 for repeats.
At 10:17 AM 6/14/99 -0400, Jim Fulton wrote:
The points you make above are entirely valid. In fact, I paid attention the first time you brought them up. ;) I took this into account when ZPublisher was reorganized some time ago (back in February). While 1.10 doesn't reflect this change, 1.11 did and 2.0 does.
Oops. Guess I should check CVSWeb before making comments like this one in future. :) Actually, I only brought up the second one before (the "late begin" problem); today was the first I noticed the "non-commit" issue. But I notice they are both fixed. Thanks! By the way, great decomposition. Looks like you split out nearly everything I could ever want to call or subclass. I'm going to go download the whole thing and print it all out to study, as it looks like I can get rid of some of the other workarounds/kludges I'm doing by just using a couple of custom routines/subclasses.
It looks like a (possibly inadvertent) change was made to the semantics of BASE1-BASE9 as of the May 18th version of ZPublisher/HTTPRequest.py. To compare examples... Older ZPublisher values ----------------------- URL0 http://telecommunity.com/ATEST/REQUEST URL1 http://telecommunity.com/ATEST URL2 http://telecommunity.com BASE0 http://telecommunity.com BASE1 http://telecommunity.com/ATEST BASE2 http://telecommunity.com/ATEST/REQUEST Newer ZPublisher values ----------------------- URL0 http://teams.internal.rapidsite.net/ATEST/REQUEST URL1 http://teams.internal.rapidsite.net/ATEST URL2 http://teams.internal.rapidsite.net BASE0 http://teams.internal.rapidsite.net BASE1 http://teams.internal.rapidsite.net/ATEST/REQUEST BASE2 undefined This looks to me like an error, although it may not affect Zope or ZServer. Or is this an intended change in semantics? Looks to me like it would break existing DTML. I suspect this is an unintentional side effect of other changes in that version. Here's a diff that restores the old semantics: diff -u -r1.6 HTTPRequest.py --- HTTPRequest.py 1999/05/25 12:58:07 1.6 +++ HTTPRequest.py 1999/06/14 19:09:44 @@ -426,11 +426,11 @@ if key[:1]=='B' and BASEmatch(key) >= 0: n=ord(key[4])-ord('0') if n: - if len(self.steps) < n: + if len(self.steps) < n-1: raise KeyError, key v=self.script while v[-1:]=='/': v=v[:-1] - v=join([v]+self.steps[:n],'/') + v=join([v]+self.steps[:n-1],'/') else: v=self.base while v[-1:]=='/': v=v[:-1]
participants (2)
-
Jim Fulton -
Phillip J. Eby