By the way one might rename it from isNested to isNestedOrNoTransaction.
Same behavior more precise in the naming
2013/8/23 Martin Kersten <martin.kersten...@gmail.com>
> >>> I disagree. @CommitAfter never claimed to implement nested
> transactions, so, if someone expects it with @CommitAfter, they're wrong,
> not @CommitAfter. I agree that its documentation should be more explicit.
> <<<
>
> We had this conversation in 2008 already, if I remember correctly. Same
> problem, same question. The problem is that this is so easy to run into
> such a bug. And you can consider it as being a broken feature. I am quite
> sure if we check all of our projects there should be at least one of us
> having this problem introduced just by accident (I did). I just wanted to
> store a message in the system using it inside an administration service
> doing something else too. So this is simple to run into it.
>
>
> So lets solve this once and for all and look into the code:
>
>
> current version:
>
> public void advise(final MethodInvocation invocation)
> {
> final EntityTransaction transaction = getTransaction();
>
> if (transaction != null && !transaction.isActive())
> {
> transaction.begin();
> }
>
> try
> {
> invocation.proceed();
> } catch (final RuntimeException e)
> {
> if (transaction != null && transaction.isActive())
> {
> rollbackTransaction(transaction);
> }
>
> throw e;
> }
>
> // Success or checked exception:
>
> if (transaction != null && transaction.isActive())
> {
> transaction.commit();
> }
>
> }
>
> New version:
>
> public void advise(final MethodInvocation invocation)
> {
> final EntityTransaction transaction = getTransaction();
>
> boolean isNested = false; //<-- Change
>
> if (transaction != null && !transaction.isActive())
> {
> transaction.begin();
> }
> else
> isNested = true; //<-- Change
>
> try
> {
> invocation.proceed();
> } catch (final RuntimeException e)
> {
> if (transaction != null && transaction.isActive() &&
> !isNested) //<-- Change
> {
> rollbackTransaction(transaction);
> }
>
> throw e;
> }
>
> // Success or checked exception:
>
> if (transaction != null && transaction.isActive() && !isNested)
> //<-- Change
> {
> transaction.commit();
> }
>
> }
>
> So please come on. Lets get rid of this bug! Thats all nothing more to
> see... .
> And its totally fools proof and downward compatible (doesnt change current
> behavior).
>
>
> Lets change this! And yes we can!
>
>
>
> Cheeeers,
>
> Martin (Kersten),
> Germany
>
>
> 2013/8/23 Thiago H de Paula Figueiredo <thiag...@gmail.com>
>
>> On Fri, 23 Aug 2013 10:45:27 -0300, Martin Kersten <
>> martin.kersten...@gmail.com> wrote:
>>
>> @Dimitry
>>> But using this annotation would introduce a good chance that later in
>>> your project you introduce bugs this way. So I would consider @CommitAfter
>>> to be a harmful feature. The problem with this transaction behavior it
>>> introduces bugs that might happen in so rare circumstances that you always
>>> have no
>>> chance to reproduce or spot it during debug/testing sessions.
>>>
>>
>> I disagree. @CommitAfter never claimed to implement nested transactions,
>> so, if someone expects it with @CommitAfter, they're wrong, not
>> @CommitAfter. I agree that its documentation should be more explicit.
>>
>>
>> Is there a reason why there is no @Transactional annotation dealing with
>>> this issue like the JPA version does? The link I mentioned earlier
>>> provided such implementation. Would be create if Tapestry 5.4 might ship
>>> along with this. In the end I would only need this behavior that if a
>>> transaction is
>>> present the 'inner transaction' would be ignored and only the outter
>>> transaction remains handled. So if transaction exists, do nothing.
>>>
>>
>> Because implementing full transaction support, like Spring and EJB do, is
>> a very complex to do. Have you ever taken a look at spring-tx code? I did.
>> It's quite large. Tapestry-IoC never promised to implement a full
>> transaction manager. Just tapestry-hibernate and tapestry-jpa that provide
>> simple transaction management because both Hibernate and JPA require
>> changes to the database to be done inside a transaction.
>>
>> --
>> Thiago H. de Paula Figueiredo
>>
>>
>> ------------------------------**------------------------------**---------
>> To unsubscribe, e-mail:
>> users-unsubscribe@tapestry.**apache.org<users-unsubscr...@tapestry.apache.org>
>> For additional commands, e-mail: users-h...@tapestry.apache.org
>>
>>
>
