From 773b2f06a10679979ee747ad2ee2182eaafcc8cd Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Oct 2019 00:25:17 +0100 Subject: [PATCH] Avoid Connection.isReadOnly() call in resetConnectionAfterTransaction Closes gh-23747 --- .../DataSourceTransactionManager.java | 4 +- .../jdbc/datasource/DataSourceUtils.java | 42 ++++++++++++++++++ .../JdbcTransactionObjectSupport.java | 43 ++++++++++++++++++- .../DataSourceTransactionManagerTests.java | 7 +-- .../HibernateTransactionManager.java | 6 ++- .../orm/jpa/JpaTransactionManager.java | 3 +- .../orm/jpa/vendor/HibernateJpaDialect.java | 15 ++++--- 7 files changed, 107 insertions(+), 13 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java index ad60d673ed3..e7a66c53d1e 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java @@ -272,6 +272,7 @@ public class DataSourceTransactionManager extends AbstractPlatformTransactionMan Integer previousIsolationLevel = DataSourceUtils.prepareConnectionForTransaction(con, definition); txObject.setPreviousIsolationLevel(previousIsolationLevel); + txObject.setReadOnly(definition.isReadOnly()); // Switch to manual commit if necessary. This is very expensive in some JDBC drivers, // so we don't want to do it unnecessarily (for example if we've explicitly @@ -374,7 +375,8 @@ public class DataSourceTransactionManager extends AbstractPlatformTransactionMan if (txObject.isMustRestoreAutoCommit()) { con.setAutoCommit(true); } - DataSourceUtils.resetConnectionAfterTransaction(con, txObject.getPreviousIsolationLevel()); + DataSourceUtils.resetConnectionAfterTransaction( + con, txObject.getPreviousIsolationLevel(), txObject.isReadOnly()); } catch (Throwable ex) { logger.debug("Could not reset JDBC Connection after transaction", ex); diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java index 606ff09ab4b..3f1fcade31e 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java @@ -169,6 +169,8 @@ public abstract class DataSourceUtils { * @return the previous isolation level, if any * @throws SQLException if thrown by JDBC methods * @see #resetConnectionAfterTransaction + * @see Connection#setTransactionIsolation + * @see Connection#setReadOnly */ @Nullable public static Integer prepareConnectionForTransaction(Connection con, @Nullable TransactionDefinition definition) @@ -220,8 +222,48 @@ public abstract class DataSourceUtils { * regarding read-only flag and isolation level. * @param con the Connection to reset * @param previousIsolationLevel the isolation level to restore, if any + * @param resetReadOnly whether to reset the connection's read-only flag + * @since 5.2.1 * @see #prepareConnectionForTransaction + * @see Connection#setTransactionIsolation + * @see Connection#setReadOnly */ + public static void resetConnectionAfterTransaction( + Connection con, @Nullable Integer previousIsolationLevel, boolean resetReadOnly) { + + Assert.notNull(con, "No Connection specified"); + try { + // Reset transaction isolation to previous value, if changed for the transaction. + if (previousIsolationLevel != null) { + if (logger.isDebugEnabled()) { + logger.debug("Resetting isolation level of JDBC Connection [" + + con + "] to " + previousIsolationLevel); + } + con.setTransactionIsolation(previousIsolationLevel); + } + + // Reset read-only flag if we originally switched it to true on transaction begin. + if (resetReadOnly) { + if (logger.isDebugEnabled()) { + logger.debug("Resetting read-only flag of JDBC Connection [" + con + "]"); + } + con.setReadOnly(false); + } + } + catch (Throwable ex) { + logger.debug("Could not reset JDBC Connection after transaction", ex); + } + } + + /** + * Reset the given Connection after a transaction, + * regarding read-only flag and isolation level. + * @param con the Connection to reset + * @param previousIsolationLevel the isolation level to restore, if any + * @deprecated as of 5.1.11, in favor of + * {@link #resetConnectionAfterTransaction(Connection, Integer, boolean)} + */ + @Deprecated public static void resetConnectionAfterTransaction(Connection con, @Nullable Integer previousIsolationLevel) { Assert.notNull(con, "No Connection specified"); try { diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/JdbcTransactionObjectSupport.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/JdbcTransactionObjectSupport.java index bcc43a26e58..412a67af0d3 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/JdbcTransactionObjectSupport.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/JdbcTransactionObjectSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,35 +57,76 @@ public abstract class JdbcTransactionObjectSupport implements SavepointManager, @Nullable private Integer previousIsolationLevel; + private boolean readOnly = false; + private boolean savepointAllowed = false; + /** + * Set the ConnectionHolder for this transaction object. + */ public void setConnectionHolder(@Nullable ConnectionHolder connectionHolder) { this.connectionHolder = connectionHolder; } + /** + * Return the ConnectionHolder for this transaction object. + */ public ConnectionHolder getConnectionHolder() { Assert.state(this.connectionHolder != null, "No ConnectionHolder available"); return this.connectionHolder; } + /** + * Check whether this transaction object has a ConnectionHolder. + */ public boolean hasConnectionHolder() { return (this.connectionHolder != null); } + /** + * Set the previous isolation level to retain, if any. + */ public void setPreviousIsolationLevel(@Nullable Integer previousIsolationLevel) { this.previousIsolationLevel = previousIsolationLevel; } + /** + * Return the retained previous isolation level, if any. + */ @Nullable public Integer getPreviousIsolationLevel() { return this.previousIsolationLevel; } + /** + * Set the read-only status of this transaction. + * The default is {@code false}. + * @since 5.2.1 + */ + public void setReadOnly(boolean readOnly) { + this.readOnly = readOnly; + } + + /** + * Return the read-only status of this transaction. + * @since 5.2.1 + */ + public boolean isReadOnly() { + return this.readOnly; + } + + /** + * Set whether savepoints are allowed within this transaction. + * The default is {@code false}. + */ public void setSavepointAllowed(boolean savepointAllowed) { this.savepointAllowed = savepointAllowed; } + /** + * Return whether savepoints are allowed within this transaction. + */ public boolean isSavepointAllowed() { return this.savepointAllowed; } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java index 0b9f7690af8..d1cf8f5c8fe 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java @@ -921,11 +921,13 @@ public class DataSourceTransactionManagerTests { boolean condition = !TransactionSynchronizationManager.hasResource(ds); assertThat(condition).as("Hasn't thread connection").isTrue(); InOrder ordered = inOrder(con); + ordered.verify(con).setReadOnly(true); ordered.verify(con).setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE); ordered.verify(con).setAutoCommit(false); ordered.verify(con).commit(); ordered.verify(con).setAutoCommit(true); ordered.verify(con).setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED); + ordered.verify(con).setReadOnly(false); verify(con).close(); } @@ -954,11 +956,13 @@ public class DataSourceTransactionManagerTests { boolean condition = !TransactionSynchronizationManager.hasResource(ds); assertThat(condition).as("Hasn't thread connection").isTrue(); InOrder ordered = inOrder(con, stmt); + ordered.verify(con).setReadOnly(true); ordered.verify(con).setAutoCommit(false); ordered.verify(stmt).executeUpdate("SET TRANSACTION READ ONLY"); ordered.verify(stmt).close(); ordered.verify(con).commit(); ordered.verify(con).setAutoCommit(true); + ordered.verify(con).setReadOnly(false); ordered.verify(con).close(); } @@ -1437,7 +1441,6 @@ public class DataSourceTransactionManagerTests { verify(con).rollback(sp); verify(con).releaseSavepoint(sp); verify(con).commit(); - verify(con).isReadOnly(); verify(con).close(); } @@ -1498,7 +1501,6 @@ public class DataSourceTransactionManagerTests { verify(con).rollback(sp); verify(con).releaseSavepoint(sp); verify(con).commit(); - verify(con).isReadOnly(); verify(con).close(); } @@ -1559,7 +1561,6 @@ public class DataSourceTransactionManagerTests { verify(con).rollback(sp); verify(con).releaseSavepoint(sp); verify(con).commit(); - verify(con).isReadOnly(); verify(con).close(); } diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java index 382eb530e0d..da96519b48d 100644 --- a/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java +++ b/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -481,6 +481,7 @@ public class HibernateTransactionManager extends AbstractPlatformTransactionMana Connection con = ((SessionImplementor) session).connection(); Integer previousIsolationLevel = DataSourceUtils.prepareConnectionForTransaction(con, definition); txObject.setPreviousIsolationLevel(previousIsolationLevel); + txObject.setReadOnly(definition.isReadOnly()); if (this.allowResultAccessAfterCompletion && !txObject.isNewSession()) { int currentHoldability = con.getHoldability(); if (currentHoldability != ResultSet.HOLD_CURSORS_OVER_COMMIT) { @@ -712,7 +713,8 @@ public class HibernateTransactionManager extends AbstractPlatformTransactionMana if (previousHoldability != null) { con.setHoldability(previousHoldability); } - DataSourceUtils.resetConnectionAfterTransaction(con, txObject.getPreviousIsolationLevel()); + DataSourceUtils.resetConnectionAfterTransaction( + con, txObject.getPreviousIsolationLevel(), txObject.isReadOnly()); } catch (HibernateException ex) { logger.debug("Could not access JDBC Connection of Hibernate Session", ex); diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java index 5c33b474d1c..ff6db7957d1 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -402,6 +402,7 @@ public class JpaTransactionManager extends AbstractPlatformTransactionManager Object transactionData = getJpaDialect().beginTransaction(em, new JpaTransactionDefinition(definition, timeoutToUse, txObject.isNewEntityManagerHolder())); txObject.setTransactionData(transactionData); + txObject.setReadOnly(definition.isReadOnly()); // Register transaction timeout. if (timeoutToUse != TransactionDefinition.TIMEOUT_DEFAULT) { diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java b/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java index adbe30badb5..4ea48ce5e97 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -194,7 +194,8 @@ public class HibernateJpaDialect extends DefaultJpaDialect { session.setDefaultReadOnly(true); } } - return new SessionTransactionData(session, previousFlushMode, preparedCon, previousIsolationLevel); + return new SessionTransactionData( + session, previousFlushMode, preparedCon, previousIsolationLevel, definition.isReadOnly()); } @Override @@ -203,7 +204,7 @@ public class HibernateJpaDialect extends DefaultJpaDialect { Session session = getSession(entityManager); FlushMode previousFlushMode = prepareFlushMode(session, readOnly); - return new SessionTransactionData(session, previousFlushMode, null, null); + return new SessionTransactionData(session, previousFlushMode, null, null, readOnly); } @SuppressWarnings("deprecation") @@ -370,13 +371,16 @@ public class HibernateJpaDialect extends DefaultJpaDialect { @Nullable private final Integer previousIsolationLevel; + private final boolean readOnly; + public SessionTransactionData(Session session, @Nullable FlushMode previousFlushMode, - @Nullable Connection preparedCon, @Nullable Integer previousIsolationLevel) { + @Nullable Connection preparedCon, @Nullable Integer previousIsolationLevel, boolean readOnly) { this.session = session; this.previousFlushMode = previousFlushMode; this.preparedCon = preparedCon; this.previousIsolationLevel = previousIsolationLevel; + this.readOnly = readOnly; } @SuppressWarnings("deprecation") @@ -392,7 +396,8 @@ public class HibernateJpaDialect extends DefaultJpaDialect { "make sure to use connection release mode ON_CLOSE (the default) and to run against " + "Hibernate 4.2+ (or switch HibernateJpaDialect's prepareConnection flag to false"); } - DataSourceUtils.resetConnectionAfterTransaction(conToReset, this.previousIsolationLevel); + DataSourceUtils.resetConnectionAfterTransaction( + conToReset, this.previousIsolationLevel, this.readOnly); } } }