[java-plugin-storage-jdbc] 01/01: JJDBC-18 Prepare for the Java 17 stack
Rod Widdowson
rdw at steadingsoftware.com
Tue May 23 13:03:57 UTC 2023
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch dev/JJDBC-18
in repository java-plugin-storage-jdbc.
View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-storage-jdbc.git;a=commit;h=30af0ea44b5e65e049f1e5aa57286ba360b00612
commit 30af0ea44b5e65e049f1e5aa57286ba360b00612
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue May 23 13:33:14 2023 +0100
JJDBC-18 Prepare for the Java 17 stack
https://shibboleth.atlassian.net/browse/JJDBC-18
More tidy, mostly null checking
---
jdbc-storage-dist/pom.xml | 2 +-
.../storage/jdbc/impl/JDBCStorageService.java | 32 ++++++++++-------
.../storage/jdbc/impl/JDBCStorageServiceTest.java | 40 +++++++++++++++-------
pom.xml | 8 +----
4 files changed, 50 insertions(+), 32 deletions(-)
diff --git a/jdbc-storage-dist/pom.xml b/jdbc-storage-dist/pom.xml
index 0716f78..1bc7d19 100644
--- a/jdbc-storage-dist/pom.xml
+++ b/jdbc-storage-dist/pom.xml
@@ -8,7 +8,7 @@
<version>1.0.5-SNAPSHOT</version>
</parent>
- <artifactId>java-plugin-jdbc-storage</artifactId>
+ <artifactId>java-plugin-jdbc-storage-dist</artifactId>
<name>Shibboleth :: Plugins :: jdbc-storage Distribution</name>
<description>JDBC storage plugin packaging.</description>
<packaging>pom</packaging>
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 86b3446..9bd5060 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
@@ -26,7 +26,6 @@ import java.sql.Types;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
import java.util.TimerTask;
import java.util.concurrent.locks.Lock;
@@ -43,16 +42,17 @@ import org.opensaml.storage.StorageCapabilities;
import org.opensaml.storage.StorageRecord;
import org.opensaml.storage.VersionMismatchException;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.annotation.constraint.Positive;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.collection.Pair;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.logic.ConstraintViolationException;
+import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.shared.primitive.StringSupport;
/**
@@ -121,7 +121,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
@Nonnull private final Logger log = LoggerFactory.getLogger(JDBCStorageService.class);
/** Timeout of SQL queries. */
- @Nonnull private Duration queryTimeout = Duration.ofSeconds(5);
+ @Nonnull private Duration queryTimeout;
/** How many times do we try an operation before giving up? */
private int transactionRetries = 3;
@@ -136,7 +136,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
private boolean verify = true;
/** Error messages that signal a transaction should be retried. */
- @Nonnull @NonnullElements private Collection<String> retryableErrors = Collections.emptyList();
+ @Nonnull @NonnullElements private Collection<String> retryableErrors = CollectionSupport.emptyList();
/** The Data Source. */
@NonnullAfterInit private DataSource dataSource;
@@ -235,6 +235,9 @@ public final class JDBCStorageService extends AbstractStorageService implements
* Set the defaults so that they can be over-ridden by Spring.
*/
public JDBCStorageService() {
+ final Duration fiveSecs = Duration.ofSeconds(5);
+ assert fiveSecs != null;
+ queryTimeout = fiveSecs;
setContextSize(JDBCStorageRecord.CONTEXT_SIZE);
setKeySize(JDBCStorageRecord.KEY_SIZE);
setValueSize(Integer.MAX_VALUE);
@@ -293,7 +296,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
* @param errors what to set.
*/
public void setRetryableErrors(@Nonnull @NonnullElements final List<String> errors) {
- retryableErrors = Constraint.isNotNull(errors, "errors must not be null");
+ retryableErrors = CollectionSupport.copyToList(Constraint.isNotNull(errors, "errors must not be null"));
Constraint.noNullItems(errors, "errors must not have null members");
}
@@ -475,7 +478,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
final String context = results.getString(1);
final String id = results.getString(2);
final Long expires = getExpires(results, 3);
- final String value = results.getString(4);
+ final String value = Constraint.isNotNull(results.getString(4), "value field must not be null");
final Long version = results.getLong(5);
log.trace("Record: Context = '{}', Id = '{}', value = '{}', verion = '{}', expires = '{}'",
context, id, value, version, expires == null ? "<never>": expires);
@@ -510,7 +513,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
while (results.next()) {
final String id = results.getString(1);
final Long expires = getExpires(results, 2);
- final String value = results.getString(3);
+ 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);
@@ -646,7 +649,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
}
final Long returnedVersion = resultSet.getLong(1);
final Long returnedExpires = getExpires(resultSet, 2);
- final String returnedValue = resultSet.getString(3);
+ 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) {
@@ -982,6 +985,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
while (true) {
try (ConnectionWithLock connection = new ConnectionWithLock(true, true)) {
final PreparedStatement updateStmnt = connection.prepareStatement(updateExpiresByContextSQL);
+ assert updateStmnt!=null;
setExpires(updateStmnt, 1, expires);
updateStmnt.setString(2, context);
final Long newExpires = System.currentTimeMillis();
@@ -1066,12 +1070,13 @@ public final class JDBCStorageService extends AbstractStorageService implements
if (!VERIFY_STRING.equals(record.getValue())) {
throw new ComponentInitializationException("Value read back was incorrect");
}
- final String lower = VERIFY_STRING.toUpperCase();
- record = read(VERIFY_STRING, lower);
+ final String upper = VERIFY_STRING.toUpperCase();
+ assert upper!=null;
+ record = read(VERIFY_STRING, upper);
if (record != null) {
throw new ComponentInitializationException("Key Column is Case Insensitive");
}
- record = read(lower, VERIFY_STRING);
+ record = read(upper, VERIFY_STRING);
if (record != null) {
throw new ComponentInitializationException("Context Column is Case Insensitive");
}
@@ -1155,7 +1160,9 @@ public final class JDBCStorageService extends AbstractStorageService implements
* @throws SQLException if any of the SQL operations throw one
*/
public ConnectionWithLock(final boolean autoCommit, final boolean writeLock) throws SQLException {
- connection = dataSource.getConnection();
+ final Connection con = dataSource.getConnection();
+ assert con != null;
+ connection = con;
connection.setAutoCommit(autoCommit);
connection.setTransactionIsolation(transactionIsolation);
if (readWriteLock != null) {
@@ -1164,6 +1171,7 @@ public final class JDBCStorageService extends AbstractStorageService implements
} else {
threadLock = readWriteLock.readLock();
}
+ assert threadLock != null;
threadLock.lock();
} else {
threadLock = null;
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 6731887..a797340 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
@@ -43,6 +43,8 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeTest;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
@@ -51,7 +53,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
*/
public class JDBCStorageServiceTest extends StorageServiceTest {
- private JDBCStorageService storageService;
+ @NonnullBeforeTest private JDBCStorageService storageService;
private final boolean USE_SQLSERVER = false;
@@ -78,7 +80,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
/** Contexts used for testing. */
private Object[][] contexts;
- private BasicDataSource dataSource;
+ @NonnullBeforeTest private BasicDataSource dataSource;
public JDBCStorageServiceTest() {
final SecureRandom random1 = new SecureRandom();
@@ -119,7 +121,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
*/
@BeforeClass public void setUp() throws ComponentInitializationException {
try {
- dataSource = new BasicDataSource();
+ final BasicDataSource ds = dataSource = new BasicDataSource();
if (USE_SQLSERVER) {
setupSQLServer();
} else {
@@ -128,10 +130,12 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
storageService = new JDBCStorageService();
storageService.setId("test");
- storageService.setDataSource(dataSource);
- storageService.setCleanupInterval(Duration.ofSeconds(5));
+ storageService.setDataSource(ds);
+ final Duration fiveSecs = Duration.ofSeconds(5);
+ assert fiveSecs!=null;
+ storageService.setCleanupInterval(fiveSecs);
storageService.setTransactionRetries(12);
- storageService.setRetryableErrors(List.of("40001"));
+ storageService.setRetryableErrors(CollectionSupport.listOf("40001"));
storageService.setLocalLocking(true);
} catch (final SQLException | ClassNotFoundException e) {
throw new ComponentInitializationException(e);
@@ -143,7 +147,8 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
protected void tearDown() {
try {
List<String> contexts1 = storageService.readContexts();
- for (String ctx : contexts1) {
+ for (final String ctx : contexts1) {
+ assert ctx != null;
storageService.deleteContext(ctx);
}
List<?> recs = storageService.readAll();
@@ -162,14 +167,19 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
}
@Nonnull protected StorageService getStorageService() {
+ assert storageService!=null;
return storageService;
}
@Test
public void cleanup() throws ComponentInitializationException, IOException {
String context = Long.toString(random.nextLong());
+ assert context != null;
for (int i = 1; i <= 100; i++) {
- storageService.create(context, Integer.toString(i), Integer.toString(i + 1), System.currentTimeMillis() + 100);
+ final String iString = Integer.toString(i);
+ final String iPlusOneString = Integer.toString(i+1);
+ assert iString!=null && iPlusOneString!=null;
+ storageService.create(context, iString, iPlusOneString, System.currentTimeMillis() + 100);
}
try {
Thread.sleep(7500);
@@ -186,19 +196,23 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
final String value = Long.toString(random.nextLong());
final String newValue = Long.toString(random.nextLong());
final Long expiration = System.currentTimeMillis() + 10000;
+ assert context!=null && value != null && newValue!=null && expiration!=null;
storageService.create(context, context, value, expiration);
StorageRecord<Object> rec = storageService.read(context, context);
+ assert rec != null;
assertEquals(rec.getValue(), value);
assertEquals(rec.getExpiration(), expiration);
storageService.updateExpiration(context, context, expiration+50000);
rec = storageService.read(context, context);
+ assert rec != null;
assertEquals(rec.getValue(), value);
assertNotEquals(rec.getExpiration(), expiration);
storageService.update(context, context, newValue, expiration+100000);
rec = storageService.read(context, context);
+ assert rec != null;
assertEquals(rec.getValue(), newValue);
assertNotEquals(rec.getExpiration(), expiration);
}
@@ -210,7 +224,7 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
}
@Test(dataProvider = "contexts", singleThreaded = false, threadPoolSize = 25, invocationCount = 100, enabled = true)
- public void multithread(final String context) throws IOException {
+ public void multithread(@Nonnull final String context) throws IOException {
shared.create(context, "mt", "bar", System.currentTimeMillis() + 300000);
StorageRecord<?> rec = shared.read(context, "mt");
Assert.assertNotNull(rec);
@@ -336,13 +350,15 @@ public class JDBCStorageServiceTest extends StorageServiceTest {
@Test(enabled = false)
public void largeValue() throws IOException {
// hsqldb defaults LOB length to 255 chars; disabled for now
- StringBuilder sb = new StringBuilder(1000 * 36);
+ final StringBuilder sb = new StringBuilder(1000 * 36);
for (int i = 0; i < 1000; i++) {
sb.append(UUID.randomUUID());
}
- shared.create("unit_test", "large", sb.toString(), System.currentTimeMillis() + 300000);
+ final String sbString = sb.toString();
+ assert sbString!=null;
+ shared.create("unit_test", "large", sbString, System.currentTimeMillis() + 300000);
StorageRecord<?> rec = shared.read("unit_test", "large");
- Assert.assertNotNull(rec);
+ assert rec !=null;
Assert.assertEquals(sb.toString(), rec.getValue());
}
diff --git a/pom.xml b/pom.xml
index d3aa01d..70b4056 100644
--- a/pom.xml
+++ b/pom.xml
@@ -15,6 +15,7 @@
<properties>
<idp.groupId>net.shibboleth.idp</idp.groupId>
<idp.version>5.0.0-SNAPSHOT</idp.version>
+ <!-- OpenSAML used for testing only -->
<opensaml.groupId>org.opensaml</opensaml.groupId>
<opensaml.version>5.0.0-SNAPSHOT</opensaml.version>
<checkstyle.configLocation>${project.basedir}/checkstyle.xml</checkstyle.configLocation>
@@ -74,13 +75,6 @@
<type>pom</type>
<scope>import</scope>
</dependency>
- <!-- Provided shibboleth support dependencies -->
- <dependency>
- <groupId>net.shibboleth.utilities</groupId>
- <artifactId>java-support</artifactId>
- <version>${java-support.version}</version>
- <scope>provided</scope>
- </dependency>
<!-- Test bom dependencies -->
<dependency>
<groupId>${idp.groupId}</groupId>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list