Add EntityManagerFactory.runInTransaction()/callInTransaction()

[gavinking]

See #410 and #204.

This one is just a rough draft.

[dmatej]

Perhaps yet add a documentation "copy pasted" from the getTransaction method about the JTA:

     * @throws IllegalStateException if invoked on a JTA entity manager

[gavinking]

@dmatej do we really want to require implementations to do that though?

I'm not so sure about that.

[gavinking]

I've updated the PR with an overloaded version of withTransaction().

[Tomas-Kraus]

Hi Gavin, I like this PR. But it may be done a bit better. :) I was designing identical pattern for Oracle Helidon just few weeks ago. The only difference between my and yours proposal are 2 more methods. Check transaction methods in https://github.com/Tomas-Kraus/helidon/blob/helidon-data/data/data/src/main/java/io/helidon/data/HelidonData.java and POC implementation code in https://github.com/Tomas-Kraus/helidon/blob/helidon-data/data/jpa/runtime/src/main/java/io/helidon/data/jpa/runtime/JpaHelidonData.java The only difference between Callable and Function/Consumer is how the transaction is handled. Callable does it for you (rollback on any Throwable, commit otherwise), in Function/Consumer it's user response to handle the transaction.

Function/Consumer have apply/accpet methods without throwing anything. So exceptions must be handled in those methods and transaction handler must be available there to allow commit/rollback: public void withTransaction(BiConsumer<EntityManager, EntityTransaction> task) public <R> R withTransaction(BiFunction<EntityManager, EntityTransaction,R> task)

For automatic handling, we'll have to define our own functional interfaces, e.g.

    @FunctionalInterface
    interface VoidTransaction {
        /**
         * Executes the task. Throws an exception if unable to do so.
         *
         * @throws Exception when unable to compute a result
         */
        void call(EntityManager em) throws Exception;
    }

    @FunctionalInterface
    interface Transaction<R> {
        /**
         * Executes the task. Throws an exception if unable to do so.
         *
         * @throws Exception when unable to compute a result
         */
        R call(EntityManager em) throws Exception;
    }

where functions are throwing something (Exception may be changed to more common Throwable) and this something will be handled by JPA framework implementing code.

[lukasj]

would it make sense to make these additional methods default?

[Tomas-Kraus]

Here is sample I made to verify this change in Eclipselink: https://github.com/gavinking/jpa-api/pull/1 Edit: Added as PR to gavin's fork to make it properly. So default implementation may not be a problem.

[dmatej]

@dmatej do we really want to require implementations to do that though?

I'm not so sure about that.

Here is sample I made to verify this change in Eclipselink: https://github.com/Tomas-Kraus/jpa-api/blob/emf_transaction/api/src/main/java/jakarta/persistence/EntityManagerFactory.java#L227 So default implementation may not be a problem.

With JTA the call of em.getTransaction(); will throw IllegalStateException. I still think it should be documented.

Perhaps we should separate EntityManager to JtaEntityManager and JpaEntityManager or something like that in the future. There's already too many methods throwing ISE for JTA or methods usable just with non-JTA tx XOR with JTA tx.

[gavinking]

@Tomas-Kraus so the essential differences with your proposal are that:

1. we also accept work that throws a checked exception, and
2. we also give you the option of receiving a reference to the EntityTransaction in the work lambda?

Is that right?

I'm not sure what I thin about 2. If we're managing the transaction for you, what are you going to do with the reference to the EntityTransaction? It seems like you would not be allowed to call any of its methods?

[gavinking]

(P.S. Apologies for not noticing the comments on the PR. For some reason I did not get notified.)

[Tomas-Kraus]

@gavinking Looks like my link is broken, here is another one: https://github.com/Tomas-Kraus/jpa-api-gavin/blob/af4248c1cdbc12fc0f72d59d84795846485944b6/api/src/main/java/jakarta/persistence/EntityManagerFactory.java There is default implementation in the methods.

1. Transaction for TransactionVoidWork/TransactionWork is handled by JPA so there shall be checked exception (like SQLException) too and no user access to transaction handler is needed. Maybe catch (Throwable e) would be better in the implementation to rollback on any Throwable that may happen.

2. Transaction for BiConsumer/BiFunction is not handled by JPA, only begin() is called before running user code. that's why EntityTransaction instance is passed to the user. It's user's responsibility to do commit/rollback so JPA does not need to know about any checked exceptions. Also we may not pass EntityTransaction as is, but hidden behind some interface which gives access only to commit/rollback methods. But think that anything except begin may be useful so I'm passing it without any limitations.

I added both options because there are use cases when users would like to have control over the transaction.

[gavinking]

Add a definition also for JTA EMs to handle:

• no transaction
• existing transaction

I have updated the proposal with this and other fixes. Please take a look.

[Tomas-Kraus]

I had some conversation with Lukas about his reviews while I was on vacation so I'm synced with current status. It looks good now.

[gavinking]

Awesome, thanks @Tomas-Kraus!