Refactor ConnectionFactory to static inner holder Singleton (#2204)

* refactor ConnectionFactory to static inner holder Singleton

* remove redundant import

* make getSqlSessionFactory() method private

* fix sonar issue

* remove @Ignore of MailUtilsTest

* add MailUtilsTest to pom.xml to test

* fix MailUtilsTest path error in pom.xml

* modify test method name

* add assert and logger to unit test

* add log

* add log

* add AlertDaoTest

* move AlertDaoTest to new module

* modify test in pom.xml

* remove unnecessary log

dolphinscheduler-alert/src/test/java/org/apache/dolphinscheduler/alert/utils/MailUtilsTest.java

@@ -23,7 +23,7 @@ import org.apache.dolphinscheduler.dao.AlertDao;
 import org.apache.dolphinscheduler.dao.DaoFactory;
 import org.apache.dolphinscheduler.dao.entity.Alert;
 import org.apache.dolphinscheduler.dao.entity.User;
-import org.junit.Ignore;
+import org.junit.Assert;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -33,7 +33,6 @@ import java.util.*;
 
 /**
  */
-@Ignore
 public class MailUtilsTest {
     private static final Logger logger = LoggerFactory.getLogger(MailUtilsTest.class);
     @Test
@@ -138,8 +137,10 @@ public class MailUtilsTest {
      * Table
      */
     @Test
-    public void addAlertTable(){
+    public void testAddAlertTable(){
+        logger.info("testAddAlertTable");
         AlertDao alertDao = DaoFactory.getDaoInstance(AlertDao.class);
+        Assert.assertNotNull(alertDao);
         Alert alert = new Alert();
         alert.setTitle("Mysql Exception");
         alert.setShowType(ShowType.TABLE);
@@ -149,6 +150,7 @@ public class MailUtilsTest {
         alert.setAlertType(AlertType.EMAIL);
         alert.setAlertGroupId(1);
         alertDao.addAlert(alert);
+        logger.info("" +alert);
     }
 
     @Test

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java

@@ -50,8 +50,8 @@ public class AlertDao extends AbstractBaseDao {
 
     @Override
     protected void init() {
-        alertMapper = ConnectionFactory.getMapper(AlertMapper.class);
-        userAlertGroupMapper = ConnectionFactory.getMapper(UserAlertGroupMapper.class);
+        alertMapper = ConnectionFactory.getInstance().getMapper(AlertMapper.class);
+        userAlertGroupMapper = ConnectionFactory.getInstance().getMapper(UserAlertGroupMapper.class);
     }
 
     /**

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/datasource/ConnectionFactory.java

@@ -34,29 +34,47 @@ import javax.sql.DataSource;
 
 
 /**
- *  not spring manager connection, only use for init db, and alert module for non-spring application
+ * not spring manager connection, only use for init db, and alert module for non-spring application
  * data source connection factory
  */
-public class ConnectionFactory extends SpringConnectionFactory{
+public class ConnectionFactory extends SpringConnectionFactory {
 
     private static final Logger logger = LoggerFactory.getLogger(ConnectionFactory.class);
 
+    private static class ConnectionFactoryHolder {
+        private static final ConnectionFactory connectionFactory = new ConnectionFactory();
+    }
+
+    public static ConnectionFactory getInstance() {
+        return ConnectionFactoryHolder.connectionFactory;
+    }
+
+    private ConnectionFactory() {
+        try {
+            sqlSessionFactory = getSqlSessionFactory();
+            sqlSessionTemplate = getSqlSessionTemplate();
+        } catch (Exception e) {
+            logger.error("Initializing ConnectionFactory error", e);
+            throw new RuntimeException(e);
+        }
+    }
 
     /**
      * sql session factory
      */
-    private static SqlSessionFactory sqlSessionFactory;
+    private SqlSessionFactory sqlSessionFactory;
 
     /**
      * sql session template
      */
-    private static SqlSessionTemplate sqlSessionTemplate;
+    private SqlSessionTemplate sqlSessionTemplate;
 
     /**
      * get the data source
+     *
      * @return druid dataSource
      */
-    public static DruidDataSource getDataSource() {
+    public DruidDataSource getDataSource() {
 
         DruidDataSource druidDataSource = new DruidDataSource();
 
@@ -89,65 +107,54 @@ public class ConnectionFactory extends SpringConnectionFactory{
 
     /**
      * * get sql session factory
+     *
      * @return sqlSessionFactory
      * @throws Exception sqlSessionFactory exception
      */
-    public static SqlSessionFactory getSqlSessionFactory() throws Exception {
-        if (sqlSessionFactory == null) {
-            synchronized (ConnectionFactory.class) {
-                if (sqlSessionFactory == null) {
-                    DataSource dataSource = getDataSource();
-                    TransactionFactory transactionFactory = new JdbcTransactionFactory();
-
-                    Environment environment = new Environment("development", transactionFactory, dataSource);
-
-                    MybatisConfiguration configuration = new MybatisConfiguration();
-                    configuration.setEnvironment(environment);
-                    configuration.setLazyLoadingEnabled(true);
-                    configuration.addMappers("org.apache.dolphinscheduler.dao.mapper");
-                    configuration.addInterceptor(new PaginationInterceptor());
-
-                    MybatisSqlSessionFactoryBean sqlSessionFactoryBean = new MybatisSqlSessionFactoryBean();
-                    sqlSessionFactoryBean.setConfiguration(configuration);
-                    sqlSessionFactoryBean.setDataSource(dataSource);
-
-                    sqlSessionFactoryBean.setTypeEnumsPackage("org.apache.dolphinscheduler.*.enums");
-                    sqlSessionFactory = sqlSessionFactoryBean.getObject();
-                }
-            }
-        }
+    private SqlSessionFactory getSqlSessionFactory() throws Exception {
+        DataSource dataSource = getDataSource();
+        TransactionFactory transactionFactory = new JdbcTransactionFactory();
+
+        Environment environment = new Environment("development", transactionFactory, dataSource);
+
+        MybatisConfiguration configuration = new MybatisConfiguration();
+        configuration.setEnvironment(environment);
+        configuration.setLazyLoadingEnabled(true);
+        configuration.addMappers("org.apache.dolphinscheduler.dao.mapper");
+        configuration.addInterceptor(new PaginationInterceptor());
+
+        MybatisSqlSessionFactoryBean sqlSessionFactoryBean = new MybatisSqlSessionFactoryBean();
+        sqlSessionFactoryBean.setConfiguration(configuration);
+        sqlSessionFactoryBean.setDataSource(dataSource);
+
+        sqlSessionFactoryBean.setTypeEnumsPackage("org.apache.dolphinscheduler.*.enums");
+        sqlSessionFactory = sqlSessionFactoryBean.getObject();
 
         return sqlSessionFactory;
+}
+
+    private SqlSessionTemplate getSqlSessionTemplate() {
+        sqlSessionTemplate = new SqlSessionTemplate(sqlSessionFactory);
+        return sqlSessionTemplate;
     }
 
     /**
      * get sql session
+     *
      * @return sqlSession
      */
-    public static SqlSession getSqlSession() {
-        if (sqlSessionTemplate == null) {
-            synchronized (ConnectionFactory.class) {
-                if (sqlSessionTemplate == null) {
-                    try {
-                        sqlSessionTemplate = new SqlSessionTemplate(getSqlSessionFactory());
-                        return sqlSessionTemplate;
-                    } catch (Exception e) {
-                        logger.error("getSqlSession error", e);
-                        throw new RuntimeException(e);
-                    }
-                }
-            }
-        }
+    public SqlSession getSqlSession() {
         return sqlSessionTemplate;
     }
 
     /**
      * get mapper
+     *
      * @param type target class
-     * @param <T> generic
+     * @param <T>  generic
      * @return target object
      */
-    public static <T> T getMapper(Class<T> type) {
+    public <T> T getMapper(Class<T> type) {
         try {
             return getSqlSession().getMapper(type);
         } catch (Exception e) {

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/upgrade/UpgradeDao.java

@@ -53,7 +53,7 @@ public abstract class UpgradeDao extends AbstractBaseDao {
      * @return DruidDataSource
      */
     public static DruidDataSource getDataSource(){
-        DruidDataSource dataSource = ConnectionFactory.getDataSource();
+        DruidDataSource dataSource = ConnectionFactory.getInstance().getDataSource();
         dataSource.setInitialSize(2);
         dataSource.setMinIdle(2);
         dataSource.setMaxActive(2);

dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/AlertDaoTest.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dolphinscheduler.dao;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class AlertDaoTest {
+    private static final Logger logger = LoggerFactory.getLogger(AlertDaoTest.class);
+
+    @Test
+    public void testGetAlertDao() {
+        logger.info("testGetAlertDao start");
+        AlertDao alertDao = DaoFactory.getDaoInstance(AlertDao.class);
+        Assert.assertNotNull(alertDao);
+        logger.info("testGetAlertDao end");
+    }
+}

dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ConnectionFactoryTest.java

@@ -31,7 +31,7 @@ public class ConnectionFactoryTest {
      */
     @Test
     public void testConnection()throws Exception{
-        Connection connection = ConnectionFactory.getDataSource().getPooledConnection().getConnection();
+        Connection connection = ConnectionFactory.getInstance().getDataSource().getPooledConnection().getConnection();
         Assert.assertTrue(connection != null);
     }
 }

pom.xml

@@ -740,6 +740,8 @@
                         <include>**/server/worker/task/sqoop/SqoopTaskTest.java</include>
                         <include>**/server/utils/DataxUtilsTest.java</include>
                         <include>**/service/zk/DefaultEnsembleProviderTest.java</include>
+                        <include>**/alert/utils/MailUtilsTest.java</include>
+                        <include>**/dao/AlertDaoTest.java</include>
                     </includes>
                     <!-- <skip>true</skip> -->
                 </configuration>