This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new a727069c32 Refactor DataSourceStore to reduce duplication around
duplication
a727069c32 is described below
commit a727069c32f6d2046ef80ab7bd9be141d8560996
Author: Mark Thomas <[email protected]>
AuthorDate: Tue May 26 14:00:44 2026 +0100
Refactor DataSourceStore to reduce duplication around duplication
Also addresses a regression the previous refactoring that meant an
exception was thrown if the first attempt failed but the second
succeeded.
---
.../apache/catalina/session/DataSourceStore.java | 263 ++++++++-------------
1 file changed, 104 insertions(+), 159 deletions(-)
diff --git a/java/org/apache/catalina/session/DataSourceStore.java
b/java/org/apache/catalina/session/DataSourceStore.java
index 159318ba8a..26d740168e 100644
--- a/java/org/apache/catalina/session/DataSourceStore.java
+++ b/java/org/apache/catalina/session/DataSourceStore.java
@@ -352,101 +352,65 @@ public class DataSourceStore extends StoreBase {
*
* @return array containing the list of session IDs
*/
- private String[] keys(boolean expiredOnly) {
- String[] keys = null;
- int numberOfTries = 2;
- while (numberOfTries > 0) {
-
- Connection _conn = getConnection();
- if (_conn == null) {
- return new String[0];
- }
- try {
+ private String[] keys(boolean expiredOnly) throws IOException {
+ String sqlTmp = "SELECT " + sessionIdCol + " FROM " + sessionTable + "
WHERE " + sessionAppCol + " = ?";
+ if (expiredOnly) {
+ sqlTmp += " AND (" + sessionLastAccessedCol + " + " +
sessionMaxInactiveCol + " * 1000 < ?)";
+ }
+ final String keysSql = sqlTmp;
- String keysSql =
- "SELECT " + sessionIdCol + " FROM " + sessionTable + "
WHERE " + sessionAppCol + " = ?";
+ String[] keys = withRetry((ConnectionOperation<String[],IOException>)
conn -> {
+ 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;
}
+ return tmpkeys.toArray(new String[0]);
}
- } catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"),
e);
- keys = new String[0];
- // Close the connection so that it gets reopened next time
- } finally {
- release(_conn);
}
- numberOfTries--;
- }
- return keys;
+ });
+
+ return keys == null ? new String[0] : keys;
}
@Override
public int getSize() throws IOException {
- int size = 0;
String sizeSql = "SELECT COUNT(" + sessionIdCol + ") FROM " +
sessionTable + " WHERE " + sessionAppCol + " = ?";
- int numberOfTries = 2;
- while (numberOfTries > 0) {
- Connection _conn = getConnection();
-
- if (_conn == null) {
- return size;
- }
-
- try (PreparedStatement preparedSizeSql =
_conn.prepareStatement(sizeSql)) {
+ Integer size = withRetry((ConnectionOperation<Integer,IOException>)
conn -> {
+ try (PreparedStatement preparedSizeSql =
conn.prepareStatement(sizeSql)) {
preparedSizeSql.setString(1, getName());
try (ResultSet rst = preparedSizeSql.executeQuery()) {
if (rst.next()) {
- size = rst.getInt(1);
+ return Integer.valueOf(rst.getInt(1));
+ } else {
+ return Integer.valueOf(0);
}
- // Break out after the finally block
- numberOfTries = 0;
}
- } catch (SQLException e) {
-
manager.getContext().getLogger().error(sm.getString("dataSourceStore.SQLException"),
e);
- } finally {
- release(_conn);
}
- numberOfTries--;
- }
- return size;
+ });
+
+ return size == null ? 0 : size.intValue();
}
@Override
public Session load(String id) throws ClassNotFoundException, IOException {
- StandardSession _session = null;
org.apache.catalina.Context context = getManager().getContext();
Log contextLog = context.getLogger();
-
- 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;
- }
+ Session session =
withRetry((ConnectionOperation<StandardSession,ClassNotFoundException>) conn ->
{
ClassLoader oldThreadContextCL = context.bind(null);
- try (PreparedStatement preparedLoadSql =
_conn.prepareStatement(loadSql)) {
+ try (PreparedStatement preparedLoadSql =
conn.prepareStatement(loadSql)) {
preparedLoadSql.setString(1, id);
preparedLoadSql.setString(2, getName());
try (ResultSet rst = preparedLoadSql.executeQuery()) {
@@ -456,58 +420,29 @@ public class DataSourceStore extends StoreBase {
contextLog.trace(sm.getString("dataSourceStore.loading", id, sessionTable));
}
- _session = (StandardSession)
manager.createEmptySession();
+ StandardSession _session = (StandardSession)
manager.createEmptySession();
_session.readObjectData(ois);
_session.setManager(manager);
+ return _session;
}
} else if (context.getLogger().isDebugEnabled()) {
contextLog.debug(sm.getString("dataSourceStore.noObject", id));
}
- // Break out after the finally block
- numberOfTries = 0;
+ return null;
}
- } catch (SQLException e) {
- contextLog.error(sm.getString("dataSourceStore.SQLException"),
e);
} finally {
context.unbind(oldThreadContextCL);
- release(_conn);
}
- numberOfTries--;
- }
- return _session;
+ });
+ return session;
}
@Override
public void remove(String id) throws IOException {
-
- SQLException sqlException = null;
-
- int numberOfTries = 2;
- while (numberOfTries > 0) {
- Connection _conn = getConnection();
-
- if (_conn == null) {
- return;
- }
-
- try {
- remove(id, _conn);
- // Break out after the finally block
- numberOfTries = 0;
- } catch (SQLException e) {
- // Keep the first exception just in case
- if (sqlException == null) {
- sqlException = e;
- }
- } finally {
- release(_conn);
- }
- numberOfTries--;
- }
-
- if (sqlException != null) {
- throw new
IOException(sm.getString("dataSourceStore.SQLException"), sqlException);
- }
+ withRetry(conn -> {
+ remove(id, conn);
+ return null;
+ });
if (manager.getContext().getLogger().isTraceEnabled()) {
manager.getContext().getLogger().trace(sm.getString("dataSourceStore.removing",
id, sessionTable));
@@ -537,25 +472,13 @@ public class DataSourceStore extends StoreBase {
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;
- }
-
- try (PreparedStatement preparedClearSql =
_conn.prepareStatement(clearSql)) {
+ withRetry(conn -> {
+ 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("dataSourceStore.SQLException"),
e);
- } finally {
- release(_conn);
}
- numberOfTries--;
- }
+ return null;
+ });
}
@Override
@@ -563,7 +486,6 @@ public class DataSourceStore extends StoreBase {
String saveSql = "INSERT INTO " + sessionTable + " (" + sessionIdCol +
", " + sessionAppCol + ", " +
sessionDataCol + ", " + sessionValidCol + ", " +
sessionMaxInactiveCol + ", " + sessionLastAccessedCol +
") VALUES (?, ?, ?, ?, ?, ?)";
- SQLException sqlException = null;
synchronized (session) {
@@ -574,46 +496,24 @@ public class DataSourceStore extends StoreBase {
}
byte[] obs = bos.toByteArray();
- int numberOfTries = 2;
- while (numberOfTries > 0) {
- Connection _conn = getConnection();
- if (_conn == null) {
- return;
- }
-
- try {
- // Remove session if it exists and insert again.
- remove(session.getIdInternal(), _conn);
-
- int size = obs.length;
- try (ByteArrayInputStream bis = new
ByteArrayInputStream(obs, 0, size);
- 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);
- preparedSaveSql.setString(4, session.isValid() ? "1" :
"0");
- preparedSaveSql.setInt(5,
session.getMaxInactiveInterval());
- preparedSaveSql.setLong(6,
session.getLastAccessedTime());
- preparedSaveSql.execute();
- // Break out after the finally block
- numberOfTries = 0;
- sqlException = null;
- }
- } catch (SQLException e) {
- // Keep the first exception just in case
- if (sqlException == null) {
- sqlException = e;
- }
- } finally {
- release(_conn);
+ withRetry(conn -> {
+ // Remove session if it exists and insert again.
+ remove(session.getIdInternal(), conn);
+
+ int size = obs.length;
+ try (ByteArrayInputStream bis = new ByteArrayInputStream(obs,
0, size);
+ 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);
+ preparedSaveSql.setString(4, session.isValid() ? "1" :
"0");
+ preparedSaveSql.setInt(5,
session.getMaxInactiveInterval());
+ preparedSaveSql.setLong(6, session.getLastAccessedTime());
+ preparedSaveSql.execute();
}
- numberOfTries--;
- }
- }
-
- if (sqlException != null) {
- throw new
IOException(sm.getString("dataSourceStore.SQLException"), sqlException);
+ return null;
+ });
}
if (manager.getContext().getLogger().isTraceEnabled()) {
@@ -743,4 +643,49 @@ public class DataSourceStore extends StoreBase {
}
}
+
+ private <T, E extends Exception> T withRetry(ConnectionOperation<T,E>
operation) throws IOException, E {
+ SQLException sqlException = null;
+
+ int numberOfTries = 2;
+ while (numberOfTries > 0) {
+ /*
+ * TODO: To further improve consistency, consider refactoring
getConnection so an IOException is thrown here
+ * with a nested SQLException if a connection cannot be obtained.
This would also allow some of the null
+ * handling on return to be removed.
+ */
+ Connection _conn = getConnection();
+ if (_conn == null) {
+ return null;
+ }
+
+ try {
+ return operation.execute(_conn);
+ } catch (SQLException e) {
+ // Retain the first exception to use as the cause if all
retries fail
+ if (sqlException == null) {
+ sqlException = e;
+ }
+ } finally {
+ release(_conn);
+ }
+ numberOfTries--;
+ }
+
+ throw new IOException(sm.getString("dataSourceStore.SQLException"),
sqlException);
+ }
+
+
+ /**
+ * Functional interface for store operation. Used with {@link
DataSourceStore#withRetry(ConnectionOperation)} to
+ * reduce code duplication.
+ *
+ * @param <T> The return type for the operation
+ * @param <E> The additional exception type thrown by this operation. If
no additional exception type is thrown then
+ * specify IOException
+ */
+ @FunctionalInterface
+ private interface ConnectionOperation<T, E extends Exception> {
+ T execute(Connection connection) throws IOException, SQLException, E;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
