[java-plugin-storage-jdbc] 01/02: JJDBC-8 Update with no expiration fails

[Rod Widdowson]

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=4609d32601089784024ddbf72c6e7af28f323abe

commit 4609d32601089784024ddbf72c6e7af28f323abe
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Jun 25 11:54:54 2022 +0100

    JJDBC-8 Update with no expiration fails
    
    https://shibboleth.atlassian.net/browse/JJDBC-8
    
    Regression test and fix.
---
 .../storage/jdbc/impl/JDBCStorageService.java      | 16 +++++++-----
 .../storage/jdbc/impl/JDBCStorageServiceTest.java  | 30 +++++++++++++++++++---
 2 files changed, 36 insertions(+), 10 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 e7d97cb..67f3a27 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
@@ -92,7 +92,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
 
     /** The SQL to check whether a record exists prior to updating it. */
     static final String DEFAULT_PRE_UPDATE_QUERY_SQL =
-        "SELECT version, expires FROM StorageRecords WHERE context =? AND id=?";
+        "SELECT version, expires, value FROM StorageRecords WHERE context =? AND id=?";
 
     /** The SQL to update a record. */
     static final String DEFAULT_UPDATE_RECORD_SQL =
@@ -621,7 +621,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
 
         Constraint.isNotEmpty(Constraint.isNotNull(context, "read: context must not be null"),
                               "read: context must not be empty");
-        Constraint.isNotEmpty(Constraint.isNotNull(context, "read: key must not be null"),
+        Constraint.isNotEmpty(Constraint.isNotNull(key, "read: key must not be null"),
                               "read: key must not be empty");
 
         int retries = transactionRetries;
@@ -729,7 +729,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
     @Nullable protected Long updateImpl(@Nullable final Long version,
                                         @Nonnull @NotEmpty final String context,
                                         @Nonnull @NotEmpty final String key,
-                                        @Nonnull @NotEmpty final String value,
+                                        @Nullable final String value,
                                         @Nullable @Positive final Long expires)
                                                throws IOException, VersionMismatchException {
         
@@ -737,8 +737,6 @@ public final class JDBCStorageService extends AbstractStorageService implements
                               "update: context must not be empty");
         Constraint.isNotEmpty(Constraint.isNotNull(key, "update: key must not be null"),
                               "update: key must not be empty");
-        Constraint.isNotEmpty(Constraint.isNotNull(value, "update: value must not be null"),
-                              "update: value must not be empty");
         int retries = transactionRetries;
         while (true) {
             try (ConnectionWithLock connection = new ConnectionWithLock(false, true)) {
@@ -755,6 +753,12 @@ public final class JDBCStorageService extends AbstractStorageService implements
                 }
                 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;
@@ -768,7 +772,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
                 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, value);
+                updateStmnt.setString(1, updateValue);
                 updateStmnt.setLong(2, newVersion);
                 setExpires(updateStmnt, 3, expires);
                 updateStmnt.setString(4, context);
diff --git a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
index 3928aa4..9871da6 100644
--- a/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
+++ b/jdbc-storage-impl/src/test/java/net/shibboleth/plugin/storage/jdbc/impl/JDBCStorageServiceTest.java
@@ -17,6 +17,8 @@
 
 package net.shibboleth.plugin.storage.jdbc.impl;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertTrue;
 
 import java.io.IOException;
@@ -161,10 +163,6 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
     @Nonnull protected StorageService getStorageService() {
         return storageService;
     }
-    /*
-    @Test(enabled = false)
-    public void strings() throws IOException {
-    }*/
 
     @Test
     public void cleanup() throws ComponentInitializationException, IOException {
@@ -181,6 +179,30 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
         Assert.assertEquals(recs.size(), 0);
     }
 
+    @Test
+    public void update() throws ComponentInitializationException, IOException {
+        final String context = Long.toString(random.nextLong());
+        final String value = Long.toString(random.nextLong());
+        final String newValue = Long.toString(random.nextLong());
+        final Long expiration = System.currentTimeMillis() + 10000;
+
+        storageService.create(context, context, value, expiration);
+        StorageRecord<Object> rec = storageService.read(context, context);
+        assertEquals(rec.getValue(), value);
+        assertEquals(rec.getExpiration(), expiration);
+
+        storageService.updateExpiration(context, context, expiration+50000);
+        rec = storageService.read(context, context);
+        assertEquals(rec.getValue(), value);
+        assertNotEquals(rec.getExpiration(), expiration);
+
+        storageService.update(context, context, newValue, expiration+100000);
+        rec = storageService.read(context, context);
+        assertEquals(rec.getValue(), newValue);
+        assertNotEquals(rec.getExpiration(), expiration);
+    }
+
+
     @DataProvider(name = "contexts")
     public Object[][] contexts() throws Exception {
         return contexts;

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