NIFI-11547 Fixed DBCPConnectionPool verification

- Added unit test for AbstractDBCPConnectionPool

This closes #7249

Co-authored-by: David Handermann <exceptionfactory@apache.org>
Signed-off-by: David Handermann <exceptionfactory@apache.org>

nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/pom.xml

@@ -53,5 +53,9 @@
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-lang3</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>jcl-over-slf4j</artifactId>
+        </dependency>
     </dependencies>
 </project>

nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java

@@ -76,17 +76,17 @@ public abstract class AbstractDBCPConnectionPool extends AbstractControllerServi
                     .build());
         }
 
-        final BasicDataSource dataSource = new BasicDataSource();
+        final BasicDataSource basicDataSource = new BasicDataSource();
         try {
             final DataSourceConfiguration configuration = getDataSourceConfiguration(context);
-            configureDataSource(context, configuration);
+            configureDataSource(context, basicDataSource, configuration);
             results.add(new ConfigVerificationResult.Builder()
                     .verificationStepName("Configure Data Source")
                     .outcome(SUCCESSFUL)
                     .explanation("Successfully configured data source")
                     .build());
 
-            try (final Connection conn = getConnection(dataSource, kerberosUser)) {
+            try (final Connection conn = getConnection(basicDataSource, kerberosUser)) {
                 results.add(new ConfigVerificationResult.Builder()
                         .verificationStepName("Establish Connection")
                         .outcome(SUCCESSFUL)
@@ -114,7 +114,7 @@ public abstract class AbstractDBCPConnectionPool extends AbstractControllerServi
                     .build());
         } finally {
             try {
-                shutdown(dataSource, kerberosUser);
+                shutdown(basicDataSource, kerberosUser);
             } catch (final SQLException e) {
                 verificationLogger.error("Failed to shut down data source", e);
             }
@@ -140,7 +140,7 @@ public abstract class AbstractDBCPConnectionPool extends AbstractControllerServi
         kerberosUser = getKerberosUser(context);
         loginKerberos(kerberosUser);
         final DataSourceConfiguration configuration = getDataSourceConfiguration(context);
-        configureDataSource(context, configuration);
+        configureDataSource(context, dataSource, configuration);
     }
 
     private void loginKerberos(KerberosUser kerberosUser) throws InitializationException {
@@ -157,30 +157,30 @@ public abstract class AbstractDBCPConnectionPool extends AbstractControllerServi
 
     protected abstract DataSourceConfiguration getDataSourceConfiguration(final ConfigurationContext context);
 
-    protected void configureDataSource(final ConfigurationContext context, final DataSourceConfiguration configuration) {
+    protected void configureDataSource(final ConfigurationContext context, final BasicDataSource basicDataSource, final DataSourceConfiguration configuration) {
         final Driver driver = getDriver(configuration.getDriverName(), configuration.getUrl());
 
-        dataSource.setDriver(driver);
+        basicDataSource.setDriver(driver);
-        dataSource.setMaxWaitMillis(configuration.getMaxWaitMillis());
+        basicDataSource.setMaxWaitMillis(configuration.getMaxWaitMillis());
-        dataSource.setMaxTotal(configuration.getMaxTotal());
+        basicDataSource.setMaxTotal(configuration.getMaxTotal());
-        dataSource.setMinIdle(configuration.getMinIdle());
+        basicDataSource.setMinIdle(configuration.getMinIdle());
-        dataSource.setMaxIdle(configuration.getMaxIdle());
+        basicDataSource.setMaxIdle(configuration.getMaxIdle());
-        dataSource.setMaxConnLifetimeMillis(configuration.getMaxConnLifetimeMillis());
+        basicDataSource.setMaxConnLifetimeMillis(configuration.getMaxConnLifetimeMillis());
-        dataSource.setTimeBetweenEvictionRunsMillis(configuration.getTimeBetweenEvictionRunsMillis());
+        basicDataSource.setTimeBetweenEvictionRunsMillis(configuration.getTimeBetweenEvictionRunsMillis());
-        dataSource.setMinEvictableIdleTimeMillis(configuration.getMinEvictableIdleTimeMillis());
+        basicDataSource.setMinEvictableIdleTimeMillis(configuration.getMinEvictableIdleTimeMillis());
-        dataSource.setSoftMinEvictableIdleTimeMillis(configuration.getSoftMinEvictableIdleTimeMillis());
+        basicDataSource.setSoftMinEvictableIdleTimeMillis(configuration.getSoftMinEvictableIdleTimeMillis());
 
         final String validationQuery = configuration.getValidationQuery();
         if (StringUtils.isNotBlank(validationQuery)) {
-            dataSource.setValidationQuery(validationQuery);
+            basicDataSource.setValidationQuery(validationQuery);
-            dataSource.setTestOnBorrow(true);
+            basicDataSource.setTestOnBorrow(true);
         }
 
-        dataSource.setUrl(configuration.getUrl());
+        basicDataSource.setUrl(configuration.getUrl());
-        dataSource.setUsername(configuration.getUserName());
+        basicDataSource.setUsername(configuration.getUserName());
-        dataSource.setPassword(configuration.getPassword());
+        basicDataSource.setPassword(configuration.getPassword());
 
-        getConnectionProperties(context).forEach(dataSource::addConnectionProperty);
+        getConnectionProperties(context).forEach(basicDataSource::addConnectionProperty);
     }
 
     protected Map<String, String> getConnectionProperties(final ConfigurationContext context) {

nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/test/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPoolTest.java

@@ -0,0 +1,116 @@
+/*
+ * 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.nifi.dbcp;
+
+import org.apache.nifi.components.ConfigVerificationResult;
+import org.apache.nifi.components.PropertyValue;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.utils.DBCPProperties;
+import org.apache.nifi.dbcp.utils.DataSourceConfiguration;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.logging.ComponentLog;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class AbstractDBCPConnectionPoolTest {
+
+    private static final int MAX_TOTAL = 1;
+
+    private static final int TIMEOUT = 0;
+
+    @Mock
+    Driver driver;
+
+    @Mock
+    Connection connection;
+
+    @Mock
+    DataSourceConfiguration dataSourceConfiguration;
+
+    @Mock
+    ConfigurationContext configurationContext;
+
+    @Mock
+    ComponentLog componentLog;
+
+    @Mock
+    PropertyValue kerberosUserServiceProperty;
+
+    @Mock
+    KerberosUserService kerberosUserService;
+
+    @Test
+    void testVerifySuccessful() throws SQLException {
+        final AbstractDBCPConnectionPool connectionPool = new MockDBCPConnectionPool();
+
+        when(dataSourceConfiguration.getMaxTotal()).thenReturn(MAX_TOTAL);
+        when(configurationContext.getProperty(eq(DBCPProperties.KERBEROS_USER_SERVICE))).thenReturn(kerberosUserServiceProperty);
+        when(kerberosUserServiceProperty.asControllerService(eq(KerberosUserService.class))).thenReturn(kerberosUserService);
+        when(driver.connect(any(), any())).thenReturn(connection);
+        when(connection.isValid(eq(TIMEOUT))).thenReturn(true);
+
+        final List<ConfigVerificationResult> results = connectionPool.verify(configurationContext, componentLog, Collections.emptyMap());
+
+        assertOutcomeSuccessful(results);
+    }
+
+    private void assertOutcomeSuccessful(final List<ConfigVerificationResult> results) {
+        assertNotNull(results);
+        final Iterator<ConfigVerificationResult> resultsFound = results.iterator();
+
+        assertTrue(resultsFound.hasNext());
+        final ConfigVerificationResult firstResult = resultsFound.next();
+        assertEquals(ConfigVerificationResult.Outcome.SUCCESSFUL, firstResult.getOutcome(), firstResult.getExplanation());
+
+        assertTrue(resultsFound.hasNext());
+        final ConfigVerificationResult secondResult = resultsFound.next();
+        assertEquals(ConfigVerificationResult.Outcome.SUCCESSFUL, secondResult.getOutcome(), secondResult.getExplanation());
+
+        assertFalse(resultsFound.hasNext());
+    }
+
+    private class MockDBCPConnectionPool extends AbstractDBCPConnectionPool {
+
+        @Override
+        protected Driver getDriver(final String driverName, final String url) {
+            return driver;
+        }
+
+        @Override
+        protected DataSourceConfiguration getDataSourceConfiguration(final ConfigurationContext context) {
+            return dataSourceConfiguration;
+        }
+    }
+}