[GitHub] [guacamole-client] necouchman commented on a change in pull request #642: GUACAMOLE-1407: Automatically detect whether MySQL or MariaDB version of "Connector/J" is installed.

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #642:
URL: https://github.com/apache/guacamole-client/pull/642#discussion_r697080618



##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/java/org/apache/guacamole/auth/mysql/conf/MySQLEnvironment.java
##########
@@ -178,8 +179,10 @@ public PasswordPolicy getPasswordPolicy() {
 
     /**
      * Returns the MySQL driver that will be used to talk to the MySQL-compatible
-     * database server hosting the Guacamole Client database.  If unspecified
-     * a default value of MySQL will be used.
+     * database server hosting the Guacamole database. If unspecified, the
+     * installed MySQL driver will be automatically detected by inspecting the
+     * classes available an the classpath. If automatic detection fails, the

Review comment:
       Perhaps "in the classpath"?

##########
File path: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-mysql/src/main/java/org/apache/guacamole/auth/mysql/conf/MySQLEnvironment.java
##########
@@ -189,10 +192,30 @@ public PasswordPolicy getPasswordPolicy() {
      *     If guacamole.properties cannot be parsed.
      */
     public MySQLDriver getMySQLDriver() throws GuacamoleException {
-        return getProperty(
-            MySQLGuacamoleProperties.MYSQL_DRIVER,
-            DEFAULT_DRIVER
-        );
+
+        // Use any explicitly-specified driver
+        MySQLDriver driver = getProperty(MySQLGuacamoleProperties.MYSQL_DRIVER);
+        if (driver != null)
+            return driver;
+
+        // Attempt autodetection based on presence of JDBC driver within
+        // classpath...
+
+        if (MySQLDriver.MARIADB.isInstalled()) {
+            logger.info("Installed JDBC driver for MySQL/MariaDB detected as \"MariaDB Connector/J\".");
+            return MySQLDriver.MARIADB;
+        }
+
+        if (MySQLDriver.MYSQL.isInstalled()) {
+            logger.info("Installed JDBC driver for MySQL/MariaDB detected as \"MySQL Connector/J\".");
+            return MySQLDriver.MYSQL;
+        }
+
+        // Fallback to MySQL Connector/J if nothing can be found
+        logger.warn("JDBC driver for MySQL/MariaDB couuld not be detected "
+                + "and might not be installed. Assuming MySQL Connector/J...");
+        return FALLBACK_DEFAULT_DRIVER;

Review comment:
       If we reach the this fallback point, doesn't it mean that the MySQLDriver.MySQL has already not been found? Should we really fall back to something we already are reasonably certain doesn't exist, or would it be better to throw an exception here that tells the admin the driver is missing?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org