Avoid Connection.isReadOnly() call in resetConnectionAfterTransaction

Closes gh-23747

Author: jhoeller

spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java

@@ -272,6 +272,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 
 			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 @@ protected void doCleanupAfterCompletion(Object transaction) {
 			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);

spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java

@@ -169,6 +169,8 @@ private static Connection fetchConnection(DataSource dataSource) throws SQLExcep
 	 * @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 static Integer prepareConnectionForTransaction(Connection con, @Nullable
 	 * 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 {

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;
 	}

spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java

@@ -921,11 +921,13 @@ protected void doInTransactionWithoutResult(TransactionStatus status) {
 		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 @@ protected void doInTransactionWithoutResult(TransactionStatus status) {
 		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 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run
 		verify(con).rollback(sp);
 		verify(con).releaseSavepoint(sp);
 		verify(con).commit();
-		verify(con).isReadOnly();
 		verify(con).close();
 	}
 
@@ -1498,7 +1501,6 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run
 		verify(con).rollback(sp);
 		verify(con).releaseSavepoint(sp);
 		verify(con).commit();
-		verify(con).isReadOnly();
 		verify(con).close();
 	}
 
@@ -1559,7 +1561,6 @@ protected void doInTransactionWithoutResult(TransactionStatus status) throws Run
 		verify(con).rollback(sp);
 		verify(con).releaseSavepoint(sp);
 		verify(con).commit();
-		verify(con).isReadOnly();
 		verify(con).close();
 	}
 

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 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 					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 @@ protected void doCleanupAfterCompletion(Object transaction) {
 				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);

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 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 			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) {

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 @@ else if (isolationLevelNeeded) {
 				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 Object prepareTransaction(EntityManager entityManager, boolean readOnly,
 
 		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 @@ private static class SessionTransactionData {
 		@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 void resetSessionState() {
 							"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);
 			}
 		}
 	}