turcsanyip commented on code in PR #6935:
URL: https://github.com/apache/nifi/pull/6935#discussion_r1112089266
##########
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) {
Review Comment:
Typo:
```suggestion
protected DataSourceConfiguration createDataSourceConfiguration(final
ConfigurationContext context) {
```
##########
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();
+ }
+
+ 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);
+ }
+
+ 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();
+ }));
+ }
- final List<PropertyDescriptor> dynamicProperties =
context.getProperties()
+ 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)
{
+ KerberosUser kerberosUser;
Review Comment:
```suggestion
final KerberosUser kerberosUser;
```
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DBCPProperties.java:
##########
@@ -0,0 +1,228 @@
+/*
+ * 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.utils;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.PropertyValue;
+import org.apache.nifi.components.resource.ResourceCardinality;
+import org.apache.nifi.components.resource.ResourceType;
+import org.apache.nifi.dbcp.DBCPValidator;
+import org.apache.nifi.expression.AttributeExpression;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.kerberos.KerberosCredentialsService;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.util.concurrent.TimeUnit;
+
+public final class DBCPProperties {
+
+ private DBCPProperties() {
+ }
+
+ public static final PropertyDescriptor DATABASE_URL = new
PropertyDescriptor.Builder()
+ .name("Database Connection URL")
+ .description("A database connection URL used to connect to a
database. May contain database system name, host, port, database name and some
parameters."
+ + " The exact syntax of a database connection URL is
specified by your DBMS.")
+ .defaultValue(null)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .required(true)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor DB_USER = new
PropertyDescriptor.Builder()
+ .name("Database User")
+ .description("Database user name")
+ .defaultValue(null)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor DB_PASSWORD = new
PropertyDescriptor.Builder()
+ .name("Password")
+ .description("The password for the database user")
+ .defaultValue(null)
+ .required(false)
+ .sensitive(true)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+
+ public static final PropertyDescriptor DB_DRIVERNAME = new
PropertyDescriptor.Builder()
+ .name("Database Driver Class Name")
+ .description("Database driver class name")
+ .defaultValue(null)
+ .required(true)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor DB_DRIVER_LOCATION = new
PropertyDescriptor.Builder()
+ .name("database-driver-locations")
+ .displayName("Database Driver Location(s)")
+ .description("Comma-separated list of files/folders and/or URLs
containing the driver JAR and its dependencies (if any). For example
'/var/tmp/mariadb-java-client-1.1.7.jar'")
+ .defaultValue(null)
+ .required(false)
+ .identifiesExternalResource(ResourceCardinality.MULTIPLE,
ResourceType.FILE, ResourceType.DIRECTORY, ResourceType.URL)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .dynamicallyModifiesClasspath(true)
+ .build();
+
+ public static final PropertyDescriptor MAX_WAIT_TIME = new
PropertyDescriptor.Builder()
+ .name("Max Wait Time")
+ .description("The maximum amount of time that the pool will wait
(when there are no available connections) "
+ + " for a connection to be returned before failing, or -1
to wait indefinitely. ")
+
.defaultValue(DefaultDataSourceValues.MAX_WAIT_MILLIS.getPropertyValue())
+ .required(true)
+ .addValidator(DBCPValidator.CUSTOM_TIME_PERIOD_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .sensitive(false)
+ .build();
+
+ public static final PropertyDescriptor MAX_TOTAL_CONNECTIONS = new
PropertyDescriptor.Builder()
+ .name("Max Total Connections")
+ .description("The maximum number of active connections that can be
allocated from this pool at the same time, "
+ + " or negative for no limit.")
+
.defaultValue(DefaultDataSourceValues.MAX_TOTAL_CONNECTIONS.getPropertyValue())
+ .required(true)
+ .addValidator(StandardValidators.INTEGER_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .sensitive(false)
+ .build();
+
+ public static final PropertyDescriptor VALIDATION_QUERY = new
PropertyDescriptor.Builder()
+ .name("Validation-query")
+ .displayName("Validation query")
+ .description("Validation query used to validate connections before
returning them. "
+ + "When connection is invalid, it gets dropped and new
valid connection will be returned. "
+ + "Note!! Using validation might have some performance
penalty.")
+ .required(false)
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor MIN_IDLE = new
PropertyDescriptor.Builder()
+ .displayName("Minimum Idle Connections")
+ .name("dbcp-min-idle-conns")
+ .description("The minimum number of connections that can remain
idle in the pool without extra ones being " +
+ "created. Set to or zero to allow no idle connections.")
+ .defaultValue(DefaultDataSourceValues.MIN_IDLE.getPropertyValue())
+ .required(false)
+ .addValidator(StandardValidators.NON_NEGATIVE_INTEGER_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor MAX_IDLE = new
PropertyDescriptor.Builder()
+ .displayName("Max Idle Connections")
+ .name("dbcp-max-idle-conns")
+ .description("The maximum number of connections that can remain
idle in the pool without extra ones being " +
+ "released. Set to any negative value to allow unlimited
idle connections.")
+ .defaultValue(DefaultDataSourceValues.MAX_IDLE.getPropertyValue())
+ .required(false)
+ .addValidator(StandardValidators.INTEGER_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor MAX_CONN_LIFETIME = new
PropertyDescriptor.Builder()
+ .displayName("Max Connection Lifetime")
+ .name("dbcp-max-conn-lifetime")
+ .description("The maximum lifetime in milliseconds of a
connection. After this time is exceeded the " +
+ "connection will fail the next activation, passivation or
validation test. A value of zero or less " +
+ "means the connection has an infinite lifetime.")
+
.defaultValue(DefaultDataSourceValues.MAX_CONN_LIFETIME.getPropertyValue())
+ .required(false)
+ .addValidator(DBCPValidator.CUSTOM_TIME_PERIOD_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor EVICTION_RUN_PERIOD = new
PropertyDescriptor.Builder()
+ .displayName("Time Between Eviction Runs")
+ .name("dbcp-time-between-eviction-runs")
+ .description("The number of milliseconds to sleep between runs of
the idle connection evictor thread. When " +
+ "non-positive, no idle connection evictor thread will be
run.")
+
.defaultValue(DefaultDataSourceValues.EVICTION_RUN_PERIOD.getPropertyValue())
+ .required(false)
+ .addValidator(DBCPValidator.CUSTOM_TIME_PERIOD_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor MIN_EVICTABLE_IDLE_TIME = new
PropertyDescriptor.Builder()
+ .displayName("Minimum Evictable Idle Time")
+ .name("dbcp-min-evictable-idle-time")
+ .description("The minimum amount of time a connection may sit idle
in the pool before it is eligible for eviction.")
+
.defaultValue(DefaultDataSourceValues.MIN_EVICTABLE_IDLE_TIME_MILLIS.getPropertyValue())
+ .required(false)
+ .addValidator(DBCPValidator.CUSTOM_TIME_PERIOD_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor SOFT_MIN_EVICTABLE_IDLE_TIME = new
PropertyDescriptor.Builder()
+ .displayName("Soft Minimum Evictable Idle Time")
+ .name("dbcp-soft-min-evictable-idle-time")
+ .description("The minimum amount of time a connection may sit idle
in the pool before it is eligible for " +
+ "eviction by the idle connection evictor, with the extra
condition that at least a minimum number of" +
+ " idle connections remain in the pool. When the not-soft
version of this option is set to a positive" +
+ " value, it is examined first by the idle connection
evictor: when idle connections are visited by " +
+ "the evictor, idle time is first compared against it
(without considering the number of idle " +
+ "connections in the pool) and then against this soft
option, including the minimum idle connections " +
+ "constraint.")
+
.defaultValue(DefaultDataSourceValues.SOFT_MIN_EVICTABLE_IDLE_TIME.getPropertyValue())
+ .required(false)
+ .addValidator(DBCPValidator.CUSTOM_TIME_PERIOD_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .build();
+
+ public static final PropertyDescriptor KERBEROS_CREDENTIALS_SERVICE = new
PropertyDescriptor.Builder()
Review Comment:
The legacy kerberos properties (`KERBEROS_CREDENTIALS_SERVICE`,
`KERBEROS_PRINCIPAL`, `KERBEROS_PASSWORD`) should be moved to the existing
processor(s) where they are in still use. This common code should only contain
the current kerberos property / code.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DefaultDataSourceValues.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
+public enum DefaultDataSourceValues {
+
+
+ MAX_WAIT_MILLIS("500", "500 millis"),
Review Comment:
I think it is not necessary to define the same value in 2 formats because
the millisec value could be calculated from the property value using
`FormatUtils.getTimeDuration()` (the -1 special value needs to be handled too).
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DefaultDataSourceValues.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
+public enum DefaultDataSourceValues {
+
+
+ MAX_WAIT_MILLIS("500", "500 millis"),
Review Comment:
```suggestion
MAX_WAIT_TIME("500", "500 millis"),
```
Other constants (`MAX_CONN_LIFETIME`, `EVICTION_RUN_PERIOD`) do not have
`_MILLIS` suffix which I think is correct because the time period can be
defined in different time units.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DefaultDataSourceValues.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.utils;
+
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
+public enum DefaultDataSourceValues {
+
+
+ MAX_WAIT_MILLIS("500", "500 millis"),
+
+ MAX_TOTAL_CONNECTIONS("8", "8"),
+ /**
+ * Copied from {@link GenericObjectPoolConfig#DEFAULT_MIN_IDLE} in
Commons-DBCP 2.7.0
+ */
+ MIN_IDLE("0", "0"),
+ /**
+ * Copied from {@link GenericObjectPoolConfig#DEFAULT_MAX_IDLE} in
Commons-DBCP 2.7.0
+ */
+ MAX_IDLE("8", "8"),
+ /**
+ * Copied from private variable {@link
BasicDataSource#maxConnLifetimeMillis} in Commons-DBCP 2.7.0
+ */
+ MAX_CONN_LIFETIME("-1", "-1"),
+ /**
+ * Copied from {@link
GenericObjectPoolConfig#DEFAULT_TIME_BETWEEN_EVICTION_RUNS_MILLIS} in
Commons-DBCP 2.7.0
+ */
+ EVICTION_RUN_PERIOD("-1", "-1"),
+ /**
+ * Copied from {@link
GenericObjectPoolConfig#DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS} in Commons-DBCP
2.7.0
+ * and converted from 1800000L to "1800000 millis" to "30 mins"
+ */
+ MIN_EVICTABLE_IDLE_TIME_MILLIS("1800000", "30 mins"),
Review Comment:
```suggestion
MIN_EVICTABLE_IDLE_TIME("1800000", "30 mins"),
```
Other constants (`MAX_CONN_LIFETIME`, `EVICTION_RUN_PERIOD`) do not have
`_MILLIS` suffix which I think is correct because the time period can be
defined in different time units.
##########
nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DataSourceConfiguration.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * 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.utils;
+
+public class DataSourceConfiguration {
+
+ private final long maxWaitMillis;
+ private final int maxTotal;
+ private final int minIdle;
+ private final int maxIdle;
+ private final long maxConnLifetimeMillis;
+ private final long timeBetweenEvictionRunsMillis;
+ private final long minEvictableIdleTimeMillis;
+ private final long softMinEvictableIdleTimeMillis;
+ private final String validationQuery;
+ private final String url;
+ private final String driverName;
+ private final String userName;
+ private final String password;
+
+ public long getMaxWaitMillis() {
+ return maxWaitMillis;
+ }
+
+ public int getMaxTotal() {
+ return maxTotal;
+ }
+
+ public int getMinIdle() {
+ return minIdle;
+ }
+
+ public int getMaxIdle() {
+ return maxIdle;
+ }
+
+ public long getMaxConnLifetimeMillis() {
+ return maxConnLifetimeMillis;
+ }
+
+ public long getTimeBetweenEvictionRunsMillis() {
+ return timeBetweenEvictionRunsMillis;
+ }
+
+ public long getMinEvictableIdleTimeMillis() {
+ return minEvictableIdleTimeMillis;
+ }
+
+ public long getSoftMinEvictableIdleTimeMillis() {
+ return softMinEvictableIdleTimeMillis;
+ }
+
+ public String getValidationQuery() {
+ return validationQuery;
+ }
+
+ public String getUrl() {
+ return url;
+ }
+
+ public String getDriverName() {
+ return driverName;
+ }
+
+ public String getUserName() {
+ return userName;
+ }
+
+ public String getPassword() {
+ return password;
+ }
+
+ public DataSourceConfiguration(final Builder builder) {
+ this.maxWaitMillis = builder.maxWaitMillis;
+ this.maxTotal = builder.maxTotal;
+ this.minIdle = builder.minIdle;
+ this.maxIdle = builder.maxIdle;
+ this.maxConnLifetimeMillis = builder.maxConnLifetimeMillis;
+ this.timeBetweenEvictionRunsMillis =
builder.timeBetweenEvictionRunsMillis;
+ this.minEvictableIdleTimeMillis = builder.minEvictableIdleTimeMillis;
+ this.softMinEvictableIdleTimeMillis =
builder.softMinEvictableIdleTimeMillis;
+ this.validationQuery = builder.validationQuery;
+ this.url = builder.url;
+ this.driverName = builder.driverName;
+ this.userName = builder.userName;
+ this.password = builder.password;
+ }
+
+ public static class Builder {
+ private long maxWaitMillis =
DefaultDataSourceValues.MAX_WAIT_MILLIS.getLongValue();
+ private int maxTotal =
DefaultDataSourceValues.MAX_TOTAL_CONNECTIONS.getIntValue();
+ private int minIdle = DefaultDataSourceValues.MIN_IDLE.getIntValue();
+ private int maxIdle = DefaultDataSourceValues.MAX_IDLE.getIntValue();
+ private long maxConnLifetimeMillis =
DefaultDataSourceValues.MAX_CONN_LIFETIME.getLongValue();
+ private long timeBetweenEvictionRunsMillis =
DefaultDataSourceValues.EVICTION_RUN_PERIOD.getLongValue();
+ private long minEvictableIdleTimeMillis =
DefaultDataSourceValues.MIN_EVICTABLE_IDLE_TIME_MILLIS.getLongValue();
+ private long softMinEvictableIdleTimeMillis =
DefaultDataSourceValues.SOFT_MIN_EVICTABLE_IDLE_TIME.getLongValue();
+ private String validationQuery;
+ private final String url;
+ private final String driverName;
+ private final String userName;
+ private final String password;
Review Comment:
Minor, but I would move the more important and mandatory fields to the top
(all over the class: fields, getters, etc.). Something like:
```
private final String url;
private final String driverName;
private final String userName;
private final String password;
private String validationQuery;
// other
```
##########
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:
This method it not really reusable unless the child processor has this 4
properties and no more because the configuration object is built right here and
now.
I think the Builder should be returned instead in order to make the building
of the configuration object extendable.
--
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]
