Configure database connection via db.connection_string param

Needed for flexible configuration of connection parameters required to run MySQL8.

core/src/main/java/org/mskcc/cbio/portal/dao/JdbcDataSource.java

@@ -3,28 +3,58 @@
 import org.apache.commons.dbcp2.BasicDataSource;
 import org.mskcc.cbio.portal.util.DatabaseProperties;
 import org.apache.commons.lang3.StringUtils;
+import org.springframework.util.Assert;
 
 /**
  * Data source that self-initializes based on cBioPortal configuration.
  */
 public class JdbcDataSource extends BasicDataSource {
+
     public JdbcDataSource () {
         DatabaseProperties dbProperties = DatabaseProperties.getInstance();
+
         String host = dbProperties.getDbHost();
         String userName = dbProperties.getDbUser();
         String password = dbProperties.getDbPassword();
         String mysqlDriverClassName = dbProperties.getDbDriverClassName();
         String database = dbProperties.getDbName();
         String useSSL = (!StringUtils.isBlank(dbProperties.getDbUseSSL())) ? dbProperties.getDbUseSSL() : "false";
         String enablePooling = (!StringUtils.isBlank(dbProperties.getDbEnablePooling())) ? dbProperties.getDbEnablePooling(): "false";
-        String url ="jdbc:mysql://" + host + "/" + database +
-                        "?user=" + userName + "&password=" + password +
-                        "&zeroDateTimeBehavior=convertToNull&useSSL=" + useSSL;
+        String connectionURL = dbProperties.getConnectionURL();
+
+        Assert.hasText(userName, errorMessage("username", "db.user"));
+        Assert.hasText(password, errorMessage("password", "db.password"));
+        Assert.hasText(mysqlDriverClassName, errorMessage("driver class name", "db.driver"));
+
+        Assert.isTrue(
+            !((defined(host) || defined(database) || defined(dbProperties.getDbUseSSL())) && defined(connectionURL)),
+            "Properties define both db.connection_string and (one of) db.host, db.portal_db_name and db.use_ssl. " + 
+            "Please configure with either db.connection_string (preferred), or db.host, db.portal_db_name and db.use_ssl."
+        );
+
+        // For backward compatibility, build connection URL from individual properties.
+        if (connectionURL == null) {
+            Assert.hasText(host, errorMessage("host", "db.host") + 
+                " Or preferably, set the 'db.connection_string' and remove 'db.host', 'db.database' and 'db.use_ssl' (best practice).");
+            Assert.hasText(database, errorMessage("database", "db.database") + 
+                " Or preferably, set the 'db.connection_string' and remove 'db.host', 'db.database' and 'db.use_ssl (best practice).");
+            System.out.println("\n----------------------------------------------------------------------------------------------------------------");
+            System.out.println("-- Deprecation warning:");
+            System.out.println("-- You are connection to the database using the deprecated 'db.host', 'db.portal_db_name' and 'db.use_ssl' properties.");
+            System.out.println("-- Please use the 'db.connection_string' instead (see https://docs.cbioportal.org/deployment/customization/portal.properties-reference/).");
+            System.out.println("----------------------------------------------------------------------------------------------------------------\n");
+            connectionURL = String.format(
+                "jdbc:mysql://%s/%s?zeroDateTimeBehavior=convertToNull&useSSL=%s",
+                host, database, useSSL
+            );
+        }
+
+        this.setUrl(connectionURL);
+
         //  Set up poolable data source
         this.setDriverClassName(mysqlDriverClassName);
         this.setUsername(userName);
         this.setPassword(password);
-        this.setUrl(url);
         // Disable this to avoid caching statements
         this.setPoolPreparedStatements(Boolean.valueOf(enablePooling));
         // these are the values cbioportal has been using in their production
@@ -37,4 +67,12 @@ public JdbcDataSource () {
         this.setValidationQuery("SELECT 1");
         this.setJmxName("org.cbioportal:DataSource=" + database);
     }
+
+    private String errorMessage(String displayName, String propertyName) {
+        return String.format("No %s provided for database connection. Please set '%s' in portal.properties.", displayName, propertyName);
+    }
+
+    private boolean defined(String property) {
+        return property != null && !property.isEmpty();
+    }
 }

core/src/main/java/org/mskcc/cbio/portal/util/DatabaseProperties.java

@@ -47,6 +47,7 @@ public class DatabaseProperties {
     private String dbDriverClassName;
     private String dbUseSSL;
     private String dbEnablePooling;
+    private String connectionURL;
 
     // No production keys stored in filesystem or code: digest the key; put it in properties; load it into dbms on startup
     private static DatabaseProperties dbProperties;
@@ -63,6 +64,7 @@ public static DatabaseProperties getInstance() {
             dbProperties.setDbDriverClassName(GlobalProperties.getProperty("db.driver"));
             dbProperties.setDbUseSSL(GlobalProperties.getProperty("db.use_ssl"));
             dbProperties.setDbEnablePooling(GlobalProperties.getProperty("db.enable_pooling"));
+            dbProperties.setConnectionURL(GlobalProperties.getProperty("db.connection_string"));
         }
         return dbProperties;
     }
@@ -134,4 +136,12 @@ public void setDbEnablePooling(String dbEnablePooling) {
         this.dbEnablePooling = dbEnablePooling;
     }
 
+    public String getConnectionURL() {
+        return connectionURL;
+    }
+
+    public void setConnectionURL(String connectionURL) {
+        this.connectionURL = connectionURL;
+    }
+
 }

core/src/main/java/org/mskcc/cbio/portal/util/SpringUtil.java

@@ -38,33 +38,28 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
-import org.springframework.context.support.GenericXmlApplicationContext;
 import org.springframework.stereotype.Component;
 
 @Component
-public class SpringUtil
-{
+public class SpringUtil {
     private static final Logger log = LoggerFactory.getLogger(SpringUtil.class);
 
     private static AccessControl accessControl;
-    private static ApplicationContext context;
-    private static GenericXmlApplicationContext applicationContext;
+    private static ApplicationContext applicationContext;
 
     @Autowired
     public void setAccessControl(AccessControl accessControl) {
         log.debug("Setting access control");
         SpringUtil.accessControl = accessControl;
     }
 
-    public static AccessControl getAccessControl()
-    {
+    public static AccessControl getAccessControl() {
         return accessControl;
     }
 
-    public static synchronized void initDataSource()
-    {
-        if (SpringUtil.context == null) {
-            context = new ClassPathXmlApplicationContext("classpath:applicationContext-persistenceConnections.xml");
+    public static synchronized void initDataSource() {
+        if (SpringUtil.applicationContext == null) {
+            SpringUtil.applicationContext = new ClassPathXmlApplicationContext("classpath:applicationContext-persistenceConnections.xml");
         }
     }
 
@@ -74,7 +69,7 @@ public static synchronized void initDataSource()
      * @return the Spring Framework application context
      */
     public static ApplicationContext getApplicationContext() {
-        return context;
+        return applicationContext;
     }
 
     /**
@@ -84,7 +79,7 @@ public static ApplicationContext getApplicationContext() {
      * @param context
      */
     public static void setApplicationContext(ApplicationContext context) {
-        SpringUtil.context = context;
+        SpringUtil.applicationContext = context;
     }
 
     /**
@@ -95,6 +90,6 @@ public static void setApplicationContext(ApplicationContext context) {
      */
     public static synchronized void initDataSource(ApplicationContext context)
     {
-        SpringUtil.context = context;
+        SpringUtil.applicationContext = context;
     }
 }

core/src/main/scripts/importer/cbioportalImporter.py

@@ -419,8 +419,7 @@ def usage():
                            '--command [%s] --study_directory <path to directory> '
                            '--meta_filename <path to metafile>'
                            '--data_filename <path to datafile>'
-                           '--study_ids <cancer study ids for remove-study command, comma separated>'
-                           '--properties-filename <path to properties file> ' % (COMMANDS)), file=OUTPUT_FILE)
+                           '--study_ids <cancer study ids for remove-study command, comma separated>' % (COMMANDS)), file=OUTPUT_FILE)
 
 def check_args(command):
     if command not in COMMANDS:

core/src/main/scripts/migrate_db.py

@@ -6,19 +6,19 @@
 import contextlib
 import argparse
 from collections import OrderedDict
+import dsnparse
+
 import MySQLdb
 
 # globals
 ERROR_FILE = sys.stderr
 OUTPUT_FILE = sys.stdout
-DATABASE_HOST = 'db.host'
-DATABASE_NAME = 'db.portal_db_name'
 DATABASE_USER = 'db.user'
 DATABASE_PW = 'db.password'
-DATABASE_USE_SSL = 'db.use_ssl'
+DATABASE_URL = 'db.connection_string'
 VERSION_TABLE = 'info'
 VERSION_FIELD = 'DB_SCHEMA_VERSION'
-REQUIRED_PROPERTIES = [DATABASE_HOST, DATABASE_NAME, DATABASE_USER, DATABASE_PW, DATABASE_USE_SSL]
+REQUIRED_PROPERTIES = [DATABASE_USER, DATABASE_PW, DATABASE_URL]
 ALLOWABLE_GENOME_REFERENCES = ['37', 'hg19', 'GRCh37', '38', 'hg38', 'GRCh38', 'mm10', 'GRCm38']
 DEFAULT_GENOME_REFERENCE = 'hg19'
 MULTI_REFERENCE_GENOME_SUPPORT_MIGRATION_STEP = (2, 11, 0)
@@ -29,86 +29,63 @@
 class PortalProperties(object):
     """ Properties object class, just has fields for db conn """
 
-    def __init__(self, database_host, database_name, database_user, database_pw, database_use_ssl):
-        # default port:
-        self.database_port = 3306
-        # if there is a port added to the host name, split and use this one:
-        if ':' in database_host:
-            host_and_port = database_host.split(':')
-            self.database_host = host_and_port[0]
-            if self.database_host.strip() == 'localhost':
-                print(
-                    "Invalid host config '" + database_host + "' in properties file. If you want to specify a port on local host use '127.0.0.1' instead of 'localhost'",
-                    file=ERROR_FILE)
-                sys.exit(1)
-            self.database_port = int(host_and_port[1])
-        else:
-            self.database_host = database_host
-        self.database_name = database_name
+    def __init__(self, database_user, database_pw, database_url):
         self.database_user = database_user
         self.database_pw = database_pw
-        self.database_use_ssl = database_use_ssl
+        self.database_url = database_url
 
 def get_db_cursor(portal_properties):
     """ Establishes a MySQL connection """
     try:
-        connection_kwargs = {}
-        connection_kwargs['host'] = portal_properties.database_host
-        connection_kwargs['port'] = portal_properties.database_port
-        connection_kwargs['user'] = portal_properties.database_user
-        connection_kwargs['passwd'] = portal_properties.database_pw
-        connection_kwargs['db'] = portal_properties.database_name
-        if portal_properties.database_use_ssl == 'true':
-            connection_kwargs['ssl'] = {"ssl_mode": True}
-
+        url_elements = dsnparse.parse(portal_properties.database_url)
+        connection_kwargs = {
+            "host": url_elements.host,
+            "port": url_elements.port if url_elements.port is not None else 3306,
+            "db": url_elements.paths[0],
+            "user": portal_properties.database_user,
+            "passwd": portal_properties.database_pw,
+            "ssl": "useSSL" not in url_elements.query or url_elements.query["useSSL"] == "true"
+        }
         connection = MySQLdb.connect(**connection_kwargs)
     except MySQLdb.Error as exception:
         print(exception, file=ERROR_FILE)
-        port_info = ''
-        if portal_properties.database_host.strip() != 'localhost':
-            # only add port info if host is != localhost (since with localhost apparently sockets are used and not the given port) TODO - perhaps this applies for all names vs ips?
-            port_info = " on port " + str(portal_properties.database_port)
         message = (
-            "--> Error connecting to server "
-            + portal_properties.database_host
-            + port_info)
+            "--> Error connecting to server with URL"
+            + portal_properties.database_url)
         print(message, file=ERROR_FILE)
         raise ConnectionError(message) from exception
     if connection is not None:
         return connection, connection.cursor()
 
-def get_portal_properties(properties_filename):
-    """ Returns a properties object """
-    properties = {}
-    with open(properties_filename, 'r') as properties_file:
-        for line in properties_file:
-            line = line.strip()
-            # skip line if its blank or a comment
-            if len(line) == 0 or line.startswith('#'):
-                continue
-            try:
-                name, value = line.split('=', maxsplit=1)
-            except ValueError:
-                print(
-                    'Skipping invalid entry in property file: %s' % (line),
-                    file=ERROR_FILE)
-                continue
-            properties[name] = value.strip()
-    missing_properties = []
-    for required_property in REQUIRED_PROPERTIES:
-        if required_property not in properties or len(properties[required_property]) == 0:
-            missing_properties.append(required_property)
-    if missing_properties:
-        print(
-            'Missing required properties : (%s)' % (', '.join(missing_properties)),
-            file=ERROR_FILE)
-        return None
-    # return an instance of PortalProperties
-    return PortalProperties(properties[DATABASE_HOST],
-                            properties[DATABASE_NAME],
-                            properties[DATABASE_USER],
-                            properties[DATABASE_PW],
-                            properties[DATABASE_USE_SSL])
+def get_portal_properties(properties_filename) -> PortalProperties:
+        properties = {}
+        with open(properties_filename, 'r') as properties_file:
+            for line in properties_file:
+                line = line.strip()
+                # skip line if its blank or a comment
+                if len(line) == 0 or line.startswith('#'):
+                    continue
+                try:
+                    name, value = line.split('=', maxsplit=1)
+                except ValueError:
+                    print(
+                        'Skipping invalid entry in property file: %s' % (line),
+                        file=ERROR_FILE)
+                    continue
+                properties[name] = value.strip()
+        missing_properties = []
+        for required_property in REQUIRED_PROPERTIES:
+            if required_property not in properties or len(properties[required_property]) == 0:
+                missing_properties.append(required_property)
+        if missing_properties:
+            print(
+                'Missing required properties : (%s)' % (', '.join(missing_properties)),
+                file=ERROR_FILE)
+            return None
+        # return an instance of PortalProperties
+        return PortalProperties(properties[DATABASE_USER],
+                                properties[DATABASE_PW],
+                                properties[DATABASE_URL])
 
 def get_db_version(cursor):
     """ gets the version number of the database """
@@ -434,21 +411,13 @@ def main():
     properties_filename = parser.properties_file
     sql_filename = parser.sql
 
-    # check existence of properties file and sql file
-    if not os.path.exists(properties_filename):
-        print('properties file %s cannot be found' % (properties_filename), file=ERROR_FILE)
-        usage()
-        sys.exit(2)
     if not os.path.exists(sql_filename):
         print('sql file %s cannot be found' % (sql_filename), file=ERROR_FILE)
         usage()
         sys.exit(2)
 
     # parse properties file
     portal_properties = get_portal_properties(properties_filename)
-    if portal_properties is None:
-        print('failure reading properties file (%s)' % (properties_filename), file=ERROR_FILE)
-        sys.exit(1)
 
     # warn user
     if not parser.suppress_confirmation:

docs/deployment/customization/portal.properties-Reference.md

@@ -7,18 +7,25 @@ This page describes the main properties within portal.properties.
 ```
 db.user=
 db.password=
-db.host=[e.g. localhost to connect via socket, or e.g. 127.0.0.1:3307 to connect to a different port like 3307. Used by Java data import layer]
-db.portal_db_name=[the database name in mysql, e.g. cbiodb]
+db.connection_string=
 db.driver=[this is the name of your JDBC driver, e.g., com.mysql.jdbc.Driver]
 ```
 
-Include `db_connection_string` with the format specified below, and replace `localhost` by the value of `db.host`:
+The format of the `db.connection_string` is:
+```
+jdbc:mysql://<host>:<port>/<database name>?<parameter1>&<parameter2>&<parameter...>
+```
+
+For example:
 
 ```
-db.connection_string=jdbc:mysql://localhost/
+jdbc:mysql://localhost:3306/cbiodb?zeroDateTimeBehavior=convertToNull&useSSL=false
 ```
 
-db.tomcat\_resource\_name is required in order to work with the tomcat database connection pool and should have the default value jdbc/cbioportal in order to work correctly with the your WAR file.
+:warning: The fields `db.host` and `db.portal_db_name` are deprecated. It is recommended to configure the database connection using
+the `db.connection_string` instead.
+
+`db.tomcat_resource_name` is required in order to work with the tomcat database connection pool and should have the default value jdbc/cbioportal in order to work correctly with the your WAR file.
 
 ```
 db.tomcat_resource_name=jdbc/cbioportal

requirements.txt

@@ -3,3 +3,4 @@ markupsafe==2.0.1
 Jinja2==2.11.3
 mysqlclient==2.1.0
 PyYAML==5.4
+dsnparse==0.1.15