Connector/J commit: r4984 - in branches: branch_3_1/connector-j branch_3_1/connector-j/src/com/mysql/jdbc branch_3_1/connector-j/src/testsuite/regress...

[mmatthews]

Modified:
   branches/branch_3_1/connector-j/CHANGES
   branches/branch_3_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java
   branches/branch_3_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
   branches/branch_5_0/connector-j/CHANGES
   branches/branch_5_0/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
   branches/branch_5_1/connector-j/CHANGES
   branches/branch_5_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java
   branches/branch_5_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
Log:
Fixed BUG#15570 - ReplicationConnection incorrectly copies state,
	  doesn't transfer connection context correctly when transitioning between 
	  the same read-only states.
    

Modified: branches/branch_3_1/connector-j/CHANGES
===================================================================
--- branches/branch_3_1/connector-j/CHANGES	2006-02-24 22:04:41 UTC (rev 4983)
+++ branches/branch_3_1/connector-j/CHANGES	2006-02-24 22:40:10 UTC (rev 4984)
@@ -53,6 +53,10 @@
       found as-specified. This allows you to use "short names" for the built-in log
       factories, for example "logger=CommonsLogger" instead of 
       "logger=com.mysql.jdbc.log.CommonsLogger".
+      
+    - Fixed BUG#15570 - ReplicationConnection incorrectly copies state,
+	  doesn't transfer connection context correctly when transitioning between 
+	  the same read-only states.
     
 11-30-05 - Version 3.1.12
 

Modified: branches/branch_3_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java
===================================================================
---
branches/branch_3_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java	2006-02-24
22:04:41 UTC (rev 4983)
+++
branches/branch_3_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java	2006-02-24
22:40:10 UTC (rev 4984)
@@ -379,9 +379,13 @@
 	 */
 	public synchronized void setReadOnly(boolean readOnly) throws SQLException {
 		if (readOnly) {
-			switchToSlavesConnection();
+			if (currentConnection != slavesConnection) {
+				switchToSlavesConnection();
+			}
 		} else {
-			switchToMasterConnection();
+			if (currentConnection != masterConnection) {
+				switchToMasterConnection();
+			}
 		}
 	}
 
@@ -425,60 +429,48 @@
 	}
 
 	private synchronized void switchToMasterConnection() throws SQLException {
-		String slaveCatalog = this.slavesConnection.getCatalog();
-		String masterCatalog = this.masterConnection.getCatalog();
-
-		if (slaveCatalog != null && !slaveCatalog.equals(masterCatalog)) {
-			this.masterConnection.setCatalog(slaveCatalog);
-		} else if (masterCatalog != null) {
-			this.masterConnection.setCatalog(masterCatalog);
-		}
-
-		boolean slavesAutoCommit = this.slavesConnection.getAutoCommit();
-
-		if (this.masterConnection.getAutoCommit() != slavesAutoCommit) {
-			this.masterConnection.setAutoCommit(slavesAutoCommit);
-		}
-
-		int slavesTransactionIsolation = this.slavesConnection
-				.getTransactionIsolation();
-
-		if (this.masterConnection.getTransactionIsolation() != slavesTransactionIsolation) {
-			this.masterConnection
-					.setTransactionIsolation(slavesTransactionIsolation);
-		}
-
-		this.currentConnection = this.masterConnection;
+		swapConnections(this.masterConnection, this.slavesConnection);
 	}
 
 	private synchronized void switchToSlavesConnection() throws SQLException {
-		String slaveCatalog = this.slavesConnection.getCatalog();
-		String masterCatalog = this.masterConnection.getCatalog();
+		swapConnections(this.slavesConnection, this.masterConnection);
+	}
+	
+	/**
+	 * Swaps current context (catalog, autocommit and txn_isolation) from
+	 * sourceConnection to targetConnection, and makes targetConnection
+	 * the "current" connection that will be used for queries.
+	 * 
+	 * @param sourceConnection the connection to swap from
+	 * @param targetConnection the connection to swap to
+	 * 
+	 * @throws SQLException if an error occurs
+	 */
+	private synchronized void swapConnections(Connection sourceConnection, 
+			Connection targetConnection) throws SQLException {
+		String slaveCatalog = targetConnection.getCatalog();
+		String sourceCatalog = sourceConnection.getCatalog();
 
-		if (masterCatalog != null && !masterCatalog.equals(slaveCatalog)) {
-			this.slavesConnection.setCatalog(masterCatalog);
+		if (sourceCatalog != null && !sourceCatalog.equals(slaveCatalog)) {
+			targetConnection.setCatalog(sourceCatalog);
 		} else if (slaveCatalog != null) {
-			this.slavesConnection.setCatalog(slaveCatalog);
+			targetConnection.setCatalog(slaveCatalog);
 		}
 
-		boolean masterAutoCommit = this.masterConnection.getAutoCommit();
+		boolean sourceAutoCommit = sourceConnection.getAutoCommit();
 
-		if (this.slavesConnection.getAutoCommit() != masterAutoCommit) {
-			this.slavesConnection.setAutoCommit(masterAutoCommit);
+		if (targetConnection.getAutoCommit() != sourceAutoCommit) {
+			targetConnection.setAutoCommit(sourceAutoCommit);
 		}
 
-		int masterTransactionIsolation = this.masterConnection
+		int sourceTransactionIsolation = sourceConnection
 				.getTransactionIsolation();
 
-		if (this.slavesConnection.getTransactionIsolation() != masterTransactionIsolation) {
-			this.slavesConnection
-					.setTransactionIsolation(masterTransactionIsolation);
+		if (targetConnection.getTransactionIsolation() != sourceTransactionIsolation) {
+			targetConnection
+					.setTransactionIsolation(sourceTransactionIsolation);
 		}
-		this.currentConnection = this.slavesConnection;
-
-		this.slavesConnection.setAutoCommit(this.masterConnection
-				.getAutoCommit());
-		this.slavesConnection.setTransactionIsolation(this.masterConnection
-				.getTransactionIsolation());
+		
+		this.currentConnection = targetConnection;
 	}
 }

Modified:
branches/branch_3_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
===================================================================
---
branches/branch_3_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:04:41 UTC (rev 4983)
+++
branches/branch_3_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:40:10 UTC (rev 4984)
@@ -1562,4 +1562,38 @@
 		
 		return props;
 	}
+
+	/**
+	 * Tests fix for BUG#15570 - ReplicationConnection incorrectly copies state,
+	 * doesn't transfer connection context correctly when transitioning between 
+	 * the same read-only states.
+	 * 
+	 * (note, this test will fail if the test user doesn't have permission to 
+	 * "USE 'mysql'".
+	 * 
+	 * @throws Exception if the test fails.
+	 */
+	public void testBug15570() throws Exception {
+	    Connection replConn = getMasterSlaveReplicationConnection();
+	
+	    replConn.setReadOnly(false);
+	
+	    String currentCatalog = replConn.getCatalog();
+	    
+	    replConn.setCatalog(currentCatalog);
+	    assertEquals(currentCatalog, replConn.getCatalog());
+	
+	    replConn.setReadOnly(true);
+	    assertEquals(currentCatalog, replConn.getCatalog());
+	
+	    String newCatalog = "mysql";
+	
+	    replConn.setCatalog(newCatalog);
+	    assertEquals(newCatalog, replConn.getCatalog());
+	
+	    replConn.setReadOnly(true);
+	    assertEquals(newCatalog, replConn.getCatalog());
+	
+	    replConn.close();
+	}
 }

Modified: branches/branch_5_0/connector-j/CHANGES
===================================================================
--- branches/branch_5_0/connector-j/CHANGES	2006-02-24 22:04:41 UTC (rev 4983)
+++ branches/branch_5_0/connector-j/CHANGES	2006-02-24 22:40:10 UTC (rev 4984)
@@ -156,6 +156,10 @@
       found as-specified. This allows you to use "short names" for the built-in log
       factories, for example "logger=CommonsLogger" instead of 
       "logger=com.mysql.jdbc.log.CommonsLogger".
+      
+    - Fixed BUG#15570 - ReplicationConnection incorrectly copies state,
+	  doesn't transfer connection context correctly when transitioning between 
+	  the same read-only states.
     
 11-30-05 - Version 3.1.12
 

Modified:
branches/branch_5_0/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
===================================================================
---
branches/branch_5_0/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:04:41 UTC (rev 4983)
+++
branches/branch_5_0/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:40:10 UTC (rev 4984)
@@ -384,12 +384,9 @@
 	 *             if an error occurs.
 	 */
 	public void testBug4334() throws Exception {
-		if (isAdminConnectionConfigured()) {
-			Connection adminConnection = null;
-
+		
+			
 			try {
-				adminConnection = getAdminConnection();
-
 				int bogusPortNumber = 65534;
 
 				NonRegisteringDriver driver = new NonRegisteringDriver();
@@ -459,7 +456,7 @@
 				this.rs = portNumStmt.executeQuery("SELECT connection_id()");
 				this.rs.next();
 
-				killConnection(adminConnection, this.rs.getString(1));
+				killConnection(this.rs.getString(1));
 
 				try {
 					portNumStmt.executeQuery("SELECT connection_id()");
@@ -521,7 +518,7 @@
 				this.rs = failoverStmt.executeQuery("SELECT connection_id()");
 				this.rs.next();
 
-				killConnection(adminConnection, this.rs.getString(1));
+				killConnection(this.rs.getString(1));
 
 				try {
 					failoverStmt.executeQuery("SELECT connection_id()");
@@ -531,16 +528,14 @@
 
 				failoverStmt.executeQuery("SELECT connection_id()");
 			} finally {
-				if (adminConnection != null) {
-					adminConnection.close();
-				}
+				
 			}
 		}
-	}
+	
 
-	private static void killConnection(Connection adminConn, String threadId)
+	private void killConnection(String threadId)
 			throws SQLException {
-		adminConn.createStatement().execute("KILL " + threadId);
+		getConnectionWithProps(null).createStatement().execute("KILL " + threadId);
 	}
 
 	/**
@@ -621,7 +616,7 @@
 
 			System.out.println("Connection id: " + failoverConnectionId);
 
-			killConnection(killerConnection, failoverConnectionId);
+			killConnection(failoverConnectionId);
 
 			Thread.sleep(3000); // This can take some time....
 
@@ -1583,4 +1578,38 @@
 			}
 		}
 	}
+	
+	/**
+	 * Tests fix for BUG#15570 - ReplicationConnection incorrectly copies state,
+	 * doesn't transfer connection context correctly when transitioning between 
+	 * the same read-only states.
+	 * 
+	 * (note, this test will fail if the test user doesn't have permission to 
+	 * "USE 'mysql'".
+	 * 
+	 * @throws Exception if the test fails.
+	 */
+	public void testBug15570() throws Exception {
+        Connection replConn = getMasterSlaveReplicationConnection();
+
+        replConn.setReadOnly(false);
+
+        String currentCatalog = replConn.getCatalog();
+        
+        replConn.setCatalog(currentCatalog);
+        assertEquals(currentCatalog, replConn.getCatalog());
+
+        replConn.setReadOnly(true);
+        assertEquals(currentCatalog, replConn.getCatalog());
+
+        String newCatalog = "mysql";
+
+        replConn.setCatalog(newCatalog);
+        assertEquals(newCatalog, replConn.getCatalog());
+
+        replConn.setReadOnly(true);
+        assertEquals(newCatalog, replConn.getCatalog());
+
+        replConn.close();
+	}
 }

Modified: branches/branch_5_1/connector-j/CHANGES
===================================================================
--- branches/branch_5_1/connector-j/CHANGES	2006-02-24 22:04:41 UTC (rev 4983)
+++ branches/branch_5_1/connector-j/CHANGES	2006-02-24 22:40:10 UTC (rev 4984)
@@ -158,6 +158,10 @@
       found as-specified. This allows you to use "short names" for the built-in log
       factories, for example "logger=CommonsLogger" instead of 
       "logger=com.mysql.jdbc.log.CommonsLogger".
+      
+    - Fixed BUG#15570 - ReplicationConnection incorrectly copies state,
+	  doesn't transfer connection context correctly when transitioning between 
+	  the same read-only states.
     
 11-30-05 - Version 3.1.12
 

Modified: branches/branch_5_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java
===================================================================
---
branches/branch_5_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java	2006-02-24
22:04:41 UTC (rev 4983)
+++
branches/branch_5_1/connector-j/src/com/mysql/jdbc/ReplicationConnection.java	2006-02-24
22:40:10 UTC (rev 4984)
@@ -429,11 +429,15 @@
 	 */
 	public synchronized void setReadOnly(boolean readOnly) throws SQLException {
 		if (readOnly) {
+			if (currentConnection != slavesConnection) {
 			switchToSlavesConnection();
+			}
 		} else {
+			if (currentConnection != masterConnection) {
 			switchToMasterConnection();
 		}
 	}
+	}
 
 	/*
 	 * (non-Javadoc)
@@ -475,61 +479,49 @@
 	}
 
 	private synchronized void switchToMasterConnection() throws SQLException {
-		String slaveCatalog = this.slavesConnection.getCatalog();
-		String masterCatalog = this.masterConnection.getCatalog();
-
-		if (slaveCatalog != null && !slaveCatalog.equals(masterCatalog)) {
-			this.masterConnection.setCatalog(slaveCatalog);
-		} else if (masterCatalog != null) {
-			this.masterConnection.setCatalog(masterCatalog);
-		}
-
-		boolean slavesAutoCommit = this.slavesConnection.getAutoCommit();
-
-		if (this.masterConnection.getAutoCommit() != slavesAutoCommit) {
-			this.masterConnection.setAutoCommit(slavesAutoCommit);
-		}
-
-		int slavesTransactionIsolation = this.slavesConnection
-				.getTransactionIsolation();
-
-		if (this.masterConnection.getTransactionIsolation() != slavesTransactionIsolation) {
-			this.masterConnection
-					.setTransactionIsolation(slavesTransactionIsolation);
-		}
-
-		this.currentConnection = this.masterConnection;
+		swapConnections(this.masterConnection, this.slavesConnection);
 	}
 
 	private synchronized void switchToSlavesConnection() throws SQLException {
-		String slaveCatalog = this.slavesConnection.getCatalog();
-		String masterCatalog = this.masterConnection.getCatalog();
+		swapConnections(this.slavesConnection, this.masterConnection);
+	}
+	
+	/**
+	 * Swaps current context (catalog, autocommit and txn_isolation) from
+	 * sourceConnection to targetConnection, and makes targetConnection
+	 * the "current" connection that will be used for queries.
+	 * 
+	 * @param sourceConnection the connection to swap from
+	 * @param targetConnection the connection to swap to
+	 * 
+	 * @throws SQLException if an error occurs
+	 */
+	private synchronized void swapConnections(Connection sourceConnection, 
+			Connection targetConnection) throws SQLException {
+		String slaveCatalog = targetConnection.getCatalog();
+		String sourceCatalog = sourceConnection.getCatalog();
 
-		if (masterCatalog != null && !masterCatalog.equals(slaveCatalog)) {
-			this.slavesConnection.setCatalog(masterCatalog);
+		if (sourceCatalog != null && !sourceCatalog.equals(slaveCatalog)) {
+			targetConnection.setCatalog(sourceCatalog);
 		} else if (slaveCatalog != null) {
-			this.slavesConnection.setCatalog(slaveCatalog);
+			targetConnection.setCatalog(slaveCatalog);
 		}
 
-		boolean masterAutoCommit = this.masterConnection.getAutoCommit();
+		boolean sourceAutoCommit = sourceConnection.getAutoCommit();
 
-		if (this.slavesConnection.getAutoCommit() != masterAutoCommit) {
-			this.slavesConnection.setAutoCommit(masterAutoCommit);
+		if (targetConnection.getAutoCommit() != sourceAutoCommit) {
+			targetConnection.setAutoCommit(sourceAutoCommit);
 		}
 
-		int masterTransactionIsolation = this.masterConnection
+		int sourceTransactionIsolation = sourceConnection
 				.getTransactionIsolation();
 
-		if (this.slavesConnection.getTransactionIsolation() != masterTransactionIsolation) {
-			this.slavesConnection
-					.setTransactionIsolation(masterTransactionIsolation);
+		if (targetConnection.getTransactionIsolation() != sourceTransactionIsolation) {
+			targetConnection
+					.setTransactionIsolation(sourceTransactionIsolation);
 		}
-		this.currentConnection = this.slavesConnection;
-
-		this.slavesConnection.setAutoCommit(this.masterConnection
-				.getAutoCommit());
-		this.slavesConnection.setTransactionIsolation(this.masterConnection
-				.getTransactionIsolation());
+		
+		this.currentConnection = targetConnection;
 	}
 
 	public Object unwrap(Class arg0) throws SQLException {

Modified:
branches/branch_5_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java
===================================================================
---
branches/branch_5_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:04:41 UTC (rev 4983)
+++
branches/branch_5_1/connector-j/src/testsuite/regression/ConnectionRegressionTest.java	2006-02-24
22:40:10 UTC (rev 4984)
@@ -1583,4 +1583,38 @@
 			}
 		}
 	}
+
+	/**
+	 * Tests fix for BUG#15570 - ReplicationConnection incorrectly copies state,
+	 * doesn't transfer connection context correctly when transitioning between 
+	 * the same read-only states.
+	 * 
+	 * (note, this test will fail if the test user doesn't have permission to 
+	 * "USE 'mysql'".
+	 * 
+	 * @throws Exception if the test fails.
+	 */
+	public void testBug15570() throws Exception {
+	    Connection replConn = getMasterSlaveReplicationConnection();
+	
+	    replConn.setReadOnly(false);
+	
+	    String currentCatalog = replConn.getCatalog();
+	    
+	    replConn.setCatalog(currentCatalog);
+	    assertEquals(currentCatalog, replConn.getCatalog());
+	
+	    replConn.setReadOnly(true);
+	    assertEquals(currentCatalog, replConn.getCatalog());
+	
+	    String newCatalog = "mysql";
+	
+	    replConn.setCatalog(newCatalog);
+	    assertEquals(newCatalog, replConn.getCatalog());
+	
+	    replConn.setReadOnly(true);
+	    assertEquals(newCatalog, replConn.getCatalog());
+	
+	    replConn.close();
+	}
 }