[java-plugin-storage-jdbc] branch main updated: JJDBC-30 Incomplete SQL transaction in create()

Rod Widdowson rdw at steadingsoftware.com
Fri Sep 20 14:04:43 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=619c96d9d17a7e429ea171b97d450567d29d24d3

The following commit(s) were added to refs/heads/main by this push:
     new 619c96d  JJDBC-30 Incomplete SQL transaction in create()
619c96d is described below

commit 619c96d9d17a7e429ea171b97d450567d29d24d3
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Sep 20 15:01:36 2024 +0100

    JJDBC-30 Incomplete SQL transaction in create()
    
    https://shibboleth.atlassian.net/browse/JJDBC-30
    
    Add explicit rollback() to connections in paths where we return false (and have not written
    anything to the database).  This happens *even if the connection was marked autocommit*.
    
    Add explicit commit() to some to autocommit connections.
    
    Neither of these should have any semantic difference but some Pooling connectors seem to
    expect this
---
 .../storage/jdbc/impl/JDBCStorageService.java      | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

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 b8ba682..35fd5ee 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
@@ -484,6 +484,7 @@ public final class JDBCStorageService extends AbstractStorageService
                     result.add(context);
                 }
             }
+            connection.rollback();
             return result;
         } catch (final SQLException e) {
             log.error("ReadContexts failed", e);
@@ -514,6 +515,7 @@ public final class JDBCStorageService extends AbstractStorageService
                           context, id, value, version, expires == null ? "<never>": expires);
                 result.add(new JDBCStorageRecord<>(value, expires, version));
             }
+            connection.rollback();
             return result;            
         } catch (final SQLException e) {
             log.error("ReadAll failed", e);
@@ -549,6 +551,7 @@ public final class JDBCStorageService extends AbstractStorageService
                     result.add(new JDBCStorageRecord<>(value, expires, version));
                 }
             }
+            connection.rollback();
             return result;
             
         } catch (final SQLException e) {
@@ -595,6 +598,7 @@ public final class JDBCStorageService extends AbstractStorageService
                         final Long returnedExpiration = getExpires(resultSet, 1);
                         if (returnedExpiration == null || System.currentTimeMillis() < returnedExpiration) {
                             log.debug("Duplicate record '{}' in context '{}'", key, context);
+                            connection.rollback();
                             return false;
                         }
                     }
@@ -679,6 +683,7 @@ public final class JDBCStorageService extends AbstractStorageService
                 try (final ResultSet resultSet = stmnt.executeQuery()) {
                     if (!resultSet.next()) {
                         log.debug("Nothing returned");
+                        connection.rollback();
                         return new Pair<>();
                     }
                     final Long returnedVersion = resultSet.getLong(1);
@@ -689,6 +694,7 @@ public final class JDBCStorageService extends AbstractStorageService
                               returnedVersion, returnedExpires, returnedValue);
                     if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
                         log.debug("Read failed, key '{}' expired in context '{}'", key, context);
+                        connection.rollback();
                         return new Pair<>();
                     }
                     if (version != null && version.equals(returnedVersion)) {
@@ -700,6 +706,7 @@ public final class JDBCStorageService extends AbstractStorageService
                     }
                     final MutableStorageRecord<T> result =
                         new JDBCStorageRecord<>(returnedValue, returnedExpires, returnedVersion);
+                    connection.rollback();
                     return new Pair<>(version, result);
                 }
             } catch (final SQLException e) {
@@ -796,6 +803,7 @@ public final class JDBCStorageService extends AbstractStorageService
 
                 try (final ResultSet resultSet = selectStmnt.executeQuery()) {
                     if (!resultSet.next()) {
+                        connection.rollback();
                         log.debug("Nothing returned");
                         return null;
                     }
@@ -808,6 +816,7 @@ public final class JDBCStorageService extends AbstractStorageService
                         updateValue = value;
                     }
                     if (returnedExpires != null && System.currentTimeMillis() >= returnedExpires) {
+                        connection.rollback();
                         log.debug("Update failed, key '{}' expired in context '{}'", key, context);
                         return null;
                     }
@@ -900,10 +909,12 @@ public final class JDBCStorageService extends AbstractStorageService
                 try (final ResultSet resultSet = selectStmnt.executeQuery()) {
                     if (!resultSet.next()) {
                         log.debug("Nothing returned");
+                        connection.rollback();
                         return false;
                     }
                     final Long returnedVersion = resultSet.getLong(1);
                     if (version != null && !version.equals(returnedVersion)) {
+                        connection.rollback();
                         throw new VersionMismatchException();
                     }
                 }
@@ -995,6 +1006,7 @@ public final class JDBCStorageService extends AbstractStorageService
                 setExpires(updateStmnt, 2, expires);
                 log.trace("Reap:: '{}':  1: '{}' ; 2: '{}' ;", deleteByContextExpiredSQL, context, expires);
                 updateStmnt.execute();
+                connection.commit();
                 return;
             } catch (final SQLException e) {
                 boolean retry = false;
@@ -1036,6 +1048,7 @@ public final class JDBCStorageService extends AbstractStorageService
                 log.trace("UpdateContextExpiration:: '{}':  1: '{}' ; 2: '{}' ; 3: '{}' ;",
                           updateExpiresByContextSQL, expires, context, newExpires);
                 updateStmnt.execute();
+                connection.commit();
                 return;
             } catch (final SQLException e) {
                 boolean retry = false;
@@ -1072,6 +1085,7 @@ public final class JDBCStorageService extends AbstractStorageService
                 log.trace("UpdateContextExpiration:: '{}': 1: '{}'", deleteByContextSQL, context);
 
                 updateStmnt.execute();
+                connection.commit();
                 return;
             } catch (final SQLException e) {
                 boolean retry = false;
@@ -1126,6 +1140,7 @@ public final class JDBCStorageService extends AbstractStorageService
                     result.add(key);
                 }
             }
+            connection.rollback();
             return result;
             
         } catch (final SQLException e) {
@@ -1281,6 +1296,14 @@ public final class JDBCStorageService extends AbstractStorageService
             connection.commit();
         }
 
+        /** Delegated operation to the encapsulated {@link Connection}.
+         * @throws SQLException if encapsulated {@link Connection} does
+         */
+        public void rollback() throws SQLException {
+            connection.rollback();
+        }
+
+
         @Override
         public void close()  {
             try {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list