This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new c38af7c  Remove legacy JDBC code from session store
c38af7c is described below

commit c38af7c7305ba3942ca441496be9f703beef9a86
Author: remm <r...@apache.org>
AuthorDate: Mon Nov 9 22:31:50 2020 +0100

    Remove legacy JDBC code from session store
    
    Remove bottlenecks caused by single JDBC connection code. The data
    source configuration is now the only one supported.
    Rename the store to DataSourceStore.
    Some code cleanups.
    Code submitted by Philippe Mouawad (removed added author tag since
    Tomcat does not allow new ones, sorry).
---
 .../{JDBCStore.java => DataSourceStore.java}       | 571 ++++++---------------
 .../catalina/session/LocalStrings.properties       |  25 +-
 .../catalina/storeconfig/server-registry.xml       |   2 +-
 res/findbugs/filter-false-positives.xml            |   4 +-
 webapps/docs/changelog.xml                         |   6 +
 webapps/docs/config/manager.xml                    |  51 +-
 6 files changed, 175 insertions(+), 484 deletions(-)

diff --git a/java/org/apache/catalina/session/JDBCStore.java 
b/java/org/apache/catalina/session/DataSourceStore.java
similarity index 56%
rename from java/org/apache/catalina/session/JDBCStore.java
rename to java/org/apache/catalina/session/DataSourceStore.java
index 4ad4b06..d6184ea 100644
--- a/java/org/apache/catalina/session/JDBCStore.java
+++ b/java/org/apache/catalina/session/DataSourceStore.java
@@ -26,13 +26,11 @@ import java.io.InputStream;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.sql.Connection;
-import java.sql.Driver;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Properties;
 
 import javax.naming.Context;
 import javax.naming.InitialContext;
@@ -41,10 +39,8 @@ import javax.sql.DataSource;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.Globals;
-import org.apache.catalina.LifecycleException;
 import org.apache.catalina.Session;
 import org.apache.juli.logging.Log;
-import org.apache.tomcat.util.ExceptionUtils;
 
 /**
  * Implementation of the {@link org.apache.catalina.Store Store}
@@ -54,7 +50,7 @@ import org.apache.tomcat.util.ExceptionUtils;
  *
  * @author Bip Thelin
  */
-public class JDBCStore extends StoreBase {
+public class DataSourceStore extends StoreBase {
 
     /**
      * Context name associated with this Store
@@ -64,38 +60,7 @@ public class JDBCStore extends StoreBase {
     /**
      * Name to register for this Store, used for logging.
      */
-    protected static final String storeName = "JDBCStore";
-
-    /**
-     * The connection username to use when trying to connect to the database.
-     */
-    protected String connectionName = null;
-
-
-    /**
-     * The connection URL to use when trying to connect to the database.
-     */
-    protected String connectionPassword = null;
-
-    /**
-     * Connection string to use when connecting to the DB.
-     */
-    protected String connectionURL = null;
-
-    /**
-     * The database connection.
-     */
-    private Connection dbConnection = null;
-
-    /**
-     * Instance of the JDBC Driver class we use as a connection factory.
-     */
-    protected Driver driver = null;
-
-    /**
-     * Driver to use.
-     */
-    protected String driverName = null;
+    protected static final String storeName = "dataSourceStore";
 
     /**
      * name of the JNDI resource
@@ -150,35 +115,6 @@ public class JDBCStore extends StoreBase {
      */
     protected String sessionLastAccessedCol = "lastaccess";
 
-
-    // ----------------------------------------------------------- SQL 
Variables
-
-    /**
-     * Variable to hold the <code>getSize()</code> prepared statement.
-     */
-    protected PreparedStatement preparedSizeSql = null;
-
-    /**
-     * Variable to hold the <code>save()</code> prepared statement.
-     */
-    protected PreparedStatement preparedSaveSql = null;
-
-    /**
-     * Variable to hold the <code>clear()</code> prepared statement.
-     */
-    protected PreparedStatement preparedClearSql = null;
-
-    /**
-     * Variable to hold the <code>remove()</code> prepared statement.
-     */
-    protected PreparedStatement preparedRemoveSql = null;
-
-    /**
-     * Variable to hold the <code>load()</code> prepared statement.
-     */
-    protected PreparedStatement preparedLoadSql = null;
-
-
     // -------------------------------------------------------------- 
Properties
 
     /**
@@ -215,79 +151,6 @@ public class JDBCStore extends StoreBase {
     }
 
     /**
-     * Set the driver for this Store.
-     *
-     * @param driverName The new driver
-     */
-    public void setDriverName(String driverName) {
-        String oldDriverName = this.driverName;
-        this.driverName = driverName;
-        support.firePropertyChange("driverName",
-                oldDriverName,
-                this.driverName);
-        this.driverName = driverName;
-    }
-
-    /**
-     * @return the driver for this Store.
-     */
-    public String getDriverName() {
-        return driverName;
-    }
-
-    /**
-     * @return the username to use to connect to the database.
-     */
-    public String getConnectionName() {
-        return connectionName;
-    }
-
-    /**
-     * Set the username to use to connect to the database.
-     *
-     * @param connectionName Username
-     */
-    public void setConnectionName(String connectionName) {
-        this.connectionName = connectionName;
-    }
-
-    /**
-     * @return the password to use to connect to the database.
-     */
-    public String getConnectionPassword() {
-        return connectionPassword;
-    }
-
-    /**
-     * Set the password to use to connect to the database.
-     *
-     * @param connectionPassword User password
-     */
-    public void setConnectionPassword(String connectionPassword) {
-        this.connectionPassword = connectionPassword;
-    }
-
-    /**
-     * Set the Connection URL for this Store.
-     *
-     * @param connectionURL The new Connection URL
-     */
-    public void setConnectionURL(String connectionURL) {
-        String oldConnString = this.connectionURL;
-        this.connectionURL = connectionURL;
-        support.firePropertyChange("connectionURL",
-                oldConnString,
-                this.connectionURL);
-    }
-
-    /**
-     * @return the Connection URL for this Store.
-     */
-    public String getConnectionURL() {
-        return connectionURL;
-    }
-
-    /**
      * Set the table for this Store.
      *
      * @param sessionTable The new table
@@ -491,50 +354,46 @@ public class JDBCStore extends StoreBase {
      */
     private String[] keys(boolean expiredOnly) throws IOException {
         String keys[] = null;
-        synchronized (this) {
-            int numberOfTries = 2;
-            while (numberOfTries > 0) {
+        int numberOfTries = 2;
+        while (numberOfTries > 0) {
 
-                Connection _conn = getConnection();
-                if (_conn == null) {
-                    return new String[0];
-                }
-                try {
+            Connection _conn = getConnection();
+            if (_conn == null) {
+                return new String[0];
+            }
+            try {
 
-                    String keysSql = "SELECT " + sessionIdCol + " FROM "
-                            + sessionTable + " WHERE " + sessionAppCol + " = 
?";
+                String keysSql = "SELECT " + sessionIdCol + " FROM "
+                        + sessionTable + " WHERE " + sessionAppCol + " = ?";
+                if (expiredOnly) {
+                    keysSql += " AND (" + sessionLastAccessedCol + " + "
+                            + sessionMaxInactiveCol + " * 1000 < ?)";
+                }
+                try (PreparedStatement preparedKeysSql = 
_conn.prepareStatement(keysSql)) {
+                    preparedKeysSql.setString(1, getName());
                     if (expiredOnly) {
-                        keysSql += " AND (" + sessionLastAccessedCol + " + "
-                                + sessionMaxInactiveCol + " * 1000 < ?)";
+                        preparedKeysSql.setLong(2, System.currentTimeMillis());
                     }
-                    try (PreparedStatement preparedKeysSql = 
_conn.prepareStatement(keysSql)) {
-                        preparedKeysSql.setString(1, getName());
-                        if (expiredOnly) {
-                            preparedKeysSql.setLong(2, 
System.currentTimeMillis());
-                        }
-                        try (ResultSet rst = preparedKeysSql.executeQuery()) {
-                            List<String> tmpkeys = new ArrayList<>();
-                            if (rst != null) {
-                                while (rst.next()) {
-                                    tmpkeys.add(rst.getString(1));
-                                }
+                    try (ResultSet rst = preparedKeysSql.executeQuery()) {
+                        List<String> tmpkeys = new ArrayList<>();
+                        if (rst != null) {
+                            while (rst.next()) {
+                                tmpkeys.add(rst.getString(1));
                             }
-                            keys = tmpkeys.toArray(new String[0]);
-                            // Break out after the finally block
-                            numberOfTries = 0;
                         }
+                        keys = tmpkeys.toArray(new String[0]);
+                        // Break out after the finally block
+                        numberOfTries = 0;
                     }
-                } catch (SQLException e) {
-                    
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
-                    keys = new String[0];
-                    // Close the connection so that it gets reopened next time
-                    if (dbConnection != null)
-                        close(dbConnection);
-                } finally {
-                    release(_conn);
                 }
-                numberOfTries--;
+            } catch (SQLException e) {
+                
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
+                keys = new String[0];
+                // Close the connection so that it gets reopened next time
+            } finally {
+                release(_conn);
             }
+            numberOfTries--;
         }
         return keys;
     }
@@ -551,41 +410,33 @@ public class JDBCStore extends StoreBase {
     @Override
     public int getSize() throws IOException {
         int size = 0;
+        String sizeSql = "SELECT COUNT(" + sessionIdCol
+                + ") FROM " + sessionTable + " WHERE "
+                + sessionAppCol + " = ?";
 
-        synchronized (this) {
-            int numberOfTries = 2;
-            while (numberOfTries > 0) {
-                Connection _conn = getConnection();
+        int numberOfTries = 2;
+        while (numberOfTries > 0) {
+            Connection _conn = getConnection();
 
-                if (_conn == null) {
-                    return size;
-                }
-
-                try {
-                    if (preparedSizeSql == null) {
-                        String sizeSql = "SELECT COUNT(" + sessionIdCol
-                                + ") FROM " + sessionTable + " WHERE "
-                                + sessionAppCol + " = ?";
-                        preparedSizeSql = _conn.prepareStatement(sizeSql);
-                    }
+            if (_conn == null) {
+                return size;
+            }
 
-                    preparedSizeSql.setString(1, getName());
-                    try (ResultSet rst = preparedSizeSql.executeQuery()) {
-                        if (rst.next()) {
-                            size = rst.getInt(1);
-                        }
-                        // Break out after the finally block
-                        numberOfTries = 0;
+            try (PreparedStatement preparedSizeSql = 
_conn.prepareStatement(sizeSql)){
+                preparedSizeSql.setString(1, getName());
+                try (ResultSet rst = preparedSizeSql.executeQuery()) {
+                    if (rst.next()) {
+                        size = rst.getInt(1);
                     }
-                } catch (SQLException e) {
-                    
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
-                    if (dbConnection != null)
-                        close(dbConnection);
-                } finally {
-                    release(_conn);
+                    // Break out after the finally block
+                    numberOfTries = 0;
                 }
-                numberOfTries--;
+            } catch (SQLException e) {
+                
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
+            } finally {
+                release(_conn);
             }
+            numberOfTries--;
         }
         return size;
     }
@@ -605,58 +456,49 @@ public class JDBCStore extends StoreBase {
         org.apache.catalina.Context context = getManager().getContext();
         Log contextLog = context.getLogger();
 
-        synchronized (this) {
-            int numberOfTries = 2;
-            while (numberOfTries > 0) {
-                Connection _conn = getConnection();
-                if (_conn == null) {
-                    return null;
-                }
-
-                ClassLoader oldThreadContextCL = 
context.bind(Globals.IS_SECURITY_ENABLED, null);
-
-                try {
-                    if (preparedLoadSql == null) {
-                        String loadSql = "SELECT " + sessionIdCol + ", "
-                                + sessionDataCol + " FROM " + sessionTable
-                                + " WHERE " + sessionIdCol + " = ? AND "
-                                + sessionAppCol + " = ?";
-                        preparedLoadSql = _conn.prepareStatement(loadSql);
-                    }
+        int numberOfTries = 2;
+        String loadSql = "SELECT " + sessionIdCol + ", "
+                + sessionDataCol + " FROM " + sessionTable
+                + " WHERE " + sessionIdCol + " = ? AND "
+                + sessionAppCol + " = ?";
+        while (numberOfTries > 0) {
+            Connection _conn = getConnection();
+            if (_conn == null) {
+                return null;
+            }
 
-                    preparedLoadSql.setString(1, id);
-                    preparedLoadSql.setString(2, getName());
-                    try (ResultSet rst = preparedLoadSql.executeQuery()) {
-                        if (rst.next()) {
-                            try (ObjectInputStream ois =
-                                    
getObjectInputStream(rst.getBinaryStream(2))) {
-                                if (contextLog.isDebugEnabled()) {
-                                    contextLog.debug(sm.getString(
-                                            getStoreName() + ".loading", id, 
sessionTable));
-                                }
-
-                                _session = (StandardSession) 
manager.createEmptySession();
-                                _session.readObjectData(ois);
-                                _session.setManager(manager);
+            ClassLoader oldThreadContextCL = 
context.bind(Globals.IS_SECURITY_ENABLED, null);
+
+            try (PreparedStatement preparedLoadSql = 
_conn.prepareStatement(loadSql)){
+                preparedLoadSql.setString(1, id);
+                preparedLoadSql.setString(2, getName());
+                try (ResultSet rst = preparedLoadSql.executeQuery()) {
+                    if (rst.next()) {
+                        try (ObjectInputStream ois =
+                                getObjectInputStream(rst.getBinaryStream(2))) {
+                            if (contextLog.isDebugEnabled()) {
+                                contextLog.debug(sm.getString(
+                                        getStoreName() + ".loading", id, 
sessionTable));
                             }
-                        } else if (context.getLogger().isDebugEnabled()) {
-                            contextLog.debug(getStoreName() + ": No persisted 
data object found");
+
+                            _session = (StandardSession) 
manager.createEmptySession();
+                            _session.readObjectData(ois);
+                            _session.setManager(manager);
                         }
-                        // Break out after the finally block
-                        numberOfTries = 0;
+                    } else if (context.getLogger().isDebugEnabled()) {
+                        contextLog.debug(getStoreName() + ": No persisted data 
object found");
                     }
-                } catch (SQLException e) {
-                    contextLog.error(sm.getString(getStoreName() + 
".SQLException", e));
-                    if (dbConnection != null)
-                        close(dbConnection);
-                } finally {
-                    context.unbind(Globals.IS_SECURITY_ENABLED, 
oldThreadContextCL);
-                    release(_conn);
+                    // Break out after the finally block
+                    numberOfTries = 0;
                 }
-                numberOfTries--;
+            } catch (SQLException e) {
+                contextLog.error(sm.getString(getStoreName() + 
".SQLException", e));
+            } finally {
+                context.unbind(Globals.IS_SECURITY_ENABLED, 
oldThreadContextCL);
+                release(_conn);
             }
+            numberOfTries--;
         }
-
         return _session;
     }
 
@@ -672,28 +514,24 @@ public class JDBCStore extends StoreBase {
     @Override
     public void remove(String id) throws IOException {
 
-        synchronized (this) {
-            int numberOfTries = 2;
-            while (numberOfTries > 0) {
-                Connection _conn = getConnection();
+        int numberOfTries = 2;
+        while (numberOfTries > 0) {
+            Connection _conn = getConnection();
 
-                if (_conn == null) {
-                    return;
-                }
+            if (_conn == null) {
+                return;
+            }
 
-                try {
-                    remove(id, _conn);
-                    // Break out after the finally block
-                    numberOfTries = 0;
-                } catch (SQLException e) {
-                    
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
-                    if (dbConnection != null)
-                        close(dbConnection);
-                } finally {
-                    release(_conn);
-                }
-                numberOfTries--;
+            try {
+                remove(id, _conn);
+                // Break out after the finally block
+                numberOfTries = 0;
+            } catch (SQLException e) {
+                
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
+            } finally {
+                release(_conn);
             }
+            numberOfTries--;
         }
 
         if (manager.getContext().getLogger().isDebugEnabled()) {
@@ -711,16 +549,14 @@ public class JDBCStore extends StoreBase {
      * @throws SQLException if an error occurs while talking to the database
      */
     private void remove(String id, Connection _conn) throws SQLException {
-        if (preparedRemoveSql == null) {
-            String removeSql = "DELETE FROM " + sessionTable
-                    + " WHERE " + sessionIdCol + " = ?  AND "
-                    + sessionAppCol + " = ?";
-            preparedRemoveSql = _conn.prepareStatement(removeSql);
+        String removeSql = "DELETE FROM " + sessionTable
+                + " WHERE " + sessionIdCol + " = ?  AND "
+                + sessionAppCol + " = ?";
+        try (PreparedStatement preparedRemoveSql = 
_conn.prepareStatement(removeSql)) {
+            preparedRemoveSql.setString(1, id);
+            preparedRemoveSql.setString(2, getName());
+            preparedRemoveSql.execute();
         }
-
-        preparedRemoveSql.setString(1, id);
-        preparedRemoveSql.setString(2, getName());
-        preparedRemoveSql.execute();
     }
 
     /**
@@ -730,35 +566,27 @@ public class JDBCStore extends StoreBase {
      */
     @Override
     public void clear() throws IOException {
+        String clearSql = "DELETE FROM " + sessionTable
+                + " WHERE " + sessionAppCol + " = ?";
+
+        int numberOfTries = 2;
+        while (numberOfTries > 0) {
+            Connection _conn = getConnection();
+            if (_conn == null) {
+                return;
+            }
 
-        synchronized (this) {
-            int numberOfTries = 2;
-            while (numberOfTries > 0) {
-                Connection _conn = getConnection();
-                if (_conn == null) {
-                    return;
-                }
-
-                try {
-                    if (preparedClearSql == null) {
-                        String clearSql = "DELETE FROM " + sessionTable
-                             + " WHERE " + sessionAppCol + " = ?";
-                        preparedClearSql = _conn.prepareStatement(clearSql);
-                    }
-
-                    preparedClearSql.setString(1, getName());
-                    preparedClearSql.execute();
-                    // Break out after the finally block
-                    numberOfTries = 0;
-                } catch (SQLException e) {
-                    
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
-                    if (dbConnection != null)
-                        close(dbConnection);
-                } finally {
-                    release(_conn);
-                }
-                numberOfTries--;
+            try (PreparedStatement preparedClearSql = 
_conn.prepareStatement(clearSql)){
+                preparedClearSql.setString(1, getName());
+                preparedClearSql.execute();
+                // Break out after the finally block
+                numberOfTries = 0;
+            } catch (SQLException e) {
+                
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
+            } finally {
+                release(_conn);
             }
+            numberOfTries--;
         }
     }
 
@@ -771,8 +599,14 @@ public class JDBCStore extends StoreBase {
     @Override
     public void save(Session session) throws IOException {
         ByteArrayOutputStream bos = null;
-
-        synchronized (this) {
+        String saveSql = "INSERT INTO " + sessionTable + " ("
+                + sessionIdCol + ", " + sessionAppCol + ", "
+                + sessionDataCol + ", " + sessionValidCol
+                + ", " + sessionMaxInactiveCol + ", "
+                + sessionLastAccessedCol
+                + ") VALUES (?, ?, ?, ?, ?, ?)";
+
+        synchronized (session) {
             int numberOfTries = 2;
             while (numberOfTries > 0) {
                 Connection _conn = getConnection();
@@ -794,17 +628,8 @@ public class JDBCStore extends StoreBase {
                     byte[] obs = bos.toByteArray();
                     int size = obs.length;
                     try (ByteArrayInputStream bis = new 
ByteArrayInputStream(obs, 0, size);
-                            InputStream in = new BufferedInputStream(bis, 
size)) {
-                        if (preparedSaveSql == null) {
-                            String saveSql = "INSERT INTO " + sessionTable + " 
("
-                               + sessionIdCol + ", " + sessionAppCol + ", "
-                               + sessionDataCol + ", " + sessionValidCol
-                               + ", " + sessionMaxInactiveCol + ", "
-                               + sessionLastAccessedCol
-                               + ") VALUES (?, ?, ?, ?, ?, ?)";
-                           preparedSaveSql = _conn.prepareStatement(saveSql);
-                        }
-
+                            InputStream in = new BufferedInputStream(bis, 
size);
+                            PreparedStatement preparedSaveSql = 
_conn.prepareStatement(saveSql)) {
                         preparedSaveSql.setString(1, session.getIdInternal());
                         preparedSaveSql.setString(2, getName());
                         preparedSaveSql.setBinaryStream(3, in, size);
@@ -817,8 +642,6 @@ public class JDBCStore extends StoreBase {
                     }
                 } catch (SQLException e) {
                     
manager.getContext().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
-                    if (dbConnection != null)
-                        close(dbConnection);
                 } catch (IOException e) {
                     // Ignore
                 } finally {
@@ -872,11 +695,6 @@ public class JDBCStore extends StoreBase {
      * @exception SQLException if a database error occurs
      */
     protected Connection open() throws SQLException {
-
-        // Do nothing if there is a database connection already open
-        if (dbConnection != null)
-            return dbConnection;
-
         if (dataSourceName != null && dataSource == null) {
             org.apache.catalina.Context context = getManager().getContext();
             ClassLoader oldThreadContextCL = null;
@@ -902,34 +720,9 @@ public class JDBCStore extends StoreBase {
 
         if (dataSource != null) {
             return dataSource.getConnection();
+        } else {
+            throw new IllegalStateException(sm.getString(getStoreName() + 
".missingDataSource"));
         }
-
-        // Instantiate our database driver if necessary
-        if (driver == null) {
-            try {
-                Class<?> clazz = Class.forName(driverName);
-                driver = (Driver) clazz.getConstructor().newInstance();
-            } catch (ReflectiveOperationException e) {
-                manager.getContext().getLogger().error(
-                        sm.getString(getStoreName() + 
".checkConnectionClassNotFoundException",
-                        e.toString()));
-                throw new SQLException(e);
-            }
-        }
-
-        // Open a new connection
-        Properties props = new Properties();
-        if (connectionName != null)
-            props.put("user", connectionName);
-        if (connectionPassword != null)
-            props.put("password", connectionPassword);
-        dbConnection = driver.connect(connectionURL, props);
-        if (dbConnection == null) {
-            throw new SQLException(sm.getString(getStoreName() + 
".connectError", connectionURL));
-        }
-        dbConnection.setAutoCommit(true);
-        return dbConnection;
-
     }
 
     /**
@@ -943,41 +736,6 @@ public class JDBCStore extends StoreBase {
         if (dbConnection == null)
             return;
 
-        // Close our prepared statements (if any)
-        try {
-            preparedSizeSql.close();
-        } catch (Throwable f) {
-            ExceptionUtils.handleThrowable(f);
-        }
-        this.preparedSizeSql = null;
-
-        try {
-            preparedSaveSql.close();
-        } catch (Throwable f) {
-            ExceptionUtils.handleThrowable(f);
-        }
-        this.preparedSaveSql = null;
-
-        try {
-            preparedClearSql.close();
-        } catch (Throwable f) {
-            ExceptionUtils.handleThrowable(f);
-        }
-
-        try {
-            preparedRemoveSql.close();
-        } catch (Throwable f) {
-            ExceptionUtils.handleThrowable(f);
-        }
-        this.preparedRemoveSql = null;
-
-        try {
-            preparedLoadSql.close();
-        } catch (Throwable f) {
-            ExceptionUtils.handleThrowable(f);
-        }
-        this.preparedLoadSql = null;
-
         // Commit if autoCommit is false
         try {
             if (!dbConnection.getAutoCommit()) {
@@ -992,10 +750,7 @@ public class JDBCStore extends StoreBase {
             dbConnection.close();
         } catch (SQLException e) {
             manager.getContext().getLogger().error(sm.getString(getStoreName() 
+ ".close", e.toString())); // Just log it here
-        } finally {
-            this.dbConnection = null;
         }
-
     }
 
     /**
@@ -1010,44 +765,4 @@ public class JDBCStore extends StoreBase {
         }
     }
 
-    /**
-     * Start this component and implement the requirements
-     * of {@link org.apache.catalina.util.LifecycleBase#startInternal()}.
-     *
-     * @exception LifecycleException if this component detects a fatal error
-     *  that prevents this component from being used
-     */
-    @Override
-    protected synchronized void startInternal() throws LifecycleException {
-
-        if (dataSourceName == null) {
-            // If not using a connection pool, open a connection to the 
database
-            this.dbConnection = getConnection();
-        }
-
-        super.startInternal();
-    }
-
-    /**
-     * Stop this component and implement the requirements
-     * of {@link org.apache.catalina.util.LifecycleBase#stopInternal()}.
-     *
-     * @exception LifecycleException if this component detects a fatal error
-     *  that prevents this component from being used
-     */
-    @Override
-    protected synchronized void stopInternal() throws LifecycleException {
-
-        super.stopInternal();
-
-        // Close and release everything associated with our db.
-        if (dbConnection != null) {
-            try {
-                dbConnection.commit();
-            } catch (SQLException e) {
-                // Ignore
-            }
-            close(dbConnection);
-        }
-    }
 }
diff --git a/java/org/apache/catalina/session/LocalStrings.properties 
b/java/org/apache/catalina/session/LocalStrings.properties
index dae4f68..eba297f 100644
--- a/java/org/apache/catalina/session/LocalStrings.properties
+++ b/java/org/apache/catalina/session/LocalStrings.properties
@@ -13,19 +13,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-JDBCStore.SQLException=SQL Error [{0}]
-JDBCStore.checkConnectionClassNotFoundException=JDBC driver class not found 
[{0}]
-JDBCStore.checkConnectionDBClosed=The database connection is null or was found 
to be closed. Trying to re-open it.
-JDBCStore.checkConnectionDBReOpenFail=The re-open on the database failed. The 
database could be down.
-JDBCStore.checkConnectionSQLException=A SQL exception occurred [{0}]
-JDBCStore.close=Exception closing database connection [{0}]
-JDBCStore.commitSQLException=SQLException committing connection before closing
-JDBCStore.loading=Loading Session [{0}] from database [{1}]
-JDBCStore.missingDataSourceName=No valid JNDI name was given.
-JDBCStore.removing=Removing Session [{0}] at database [{1}]
-JDBCStore.saving=Saving Session [{0}] to database [{1}]
-JDBCStore.wrongDataSource=Cannot open JNDI DataSource [{0}]
-JDBCStore.connectError=Cannot connect to database [{0}]
+dataSourceStore.SQLException=SQL Error [{0}]
+dataSourceStore.checkConnectionDBClosed=The database connection is null or was 
found to be closed. Trying to re-open it.
+dataSourceStore.checkConnectionDBReOpenFail=The re-open on the database 
failed. The database could be down.
+dataSourceStore.checkConnectionSQLException=A SQL exception occurred [{0}]
+dataSourceStore.close=Exception closing database connection [{0}]
+dataSourceStore.commitSQLException=SQLException committing connection before 
closing
+dataSourceStore.loading=Loading Session [{0}] from database [{1}]
+dataSourceStore.missingDataSource=No data source available
+dataSourceStore.missingDataSourceName=No valid JNDI name was given
+dataSourceStore.removing=Removing Session [{0}] at database [{1}]
+dataSourceStore.saving=Saving Session [{0}] to database [{1}]
+dataSourceStore.wrongDataSource=Cannot open JNDI DataSource [{0}]
 
 fileStore.createFailed=Unable to create directory [{0}] for the storage of 
session data
 fileStore.deleteFailed=Unable to delete file [{0}] which is preventing the 
creation of the session storage location
diff --git a/java/org/apache/catalina/storeconfig/server-registry.xml 
b/java/org/apache/catalina/storeconfig/server-registry.xml
index cdcff27..4dd7dde 100644
--- a/java/org/apache/catalina/storeconfig/server-registry.xml
+++ b/java/org/apache/catalina/storeconfig/server-registry.xml
@@ -180,7 +180,7 @@
         tag="Store"
         standard="false"
         default="false"
-        tagClass="org.apache.catalina.session.JDBCStore"
+        tagClass="org.apache.catalina.session.DataSourceStore"
         storeFactoryClass="org.apache.catalina.storeconfig.StoreFactoryBase">
      </Description>
      <Description
diff --git a/res/findbugs/filter-false-positives.xml 
b/res/findbugs/filter-false-positives.xml
index 4c718a3..e4b6005 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -416,7 +416,7 @@
   <Match>
     <!-- Non-constant strings are configuration settings rather than client
          supplied -->
-    <Class name="org.apache.catalina.session.JDBCStore" />
+    <Class name="org.apache.catalina.session.DataSourceStore" />
     <Or>
       <Method name="clear" />
       <Method name="getSize" />
@@ -429,7 +429,7 @@
   </Match>
   <Match>
     <!-- Syncs aren't intended to protect these fields -->
-    <Class name="org.apache.catalina.session.JDBCStore" />
+    <Class name="org.apache.catalina.session.DataSourceStore" />
     <Or>
       <Field name="dataSourceName" />
       <Field name="sessionAppCol" />
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 57af76f..fd14274 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -96,6 +96,12 @@
         <bug>64871</bug>: Log a warning if Tomcat blocks access to a file
         because it uses symlinks. (markt)
       </add>
+      <update>
+        Rename <code>JDBCStore</code> to <code>DataSourceStore</code>
+        and remove bottlenecks for database backed session store. Legacy JDBC
+        driver configuration is no longer supported. Patch submitted by
+        Philippe Mouawad. (remm)
+      </update>
     </changelog>
   </subsection>
   <subsection name="Coyote">
diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index d70e7b5..93489f8 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -438,11 +438,11 @@
   </attributes>
 
 
-  <h5>JDBC Based Store</h5>
+  <h5>Data source Based Store</h5>
 
-  <p>The <em>JDBC Based Store</em> implementation saves swapped out
+  <p>The <em>Data source Based Store</em> implementation saves swapped out
   sessions in individual rows of a preconfigured table in a database
-  that is accessed via a JDBC driver.  With large numbers of swapped out
+  that is accessed via a data sourcer.  With large numbers of swapped out
   sessions, this implementation will exhibit improved performance over
   the File Based Store described above.</p>
 
@@ -456,41 +456,18 @@
       <p>Java class name of the implementation to use.  This class must
       implement the <code>org.apache.catalina.Store</code> interface.  You
       <strong>must</strong> specify
-      <code>org.apache.catalina.session.JDBCStore</code>
+      <code>org.apache.catalina.session.DataSourceStore</code>
       to use this implementation.</p>
     </attribute>
 
-    <attribute name="connectionName" required="true">
-      <p>The user name that will be handed to the configured JDBC driver to
-      establish a connection to the database containing the session table.</p>
-    </attribute>
-
-    <attribute name="connectionPassword" required="true">
-      <p>The password that will be handed to the configured JDBC driver to
-      establish a connection to the database containing the session table.</p>
-    </attribute>
-
-    <attribute name="connectionURL" required="true">
-      <p>The connection URL that will be handed to the configured JDBC
-      driver to establish a connection to the database containing our
-      session table.</p>
-    </attribute>
-
     <attribute name="dataSourceName" required="false">
-      <p>Name of the JNDI resource for a JDBC DataSource-factory. If this 
option
-      is given and a valid JDBC resource can be found, it will be used and any
-      direct configuration of a JDBC connection via <code>connectionURL</code>,
-      <code>connectionName</code>, <code>connectionPassword</code> and
-      <code>driverName</code> will be ignored. Since this code uses prepared
-      statements, you might want to configure pooled prepared statements as
-      shown in <a href="../jndi-resources-howto.html">the JNDI resources
+      <p>Name of the JNDI resource for a JDBC DataSource-factory. Since this
+      code uses prepared statements, you might want to configure pooled
+      prepared statements as shown in
+       <a href="../jndi-resources-howto.html">the JNDI resources
       How-To</a>.</p>
     </attribute>
 
-    <attribute name="driverName" required="true">
-      <p>Java class name of the JDBC driver to be used.</p>
-    </attribute>
-
     <attribute name="localDataSource" required="false">
       <p>This allows the Store to use a DataSource defined for the Context
       rather than a global DataSource. If not specified, the default is
@@ -552,7 +529,7 @@
 
   </attributes>
 
-  <p>Before attempting to use the JDBC Based Store for the first time,
+  <p>Before attempting to use the data source Store for the first time,
   you must create the table that will be used to store swapped out sessions.
   Detailed SQL commands vary depending on the database you are using, but
   a script like this will generally be required:</p>
@@ -568,14 +545,8 @@
 );</source>
 
   <p>Note: The SQL command above does not use the default names for either the
-  table or the columns so the JDBC Store would need to be configured to reflect
-  this.</p>
-
-  <p>In order for the JDBC Based Store to successfully connect to your
-  database, the JDBC driver you configure must be visible to Tomcat's
-  internal class loader.  Generally, that means you must place the JAR
-  file containing this driver into the <code>$CATALINA_HOME/lib</code>
-  directory.</p>
+  table or the columns so the data source Store would need to be configured
+  to reflect this.</p>
 
 </section>
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to