Hi everyone

There are two @CommitAfters and both work differently from each other.

Hibernate's @CommitAfter.

Here is the advisor

http://tapestry.apache.org/current/apidocs/src-html/org/apache/tapestry5/internal/hibernate/HibernateTransactionAdvisorImpl.html#line.26

It just starts a transaction before the method is invoked and commits/rollbacks 
at the end of the invocation. So nested transactions will work here.  but the 
problems that Martin mentioned will be there. Dimitry is right to point out 
that it won't be a problem as long as the service implementation is calling its 
own methods(annotated with @CommitAfter in the interface). 

JPA's @CommitAfter

Here is the advice

http://tapestry.apache.org/current/apidocs/src-html/org/apache/tapestry5/internal/jpa/CommitAfterMethodAdvice.html

I may be wrong but I think this one has a flaw. It doesn't start a transaction 
if one exists but commits. so it won't work with nested transactions. if we 
have A() calling B() where A and B are methods annotated with @CommitAfter 
belonging to two different services, A with start a transaction and B wont 
start another as A has already started but will commit it at the end of the 
method invocation. 

Now there are some issues.

1. We should have common implementation for both
2. It should be properly documented.
3. We have to choose between whether we want to implement @CommitAfter in a way 
so that only one transaction takes place even in nested methods or we want a 
beginTransaction() and commit()/rollback() for each call like hibernate's 
implementation does. In any case JPA's implementation needs fixing.


I am moving the discussion to development.

regards
Taha



On Aug 23, 2013, at 9:05 PM, Martin Kersten wrote:

> http://apache-tapestry-mailing-list-archives.1045711.n5.nabble.com/Transactions-and-AfterCommit-td5722996.html
> 
> If you look at this solution, I also ask myself why there is no direct
> support. I can not believe that this is not needed in any way. Why should I
> go for spring or fuddle with my code to just be able to mark something as
> transactional and maybe do something like read-only transaction in
> hibernate?
> 
> This would save my day.
> 
> As far as I remember I barely saw someone using something different than
> hibernate (or jdbc) when it comes to sql. So if tapestry would support
> Hibernate's transactional capabilities (and be quite fair all we need are
> nested transaction awareness (well its not a true nested transaction just a
> nested annotation of CommitAfter, nested transactions are a bit more
> complicated and are supported nowhere :)) and ReadOnly transactions thats
> all).
> 
> An Idea:
> @Transaction
> and
> @ReadOnly @Transaction
> 
> Transaction works like CommitAfter and ReadOnly is just a marker for the
> transaction to begin a read only transaction. That's it. It shouldnt be
> hard to support this.
> 
> Maybe ReadOnly might be renamed like @ReadOnlyHibernateTransaction and
> @HibernateTransaction. Who knows.
> 
> What do you think. Anyone likes this or not? Or is there something similar
> already present. Would love to know.
> 
> I dont need JTA transactions at all.
> 
> Also ReadOnlyHibernateTransaction can be implented as getting the session
> and set the connection to read only and session to FLUSH_NEVER.
> 
> In case of a Master/Slave replication how does tapestry's current solution
> work in terms of ReadOnly transactions or is everything annotated with
> CommitAfter is automatically hitting the master all the time? What happens
> with the session interaction that is not annotated with @CommitAfter. Is it
> executed statement by statement (autocommit?) and are those hitting always
> the master too?
> 
> 
> 2013/8/23 Martin Kersten <[email protected]>
> 
>> By the way one might rename it from isNested to isNestedOrNoTransaction.
>> Same behavior more precise in the naming
>> 
>> 
>> 2013/8/23 Martin Kersten <[email protected]>
>> 
>>>>>> 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 <[email protected]>
>>> 
>>>> On Fri, 23 Aug 2013 10:45:27 -0300, Martin Kersten <
>>>> [email protected]> 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<[email protected]>
>>>> For additional commands, e-mail: [email protected]
>>>> 
>>>> 
>>> 
>> 

Reply via email to