Lehel44 commented on code in PR #6935:
URL: https://github.com/apache/nifi/pull/6935#discussion_r1112304213
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/AbstractDBCPConnectionPool.java:
##########
@@ -365,168 +121,151 @@ 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 =
createDataSourceConfiguraton(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);
}
}
+ }
- 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 Driver getDriver(final String driverName, final String
url);
- if (validationQuery != null && !validationQuery.isEmpty()) {
+ protected DataSourceConfiguration createDataSourceConfiguraton(final
ConfigurationContext context) {
+ final String url =
context.getProperty(DBCPProperties.DATABASE_URL).getValue();
+ final String user =
context.getProperty(DBCPProperties.DB_USER).getValue();
+ final String password =
context.getProperty(DBCPProperties.DB_PASSWORD).getValue();
+ final String driverName =
context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue();
+
+ return new DataSourceConfiguration.Builder(url, driverName, user,
password)
+ .build();
+ }
Review Comment:
I agree that it is less reusable. The problem with returning with builder is
that the mandatory fields can't be changed afterwards. The url is mandatory and
is overwritten by several descendant classes. You could override a getUrl
method, but I think it would be an overdesign, so I would just make the method
plain abstract.
--
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]
