[xwiki-dev] Re: Help with code review of XWikiHibernateVersioningStore

[Ludovic Dubost]

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 at objectweb.org mailing list.
> To unsubscribe: mailto:xwiki-dev-unsubscribe at objectweb.org
> For general help: mailto:sympa at 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