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]