[GitHub] [nifi] turcsanyip commented on a diff in pull request #6935: NIFI-11151: Improving code reusability of DBCP services

["turcsanyip (via GitHub)" <gi...@apache.org>]

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


##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java:
##########
@@ -365,168 +119,136 @@ 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);

Review Comment:
   It does not return a builder but the configuration object itself.
   ```suggestion
       protected abstract DataSourceConfiguration getDataSourceConfiguration(final ConfigurationContext context);
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org