From 8f65d51889ef63667e7a3d6ebb898f1c6744af21 Mon Sep 17 00:00:00 2001 From: Peter Winckles Date: Thu, 19 Nov 2020 17:26:22 -0600 Subject: [PATCH] added the ability to change the name of the db tables; improved concurrency tests --- ocfl-java-core/pom.xml | 4 + .../core/db/BaseObjectDetailsDatabase.java | 51 +++-- .../ocfl/core/db/H2ObjectDetailsDatabase.java | 4 +- .../core/db/ObjectDetailsDatabaseBuilder.java | 22 +- .../db/PostgresObjectDetailsDatabase.java | 4 +- .../library/ocfl/core/db/TableCreator.java | 25 ++- .../library/ocfl/core/lock/H2ObjectLock.java | 19 +- .../ocfl/core/lock/ObjectLockBuilder.java | 23 +- .../ocfl/core/lock/PostgresObjectLock.java | 20 +- ...tails.sql => ocfl_object_details.ddl.tmpl} | 2 +- .../resources/db/h2/ocfl_object_lock.ddl.tmpl | 3 + .../main/resources/db/h2/ocfl_object_lock.sql | 3 - ...tails.sql => ocfl_object_details.ddl.tmpl} | 2 +- .../db/postgresql/ocfl_object_lock.ddl.tmpl | 3 + .../db/postgresql/ocfl_object_lock.sql | 3 - .../core/db/ObjectDetailsDatabaseTest.java | 210 +++++++++++------- .../ocfl/core/lock/DbObjectLockTest.java | 63 +++--- .../core/lock/InMemoryObjectLockTest.java | 27 ++- 18 files changed, 322 insertions(+), 166 deletions(-) rename ocfl-java-core/src/main/resources/db/h2/{ocfl_object_details.sql => ocfl_object_details.ddl.tmpl} (86%) create mode 100644 ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.ddl.tmpl delete mode 100644 ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.sql rename ocfl-java-core/src/main/resources/db/postgresql/{ocfl_object_details.sql => ocfl_object_details.ddl.tmpl} (85%) create mode 100644 ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.ddl.tmpl delete mode 100644 ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.sql diff --git a/ocfl-java-core/pom.xml b/ocfl-java-core/pom.xml index 35b32389..47fa967e 100644 --- a/ocfl-java-core/pom.xml +++ b/ocfl-java-core/pom.xml @@ -140,6 +140,10 @@ junit-jupiter-engine test + + org.junit.jupiter + junit-jupiter-params + org.mockito mockito-junit-jupiter diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/BaseObjectDetailsDatabase.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/BaseObjectDetailsDatabase.java index 629ef340..3e9954de 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/BaseObjectDetailsDatabase.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/BaseObjectDetailsDatabase.java @@ -54,6 +54,7 @@ public abstract class BaseObjectDetailsDatabase implements ObjectDetailsDatabase private static final Logger LOG = LoggerFactory.getLogger(BaseObjectDetailsDatabase.class); + private final String tableName; private final DataSource dataSource; private final boolean storeInventory; private final long waitMillis; @@ -61,13 +62,40 @@ public abstract class BaseObjectDetailsDatabase implements ObjectDetailsDatabase private final String lockFailCode; private final String duplicateKeyCode; - public BaseObjectDetailsDatabase(DataSource dataSource, boolean storeInventory, long waitTime, TimeUnit timeUnit, - String lockFailCode, String duplicateKeyCode) { + private final String selectDetailsQuery; + private final String deleteDetailsQuery; + private final String rowLockQuery; + private final String updateDetailsQuery; + private final String insertDetailsQuery; + private final String selectDigestQuery; + + public BaseObjectDetailsDatabase(String tableName, + DataSource dataSource, + boolean storeInventory, + long waitTime, + TimeUnit timeUnit, + String lockFailCode, + String duplicateKeyCode) { + this.tableName = Enforce.notBlank(tableName, "tableName cannot be blank"); this.dataSource = Enforce.notNull(dataSource, "dataSource cannot be null"); this.storeInventory = storeInventory; this.lockFailCode = Enforce.notBlank(lockFailCode, "lockFailCode cannot be blank"); this.duplicateKeyCode = Enforce.notBlank(duplicateKeyCode, "duplicateKeyCode cannot be blank"); this.waitMillis = timeUnit.toMillis(waitTime); + + this.selectDetailsQuery = String.format("SELECT" + + " object_id, version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp" + + " FROM %s WHERE object_id = ?", tableName); + this.deleteDetailsQuery = String.format("DELETE FROM %s WHERE object_id = ?", tableName); + this.rowLockQuery = String.format("SELECT version_id, revision_id FROM %s WHERE object_id = ? FOR UPDATE", tableName); + this.updateDetailsQuery = String.format("UPDATE %s SET" + + " (version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp)" + + " = (?, ?, ?, ?, ?, ?, ?)" + + " WHERE object_id = ?", tableName); + this.insertDetailsQuery = String.format("INSERT INTO %s" + + " (object_id, version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp)" + + " VALUES (?, ?, ?, ?, ?, ?, ?, ?)", tableName); + this.selectDigestQuery = String.format("SELECT inventory_digest FROM %s WHERE object_id = ?", tableName); } /** @@ -89,9 +117,7 @@ public OcflObjectDetails retrieveObjectDetails(String objectId) { OcflObjectDetails details = null; try (var connection = dataSource.getConnection()) { - try (var statement = connection.prepareStatement("SELECT" + - " object_id, version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp" + - " FROM ocfl_object_details WHERE object_id = ?")) { + try (var statement = connection.prepareStatement(selectDetailsQuery)) { statement.setString(1, objectId); try (var rs = statement.executeQuery()) { @@ -164,7 +190,7 @@ public void deleteObjectDetails(String objectId) { connection.setAutoCommit(false); setLockWaitTimeout(connection, waitMillis); - try (var statement = connection.prepareStatement("DELETE FROM ocfl_object_details WHERE object_id = ?")) { + try (var statement = connection.prepareStatement(deleteDetailsQuery)) { statement.setString(1, objectId); statement.executeUpdate(); connection.commit(); @@ -201,7 +227,7 @@ private void updateObjectDetailsInternal(Inventory inventory, String inventoryDi } private void insertInventory(Connection connection, Inventory inventory, String inventoryDigest, InputStream inventoryStream) throws SQLException { - try (var lockStatement = connection.prepareStatement("SELECT version_id, revision_id FROM ocfl_object_details WHERE object_id = ? FOR UPDATE")) { + try (var lockStatement = connection.prepareStatement(rowLockQuery)) { lockStatement.setString(1, inventory.getId()); try (var lockResult = lockStatement.executeQuery()) { @@ -222,10 +248,7 @@ private void insertInventory(Connection connection, Inventory inventory, String } private void executeUpdateDetails(Connection connection, Inventory inventory, String inventoryDigest, InputStream inventoryStream) throws SQLException { - try (var insertStatement = connection.prepareStatement("UPDATE ocfl_object_details SET" + - " (version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp)" + - " = (?, ?, ?, ?, ?, ?, ?)" + - " WHERE object_id = ?")) { + try (var insertStatement = connection.prepareStatement(updateDetailsQuery)) { insertStatement.setString(1, inventory.getHead().toString()); insertStatement.setString(2, inventory.getObjectRootPath()); insertStatement.setString(3, revisionNumStr(inventory.getRevisionNum())); @@ -244,9 +267,7 @@ private void executeUpdateDetails(Connection connection, Inventory inventory, St } private void executeInsertDetails(Connection connection, Inventory inventory, String inventoryDigest, InputStream inventoryStream) throws SQLException { - try (var insertStatement = connection.prepareStatement("INSERT INTO ocfl_object_details" + - " (object_id, version_id, object_root_path, revision_id, inventory_digest, digest_algorithm, inventory, update_timestamp)" + - " VALUES (?, ?, ?, ?, ?, ?, ?, ?)")) { + try (var insertStatement = connection.prepareStatement(insertDetailsQuery)) { insertStatement.setString(1, inventory.getId()); insertStatement.setString(2, inventory.getHead().toString()); insertStatement.setString(3, inventory.getObjectRootPath()); @@ -271,7 +292,7 @@ private void executeInsertDetails(Connection connection, Inventory inventory, St private String retrieveDigest(String objectId) { try (var connection = dataSource.getConnection()) { - try (var statement = connection.prepareStatement("SELECT inventory_digest FROM ocfl_object_details WHERE object_id = ?")) { + try (var statement = connection.prepareStatement(selectDigestQuery)) { statement.setString(1, objectId); try (var resultSet = statement.executeQuery()) { diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/H2ObjectDetailsDatabase.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/H2ObjectDetailsDatabase.java index 942821b3..99b68b82 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/H2ObjectDetailsDatabase.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/H2ObjectDetailsDatabase.java @@ -34,8 +34,8 @@ public class H2ObjectDetailsDatabase extends BaseObjectDetailsDatabase { private static final String LOCK_FAIL_STATE = "HYT00"; private static final String DUPLICATE_KEY_STATE = "23505"; - public H2ObjectDetailsDatabase(DataSource dataSource, boolean storeInventory, long waitTime, TimeUnit timeUnit) { - super(dataSource, storeInventory, waitTime, timeUnit, LOCK_FAIL_STATE, DUPLICATE_KEY_STATE); + public H2ObjectDetailsDatabase(String tableName, DataSource dataSource, boolean storeInventory, long waitTime, TimeUnit timeUnit) { + super(tableName, dataSource, storeInventory, waitTime, timeUnit, LOCK_FAIL_STATE, DUPLICATE_KEY_STATE); } protected void setLockWaitTimeout(Connection connection, long waitMillis) throws SQLException { diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseBuilder.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseBuilder.java index 995b3cda..c28642df 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseBuilder.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseBuilder.java @@ -35,10 +35,13 @@ */ public class ObjectDetailsDatabaseBuilder { + private static final String DEFAULT_TABLE_NAME = "ocfl_object_details"; + private boolean storeInventory; private long waitTime; private TimeUnit timeUnit; private DataSource dataSource; + private String tableName; public ObjectDetailsDatabaseBuilder() { storeInventory = true; @@ -81,6 +84,17 @@ public ObjectDetailsDatabaseBuilder dataSource(DataSource dataSource) { return this; } + /** + * Sets the name of the table to use to store object details. Default: ocfl_object_details + * + * @param tableName the table name to use + * @return builder + */ + public ObjectDetailsDatabaseBuilder tableName(String tableName) { + this.tableName = tableName; + return this; + } + /** * Constructs a new {@link ObjectDetailsDatabase} instance using the given dataSource. If the database does not * already contain an object details table, it attempts to create one. @@ -90,21 +104,23 @@ public ObjectDetailsDatabaseBuilder dataSource(DataSource dataSource) { public ObjectDetailsDatabase build() { Enforce.notNull(dataSource, "dataSource cannot be null"); + var resolvedTableName = tableName == null ? DEFAULT_TABLE_NAME : tableName; + var dbType = DbType.fromDataSource(dataSource); ObjectDetailsDatabase database; switch (dbType) { case POSTGRES: - database = new PostgresObjectDetailsDatabase(dataSource, storeInventory, waitTime, timeUnit); + database = new PostgresObjectDetailsDatabase(resolvedTableName, dataSource, storeInventory, waitTime, timeUnit); break; case H2: - database = new H2ObjectDetailsDatabase(dataSource, storeInventory, waitTime, timeUnit); + database = new H2ObjectDetailsDatabase(resolvedTableName, dataSource, storeInventory, waitTime, timeUnit); break; default: throw new OcflJavaException(String.format("Database type %s is not mapped to an ObjectDetailsDatabase implementation.", dbType)); } - new TableCreator(dbType, dataSource).createObjectDetailsTable(); + new TableCreator(dbType, dataSource).createObjectDetailsTable(resolvedTableName); return database; } diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/PostgresObjectDetailsDatabase.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/PostgresObjectDetailsDatabase.java index 6669b396..900ed720 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/PostgresObjectDetailsDatabase.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/PostgresObjectDetailsDatabase.java @@ -34,8 +34,8 @@ public class PostgresObjectDetailsDatabase extends BaseObjectDetailsDatabase { private static final String LOCK_FAIL_STATE = "55P03"; private static final String DUPLICATE_KEY_STATE = "23505"; - public PostgresObjectDetailsDatabase(DataSource dataSource, boolean storeInventory, long waitTime, TimeUnit timeUnit) { - super(dataSource, storeInventory, waitTime, timeUnit, LOCK_FAIL_STATE, DUPLICATE_KEY_STATE); + public PostgresObjectDetailsDatabase(String tableName, DataSource dataSource, boolean storeInventory, long waitTime, TimeUnit timeUnit) { + super(tableName, dataSource, storeInventory, waitTime, timeUnit, LOCK_FAIL_STATE, DUPLICATE_KEY_STATE); } protected void setLockWaitTimeout(Connection connection, long waitMillis) throws SQLException { diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/TableCreator.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/TableCreator.java index f7f9aa0b..f6eb67f6 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/TableCreator.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/db/TableCreator.java @@ -44,37 +44,40 @@ public class TableCreator { private static final Logger LOG = LoggerFactory.getLogger(TableCreator.class); - private static final String LOCK_TABLE_FILE = "ocfl_object_lock.sql"; - private static final String OBJECT_DETAILS_TABLE_FILE = "ocfl_object_details.sql"; + private static final String LOCK_TABLE_FILE = "ocfl_object_lock.ddl.tmpl"; + private static final String OBJECT_DETAILS_TABLE_FILE = "ocfl_object_details.ddl.tmpl"; - private Map dbScriptDir = Map.of( + private final Map dbScriptDir = Map.of( DbType.POSTGRES, "db/postgresql", DbType.H2, "db/h2" ); - private DbType dbType; - private DataSource dataSource; + private final DbType dbType; + private final DataSource dataSource; public TableCreator(DbType dbType, DataSource dataSource) { this.dbType = Enforce.notNull(dbType, "dbType cannot be null"); this.dataSource = Enforce.notNull(dataSource, "dataSource cannot be null"); } - public void createObjectLockTable() { - createTable(LOCK_TABLE_FILE); + public void createObjectLockTable(String tableName) { + createTable(tableName, LOCK_TABLE_FILE); } - public void createObjectDetailsTable() { - createTable(OBJECT_DETAILS_TABLE_FILE); + public void createObjectDetailsTable(String tableName) { + createTable(tableName, OBJECT_DETAILS_TABLE_FILE); } - private void createTable(String fileName) { + private void createTable(String tableName, String fileName) { + Enforce.notBlank(tableName, "tableName cannot be blank"); try (var connection = dataSource.getConnection()) { var filePath = getSqlFilePath(fileName); LOG.debug("Loading {}", filePath); if (filePath != null) { try (var stream = this.getClass().getResourceAsStream("/" + filePath)) { - try (var statement = connection.prepareStatement(streamToString(stream))) { + var ddlTemplate = streamToString(stream); + var ddl = String.format(ddlTemplate, tableName); + try (var statement = connection.prepareStatement(ddl)) { statement.executeUpdate(); } } diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/H2ObjectLock.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/H2ObjectLock.java index 1725c769..cfeb4e9b 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/H2ObjectLock.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/H2ObjectLock.java @@ -44,14 +44,22 @@ public class H2ObjectLock implements ObjectLock { private static final String OBJECT_LOCK_FAIL = "HYT00"; - private DataSource dataSource; - private long waitMillis; + private final String tableName; + private final DataSource dataSource; + private final long waitMillis; - public H2ObjectLock(DataSource dataSource, long waitTime, TimeUnit timeUnit) { + private final String createRowLockQuery; + private final String acquireLockQuery; + + public H2ObjectLock(String tableName, DataSource dataSource, long waitTime, TimeUnit timeUnit) { + this.tableName = Enforce.notBlank(tableName, "tableName cannot be blank"); this.dataSource = Enforce.notNull(dataSource, "dataSource cannot be null"); Enforce.expressionTrue(waitTime > -1, waitTime, "waitTime cannot be negative"); Enforce.notNull(timeUnit, "timeUnit cannot be null"); this.waitMillis = timeUnit.toMillis(waitTime); + + createRowLockQuery = String.format("MERGE INTO %s (object_id) VALUES (?)", tableName); + acquireLockQuery = String.format("SELECT object_id FROM %s WHERE object_id = ? FOR UPDATE", tableName); } /** @@ -106,8 +114,7 @@ public T doInWriteLock(String objectId, Callable doInLock) { } private void createLockRow(String objectId, Connection connection) throws SQLException { - try (var statement = connection.prepareStatement("MERGE INTO ocfl_object_lock" + - " (object_id) VALUES (?)")) { + try (var statement = connection.prepareStatement(createRowLockQuery)) { statement.setString(1, objectId); statement.executeUpdate(); } @@ -124,7 +131,7 @@ private LockException failedToAcquireLock(String objectId) { } private PreparedStatement acquireLock(Connection connection) throws SQLException { - return connection.prepareStatement("SELECT object_id FROM ocfl_object_lock WHERE object_id = ? FOR UPDATE"); + return connection.prepareStatement(acquireLockQuery); } private void safeCleanup(Connection connection) { diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/ObjectLockBuilder.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/ObjectLockBuilder.java index ce73a088..b0e362ea 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/ObjectLockBuilder.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/ObjectLockBuilder.java @@ -27,6 +27,7 @@ import edu.wisc.library.ocfl.api.exception.OcflJavaException; import edu.wisc.library.ocfl.api.util.Enforce; import edu.wisc.library.ocfl.core.db.DbType; +import edu.wisc.library.ocfl.core.db.ObjectDetailsDatabaseBuilder; import edu.wisc.library.ocfl.core.db.TableCreator; import javax.sql.DataSource; @@ -37,9 +38,12 @@ */ public class ObjectLockBuilder { + private static final String DEFAULT_TABLE_NAME = "ocfl_object_lock"; + private long waitTime; private TimeUnit timeUnit; private DataSource dataSource; + private String tableName; public ObjectLockBuilder() { waitTime = 10; @@ -70,6 +74,17 @@ public ObjectLockBuilder dataSource(DataSource dataSource) { return this; } + /** + * Sets the name of the table to use for object locking. Default: ocfl_object_lock + * + * @param tableName the table name to use + * @return builder + */ + public ObjectLockBuilder tableName(String tableName) { + this.tableName = tableName; + return this; + } + /** * Constructs a new {@link ObjectLock}. If a DataSource was set, then a DB lock is created; otherwise, an in-memory * lock is used. @@ -87,21 +102,23 @@ public ObjectLock build() { private ObjectLock buildDbLock() { Enforce.notNull(dataSource, "dataSource cannot be null"); + var resolvedTableName = tableName == null ? DEFAULT_TABLE_NAME : tableName; + var dbType = DbType.fromDataSource(dataSource); ObjectLock lock; switch (dbType) { case POSTGRES: - lock = new PostgresObjectLock(dataSource, waitTime, timeUnit); + lock = new PostgresObjectLock(resolvedTableName, dataSource, waitTime, timeUnit); break; case H2: - lock = new H2ObjectLock(dataSource, waitTime, timeUnit); + lock = new H2ObjectLock(resolvedTableName, dataSource, waitTime, timeUnit); break; default: throw new OcflJavaException(String.format("Database type %s is not mapped to an ObjectLock implementation.", dbType)); } - new TableCreator(dbType, dataSource).createObjectLockTable(); + new TableCreator(dbType, dataSource).createObjectLockTable(resolvedTableName); return lock; } diff --git a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/PostgresObjectLock.java b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/PostgresObjectLock.java index f053f25c..67dc00b0 100644 --- a/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/PostgresObjectLock.java +++ b/ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/lock/PostgresObjectLock.java @@ -44,14 +44,22 @@ public class PostgresObjectLock implements ObjectLock { private static final String OBJECT_LOCK_FAIL = "55P03"; - private DataSource dataSource; - private long waitMillis; + private final String tableName; + private final DataSource dataSource; + private final long waitMillis; - public PostgresObjectLock(DataSource dataSource, long waitTime, TimeUnit timeUnit) { + private final String createRowLockQuery; + private final String acquireLockQuery; + + public PostgresObjectLock(String tableName, DataSource dataSource, long waitTime, TimeUnit timeUnit) { + this.tableName = Enforce.notBlank(tableName, "tableName cannot be blank"); this.dataSource = Enforce.notNull(dataSource, "dataSource cannot be null"); Enforce.expressionTrue(waitTime > -1, waitTime, "waitTime cannot be negative"); Enforce.notNull(timeUnit, "timeUnit cannot be null"); this.waitMillis = timeUnit.toMillis(waitTime); + + this.createRowLockQuery = String.format("INSERT INTO %s (object_id) VALUES (?) ON CONFLICT (object_id) DO NOTHING", tableName); + this.acquireLockQuery = String.format("SELECT object_id FROM %s WHERE object_id = ? FOR UPDATE", tableName); } /** @@ -103,9 +111,7 @@ public T doInWriteLock(String objectId, Callable doInLock) { } private void createLockRow(String objectId, Connection connection) throws SQLException { - try (var statement = connection.prepareStatement("INSERT INTO ocfl_object_lock" + - " (object_id) VALUES (?)" + - " ON CONFLICT (object_id) DO NOTHING")) { + try (var statement = connection.prepareStatement(createRowLockQuery)) { statement.setString(1, objectId); statement.executeUpdate(); } @@ -122,7 +128,7 @@ private LockException failedToAcquireLock(String objectId) { } private PreparedStatement acquireLock(Connection connection) throws SQLException { - return connection.prepareStatement("SELECT object_id FROM ocfl_object_lock WHERE object_id = ? FOR UPDATE"); + return connection.prepareStatement(acquireLockQuery); } private void safeCleanup(Connection connection) { diff --git a/ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.sql b/ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.ddl.tmpl similarity index 86% rename from ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.sql rename to ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.ddl.tmpl index c6034da5..7c469f6d 100644 --- a/ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.sql +++ b/ocfl-java-core/src/main/resources/db/h2/ocfl_object_details.ddl.tmpl @@ -1,4 +1,4 @@ -CREATE TABLE IF NOT EXISTS ocfl_object_details ( +CREATE TABLE IF NOT EXISTS %s ( object_id varchar(1024) PRIMARY KEY NOT NULL, version_id varchar(255) NOT NULL, object_root_path varchar(2048) NOT NULL, diff --git a/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.ddl.tmpl b/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.ddl.tmpl new file mode 100644 index 00000000..8e3d3642 --- /dev/null +++ b/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.ddl.tmpl @@ -0,0 +1,3 @@ +CREATE TABLE IF NOT EXISTS %s ( + object_id varchar(1024) PRIMARY KEY +); \ No newline at end of file diff --git a/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.sql b/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.sql deleted file mode 100644 index f1e2eaa9..00000000 --- a/ocfl-java-core/src/main/resources/db/h2/ocfl_object_lock.sql +++ /dev/null @@ -1,3 +0,0 @@ -CREATE TABLE IF NOT EXISTS ocfl_object_lock ( - object_id varchar(1024) PRIMARY KEY -); \ No newline at end of file diff --git a/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.sql b/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.ddl.tmpl similarity index 85% rename from ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.sql rename to ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.ddl.tmpl index 9de58b5b..f65037c5 100644 --- a/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.sql +++ b/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_details.ddl.tmpl @@ -1,4 +1,4 @@ -CREATE TABLE IF NOT EXISTS ocfl_object_details ( +CREATE TABLE IF NOT EXISTS %s ( object_id varchar(1024) PRIMARY KEY, version_id varchar(255) NOT NULL, object_root_path varchar(2048) NOT NULL, diff --git a/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.ddl.tmpl b/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.ddl.tmpl new file mode 100644 index 00000000..8e3d3642 --- /dev/null +++ b/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.ddl.tmpl @@ -0,0 +1,3 @@ +CREATE TABLE IF NOT EXISTS %s ( + object_id varchar(1024) PRIMARY KEY +); \ No newline at end of file diff --git a/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.sql b/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.sql deleted file mode 100644 index f1e2eaa9..00000000 --- a/ocfl-java-core/src/main/resources/db/postgresql/ocfl_object_lock.sql +++ /dev/null @@ -1,3 +0,0 @@ -CREATE TABLE IF NOT EXISTS ocfl_object_lock ( - object_id varchar(1024) PRIMARY KEY -); \ No newline at end of file diff --git a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseTest.java b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseTest.java index 14d371d8..3ee8e62b 100644 --- a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseTest.java +++ b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/db/ObjectDetailsDatabaseTest.java @@ -11,8 +11,9 @@ import edu.wisc.library.ocfl.test.OcflAsserts; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import javax.sql.DataSource; import java.io.ByteArrayOutputStream; @@ -24,6 +25,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Phaser; import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -34,10 +36,12 @@ public class ObjectDetailsDatabaseTest { + public static final String TABLE_1 = "ocfl_object_details"; + public static final String TABLE_2 = "obj_details_2"; + @TempDir public Path tempDir; - private ObjectDetailsDatabase database; private ComboPooledDataSource dataSource; private InventoryMapper inventoryMapper; private ExecutorService executor; @@ -46,24 +50,21 @@ public class ObjectDetailsDatabaseTest { public void setup() { dataSource = new ComboPooledDataSource(); dataSource.setJdbcUrl("jdbc:h2:mem:test"); -// dataSource.setJdbcUrl( "jdbc:postgresql://localhost/ocfl" ); -// dataSource.setUser("pwinckles"); -// dataSource.setPassword(""); - database = new ObjectDetailsDatabaseBuilder().dataSource(dataSource).build(); inventoryMapper = InventoryMapper.prettyPrintMapper(); - executor = Executors.newFixedThreadPool(2); - - truncateObjectDetails(dataSource); + executor = Executors.newCachedThreadPool(); } @AfterEach public void after() { + dataSource.close(); executor.shutdown(); } - @Test - public void shouldAddDetailsWhenDoNotExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldAddDetailsWhenDoNotExist(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -74,8 +75,10 @@ public void shouldAddDetailsWhenDoNotExist() { assertObjectDetails(inventory, digest, invBytes, details); } - @Test - public void shouldUpdateDetailsWhenDetailsExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldUpdateDetailsWhenDetailsExist(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -97,14 +100,18 @@ public void shouldUpdateDetailsWhenDetailsExist() { assertObjectDetails(inventory, digest, invBytes, details); } - @Test - public void shouldReturnNullWhenDetailsDoNotExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldReturnNullWhenDetailsDoNotExist(String tableName) { + var database = createDatabase(tableName); var details = database.retrieveObjectDetails("o1"); assertNull(details); } - @Test - public void shouldApplyUpdateWhenRunnableSucceeds() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldApplyUpdateWhenRunnableSucceeds(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -127,8 +134,10 @@ public void shouldApplyUpdateWhenRunnableSucceeds() { assertObjectDetails(inventory, digest, invBytes, details); } - @Test - public void shouldRollbackDbChangesWhenRunnableFails() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRollbackDbChangesWhenRunnableFails(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -156,8 +165,10 @@ public void shouldRollbackDbChangesWhenRunnableFails() { assertObjectDetails(inventory, digest, invBytes, details); } - @Test - public void shouldDeleteDetailsWhenExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldDeleteDetailsWhenExist(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -173,15 +184,19 @@ public void shouldDeleteDetailsWhenExist() { assertNull(details); } - @Test - public void shouldDoNothingWhenDeleteAndDetailsDoNotExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldDoNothingWhenDeleteAndDetailsDoNotExist(String tableName) { + var database = createDatabase(tableName); database.deleteObjectDetails("o1"); var details = database.retrieveObjectDetails("o1"); assertNull(details); } - @Test - public void shouldNotStoreInventoryBytesWhenFeatureDisabled() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldNotStoreInventoryBytesWhenFeatureDisabled(String tableName) { + var database = createDatabase(tableName); database = new ObjectDetailsDatabaseBuilder().storeInventory(false).dataSource(dataSource).build(); var inventory = basicInventory(); @@ -194,8 +209,10 @@ public void shouldNotStoreInventoryBytesWhenFeatureDisabled() { assertObjectDetails(inventory, digest, null, details); } - @Test - public void shouldRejectUpdateWhenNewInventoryVersionIsNotNextVersion() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRejectUpdateWhenNewInventoryVersionIsNotNextVersion(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -220,8 +237,10 @@ public void shouldRejectUpdateWhenNewInventoryVersionIsNotNextVersion() { }); } - @Test - public void shouldRejectUpdateWhenNewInventoryVersionIsOldVersion() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRejectUpdateWhenNewInventoryVersionIsOldVersion(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -246,8 +265,10 @@ public void shouldRejectUpdateWhenNewInventoryVersionIsOldVersion() { }); } - @Test - public void shouldRejectUpdateWhenNewRevisionButNotR1() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRejectUpdateWhenNewRevisionButNotR1(String tableName) { + var database = createDatabase(tableName); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); @@ -273,8 +294,10 @@ public void shouldRejectUpdateWhenNewRevisionButNotR1() { }); } - @Test - public void shouldRejectUpdateWhenRevisionAndUpdateDifferentVersion() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRejectUpdateWhenRevisionAndUpdateDifferentVersion(String tableName) { + var database = createDatabase(tableName); var inventory = Inventory.builderFromStub("o1", new OcflConfig(), "o1") .mutableHead(true) .addFileToManifest("f1", "v1/content/file1.txt") @@ -303,8 +326,10 @@ public void shouldRejectUpdateWhenRevisionAndUpdateDifferentVersion() { }); } - @Test - public void shouldRejectUpdateWhenRevisionAndUpdateNotNextRevision() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldRejectUpdateWhenRevisionAndUpdateNotNextRevision(String tableName) { + var database = createDatabase(tableName); var inventory = Inventory.builderFromStub("o1", new OcflConfig(), "o1") .mutableHead(true) .addFileToManifest("f1", "v1/content/file1.txt") @@ -336,26 +361,34 @@ public void shouldRejectUpdateWhenRevisionAndUpdateNotNextRevision() { }); } - @Test - public void shouldFailWhenCannotAcquireLock() throws InterruptedException, ExecutionException { - database = new ObjectDetailsDatabaseBuilder().waitTime(500, TimeUnit.MILLISECONDS).dataSource(dataSource).build(); + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldFailWhenCannotAcquireLock(String tableName) throws InterruptedException, ExecutionException { + var database = new ObjectDetailsDatabaseBuilder() + .waitTime(250, TimeUnit.MILLISECONDS) + .dataSource(dataSource) + .tableName(tableName) + .build(); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); var invPath = writeInventory(invBytes); + var phaser = new Phaser(2); + var future = executor.submit(() -> { database.updateObjectDetails(inventory, digest, invPath, () -> { try { - Thread.sleep(TimeUnit.SECONDS.toMillis(3)); + phaser.arriveAndAwaitAdvance(); + TimeUnit.MILLISECONDS.sleep(500); } catch (InterruptedException e) { Thread.interrupted(); } }); }); - Thread.sleep(TimeUnit.SECONDS.toMillis(1)); + phaser.arriveAndAwaitAdvance(); assertThrows(LockException.class, () -> { database.addObjectDetails(inventory, digest, invBytes); @@ -364,9 +397,14 @@ public void shouldFailWhenCannotAcquireLock() throws InterruptedException, Execu future.get(); } - @Test - public void shouldFailDeleteWhenCannotAcquireLock() throws InterruptedException, ExecutionException { - database = new ObjectDetailsDatabaseBuilder().waitTime(500, TimeUnit.MILLISECONDS).dataSource(dataSource).build(); + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldFailDeleteWhenCannotAcquireLock(String tableName) throws InterruptedException, ExecutionException { + var database = new ObjectDetailsDatabaseBuilder() + .waitTime(250, TimeUnit.MILLISECONDS) + .dataSource(dataSource) + .tableName(tableName) + .build(); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); @@ -384,17 +422,20 @@ public void shouldFailDeleteWhenCannotAcquireLock() throws InterruptedException, var digest2 = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes2); var invPath = writeInventory(invBytes2); + var phaser = new Phaser(2); + var future = executor.submit(() -> { database.updateObjectDetails(inv2, digest2, invPath, () -> { try { - Thread.sleep(TimeUnit.SECONDS.toMillis(3)); + phaser.arriveAndAwaitAdvance(); + TimeUnit.MILLISECONDS.sleep(2000); } catch (InterruptedException e) { Thread.interrupted(); } }); }); - Thread.sleep(TimeUnit.SECONDS.toMillis(1)); + phaser.arriveAndAwaitAdvance(); assertThrows(LockException.class, () -> { database.deleteObjectDetails(inventory.getId()); @@ -403,9 +444,14 @@ public void shouldFailDeleteWhenCannotAcquireLock() throws InterruptedException, future.get(); } - @Test - public void shouldFailWhenConcurrentUpdateAndNew() throws InterruptedException, ExecutionException { - database = new ObjectDetailsDatabaseBuilder().waitTime(500, TimeUnit.MILLISECONDS).dataSource(dataSource).build(); + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldFailWhenConcurrentUpdateAndNew(String tableName) throws InterruptedException, ExecutionException { + var database = new ObjectDetailsDatabaseBuilder() + .waitTime(250, TimeUnit.MILLISECONDS) + .dataSource(dataSource) + .tableName(tableName) + .build(); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); @@ -422,26 +468,23 @@ public void shouldFailWhenConcurrentUpdateAndNew() throws InterruptedException, var digest2 = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes2); var invPath2 = writeInventory(invBytes2); + var phaser = new Phaser(2); + var future = executor.submit(() -> { database.updateObjectDetails(inv2, digest2, invPath2, () -> { try { - Thread.sleep(TimeUnit.SECONDS.toMillis(3)); + phaser.arriveAndAwaitAdvance(); + TimeUnit.MILLISECONDS.sleep(500); } catch (InterruptedException e) { Thread.interrupted(); } }); }); - Thread.sleep(TimeUnit.SECONDS.toMillis(1)); + phaser.arriveAndAwaitAdvance(); assertThrows(LockException.class, () -> { - database.updateObjectDetails(inventory, digest, invPath, () -> { - try { - Thread.sleep(TimeUnit.SECONDS.toMillis(3)); - } catch (InterruptedException e) { - Thread.interrupted(); - } - }); + database.updateObjectDetails(inventory, digest, invPath, () -> {}); }); future.get(); @@ -451,18 +494,28 @@ public void shouldFailWhenConcurrentUpdateAndNew() throws InterruptedException, assertObjectDetails(inv2, digest2, invBytes2, details); } - @Test - public void shouldSucceedWhenConcurrentAddAndSameDigest() throws InterruptedException, ExecutionException { - database = new ObjectDetailsDatabaseBuilder().waitTime(500, TimeUnit.MILLISECONDS).dataSource(dataSource).build(); + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldSucceedWhenConcurrentAddAndSameDigest(String tableName) throws InterruptedException, ExecutionException { + var database = new ObjectDetailsDatabaseBuilder() + .waitTime(500, TimeUnit.MILLISECONDS) + .dataSource(dataSource) + .tableName(tableName) + .build(); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); + var phaser = new Phaser(2); + var future = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); database.addObjectDetails(inventory, digest, invBytes); }); + phaser.arriveAndAwaitAdvance(); + database.addObjectDetails(inventory, digest, invBytes); future.get(); @@ -472,30 +525,36 @@ public void shouldSucceedWhenConcurrentAddAndSameDigest() throws InterruptedExce assertObjectDetails(inventory, digest, invBytes, details); } - @Test - public void shouldFailWhenConcurrentAddAndDifferentDigest() { - database = new ObjectDetailsDatabaseBuilder().waitTime(1, TimeUnit.SECONDS).dataSource(dataSource).build(); + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldFailWhenConcurrentAddAndDifferentDigest(String tableName) throws InterruptedException { + var database = new ObjectDetailsDatabaseBuilder() + .waitTime(1, TimeUnit.SECONDS) + .dataSource(dataSource) + .tableName(tableName) + .build(); var inventory = basicInventory(); var invBytes = inventoryBytes(inventory); var digest = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes); - var inv2 = inventory.buildFrom() - .addHeadVersion(Version.builder() - .created(OffsetDateTime.now()) - .addFile("f1", "file2.txt") - .build()) - .build(); + var inv2 = inventory.buildFrom().build(); var invBytes2 = inventoryBytes(inv2); - var digest2 = DigestUtil.computeDigestHex(inventory.getDigestAlgorithm(), invBytes2); + var digest2 = "bogus"; + + var phaser = new Phaser(3); var future = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); database.addObjectDetails(inv2, digest2, invBytes2); }); var future2 = executor.submit(() -> { + phaser.arriveAndAwaitAdvance(); database.addObjectDetails(inventory, digest, invBytes); }); + phaser.arriveAndAwaitAdvance(); + assertThrows(ObjectOutOfSyncException.class, () -> { try { future.get(); @@ -543,13 +602,10 @@ private Path writeInventory(byte[] invBytes) { } } - private void truncateObjectDetails(DataSource dataSource) { - try (var connection = dataSource.getConnection(); - var statement = connection.prepareStatement("TRUNCATE TABLE ocfl_object_details")) { - statement.executeUpdate(); - } catch (SQLException e) { - throw new RuntimeException(e); - } + private ObjectDetailsDatabase createDatabase(String tableName) { + // want to make sure the defaulting works + var name = TABLE_1.equals(tableName) ? null : tableName; + return new ObjectDetailsDatabaseBuilder().dataSource(dataSource).tableName(name).build(); } } diff --git a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/DbObjectLockTest.java b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/DbObjectLockTest.java index 75e7d55a..356b02d0 100644 --- a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/DbObjectLockTest.java +++ b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/DbObjectLockTest.java @@ -2,14 +2,15 @@ import com.mchange.v2.c3p0.ComboPooledDataSource; import edu.wisc.library.ocfl.api.exception.LockException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; -import javax.sql.DataSource; -import java.sql.SQLException; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Phaser; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -19,7 +20,9 @@ public class DbObjectLockTest { - private ObjectLock lock; + public static final String TABLE_1 = "ocfl_object_lock"; + public static final String TABLE_2 = "obj_lock_2"; + private ComboPooledDataSource dataSource; private ExecutorService executor; @@ -27,18 +30,20 @@ public class DbObjectLockTest { public void setup() { dataSource = new ComboPooledDataSource(); dataSource.setJdbcUrl("jdbc:h2:mem:test"); -// dataSource.setJdbcUrl( "jdbc:postgresql://localhost/ocfl" ); -// dataSource.setUser("pwinckles"); -// dataSource.setPassword(""); - lock = new ObjectLockBuilder().waitTime(500, TimeUnit.MILLISECONDS).dataSource(dataSource).build(); - executor = Executors.newFixedThreadPool(2); + executor = Executors.newCachedThreadPool(); + } - truncateObjectLock(dataSource); + @AfterEach + public void after() { + dataSource.close(); + executor.shutdown(); } - @Test - public void shouldAcquireLockWhenDoesNotExist() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldAcquireLockWhenDoesNotExist(String tableName) { + var lock = createLock(tableName); var result = new AtomicBoolean(false); lock.doInWriteLock("obj1", () -> { result.set(true); @@ -46,8 +51,10 @@ public void shouldAcquireLockWhenDoesNotExist() { assertTrue(result.get()); } - @Test - public void shouldAcquireLockWhenAlreadyExistsButNotHeld() { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldAcquireLockWhenAlreadyExistsButNotHeld(String tableName) { + var lock = createLock(tableName); var result = new AtomicBoolean(false); lock.doInWriteLock("obj1", () -> { result.set(true); @@ -60,19 +67,25 @@ public void shouldAcquireLockWhenAlreadyExistsButNotHeld() { assertFalse(result.get()); } - @Test - public void shouldThrowExceptionWhenCannotAcquireLock() throws ExecutionException, InterruptedException { + @ParameterizedTest + @ValueSource(strings = {TABLE_1, TABLE_2}) + public void shouldThrowExceptionWhenCannotAcquireLock(String tableName) throws ExecutionException, InterruptedException { + var lock = createLock(tableName); + + var phaser = new Phaser(2); + var future = executor.submit(() -> { lock.doInWriteLock("obj1", () -> { + phaser.arriveAndAwaitAdvance(); try { - Thread.sleep(TimeUnit.SECONDS.toMillis(3)); + Thread.sleep(TimeUnit.MILLISECONDS.toMillis(500)); } catch (InterruptedException e) { Thread.interrupted(); } }); }); - Thread.sleep(TimeUnit.SECONDS.toMillis(1)); + phaser.arriveAndAwaitAdvance(); var result = new AtomicBoolean(false); assertThrows(LockException.class, () -> { @@ -85,13 +98,13 @@ public void shouldThrowExceptionWhenCannotAcquireLock() throws ExecutionExceptio future.get(); } - private void truncateObjectLock(DataSource dataSource) { - try (var connection = dataSource.getConnection(); - var statement = connection.prepareStatement("TRUNCATE TABLE ocfl_object_lock")) { - statement.executeUpdate(); - } catch (SQLException e) { - throw new RuntimeException(e); - } + private ObjectLock createLock(String tableName) { + var name = TABLE_1.equals(tableName) ? null : tableName; + return new ObjectLockBuilder() + .waitTime(250, TimeUnit.MILLISECONDS) + .dataSource(dataSource) + .tableName(name) + .build(); } } diff --git a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/InMemoryObjectLockTest.java b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/InMemoryObjectLockTest.java index 5e290337..20a4cd3c 100644 --- a/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/InMemoryObjectLockTest.java +++ b/ocfl-java-core/src/test/java/edu/wisc/library/ocfl/core/lock/InMemoryObjectLockTest.java @@ -3,11 +3,13 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import edu.wisc.library.ocfl.api.exception.LockException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Phaser; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; @@ -25,25 +27,33 @@ public class InMemoryObjectLockTest { @BeforeEach public void setup() { cache = Caffeine.newBuilder().weakValues().build(); - lock = new InMemoryObjectLock(cache, 2, TimeUnit.SECONDS); - executor = Executors.newFixedThreadPool(2); + lock = new InMemoryObjectLock(cache, 250, TimeUnit.MILLISECONDS); + executor = Executors.newCachedThreadPool(); + } + + @AfterEach + public void after() { + executor.shutdown(); } @Test public void shouldRemoveValuesWhenNoLongerReferenced() throws Exception { var id = "obj1"; + var phaser = new Phaser(2); + var future = executor.submit(() -> { lock.doInWriteLock(id, () -> { + phaser.arriveAndAwaitAdvance(); try { - TimeUnit.SECONDS.sleep(3); + TimeUnit.MILLISECONDS.sleep(500); } catch (InterruptedException e) { throw new RuntimeException(e); } }); }); - TimeUnit.MILLISECONDS.sleep(100); + phaser.arriveAndAwaitAdvance(); assertTrue(cache.asMap().containsKey(id)); @@ -58,20 +68,23 @@ public void shouldRemoveValuesWhenNoLongerReferenced() throws Exception { } @Test - public void shouldBlockWhenLockAlreadyHeld() throws Exception { + public void shouldBlockWhenLockAlreadyHeld() { var id = "obj1"; + var phaser = new Phaser(2); + executor.submit(() -> { lock.doInWriteLock(id, () -> { + phaser.arriveAndAwaitAdvance(); try { - TimeUnit.SECONDS.sleep(3); + TimeUnit.MILLISECONDS.sleep(500); } catch (InterruptedException e) { throw new RuntimeException(e); } }); }); - TimeUnit.MILLISECONDS.sleep(100); + phaser.arriveAndAwaitAdvance(); assertThrows(LockException.class, () -> { lock.doInWriteLock(id, () -> {