Author: fhanik
Date: Fri Aug 8 14:02:54 2014
New Revision: 1616760
URL: http://svn.apache.org/r1616760
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54227
Proper fix for max age evaluation at time of borrow. It should be done during
borrow, not during setup. Setup does not perform proper initialization
Rename checkUser, poor name, hard to understand what the return value is
Fix unit test around pool thread renaming
Added:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java
(with props)
Modified:
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/PoolCleanerTest.java
Modified:
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java?rev=1616760&r1=1616759&r2=1616760&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
Fri Aug 8 14:02:54 2014
@@ -279,10 +279,6 @@ public class ConnectionPool {
* @throws SQLException if an interceptor can't be configured, if the
proxy can't be instantiated
*/
protected Connection setupConnection(PooledConnection con) throws
SQLException {
- //check if it's been sitting in the pool too long
- if (con.isMaxAgeExpired()) {
- con.reconnect();
- }
//fetch previously cached interceptor proxy - one per connection
JdbcInterceptor handler = con.getHandler();
if (handler==null) {
@@ -750,30 +746,20 @@ public class ConnectionPool {
boolean setToNull = false;
try {
con.lock();
- boolean usercheck = con.checkUser(username, password);
-
if (con.isReleased()) {
return null;
}
+ //evaluate username/password change as well as max age
functionality
+ boolean forceReconnect = con.shouldForceReconnect(username,
password) || con.isMaxAgeExpired();
+
if (!con.isDiscarded() && !con.isInitialized()) {
- //attempt to connect
- try {
- con.connect();
- } catch (Exception x) {
- release(con);
- setToNull = true;
- if (x instanceof SQLException) {
- throw (SQLException)x;
- } else {
- SQLException ex = new SQLException(x.getMessage());
- ex.initCause(x);
- throw ex;
- }
- }
+ //here it states that the connection not discarded, but the
connection is null
+ //don't attempt a connect here. It will be done during the
reconnect.
+ forceReconnect = true;
}
- if (usercheck) {
+ if (!forceReconnect) {
if ((!con.isDiscarded()) &&
con.validate(PooledConnection.VALIDATE_BORROW)) {
//set the timestamp
con.setTimestamp(now);
Modified:
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java?rev=1616760&r1=1616759&r2=1616760&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
Fri Aug 8 14:02:54 2014
@@ -134,8 +134,24 @@ public class PooledConnection {
return connectionVersion;
}
+ /**
+ * @deprecated use {@link #shouldForceReconnect(String, String)}
+ * method kept since it was public, to avoid changing interface. name was
pooo
+ */
public boolean checkUser(String username, String password) {
- if (!getPoolProperties().isAlternateUsernameAllowed()) return true;
+ return !shouldForceReconnect(username, password);
+ }
+ /**
+ * Returns true if we must force reconnect based on credentials passed in.
+ * Returns false if {@link PoolConfiguration#isAlternateUsernameAllowed()}
method returns false.
+ * Returns false if the username/password has not changed since this
connection was connected
+ * @param username the username you wish to connect with, pass in null to
accept the default username from {@link PoolConfiguration#getUsername()}
+ * @param password the password you wish to connect with, pass in null to
accept the default username from {@link
org.apache.tomcat.jdbc.pool.PoolConfiguration#getPassword()}
+ * @return true is the pool must reconnect
+ */
+ public boolean shouldForceReconnect(String username, String password) {
+
+ if (!getPoolProperties().isAlternateUsernameAllowed()) return false;
if (username==null) username = poolProperties.getUsername();
if (password==null) password = poolProperties.getPassword();
@@ -143,15 +159,15 @@ public class PooledConnection {
String storedUsr = (String)getAttributes().get(PROP_USER);
String storedPwd = (String)getAttributes().get(PROP_PASSWORD);
- boolean result = (username==null && storedUsr==null);
- result = (result || (username!=null && username.equals(storedUsr)));
+ boolean noChangeInCredentials = (username==null && storedUsr==null);
+ noChangeInCredentials = (noChangeInCredentials || (username!=null &&
username.equals(storedUsr)));
- result = result && ((password==null && storedPwd==null) ||
(password!=null && password.equals(storedPwd)));
+ noChangeInCredentials = noChangeInCredentials && ((password==null &&
storedPwd==null) || (password!=null && password.equals(storedPwd)));
if (username==null) getAttributes().remove(PROP_USER); else
getAttributes().put(PROP_USER, username);
if (password==null) getAttributes().remove(PROP_PASSWORD); else
getAttributes().put(PROP_PASSWORD, password);
- return result;
+ return !noChangeInCredentials;
}
/**
Added:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java?rev=1616760&view=auto
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java
(added)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java
Fri Aug 8 14:02:54 2014
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jdbc.pool;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class ShouldForceReconnectTest {
+
+ private ConnectionPool pool;
+ private PoolProperties properties;
+
+ private static final String DEFAULT_USER = "username_def";
+ private static final String DEFAULT_PASSWD = "password_def";
+ private static final String ALT_USER = "username_alt";
+ private static final String ALT_PASSWD = "password_alt";
+
+ @Before
+ public void setUp() throws Exception {
+ properties = new PoolProperties();
+ properties.setUsername(DEFAULT_USER);
+ properties.setPassword(DEFAULT_PASSWD);
+ properties.setAlternateUsernameAllowed(true);
+ properties.setInitialSize(0);
+ properties.setRemoveAbandoned(false);
+ properties.setTimeBetweenEvictionRunsMillis(-1);
+ pool = new ConnectionPool(properties);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+
+
+
+
+ }
+
+ @Test
+ public void testShouldForceReconnect() throws Exception {
+ PooledConnection con = new PooledConnection(properties, pool);
+
+ //connection previously connect with default
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(null, null));
+
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(null,DEFAULT_PASSWD));
+
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(DEFAULT_USER, null));
+
+ configureDefault(con);
+ assertTrue(con.shouldForceReconnect(ALT_USER,ALT_PASSWD));
+
+ configureDefault(con);
+ assertTrue(con.shouldForceReconnect(null,ALT_PASSWD));
+
+ configureDefault(con);
+ assertTrue(con.shouldForceReconnect(ALT_USER,null));
+
+ //connection previously connect with alternate
+ configureAlt(con);
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(null, null));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(null, DEFAULT_PASSWD));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, null));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(null,ALT_PASSWD));
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(ALT_USER,null));
+
+ //test changes in username password
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(null, null));
+ assertTrue(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertTrue(con.shouldForceReconnect(null, null));
+
+ configureDefault(con);
+ assertFalse(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+ assertTrue(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+
+ configureAlt(con);
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+ assertFalse(con.shouldForceReconnect(null, null));
+ assertTrue(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+
+
+ configureAlt(con);
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+ assertTrue(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertFalse(con.shouldForceReconnect(ALT_USER, ALT_PASSWD));
+ assertTrue(con.shouldForceReconnect(DEFAULT_USER, DEFAULT_PASSWD));
+
+ }
+
+ private void configureAlt(PooledConnection con) {
+ con.getAttributes().put(PooledConnection.PROP_USER, ALT_USER);
+ con.getAttributes().put(PooledConnection.PROP_PASSWORD, ALT_PASSWD);
+ }
+
+ private void configureDefault(PooledConnection con) {
+ con.getAttributes().put(PooledConnection.PROP_USER, DEFAULT_USER);
+ con.getAttributes().put(PooledConnection.PROP_PASSWORD,
DEFAULT_PASSWD);
+ }
+}
\ No newline at end of file
Propchange:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/ShouldForceReconnectTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/PoolCleanerTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/PoolCleanerTest.java?rev=1616760&r1=1616759&r2=1616760&view=diff
==============================================================================
---
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/PoolCleanerTest.java
(original)
+++
tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/PoolCleanerTest.java
Fri Aug 8 14:02:54 2014
@@ -30,7 +30,7 @@ public class PoolCleanerTest extends Def
Map<Thread, StackTraceElement[]> threadmap =
Thread.getAllStackTraces();
int result = 0;
for (Thread t : threadmap.keySet()) {
- if (t.getName().startsWith("PoolCleaner[")) result++;
+ if (t.getName().startsWith("Tomcat JDBC Pool Cleaner[")) result++;
}
return result;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
