From 1a38bf003ea23ccca6e04efb6afe64a52d1d61f5 Mon Sep 17 00:00:00 2001 From: Lehel Date: Mon, 15 May 2023 23:05:11 +0200 Subject: [PATCH] NIFI-11547 Fixed DBCPConnectionPool verification - Added unit test for AbstractDBCPConnectionPool This closes #7249 Co-authored-by: David Handermann Signed-off-by: David Handermann --- .../nifi-dbcp-base/pom.xml | 4 + .../nifi/dbcp/AbstractDBCPConnectionPool.java | 42 +++---- .../dbcp/AbstractDBCPConnectionPoolTest.java | 116 ++++++++++++++++++ 3 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/test/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPoolTest.java diff --git a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/pom.xml b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/pom.xml index 11738f9232..cc639c5b3d 100644 --- a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/pom.xml +++ b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/pom.xml @@ -53,5 +53,9 @@ org.apache.commons commons-lang3 + + org.slf4j + jcl-over-slf4j + \ No newline at end of file diff --git a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java index cc15e36c11..ac7caa8cf1 100644 --- a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java +++ b/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); - dataSource.setMaxWaitMillis(configuration.getMaxWaitMillis()); - dataSource.setMaxTotal(configuration.getMaxTotal()); - dataSource.setMinIdle(configuration.getMinIdle()); - dataSource.setMaxIdle(configuration.getMaxIdle()); - dataSource.setMaxConnLifetimeMillis(configuration.getMaxConnLifetimeMillis()); - dataSource.setTimeBetweenEvictionRunsMillis(configuration.getTimeBetweenEvictionRunsMillis()); - dataSource.setMinEvictableIdleTimeMillis(configuration.getMinEvictableIdleTimeMillis()); - dataSource.setSoftMinEvictableIdleTimeMillis(configuration.getSoftMinEvictableIdleTimeMillis()); + basicDataSource.setDriver(driver); + basicDataSource.setMaxWaitMillis(configuration.getMaxWaitMillis()); + basicDataSource.setMaxTotal(configuration.getMaxTotal()); + basicDataSource.setMinIdle(configuration.getMinIdle()); + basicDataSource.setMaxIdle(configuration.getMaxIdle()); + basicDataSource.setMaxConnLifetimeMillis(configuration.getMaxConnLifetimeMillis()); + basicDataSource.setTimeBetweenEvictionRunsMillis(configuration.getTimeBetweenEvictionRunsMillis()); + basicDataSource.setMinEvictableIdleTimeMillis(configuration.getMinEvictableIdleTimeMillis()); + basicDataSource.setSoftMinEvictableIdleTimeMillis(configuration.getSoftMinEvictableIdleTimeMillis()); final String validationQuery = configuration.getValidationQuery(); if (StringUtils.isNotBlank(validationQuery)) { - dataSource.setValidationQuery(validationQuery); - dataSource.setTestOnBorrow(true); + basicDataSource.setValidationQuery(validationQuery); + basicDataSource.setTestOnBorrow(true); } - dataSource.setUrl(configuration.getUrl()); - dataSource.setUsername(configuration.getUserName()); - dataSource.setPassword(configuration.getPassword()); + basicDataSource.setUrl(configuration.getUrl()); + basicDataSource.setUsername(configuration.getUserName()); + basicDataSource.setPassword(configuration.getPassword()); - getConnectionProperties(context).forEach(dataSource::addConnectionProperty); + getConnectionProperties(context).forEach(basicDataSource::addConnectionProperty); } protected Map getConnectionProperties(final ConfigurationContext context) { diff --git a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/test/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPoolTest.java b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/test/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPoolTest.java new file mode 100644 index 0000000000..7c85b876b9 --- /dev/null +++ b/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 results = connectionPool.verify(configurationContext, componentLog, Collections.emptyMap()); + + assertOutcomeSuccessful(results); + } + + private void assertOutcomeSuccessful(final List results) { + assertNotNull(results); + final Iterator 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; + } + } +}