From c4e6b046aeff38cec847a480bef2a47c61ee1b8b Mon Sep 17 00:00:00 2001
From: James Muehlner <james.muehlner@guac-dev.org>
Date: Thu, 11 May 2023 00:12:43 +0000
Subject: [PATCH] GUACAMOLE-926: Disable batch executor for SQL Server JDBC
 extension - it doesn't work.

---
 .../jdbc/JDBCAuthenticationProviderModule.java |  9 +++++----
 .../guacamole/auth/jdbc/JDBCEnvironment.java   | 18 ++++++++++++++++++
 .../sqlserver/conf/SQLServerEnvironment.java   | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java
index 7b735bdf4..1b361e561 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java
@@ -127,10 +127,11 @@ public class JDBCAuthenticationProviderModule extends MyBatisModule {
         // Transaction factory
         bindTransactionFactoryType(JdbcTransactionFactory.class);
         
-        // Set the JDBC Auth provider to use batch execution when possible
-        bindConfigurationSetting(configuration -> {
-            configuration.setDefaultExecutorType(ExecutorType.BATCH);
-        });
+        // Set the JDBC Auth provider to use batch execution if enabled
+        if (environment.shouldUseBatchExecutor())
+            bindConfigurationSetting(configuration -> {
+                configuration.setDefaultExecutorType(ExecutorType.BATCH);
+            });
 
         // Add MyBatis mappers
         addMapperClass(ConnectionMapper.class);
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java
index f7a890e49..27f3d0563 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java
@@ -254,4 +254,22 @@ public abstract class JDBCEnvironment extends DelegatingEnvironment {
      */
     public abstract boolean enforceAccessWindowsForActiveSessions() throws GuacamoleException;
 
+    /**
+     * Returns true if the JDBC batch executor should be used by default, false
+     * otherwise. The batch executor allows repeated updates to be batched
+     * together for improved performance. 
+     * See https://mybatis.org/mybatis-3/java-api.html#sqlSessions
+     *
+     * @return
+     *     true if the batch executor should be used by default, false otherwise.
+     */
+    public boolean shouldUseBatchExecutor() {
+
+        // Unless otherwise overwritten due to implementation-specific problems,
+        // all JDBC extensions should use the batch executor if possible to
+        // ensure the best performance for repetitive queries
+        return true;
+
+    }
+
 }
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java
index 612fa16d8..3f3133743 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java
@@ -295,6 +295,21 @@ public class SQLServerEnvironment extends JDBCEnvironment {
                 true);
     }
 
+    @Override
+    public boolean shouldUseBatchExecutor() {
+
+        // The SQL Server driver does not work when batch execution is enabled.
+        // Specifically, inserts fail with com.microsoft.sqlserver.jdbc.SQLServerException:
+        // The statement must be executed before any results can be obtained.
+        // See https://github.com/microsoft/mssql-jdbc/issues/358 for more.
+        logger.warn(
+                "JDBC batch executor is disabled for SQL Server Connections. "
+                + "Large batched updates may run slower."
+        );
+        return false;
+        
+    }
+
     /**
      * Returns true if all server certificates should be trusted, including
      * those signed by an unknown certificate authority, such as self-signed