Merge branch 'develop' into fb_issue51263

api/src/org/labkey/api/data/AsyncQueryRequest.java

@@ -142,7 +142,7 @@ synchronized public T waitForResult(final Callable<T> callable) throws SQLExcept
         Thread thread = new Thread(runnable, "AsyncQueryRequest: " + Thread.currentThread().getName());
         // We want the async thread to use the same database connection, in case we have a transaction open, and
         // so that when the original thread finishes processing the results it ends up closing the right connection
-        try (DbScope.ConnectionSharingCloseable ignored = DbScope.shareConnections(thread);
+        try (DbScope.ConnectionSharingCloseable ignored = DbScope.shareConnections(Thread.currentThread(), thread);
              Scope ignore = tracer.activateSpan(span))
         {
             thread.start();

api/src/org/labkey/api/data/ConnectionWrapper.java

@@ -34,7 +34,6 @@
 import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.ViewServlet;
 
-import java.lang.reflect.Method;
 import java.sql.Array;
 import java.sql.Blob;
 import java.sql.CallableStatement;
@@ -96,9 +95,6 @@ public class ConnectionWrapper implements java.sql.Connection
     /** Remember code that closed the Transaction but didn't commit - it may be faulty if it doesn't signal the failure back to the caller */
     private Throwable _suspiciousCloseStackTrace;
 
-    /** For debugging issue 23044 */
-    private static Method _transactionStateMethod;
-
     private volatile boolean _allowClose = true;
 
     private static boolean initializeExplicitLogger()
@@ -177,8 +173,9 @@ public static boolean dumpOpenConnections(@NotNull LoggerWriter logWriter)
         return true;
     }
 
-    public static void closeConnections(Thread thread)
+    public static void closeConnections()
     {
+        Thread thread = DbScope.getEffectiveThread();
         List<ConnectionWrapper> toClose = new ArrayList<>();
 
         synchronized (_openConnections)
@@ -196,7 +193,7 @@ public static void closeConnections(Thread thread)
         {
             try
             {
-                connectionWrapper.close(thread);
+                connectionWrapper.close();
             }
             catch (SQLException ignored) {}
         }
@@ -395,15 +392,10 @@ public interface Closer
 
     @Override
     public void close() throws SQLException
-    {
-        close(Thread.currentThread());
-    }
-
-    public void close(Thread thread) throws SQLException
     {
         DbScope.Transaction t;
 
-        if (_type != ConnectionType.Pooled && null != (t = _scope.getTransactionImpl(thread)))
+        if (_type != ConnectionType.Pooled && null != (t = _scope.getCurrentTransactionImpl()))
         {
             assert t.getConnection() == this : "Attempting to close a different connection from the one associated with this thread: " + this + " vs " + t.getConnection(); //Should release same conn we handed out
         }
@@ -428,31 +420,6 @@ private void realClose() throws SQLException
         // if it's already been closed instead of doing a no-op
         if (null != _connection && !isClosed())
         {
-            /* For debugging issue 23044, look for connections that are set to be autoCommit but the driver thinks are mid-transaction */
-            Connection connection = DbScope.getDelegate(_connection);
-            if (connection != null && connection.getAutoCommit() && "org.postgresql.jdbc4.Jdbc4Connection".equals(connection.getClass().getName()))
-            {
-                try
-                {
-                    if (_transactionStateMethod == null)
-                    {
-                        _transactionStateMethod = connection.getClass().getMethod("getTransactionState");
-                        _log.debug("Got transactionStateMethod from org.postgresql.jdbc4.Jdbc4Connection. " + _transactionStateMethod);
-                    }
-                    Object state = _transactionStateMethod.invoke(connection);
-
-                    // org.postgresql.core.ProtocolConnection.TRANSACTION_OPEN = 1
-                    if (1 == ((Number) state).intValue())
-                    {
-                        _log.error("Transaction state does not match autoCommit for " + this, new Throwable());
-                    }
-                }
-                catch (Exception e)
-                {
-                    _log.error("Reflection error", e);
-                }
-            }
-
             try
             {
                 _runOnClose.close();
@@ -1051,15 +1018,15 @@ public void abort(Executor executor)
     }
 
     @Override
-    public void setNetworkTimeout(Executor executor, int milliseconds)
+    public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException
     {
-        throw new UnsupportedOperationException();
+        _connection.setNetworkTimeout(executor, milliseconds);
     }
 
     @Override
-    public int getNetworkTimeout()
+    public int getNetworkTimeout() throws SQLException
     {
-        throw new UnsupportedOperationException();
+        return _connection.getNetworkTimeout();
     }
 
     private void checkForSuspiciousClose() throws SQLException

api/src/org/labkey/api/data/ContainerManager.java

@@ -30,6 +30,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.labkey.api.Constants;
+import org.labkey.api.action.ApiUsageException;
 import org.labkey.api.action.SpringActionController;
 import org.labkey.api.admin.FolderExportContext;
 import org.labkey.api.admin.FolderImportContext;
@@ -1817,12 +1818,12 @@ private static boolean delete(final Collection<Container> containers, User user,
                 {
                     if (!isDeletable(container))
                     {
-                        throw new IllegalArgumentException("Cannot delete container: " + container.getPath());
+                        throw new ApiUsageException("Cannot delete container: " + container.getPath());
                     }
 
                     if (deletingContainers.containsKey(container.getId()))
                     {
-                        throw new IllegalArgumentException("Container is already being deleted: " + container.getPath());
+                        throw new ApiUsageException("Container is already being deleted: " + container.getPath());
                     }
                 }
                 containerIds.forEach(id -> deletingContainers.put(id, user.getUserId()));
@@ -2666,7 +2667,7 @@ private static boolean isValidTargetContainer(Container current, Container targe
 
     public static int updateContainer(TableInfo dataTable, String idField, Collection<?> ids, Container targetContainer, User user, boolean withModified)
     {
-        try (DbScope.Transaction transaction = ensureTransaction())
+        try (DbScope.Transaction transaction = dataTable.getSchema().getScope().ensureTransaction())
         {
             SQLFragment dataUpdate = new SQLFragment("UPDATE ").append(dataTable)
                     .append(" SET container = ").appendValue(targetContainer.getEntityId());

api/src/org/labkey/api/data/DbScope.java

@@ -1038,17 +1038,13 @@ private <ReturnType> ReturnType _executeWithRetry(boolean useTx, RetryFn<ReturnT
         }
     }
 
-    private static Thread getEffectiveThread()
-    {
-        return getEffectiveThread(Thread.currentThread());
-    }
-
     /**
      * @return the thread that owns the connections for the given thread. Background threads may piggyback on an
-     * HTTP request thread, for example. If no sharing has been established, the same thread as passed
+     * HTTP request thread, for example. If no sharing has been established, use the current thread
      */
-    private static Thread getEffectiveThread(Thread thread)
+    public static Thread getEffectiveThread()
     {
+        Thread thread = Thread.currentThread();
         synchronized (_sharedConnections)
         {
             Thread result = _sharedConnections.get(thread);
@@ -1073,12 +1069,7 @@ public boolean isTransactionActive()
     /* package */
     @Nullable TransactionImpl getCurrentTransactionImpl()
     {
-        return getTransactionImpl(Thread.currentThread());
-    }
-
-    /* package */ @Nullable TransactionImpl getTransactionImpl(Thread thread)
-    {
-        thread = getEffectiveThread(thread);
+        Thread thread = getEffectiveThread();
         synchronized (_transaction)
         {
             List<TransactionImpl> transactions = _transaction.get(thread);
@@ -2084,21 +2075,15 @@ public static void clearFailedDbScopes()
     }
 
     /**
-     * Shuts down any connections associated with DbScopes that have been handed out to the current thread
+     * Shuts down any connections associated with DbScopes that have been handed out to the current thread or its
+     * associated/effective thread. Also releases locks acquired as part of opening the transaction.
      */
     public static void closeAllConnectionsForCurrentThread()
     {
-        closeAllConnectionsForThread(getEffectiveThread());
-    }
-    /**
-     * Shuts down any connections associated with DbScopes that have been handed out to the current thread. Also
-     * releases locks acquired as part of opening the transaction.
-     */
-    public static void closeAllConnectionsForThread(Thread thread)
-    {
+        Thread thread = getEffectiveThread();
         for (DbScope scope : getInitializedDbScopes())
         {
-            TransactionImpl t = scope.getTransactionImpl(thread);
+            TransactionImpl t = scope.getCurrentTransactionImpl();
             int count = 0;
             while (t != null)
             {
@@ -2120,13 +2105,13 @@ public static void closeAllConnectionsForThread(Thread thread)
                 }
 
                 // We may have nested concurrent transactions for a given scope, so be sure we close them all
-                TransactionImpl t2 = scope.getTransactionImpl(thread);
+                TransactionImpl t2 = scope.getCurrentTransactionImpl();
                 t = (t2 == null || t2 == t) ? null : t2;
             }
         }
 
         // Also close down connections that might not have been connected with a Transaction object
-        ConnectionWrapper.closeConnections(thread);
+        ConnectionWrapper.closeConnections();
     }
 
     /**
@@ -2143,21 +2128,22 @@ public static void finishedWithThread()
      * Causes any connections associated with the current thread to be used by the passed-in thread. This allows
      * an async thread to participate in the same transaction as the original HTTP request processing thread, for
      * example
-     * @param asyncThread the thread that should use the database connections of the current thread
+     * @param primaryThread the thread that should be treated as the owner for any connections used
+     * @param piggybackingThread the thread that should use the database connections of the primary thread
      * @return an AutoCloseable that will stop sharing the database connection with the other thread
      */
-    public static ConnectionSharingCloseable shareConnections(final Thread asyncThread)
+    public static ConnectionSharingCloseable shareConnections(Thread primaryThread, final Thread piggybackingThread)
     {
         synchronized (_sharedConnections)
         {
-            if (_sharedConnections.containsKey(asyncThread))
+            if (_sharedConnections.containsKey(piggybackingThread))
             {
-                throw new IllegalStateException("Thread '" + asyncThread.getName() + "' is already sharing the connections of thread '" + _sharedConnections.get(asyncThread) + "'");
+                throw new IllegalStateException("Thread '" + piggybackingThread.getName() + "' is already sharing the connections of thread '" + _sharedConnections.get(piggybackingThread) + "'");
             }
-            _sharedConnections.put(asyncThread, Thread.currentThread());
+            _sharedConnections.put(piggybackingThread, primaryThread);
         }
 
-        return new ConnectionSharingCloseable(asyncThread);
+        return new ConnectionSharingCloseable(piggybackingThread);
     }
 
     interface ConnectionMap
@@ -2441,8 +2427,9 @@ public void setAuditEvent(TransactionAuditProvider.TransactionAuditEvent event)
     };
 
 
-    private void popCurrentTransaction(Thread thread)
+    private void popCurrentTransaction()
     {
+        Thread thread = getEffectiveThread();
         synchronized (_transaction)
         {
             List<TransactionImpl> transactions = _transaction.get(thread);
@@ -2601,7 +2588,7 @@ public void close(Thread thread)
                 {
                     // Don't pop until locks are empty, because other closes have yet to occur,
                     // and we want to use _abort to ensure no one tries to commit this transaction
-                    popCurrentTransaction(thread);
+                    popCurrentTransaction();
 
                     if (_aborted)
                     {
@@ -2666,7 +2653,7 @@ public void commit()
                                 conn.internalClose();
                         }
 
-                        popCurrentTransaction(getEffectiveThread());
+                        popCurrentTransaction();
 
                         CommitTaskOption.POSTCOMMIT.run(this);
                     }

api/src/org/labkey/api/reports/report/ReportDescriptor.java

@@ -288,6 +288,11 @@ public void setWasShared()
         _wasShared = true;
     }
 
+    public boolean isInheritable()
+    {
+        return (getFlags() & FLAG_INHERITABLE) != 0;
+    }
+
     @Nullable
     public Integer getAuthor()
     {
@@ -780,7 +785,7 @@ public boolean isInherited(Container c)
             // if the report has been configured to be shared to child folders or is in the shared folder then
             // flag it as inherited.
             //
-            if (((getFlags() & ReportDescriptor.FLAG_INHERITABLE) != 0) || (ContainerManager.getSharedContainer().equals(srcContainer)))
+            if (isInheritable() || (ContainerManager.getSharedContainer().equals(srcContainer)))
             {
                 return !c.equals(srcContainer);
             }

api/src/org/labkey/api/reports/report/view/ReportDesignBean.java

@@ -292,8 +292,8 @@ void populateFromDescriptor(ReportDescriptor descriptor)
         setCached(BooleanUtils.toBoolean(descriptor.getProperty(ReportDescriptor.Prop.cached)));
 
         setReportAccess(descriptor.getAccess());
-        setShareReport((descriptor.isShared()));
-        setInheritable((descriptor.getFlags() & ReportDescriptor.FLAG_INHERITABLE) != 0);
+        setShareReport(descriptor.isShared());
+        setInheritable(descriptor.isInheritable());
         setRedirectUrl(getViewContext().getActionURL().getParameter(ReportDescriptor.Prop.redirectUrl.name()));
     }
 }

api/src/org/labkey/api/reports/report/view/ReportUtil.java

@@ -325,7 +325,7 @@ public static boolean isReportInherited(Container c, Report report)
                 return !ContainerManager.getSharedContainer().equals(c);
             }
 
-            if ((report.getDescriptor().getFlags() & ReportDescriptor.FLAG_INHERITABLE) != 0)
+            if (report.getDescriptor().isInheritable())
             {
                 Container reportContainer = ContainerManager.getForId(report.getDescriptor().getContainerId());
                 if (!c.equals(reportContainer))

api/src/org/labkey/api/security/SectionHeader.java

@@ -0,0 +1,15 @@
+package org.labkey.api.security;
+
+import org.jetbrains.annotations.NotNull;
+
+public class SectionHeader extends SettingsField
+{
+    public static SectionHeader of(@NotNull String caption)
+    {
+        SectionHeader of = new SectionHeader();
+        of.put("type", FieldType.section);
+        of.put("caption", caption);
+
+        return of;
+    }
+}

api/src/org/labkey/api/security/SecurityManager.java

@@ -224,6 +224,10 @@ public static UsageMetricsProvider getMetricsProvider()
         };
     }
 
+    /**
+     * @param key short description of the purpose of the allowed connection. Mostly to make it possible to update
+     *            afterward in the case of dynamically configured sources.
+     */
     public static void registerAllowedConnectionSource(String key, String serviceURL)
     {
         if (StringUtils.trimToNull(serviceURL) == null)

api/src/org/labkey/api/security/SettingsField.java

@@ -8,6 +8,7 @@ public class SettingsField extends HashMap<String, Object>
 {
     public enum FieldType
     {
+        section,
         checkbox,
         fixedHtml,
         input,
@@ -16,6 +17,15 @@ public enum FieldType
         pem  // file picker that allows upload of a PEM file (used on SAML settings page)
     }
 
+    public static SettingsField of(@NotNull String name, @NotNull FieldType type, @NotNull String caption)
+    {
+        SettingsField sf = new SettingsField();
+        sf.put("name", name);
+        sf.put("type", type.toString());
+        sf.put("caption", caption);
+
+        return sf;
+    }
     public static SettingsField of(@NotNull String name, @NotNull FieldType type, @NotNull String caption, @NotNull String description, boolean required, Object defaultValue)
     {
         SettingsField sf = of(name, type, caption, required, defaultValue);