[java-plugin-storage-jdbc] branch main updated: JJDBC-25 Statements are not closed after execution in all cases
Rod Widdowson
rdw at steadingsoftware.com
Fri Aug 16 13:48:13 UTC 2024
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-plugin-storage-jdbc.
View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-storage-jdbc.git;a=commit;h=b558c76467908ed7d1cf96addfddeb8e4bcdd279
The following commit(s) were added to refs/heads/main by this push:
new b558c76 JJDBC-25 Statements are not closed after execution in all cases
b558c76 is described below
commit b558c76467908ed7d1cf96addfddeb8e4bcdd279
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Aug 16 14:46:14 2024 +0100
JJDBC-25 Statements are not closed after execution in all cases
https://shibboleth.atlassian.net/browse/JJDBC-25
Added try-with around all:
java.sql.Connection;
java.sql.PreparedStatement;
java.sql.ResultSet;
---
.../storage/jdbc/impl/JDBCStorageService.java | 306 +++++++++++----------
1 file changed, 162 insertions(+), 144 deletions(-)
diff --git a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
index 61eadc2..f5ecaca 100644
--- a/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
+++ b/jdbc-storage-impl/src/main/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageService.java
@@ -1,3 +1,4 @@
+// Checkstyle: FileLength|Header OFF
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -474,15 +475,16 @@ public final class JDBCStorageService extends AbstractStorageService
final List<String> result = new ArrayList<>();
try (final ConnectionWithLock connection= new ConnectionWithLock(true, false)) {
log.trace("ReadContexts:: ", readContextsSQL);
- final PreparedStatement query = connection.prepareStatement(readContextsSQL);
- final ResultSet results = query.executeQuery();
- while (results.next()) {
- final String context = results.getString(1);
- log.trace("Context = '{}'", context);
- result.add(context);
+ try (final PreparedStatement query = connection.prepareStatement(readContextsSQL);
+ final ResultSet results = query.executeQuery()) {
+
+ while (results.next()) {
+ final String context = results.getString(1);
+ log.trace("Context = '{}'", context);
+ result.add(context);
+ }
}
return result;
-
} catch (final SQLException e) {
log.error("ReadContexts failed", e);
throw new IOException(e);
@@ -497,10 +499,11 @@ public final class JDBCStorageService extends AbstractStorageService
*/
@Nonnull @NonnullElements protected List<?> readAll() throws IOException {
final List<JDBCStorageRecord<?>> result = new ArrayList<>();
- try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, false);
+ final PreparedStatement query = connection.prepareStatement(readAllSQL);
+ final ResultSet results = query.executeQuery()) {
+
log.trace("ReadAll:: '{}' ", readAllSQL);
- final PreparedStatement query = connection.prepareStatement(readAllSQL);
- final ResultSet results = query.executeQuery();
while (results.next()) {
final String context = results.getString(1);
final String id = results.getString(2);
@@ -511,8 +514,7 @@ public final class JDBCStorageService extends AbstractStorageService
context, id, value, version, expires == null ? "<never>": expires);
result.add(new JDBCStorageRecord<>(value, expires, version));
}
- return result;
-
+ return result;
} catch (final SQLException e) {
log.error("ReadAll failed", e);
throw new IOException(e);
@@ -532,19 +534,20 @@ public final class JDBCStorageService extends AbstractStorageService
final List<JDBCStorageRecord<?>> result = new ArrayList<>();
Constraint.isNotEmpty(Constraint.isNotNull(context, "ReadAll(String): context must not be null"),
"ReadAll(String): context must not be empty");
- try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, false);
+ final PreparedStatement query = connection.prepareStatement(readAllByContextSQL)) {
log.trace("ReadAll:: '{}' 1: '{}' ", readAllByContextSQL, context);
- final PreparedStatement query = connection.prepareStatement(readAllByContextSQL);
query.setString(1, context);
- final ResultSet results = query.executeQuery();
- while (results.next()) {
- final String id = results.getString(1);
- final Long expires = getExpires(results, 2);
- final String value = Constraint.isNotNull(results.getString(3), "value field must not be null");
- final Long version = results.getLong(4);
- log.trace("Record: Id = '{}', value = '{}', verion = '{}', expires = '{}'",
- id, value, version, expires == null ? "<never>": expires);
- result.add(new JDBCStorageRecord<>(value, expires, version));
+ try (final ResultSet results = query.executeQuery()) {
+ while (results.next()) {
+ final String id = results.getString(1);
+ final Long expires = getExpires(results, 2);
+ final String value = Constraint.isNotNull(results.getString(3), "value field must not be null");
+ final Long version = results.getLong(4);
+ log.trace("Record: Id = '{}', value = '{}', verion = '{}', expires = '{}'",
+ id, value, version, expires == null ? "<never>": expires);
+ result.add(new JDBCStorageRecord<>(value, expires, version));
+ }
}
return result;
@@ -573,37 +576,40 @@ public final class JDBCStorageService extends AbstractStorageService
// If so check expiration
// If not expired complain
// otherwise update
- final PreparedStatement query = connection.prepareStatement(preCreateQuerySQL);
- query.setString(1, context);
- query.setString(2, key);
- final ResultSet resultSet = query.executeQuery();
- if (!resultSet.next()) {
- log.trace("Create [Insert]:: '{}' 1: '{}' ; 2: '{}' ; 3 '{}' ; 4 '{}'", createCreateRecordSQL,
- context, key, expiration, value);
- final PreparedStatement insert = connection.prepareStatement(createCreateRecordSQL);
- insert.setString(1, context);
- insert.setString(2, key);
- setExpires(insert, 3, expiration);
- insert.setString(4,value);
- insert.executeUpdate();
+ try (final PreparedStatement query = connection.prepareStatement(preCreateQuerySQL)) {
+ query.setString(1, context);
+ query.setString(2, key);
+ try (final ResultSet resultSet = query.executeQuery();) {
+ if (!resultSet.next()) {
+ log.trace("Create [Insert]:: '{}' 1: '{}' ; 2: '{}' ; 3 '{}' ; 4 '{}'",
+ createCreateRecordSQL, context, key, expiration, value);
+ final PreparedStatement insert = connection.prepareStatement(createCreateRecordSQL);
+ insert.setString(1, context);
+ insert.setString(2, key);
+ setExpires(insert, 3, expiration);
+ insert.setString(4,value);
+ insert.executeUpdate();
+ connection.commit();
+ return true;
+ }
+ final Long returnedExpiration = getExpires(resultSet, 1);
+ if (returnedExpiration == null || System.currentTimeMillis() < returnedExpiration) {
+ log.debug("Duplicate record '{}' in context '{}'", key, context);
+ return false;
+ }
+ }
+ }
+ try (final PreparedStatement update = connection.prepareStatement(createUpdateRecordSQL)) {
+ log.trace("Create [Update]:: '{}' 1: '{}' ; 2: '{}' ; 3 '{}' ; 4 '{}'", createUpdateRecordSQL,
+ value, expiration, context, key);
+ update.setString(1, value);
+ setExpires(update, 2, expiration);
+ update.setString(3,context);
+ update.setString(4,key);
+ update.executeUpdate();
connection.commit();
return true;
}
- final Long returnedExpiration = getExpires(resultSet, 1);
- if (returnedExpiration == null || System.currentTimeMillis() < returnedExpiration) {
- log.debug("Duplicate record '{}' in context '{}'", key, context);
- return false;
- }
- final PreparedStatement update = connection.prepareStatement(createUpdateRecordSQL);
- log.trace("Create [Update]:: '{}' 1: '{}' ; 2: '{}' ; 3 '{}' ; 4 '{}'", createUpdateRecordSQL,
- value, expiration, context, key);
- update.setString(1, value);
- setExpires(update, 2, expiration);
- update.setString(3,context);
- update.setString(4,key);
- update.executeUpdate();
- connection.commit();
- return true;
} catch (final SQLException e) {
boolean retry = false;
for (final String msg : retryableErrors) {
@@ -664,36 +670,38 @@ public final class JDBCStorageService extends AbstractStorageService
int retries = transactionRetries;
while(true) {
- try (final ConnectionWithLock connection = new ConnectionWithLock(true, false)) {
- final PreparedStatement stmnt = connection.prepareStatement(readRecordSQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, false);
+ final PreparedStatement stmnt = connection.prepareStatement(readRecordSQL)) {
+
log.trace("Read:: '{}' 1: '{}' ; 2: '{}'", readRecordSQL, context, key);
stmnt.setString(1, context);
stmnt.setString(2, key);
- final ResultSet resultSet = stmnt.executeQuery();
- if (!resultSet.next()) {
- log.debug("Nothing returned");
- return new Pair<>();
- }
- final Long returnedVersion = resultSet.getLong(1);
- final Long returnedExpires = getExpires(resultSet, 2);
- final String returnedValue =
- Constraint.isNotNull(resultSet.getString(3), "value field must not be null");
- log.trace("Considering Version '{}', Expires '{}', Value '{}'",
- returnedVersion, returnedValue, returnedExpires);
- if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
- log.debug("Read failed, key '{}' expired in context '{}'", key, context);
- return new Pair<>();
- }
- if (version != null && returnedVersion == version) {
- // Nothing's changed, so just echo back the version.
- return new Pair<>(version, null);
- }
- if (resultSet.next()) {
- log.error("Multiple values returned?");
+ try (final ResultSet resultSet = stmnt.executeQuery()) {
+ if (!resultSet.next()) {
+ log.debug("Nothing returned");
+ return new Pair<>();
+ }
+ final Long returnedVersion = resultSet.getLong(1);
+ final Long returnedExpires = getExpires(resultSet, 2);
+ final String returnedValue =
+ Constraint.isNotNull(resultSet.getString(3), "value field must not be null");
+ log.trace("Considering Version '{}', Expires '{}', Value '{}'",
+ returnedVersion, returnedValue, returnedExpires);
+ if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
+ log.debug("Read failed, key '{}' expired in context '{}'", key, context);
+ return new Pair<>();
+ }
+ if (version != null && returnedVersion == version) {
+ // Nothing's changed, so just echo back the version.
+ return new Pair<>(version, null);
+ }
+ if (resultSet.next()) {
+ log.error("Multiple values returned?");
+ }
+ final MutableStorageRecord<T> result =
+ new JDBCStorageRecord<>(returnedValue, returnedExpires, returnedVersion);
+ return new Pair<>(version, result);
}
- final MutableStorageRecord<T> result =
- new JDBCStorageRecord<>(returnedValue, returnedExpires, returnedVersion);
- return new Pair<>(version, result);
} catch (final SQLException e) {
boolean retry = false;
for (final String msg : retryableErrors) {
@@ -778,47 +786,50 @@ public final class JDBCStorageService extends AbstractStorageService
"update: key must not be empty");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
- final PreparedStatement selectStmnt = connection.prepareStatement(preUpdateQuerySQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(false, true);
+ final PreparedStatement selectStmnt = connection.prepareStatement(preUpdateQuerySQL)) {
+
log.trace("Update [Query]:: '{}' 1: '{}' ; 2: '{}'", preUpdateQuerySQL, context, key);
selectStmnt.setString(1, context);
selectStmnt.setString(2, key);
- final ResultSet resultSet = selectStmnt.executeQuery();
- if (!resultSet.next()) {
- log.debug("Nothing returned");
- return null;
- }
- final Long returnedExpires = getExpires(resultSet, 2);
- final Long returnedVersion = resultSet.getLong(1);
- final String updateValue;
- if (value == null) {
- updateValue = resultSet.getString(3);
- } else {
- updateValue = value;
- }
- if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
- log.debug("Update failed, key '{}' expired in context '{}'", key, context);
- return null;
- }
-
- if (version != null && returnedVersion != version) {
- // Caller is out of sync.
- throw new VersionMismatchException();
+ try (final ResultSet resultSet = selectStmnt.executeQuery()) {
+ if (!resultSet.next()) {
+ log.debug("Nothing returned");
+ return null;
+ }
+ final Long returnedExpires = getExpires(resultSet, 2);
+ final Long returnedVersion = resultSet.getLong(1);
+ final String updateValue;
+ if (value == null) {
+ updateValue = resultSet.getString(3);
+ } else {
+ updateValue = value;
+ }
+ if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
+ log.debug("Update failed, key '{}' expired in context '{}'", key, context);
+ return null;
+ }
+
+ if (version != null && returnedVersion != version) {
+ // Caller is out of sync.
+ throw new VersionMismatchException();
+ }
+ try (final PreparedStatement updateStmnt = connection.prepareStatement(updateRecordSQL)) {
+ final Long newVersion = Long.valueOf(returnedVersion + 1);
+ log.trace("Update [Update]:: '{}': 1: '{}' ; 2: '{}' ; 3: '{}' ; 4: '{}' ; 5: '{}'",
+ updateRecordSQL, value, newVersion, expires, context, key);
+ updateStmnt.setString(1, updateValue);
+ updateStmnt.setLong(2, newVersion);
+ setExpires(updateStmnt, 3, expires);
+ updateStmnt.setString(4, context);
+ updateStmnt.setString(5, key);
+ updateStmnt.executeUpdate();
+ connection.commit();
+ return newVersion;
+ }
}
- final PreparedStatement updateStmnt = connection.prepareStatement(updateRecordSQL);
- final Long newVersion = Long.valueOf(returnedVersion + 1);
- log.trace("Update [Update]:: '{}': 1: '{}' ; 2: '{}' ; 3: '{}' ; 4: '{}' ; 5: '{}'", updateRecordSQL,
- value, newVersion, expires, context, key);
- updateStmnt.setString(1, updateValue);
- updateStmnt.setLong(2, newVersion);
- setExpires(updateStmnt, 3, expires);
- updateStmnt.setString(4, context);
- updateStmnt.setString(5, key);
- updateStmnt.executeUpdate();
- connection.commit();
- return newVersion;
} catch (final SQLException e) {
boolean retry = false;
for (final String msg : retryableErrors) {
@@ -882,27 +893,29 @@ public final class JDBCStorageService extends AbstractStorageService
Constraint.isTrue(version == null || version > 0, "delete: version should be null of > 0");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection= new ConnectionWithLock(false, true)) {
- final PreparedStatement selectStmnt = connection.prepareStatement(preDeleteQuerySQL);
+ try (final ConnectionWithLock connection= new ConnectionWithLock(false, true);
+ final PreparedStatement selectStmnt = connection.prepareStatement(preDeleteQuerySQL)) {
selectStmnt.setString(1, context);
selectStmnt.setString(2, key);
log.trace("Delete [Query]:: '{}': 1: '{}' ; 2: '{}'", preDeleteQuerySQL, context, key);
- final ResultSet resultSet = selectStmnt.executeQuery();
- if (!resultSet.next()) {
- log.debug("Nothing returned");
- return false;
+ try (final ResultSet resultSet = selectStmnt.executeQuery()) {
+ if (!resultSet.next()) {
+ log.debug("Nothing returned");
+ return false;
+ }
+ final Long returnedVersion = resultSet.getLong(1);
+ if (version != null && returnedVersion != version) {
+ throw new VersionMismatchException();
+ }
}
- final Long returnedVersion = resultSet.getLong(1);
- if (version != null && returnedVersion != version) {
- throw new VersionMismatchException();
+ try (final PreparedStatement deleteStmnt = connection.prepareStatement(deleteRecordSQL)) {
+ log.trace("Delete [Delete]:: '{}': 1: '{}' ; 2: '{}'", deleteRecordSQL, context, key);
+ deleteStmnt.setString(1, context);
+ deleteStmnt.setString(2, key);
+ deleteStmnt.execute();
+ connection.commit();
+ return true;
}
- final PreparedStatement deleteStmnt = connection.prepareStatement(deleteRecordSQL);
- log.trace("Delete [Delete]:: '{}': 1: '{}' ; 2: '{}'", deleteRecordSQL, context, key);
- deleteStmnt.setString(1, context);
- deleteStmnt.setString(2, key);
- deleteStmnt.execute();
- connection.commit();
- return true;
} catch (final SQLException e) {
boolean retry = false;
for (final String msg : retryableErrors) {
@@ -938,8 +951,9 @@ public final class JDBCStorageService extends AbstractStorageService
Constraint.isNotNull(expiration, "expiration: context must not be null");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
- final PreparedStatement updateStmnt = connection.prepareStatement(deleteByExpiredSQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(false, true);
+ final PreparedStatement updateStmnt = connection.prepareStatement(deleteByExpiredSQL)) {
+
updateStmnt.setLong(1, expiration);
log.trace("DeleteByExpired:: '{}': 1: '{}' ;", deleteByExpiredSQL, expiration);
updateStmnt.execute();
@@ -974,8 +988,9 @@ public final class JDBCStorageService extends AbstractStorageService
"reap: context must not be empty");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
- final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextExpiredSQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
+ final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextExpiredSQL)) {
+
updateStmnt.setString(1, context);
final Long expires = System.currentTimeMillis();
setExpires(updateStmnt, 2, expires);
@@ -1011,8 +1026,9 @@ public final class JDBCStorageService extends AbstractStorageService
"updateContextExpiration: context must not be empty");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
- final PreparedStatement updateStmnt = connection.prepareStatement(updateExpiresByContextSQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
+ final PreparedStatement updateStmnt = connection.prepareStatement(updateExpiresByContextSQL)) {
+
assert updateStmnt!=null;
setExpires(updateStmnt, 1, expires);
updateStmnt.setString(2, context);
@@ -1050,8 +1066,9 @@ public final class JDBCStorageService extends AbstractStorageService
"deleteContext: context must not be empty");
int retries = transactionRetries;
while (true) {
- try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
- final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextSQL);
+ try (final ConnectionWithLock connection = new ConnectionWithLock(true, true);
+ final PreparedStatement updateStmnt = connection.prepareStatement(deleteByContextSQL)) {
+
updateStmnt.setString(1, context);
log.trace("UpdateContextExpiration:: '{}': 1: '{}'", deleteByContextSQL, context);
@@ -1086,9 +1103,10 @@ public final class JDBCStorageService extends AbstractStorageService
final List<String> result = new ArrayList<>();
- try (final ConnectionWithLock connection= new ConnectionWithLock(true, false)) {
- final PreparedStatement query =
- connection.prepareStatement(prefix != null ? getContextKeysWithPrefixSQL : getContextKeysSQL);
+ try (final ConnectionWithLock connection= new ConnectionWithLock(true, false);
+ final PreparedStatement query =
+ connection.prepareStatement(prefix != null ? getContextKeysWithPrefixSQL : getContextKeysSQL)) {
+
query.setString(1, context);
final Long now = System.currentTimeMillis();
@@ -1103,11 +1121,11 @@ public final class JDBCStorageService extends AbstractStorageService
log.trace("GetContextKeys:: '{}': 1: '{}', 2: '{}'", getContextKeysSQL, context, now);
}
- final ResultSet results = query.executeQuery();
-
- while (results.next()) {
- final String key = results.getString(1);
- result.add(key);
+ try (final ResultSet results = query.executeQuery()) {
+ while (results.next()) {
+ final String key = results.getString(1);
+ result.add(key);
+ }
}
return result;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list