1/ If we are outside of a transaction this actually means we are inside a higher level call from another store function. This function would already have called checkHibernate. This is an expensive function. We don't want it to be called multiple times. 2/ getSession should not be called. It's probably a leftover avec a refactoring. 3/ The first endTransaction is a "commit" The second in the finally close is a "rollback" True a variable would work and it would be more clear.. 4/ Yes PS: It's my code. I was learning hibernate ! Vincent Massol a écrit :

On Feb 20, 2007, at 10:17 AM, Vincent Massol wrote:

...

On Feb 20, 2007, at 10:02 AM, Vincent Massol wrote:

...

Hi,

I'd like to clean a bit the XWikiHibernateVersioningStore class but I'd like to confirm my changes as I don't want to break anything. There are several methods like this one:

public void updateXWikiDocArchive(XWikiDocument doc, String text, boolean bTransaction, XWikiContext context) throws XWikiException { try { if (bTransaction) { checkHibernate(context); bTransaction = beginTransaction(false, context); } Session session = getSession(context);

XWikiDocumentArchive archivedoc = getXWikiDocumentArchive(doc, context); archivedoc.updateArchive(doc.getFullName(), text); saveXWikiDocArchive(archivedoc, bTransaction, context); if (bTransaction) { endTransaction(context, true, false); } } catch (Exception e) { Object[] args = { doc.getFullName() }; throw new XWikiException( XWikiException.MODULE_XWIKI_STORE, XWikiException.ERROR_XWIKI_STORE_HIBERNATE_SAVING_OBJECT, "Exception while updating archive {0}", e, args); } finally { try { if (bTransaction) endTransaction(context, false, false); } catch (Exception e) {} } }

1) Why is checkHibernate done only when the operation is done in a transaction? 2) Why is getSession called as the session object is not used? 3) I think the endTransaction is called twice... 4) Shouldn't we log something if the endTransaction fails?

Thus I'd like to:

a) move the checkHibernate outside of the if(bTransaction) b) remove getSession call c) remove the first endTransaction and leave the one in the finally clause

Ah just noticed the 2 endTransactions are not called with the same parameters. Is that something wanted?

ok after more reading, I think I understand why the 2 are there (although they are very confusing). The second one is always executed but does nothing in practice if the first endTransaction has executed as it has reset the transaction... If the transaction has not been completed the second endTransaction rollbacks it.

pfew.... that's really not nice coding... :)

Thanks -Vincent

___________________________________________________________________________Yahoo! Mail réinvente le mail ! Découvrez le nouveau Yahoo! Mail et son interface révolutionnaire. http://fr.mail.yahoo.com

------------------------------------------------------------------------

-- You receive this message as a subscriber of the xwiki-dev@objectweb.org mailing list. To unsubscribe: mailto:xwiki-dev-unsubscribe@objectweb.org For general help: mailto:sympa@objectweb.org?subject=help ObjectWeb mailing lists service home page: http://www.objectweb.org/wws

-- Ludovic Dubost Blog: http://www.ludovic.org/blog/ XWiki: http://www.xwiki.com Skype: ldubost GTalk: ldubost AIM: nvludo Yahoo: ludovic