TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
Hi Everyone, Romain, As JIRA is down, I'm writing to the mailing list. Recently I reported TOMEE-2057 -> modification to entities done in the @BeforeCompletion callback were not getting persisted. The root cause was that eclipselink's synchronization was executed before openejb's one. The same ca

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
Hi Svetlin, why not relying on the default? javax.transaction.Transaction.class.cast(txn).registerSynchronization(listener)? This is what your code does except it goes through the registry to find the current transaction instead of using the one already passed and bound to the entity manager - sh

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
> why not relying on the default? javax.transaction.Transaction.class.cast(txn). registerSynchronization(listener Because it registers the transaction in the "regular" synchronizations list. But if the synchronization is registered via the TransactionSynchronizationRegistry, the synchronization wi

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
2017-06-12 10:29 GMT+02:00 Svetlin Zarev : > > why not relying on the > default? javax.transaction.Transaction.class.cast(txn). > registerSynchronization(listener > > Because it registers the transaction in the "regular" synchronizations > list. But > if the synchronization is registered via the >

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
I didn't think of that, but still I prefer to do it via the proper interface, because javax.transaction.Transaction does not expose the interposed list. So it would be transaction-manager implementation dependent and it might stop working if geronimo changes its impl. But I can modify my PR to use

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
2017-06-12 10:42 GMT+02:00 Svetlin Zarev : > I didn't think of that, but still I prefer to do it via the proper > interface, > because javax.transaction.Transaction does not expose the interposed list. > So it would be transaction-manager implementation dependent and it might > stop working if ger

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
OK, I'll update my PR. I want to write a test as well. In which project should I add it, so it's executed with both OpenJPA & Eclipse link ? 2017-06-12 11:43 GMT+03:00 Romain Manni-Bucau : > 2017-06-12 10:42 GMT+02:00 Svetlin Zarev >: > > > I didn't think of that, but still I prefer to do it v

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
2017-06-12 11:02 GMT+02:00 Svetlin Zarev : > OK, I'll update my PR. > > I want to write a test as well. In which project > should I add it, so it's executed with both > OpenJPA & Eclipse link ? > > you have a few options here: 1. openejb-core 2. add an example in examples/ (we use them as itest s

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
I've added Arquilian test in arquillian-tomee-webprofile-tests. It passes on my machine :) but I'm pretty sure it runs only with OpenJPA. How can I force it to run with eclipselink ? https://github.com/apache/tomee/pull/70 2017-06-12 12:04 GMT+03:00 Romain Manni-Bucau : > 2017-06-12 11:02 GMT+02

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
if you run it with the profile all-adapters from maven command line it will run with all tomee flavors (embedded, webprofile, plus, plume) so plume run will check with eclipselinks. Romain Manni-Bucau @rmannibucau | Blog |

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
It passed with " all-adapters" as well :) 2017-06-12 16:37 GMT+03:00 Romain Manni-Bucau : > if you run it with the profile all-adapters from maven command line it will > run with all tomee flavors (embedded, webprofile, plus, plume) so plume run > will check with eclipselinks. > > > Romain Manni-

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Romain Manni-Bucau
:), so looks like you did it ;). Thanks a lot! Really appreciated the effort to test it. Romain Manni-Bucau @rmannibucau | Blog | Old Blog | Github | Linke