From b5107ce943518234c36adfac4155f28118ceb6ba Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 17 Apr 2026 13:03:08 -0600 Subject: [PATCH 1/2] More docs for DbScope.Transaction usage patterns --- api/src/org/labkey/api/data/DbScope.java | 59 +++++++++++++++--------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 022e7ef2870..e6e6a8dc752 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -19,7 +19,7 @@ import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; import org.apache.commons.collections4.IteratorUtils; -import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -110,7 +110,7 @@ /** * Class that wraps a data source and is shared amongst that data source's DbSchemas. Allows "nested" transactions, * implemented via a reference-counting style approach. Each (potentially nested) set of code should call - * {@code ensureTransaction()}. This will either start a new transaction, or join an existing one. Once the outermost + * {@code ensureTransaction()}. This will either start a new transaction or join an existing one. Once the outermost * caller calls commit(), the WHOLE transaction will be committed at once. The most common usage scenario looks * something like: * 
{@code
@@ -121,8 +121,27 @@
  *     transaction.commit();
  * }
  *
- * The DbScope.Transaction class implements AutoCloseable, so it will be cleaned up automatically by JDK 7's try {}
- * resource handling.
+ * }
+ *
+ * The DbScope.Transaction class implements AutoCloseable, so it will be cleaned up automatically via try-with-resources. + *

+ * All return pathways inside the transaction must commit, or the transaction will be considered abandoned to be rolled + * back. There should be no further database work prior to existing the try block after the commit. Example: + * 

{@code
+ * DbScope scope = dbSchemaInstance.getScope();
+ * try (DbScope.Transaction transaction = scope.ensureTransaction())
+ * {
+ *     // Do some work
+ *     if (condition) {
+ *         transaction.commit();
+ *         return true;
+ *     }
+ *     // Do some other work
+ *     boolean result = determineResult();
+ *     transaction.commit();
+ *     return result;
+ * }
+ *
  * }
  *
*/ @@ -869,7 +888,7 @@ public Transaction beginTransaction(TransactionKind transactionKind, Lock... loc int stackDepth; synchronized (_transaction) { - List transactions = _transaction.computeIfAbsent(getEffectiveThread(), k -> new ArrayList<>()); + List transactions = _transaction.computeIfAbsent(getEffectiveThread(), _ -> new ArrayList<>()); transactions.add(result); stackDepth = transactions.size(); } @@ -1093,7 +1112,7 @@ public boolean isTransactionActive() synchronized (_transaction) { List transactions = _transaction.get(thread); - return transactions == null ? null : transactions.get(transactions.size() - 1); + return transactions == null ? null : transactions.getLast(); } } @@ -1242,7 +1261,7 @@ private ConnectionHolder getConnectionHolder() // Synchronize just long enough to get a ConnectionHolder into the map synchronized (_threadConnections) { - return _threadConnections.computeIfAbsent(thread, t -> new ConnectionHolder()); + return _threadConnections.computeIfAbsent(thread, _ -> new ConnectionHolder()); } } @@ -1301,7 +1320,7 @@ private static void ensureDelegation(Connection conn) try { // Test the method to make sure we can access it - Connection test = (Connection) methodGetInnermostDelegate.invoke(conn); + var _ = (Connection) methodGetInnermostDelegate.invoke(conn); isDelegating = true; return; } @@ -2000,7 +2019,7 @@ private static void createDataBase(SqlDialect dialect, LabKeyDataSource ds) thro LOG.info("Attempting to create database \"{}\"", dbName); - String defaultUrl = StringUtils.replace(url, dbName, dialect.getDefaultDatabaseName()); + String defaultUrl = Strings.CS.replace(url, dbName, dialect.getDefaultDatabaseName()); String createSql = "(undefined)"; try (Connection conn = getRawConnection(defaultUrl, ds)) @@ -2415,8 +2434,8 @@ public T add(TransactionImpl transaction, T task) public interface Transaction extends AutoCloseable { - /* - * @return the task that was inserted or the existing object (equal to the runnable passed in) that will be run instead + /** + * @return the task that was inserted or the existing object (equal to the runnable passed in) that will be run instead */ @NotNull T addCommitTask(@NotNull T runnable, @NotNull CommitTaskOption firstOption, CommitTaskOption... additionalOptions); @@ -2796,7 +2815,7 @@ private boolean isOutermostTransaction() /** Remove current transaction nesting and unlock any locks */ private void decrement() { - List locks = _locks.remove(_locks.size() - 1); + List locks = _locks.removeLast(); for (Lock lock : locks) { // Release all the locks @@ -2853,7 +2872,7 @@ public void increment(boolean releaseOnFinalCommit, List extraLocks) if (!_locks.isEmpty() && releaseOnFinalCommit) { // Add the new locks to the outermost set of locks - _locks.get(0).addAll(extraLocks); + _locks.getFirst().addAll(extraLocks); // Add an empty list to this layer of the transaction _locks.add(new ArrayList<>()); } @@ -3328,7 +3347,7 @@ public void testNestedMissingCommit() //noinspection EmptyTryBlock try (Transaction ignored = getLabKeyScope().ensureTransaction()) { - // Intentionally don't call t2.commit(); + // Intentionally, don't call t2.commit(); } try { @@ -3359,14 +3378,8 @@ public void testMultipleTransactionKinds() try (Transaction t2 = getLabKeyScope().ensureTransaction()) { assertSame(connection, t2.getConnection()); - try (Transaction t3 = getLabKeyScope().ensureTransaction(new TransactionKind() - { - @NotNull - @Override - public String getKind() - { - return "PIPELINESTATUS"; // We can't really see PipelineStatus here, but just need something non-normal to test - } + try (Transaction t3 = getLabKeyScope().ensureTransaction(() -> { + return "PIPELINESTATUS"; // We can't really see PipelineStatus here, but just need something non-normal to test })) { assertTrue(getLabKeyScope().isTransactionActive()); @@ -3392,7 +3405,7 @@ public void testServerRowLock() Lock lockUser = new ServerPrimaryKeyLock(true, CoreSchema.getInstance().getTableInfoUsersData(), user.getUserId()); Lock lockHome = new ServerPrimaryKeyLock(true, CoreSchema.getInstance().getTableInfoContainers(), ContainerManager.getHomeContainer().getId()); - attemptToDeadlock(lockUser, lockHome, (x) -> {}, PessimisticLockingFailureException.class); + attemptToDeadlock(lockUser, lockHome, (_) -> {}, PessimisticLockingFailureException.class); } private void attemptToDeadlock(Lock lock1, Lock lock2, @NotNull Consumer transactionModifier, Class expectedException) From d80601bafeefcb21cd0a6904976f1b5db3877386 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Fri, 17 Apr 2026 21:03:07 -0600 Subject: [PATCH 2/2] Update api/src/org/labkey/api/data/DbScope.java Co-authored-by: Adam Rauch --- api/src/org/labkey/api/data/DbScope.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index e6e6a8dc752..e148df203fa 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -126,7 +126,7 @@ * The DbScope.Transaction class implements AutoCloseable, so it will be cleaned up automatically via try-with-resources. *

* All return pathways inside the transaction must commit, or the transaction will be considered abandoned to be rolled - * back. There should be no further database work prior to existing the try block after the commit. Example: + * back. There should be no further database work prior to exiting the try block after the commit. Example: * 

{@code
  * DbScope scope = dbSchemaInstance.getScope();
  * try (DbScope.Transaction transaction = scope.ensureTransaction())