[#228, #218] Introduce TransactionController#229
[#228, #218] Introduce TransactionController#229gavinking wants to merge 3 commits intojakartaee:masterfrom
Conversation
|
What do you think about my comment in #228 on making this a subinterface of |
| /** | ||
| * Run the given {@linkplain Runnable operation} in a transaction. | ||
| * If there is no active transaction associated with the current | ||
| * thread, {@linkplain #begin} a new transaction before running | ||
| * the operation. If the operation completes without throwing an | ||
| * exception, {@linkplain #commit} the transaction. If the | ||
| * operation does throw an exception, {@linkplain #rollback} the | ||
| * transaction and rethrow the exception. Otherwise, if there is | ||
| * already an active transaction associated with the current | ||
| * thread, simply call the operation. | ||
| * | ||
| * @param work Some work to be performed in a transaction | ||
| * | ||
| * @exception IllegalStateException Thrown if the current | ||
| * {@linkplain #getStatus status} is anything other than | ||
| * {@link Status#STATUS_ACTIVE STATUS_ACTIVE} or | ||
| * {@link Status#STATUS_MARKED_ROLLBACK STATUS_MARKED_ROLLBACK} | ||
| * or if committing the transaction fails. | ||
| */ | ||
| void runInTransaction(Runnable work); | ||
|
|
||
| /** | ||
| * Call the given {@linkplain Supplier supplier} in a transaction. | ||
| * If there is no active transaction associated with the current | ||
| * thread, {@linkplain #begin} a new transaction before running | ||
| * the operation. If the operation completes without throwing an | ||
| * exception, {@linkplain #commit} the transaction and return the | ||
| * value produced by the supplier. If the operation does throw an | ||
| * exception, {@linkplain #rollback} the transaction and rethrow | ||
| * the exception. Otherwise, if there is already an active | ||
| * transaction associated with the current thread, simply call the | ||
| * supplier and return the value it produces. | ||
| * | ||
| * @param work A supplier to be called in a transaction | ||
| * @return The value returned by the given supplier | ||
| * @param <R> The type of the value returned by the supplier | ||
| * | ||
| * @exception IllegalStateException Thrown if the current | ||
| * {@linkplain #getStatus status} is anything other than | ||
| * {@link Status#STATUS_ACTIVE STATUS_ACTIVE} or | ||
| * {@link Status#STATUS_MARKED_ROLLBACK STATUS_MARKED_ROLLBACK} | ||
| * or if committing the transaction fails. | ||
| */ | ||
| <R> R callInTransaction(Supplier<R> work); |
There was a problem hiding this comment.
If we didn't go with TransactionController, I guess these could go into UserTransaction (if the Jakarta Transactions community decide these are the right methods to add to solve #218 - do I follow correctly?
There was a problem hiding this comment.
The highlighting was not showing even if the lines are listed so for the sake of being clear, I am talking about runInTransaction and callInTransaction
There was a problem hiding this comment.
I guess these could go into UserTransaction
Yes. Correct.
| * @exception IllegalStateException Thrown if the current | ||
| * {@linkplain #getStatus status} is anything other than | ||
| * {@link Status#STATUS_ACTIVE STATUS_ACTIVE} or | ||
| * {@link Status#STATUS_MARKED_ROLLBACK STATUS_MARKED_ROLLBACK} | ||
| * or if committing the transaction fails. |
There was a problem hiding this comment.
We might want to be a bit more prescriptive here and say:
| * @exception IllegalStateException Thrown if the current | |
| * {@linkplain #getStatus status} is anything other than | |
| * {@link Status#STATUS_ACTIVE STATUS_ACTIVE} or | |
| * {@link Status#STATUS_MARKED_ROLLBACK STATUS_MARKED_ROLLBACK} | |
| * or if committing the transaction fails. | |
| * @exception IllegalStateException Thrown if the current | |
| * {@linkplain #getStatus status} is anything other than | |
| * {@link Status#STATUS_ACTIVE STATUS_ACTIVE} or | |
| * {@link Status#STATUS_MARKED_ROLLBACK STATUS_MARKED_ROLLBACK} | |
| * or if committing the transaction fails. | |
| * In the case of a rollback or heuristic decision, the thrown | |
| * {@link IllegalStateException} must contain a wrapped | |
| * {@linkplain Throwable#getCause cause} of type | |
| * {@link RollbackException}, {@link HeuristicMixedException}, or | |
| * {@link HeuristicRollbackException}, as appropriate. |
|
Can one of the admins verify this patch? |
|
ok to test |
|
Just to mention (though let's try to get agreement on the API choice first, if possible) that for a change like this we will need spec text and TCK test(s) for it. But let's agree the design first. |
Yes of course. And there is also impact on the platform spec. |
inspired by Quarkus see discussion in jakartaee#228
| * | ||
| * @since Jakarta Transactions 2.1 | ||
| */ | ||
| public interface TransactionController { |
There was a problem hiding this comment.
Given we are in API code without the TCK incorporated in the project I am not really sure the best way to do this but if there was a test (maybe the basis of one in a TCK fork) it would help to be able to understand how the API looks in use (even given that there is no implementation today). For example, I think I like the exception handler being separate but I am not certain. I can work to understand the code snippets shared but having something in compilable code would seem useful to me (it might even be worth just adding a simple src/test/java folder to our project and even maybe using mock objects just to see the API in "use"). Anyway just sharing that idea, there isn't something definitive I would request right now.
| */ | ||
| @FunctionalInterface | ||
| interface Runnable { | ||
| void run(RunningTransaction transaction) |
There was a problem hiding this comment.
How do we obtain a reference to a RunningTransaction?
4 participants
See #228 and #218.