exceptionfactory commented on code in PR #6935:
URL: https://github.com/apache/nifi/pull/6935#discussion_r1115047210


##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java:
##########
@@ -365,168 +119,143 @@ public List<ConfigVerificationResult> verify(final 
ConfigurationContext context,
                 verificationLogger.error("Failed to shut down data source", e);
             }
         }
-
         return results;
     }
 
     /**
      * Configures connection pool by creating an instance of the
      * {@link BasicDataSource} based on configuration provided with
      * {@link ConfigurationContext}.
-     *
+     * <p>
      * This operation makes no guarantees that the actual connection could be
      * made since the underlying system may still go off-line during normal
      * operation of the connection pool.
      *
-     * @param context
-     *            the configuration context
-     * @throws InitializationException
-     *             if unable to create a database connection
+     * @param context the configuration context
+     * @throws InitializationException if unable to create a database 
connection
      */
     @OnEnabled
     public void onConfigured(final ConfigurationContext context) throws 
InitializationException {
-        kerberosUser = getKerberosUser(context);
         dataSource = new BasicDataSource();
-        configureDataSource(dataSource, kerberosUser, context);
+        kerberosUser = getKerberosUser(context);
+        loginKerberos(kerberosUser);
+        final DataSourceConfiguration configuration = 
getDataSourceConfigurationBuilder(context);
+        configureDataSource(context, configuration);
     }
 
-    private void configureDataSource(final BasicDataSource dataSource, final 
KerberosUser kerberosUser,
-                                     final ConfigurationContext context) 
throws InitializationException {
-        final String dburl = getUrl(context);
-
-        final String driverName = 
context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue();
-        final String user = 
context.getProperty(DB_USER).evaluateAttributeExpressions().getValue();
-        final String passw = 
context.getProperty(DB_PASSWORD).evaluateAttributeExpressions().getValue();
-        final Integer maxTotal = 
context.getProperty(MAX_TOTAL_CONNECTIONS).evaluateAttributeExpressions().asInteger();
-        final String validationQuery = 
context.getProperty(VALIDATION_QUERY).evaluateAttributeExpressions().getValue();
-        final Long maxWaitMillis = 
extractMillisWithInfinite(context.getProperty(MAX_WAIT_TIME).evaluateAttributeExpressions());
-        final Integer minIdle = 
context.getProperty(MIN_IDLE).evaluateAttributeExpressions().asInteger();
-        final Integer maxIdle = 
context.getProperty(MAX_IDLE).evaluateAttributeExpressions().asInteger();
-        final Long maxConnLifetimeMillis = 
extractMillisWithInfinite(context.getProperty(MAX_CONN_LIFETIME).evaluateAttributeExpressions());
-        final Long timeBetweenEvictionRunsMillis = 
extractMillisWithInfinite(context.getProperty(EVICTION_RUN_PERIOD).evaluateAttributeExpressions());
-        final Long minEvictableIdleTimeMillis = 
extractMillisWithInfinite(context.getProperty(MIN_EVICTABLE_IDLE_TIME).evaluateAttributeExpressions());
-        final Long softMinEvictableIdleTimeMillis = 
extractMillisWithInfinite(context.getProperty(SOFT_MIN_EVICTABLE_IDLE_TIME).evaluateAttributeExpressions());
-
+    private void loginKerberos(KerberosUser kerberosUser) throws 
InitializationException {
         if (kerberosUser != null) {
             try {
                 kerberosUser.login();
             } catch (KerberosLoginException e) {
                 throw new InitializationException("Unable to authenticate 
Kerberos principal", e);
             }
         }
+    }
+
+    protected abstract Driver getDriver(final String driverName, final String 
url);
 
-        dataSource.setDriver(getDriver(driverName, dburl));
-        dataSource.setMaxWaitMillis(maxWaitMillis);
-        dataSource.setMaxTotal(maxTotal);
-        dataSource.setMinIdle(minIdle);
-        dataSource.setMaxIdle(maxIdle);
-        dataSource.setMaxConnLifetimeMillis(maxConnLifetimeMillis);
-        
dataSource.setTimeBetweenEvictionRunsMillis(timeBetweenEvictionRunsMillis);
-        dataSource.setMinEvictableIdleTimeMillis(minEvictableIdleTimeMillis);
-        
dataSource.setSoftMinEvictableIdleTimeMillis(softMinEvictableIdleTimeMillis);
+    protected abstract DataSourceConfiguration 
getDataSourceConfigurationBuilder(final ConfigurationContext context);
 
-        if (validationQuery != null && !validationQuery.isEmpty()) {
+    protected void configureDataSource(final ConfigurationContext context, 
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());
+
+        final String validationQuery = configuration.getValidationQuery();
+        if (StringUtils.isNotBlank(validationQuery)) {
             dataSource.setValidationQuery(validationQuery);
             dataSource.setTestOnBorrow(true);
         }
 
-        dataSource.setUrl(dburl);
-        dataSource.setUsername(user);
-        dataSource.setPassword(passw);
+        dataSource.setUrl(configuration.getUrl());
+        dataSource.setUsername(configuration.getUserName());
+        dataSource.setPassword(configuration.getPassword());
+
+        
getConnectionProperties(context).forEach(dataSource::addConnectionProperty);
+    }
 
-        final List<PropertyDescriptor> dynamicProperties = 
context.getProperties()
+    protected Map<String, String> getConnectionProperties(final 
ConfigurationContext context) {
+        return getDynamicProperties(context)
+                .stream()
+                .collect(Collectors.toMap(PropertyDescriptor::getName, s -> {
+                    final PropertyValue propertyValue = context.getProperty(s);
+                    return 
propertyValue.evaluateAttributeExpressions().getValue();
+                }));
+    }
+
+    protected List<PropertyDescriptor> getDynamicProperties(final 
ConfigurationContext context) {
+        return context.getProperties()
                 .keySet()
                 .stream()
                 .filter(PropertyDescriptor::isDynamic)
                 .collect(Collectors.toList());
-
-        dynamicProperties.forEach((descriptor) -> {
-            final PropertyValue propertyValue = 
context.getProperty(descriptor);
-            if (descriptor.isSensitive()) {
-                final String propertyName = 
StringUtils.substringAfter(descriptor.getName(), SENSITIVE_PROPERTY_PREFIX);
-                dataSource.addConnectionProperty(propertyName, 
propertyValue.getValue());
-            } else {
-                dataSource.addConnectionProperty(descriptor.getName(), 
propertyValue.evaluateAttributeExpressions().getValue());
-            }
-        });
-
-        
getConnectionProperties(context).forEach(dataSource::addConnectionProperty);
     }
 
-    private KerberosUser getKerberosUser(final ConfigurationContext context) {
-        KerberosUser kerberosUser = null;
-        final KerberosCredentialsService kerberosCredentialsService = 
context.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
+    protected KerberosUser getKerberosUser(final ConfigurationContext context) 
{
+        final KerberosUser kerberosUser;
         final KerberosUserService kerberosUserService = 
context.getProperty(KERBEROS_USER_SERVICE).asControllerService(KerberosUserService.class);
-        final String kerberosPrincipal = 
context.getProperty(KERBEROS_PRINCIPAL).evaluateAttributeExpressions().getValue();
-        final String kerberosPassword = 
context.getProperty(KERBEROS_PASSWORD).getValue();
 
         if (kerberosUserService != null) {
             kerberosUser = kerberosUserService.createKerberosUser();
-        } else if (kerberosCredentialsService != null) {
-            kerberosUser = new 
KerberosKeytabUser(kerberosCredentialsService.getPrincipal(), 
kerberosCredentialsService.getKeytab());
-        } else if (!StringUtils.isBlank(kerberosPrincipal) && 
!StringUtils.isBlank(kerberosPassword)) {
-            kerberosUser = new KerberosPasswordUser(kerberosPrincipal, 
kerberosPassword);
+        } else {
+            kerberosUser = getKerberosUserByCredentials(context);
         }
         return kerberosUser;
     }
 
-    protected String getUrl(ConfigurationContext context) {
-        return 
context.getProperty(DATABASE_URL).evaluateAttributeExpressions().getValue();
+    protected KerberosUser getKerberosUserByCredentials(final 
ConfigurationContext context) {
+        return null;
     }

Review Comment:
   Instead of having this method implementation return `null` in this abstract 
class, it looks like it could be removed, and the concrete implementation could 
override the `getKerberosUser()` method.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to