Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHH-18867 correctly implement the documented semantics of beginTransaction() #9287

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ public interface SharedSessionContract extends QueryProducer, AutoCloseable, Ser
* Begin a unit of work and return the associated {@link Transaction} object.
* If a new underlying transaction is required, begin the transaction. Otherwise,
* continue the new work in the context of the existing underlying transaction.
* <p>
* The JPA-standard way to begin a new transaction is by calling
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are calling out JPA-specific expectations here, might also be worth noting here that JPA disallows this in JTA environments and we also honor that with strict compliance.

* {@link #getTransaction getTransaction().begin()}. When
* {@linkplain org.hibernate.jpa.spi.JpaCompliance#isJpaTransactionComplianceEnabled
* strict JPA transaction compliance} is enabled via, for example, setting
* {@value org.hibernate.cfg.JpaComplianceSettings#JPA_TRANSACTION_COMPLIANCE},
* or when resource-local transactions are used, the call to {@code begin()}
* fails if the transaction is already {@linkplain Transaction#isActive active}.
* On the other hand, this method does not fail when a transaction is already
* active, and simply returns the {@link Transaction} object representing the
* active transaction.
*
* @return an instance of {@link Transaction} representing the new transaction
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,9 +606,13 @@ public CacheTransactionSynchronization getCacheTransactionSynchronization() {
@Override
public Transaction beginTransaction() {
checkOpen();
final Transaction result = getTransaction();
result.begin();
return result;
final Transaction transaction = getTransaction();
// only need to begin a transaction if it was not
// already active (this is the documented semantics)
if ( !transaction.isActive() ) {
transaction.begin();
}
return transaction;
}

protected void checkTransactionSynchStatus() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ public void anIllegalStateExceptionShouldBeThrownWhenBeginTxIsCalledWithAnAlread
Transaction tx = null;
try {
// A call to begin() with an active Tx should cause an IllegalStateException
tx = s.beginTransaction();
tx = s.getTransaction();
tx.begin();
}
catch (Exception e) {
if ( tx != null && tx.isActive() ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public void setUp() {
public void noIllegalStateExceptionShouldBeThrownWhenBeginTxIsCalledWithAnAlreadyActiveTx() throws Exception {
tm.begin();
try (Session s = openSession()) {
Transaction tx = s.beginTransaction();
Transaction tx = s.getTransaction();
tx.begin();
try {
s.persist( new TestEntity( "ABC" ) );
tx.commit();
Expand Down
Loading