You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2018/10/24 23:07:37 UTC
[geode] branch feature/GEODE-5927 updated: The create jndi-binding
--datasource-config-properties will now check each name for a "pool."
prefix. If found then that property, minus the prefix,
will be added to the properties that the PooledDataSourceFactory gets to
configure the pool. The other datasource config properties are still used
to configure the database. The help strings on create jndi-binding have
been updated.
This is an automated email from the ASF dual-hosted git repository.
dschneider pushed a commit to branch feature/GEODE-5927
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-5927 by this push:
new e7a326e The create jndi-binding --datasource-config-properties will now check each name for a "pool." prefix. If found then that property, minus the prefix, will be added to the properties that the PooledDataSourceFactory gets to configure the pool. The other datasource config properties are still used to configure the database. The help strings on create jndi-binding have been updated.
e7a326e is described below
commit e7a326e0fd53c0e0a8f0bc76498ab456278cc216
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Wed Oct 24 16:04:40 2018 -0700
The create jndi-binding --datasource-config-properties will now check each
name for a "pool." prefix. If found then that property, minus the prefix,
will be added to the properties that the PooledDataSourceFactory gets to
configure the pool. The other datasource config properties are still used
to configure the database.
The help strings on create jndi-binding have been updated.
---
.../geode/datasource/PooledDataSourceFactory.java | 2 +
.../internal/datasource/DataSourceFactory.java | 20 +++-
.../cli/commands/CreateJndiBindingCommand.java | 7 +-
.../internal/datasource/DataSourceFactoryTest.java | 132 +++++++++++++++++++--
4 files changed, 148 insertions(+), 13 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/datasource/PooledDataSourceFactory.java b/geode-core/src/main/java/org/apache/geode/datasource/PooledDataSourceFactory.java
index ce1350e..40f27f9 100644
--- a/geode-core/src/main/java/org/apache/geode/datasource/PooledDataSourceFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/datasource/PooledDataSourceFactory.java
@@ -24,6 +24,8 @@ import javax.sql.DataSource;
* Classes that implement this interface can be used as the class name
* specified in the jndi-binding "conn-pooled-datasource-class" when the
* jndi-binding type is "POOLED". For more information see "gfsh create jndi-binding".
+ * <p>
+ * Note: implementors of this interface must also implement a zero-arg constructor.
*/
public interface PooledDataSourceFactory {
/**
diff --git a/geode-core/src/main/java/org/apache/geode/internal/datasource/DataSourceFactory.java b/geode-core/src/main/java/org/apache/geode/internal/datasource/DataSourceFactory.java
index b0569b1..c5de19f 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/datasource/DataSourceFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/datasource/DataSourceFactory.java
@@ -54,6 +54,8 @@ public class DataSourceFactory {
private static final String DEFAULT_CONNECTION_POOL_DS_CLASS =
"org.apache.geode.connectors.jdbc.JdbcPooledDataSourceFactory";
+ private static final String POOL_PREFIX = "pool.";
+
/** Creates a new instance of DataSourceFactory */
public DataSourceFactory() {}
@@ -150,7 +152,7 @@ public class DataSourceFactory {
connpoolClassName = DEFAULT_CONNECTION_POOL_DS_CLASS;
}
try {
- Properties poolProperties = createPoolProperties(configMap);
+ Properties poolProperties = createPoolProperties(configMap, props);
Properties dataSourceProperties = createDataSourceProperties(props);
Class<?> cl = ClassPathLoader.getLatest().forName(connpoolClassName);
PooledDataSourceFactory factory = (PooledDataSourceFactory) cl.newInstance();
@@ -168,8 +170,17 @@ public class DataSourceFactory {
}
}
- static Properties createPoolProperties(Map<String, String> configMap) {
+ static Properties createPoolProperties(Map<String, String> configMap,
+ List<ConfigProperty> props) {
Properties result = new Properties();
+ if (props != null) {
+ for (ConfigProperty prop : props) {
+ if (prop.getName().toLowerCase().startsWith(POOL_PREFIX)) {
+ String poolName = prop.getName().substring(POOL_PREFIX.length());
+ result.setProperty(poolName, prop.getValue());
+ }
+ }
+ }
if (configMap != null) {
for (Map.Entry<String, String> entry : configMap.entrySet()) {
if (entry.getValue() == null || entry.getValue().equals("")) {
@@ -199,10 +210,13 @@ public class DataSourceFactory {
return result;
}
- private static Properties createDataSourceProperties(List<ConfigProperty> props) {
+ static Properties createDataSourceProperties(List<ConfigProperty> props) {
Properties result = new Properties();
if (props != null) {
for (ConfigProperty prop : props) {
+ if (prop.getName().toLowerCase().startsWith(POOL_PREFIX)) {
+ continue;
+ }
result.setProperty(prop.getName(), prop.getValue());
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
index 0b01a85..b43e09e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
@@ -50,7 +50,8 @@ public class CreateJndiBindingCommand extends SingleGfshCommand {
"This element specifies the maximum time to block while waiting for a connection before throwing an exception.";
static final String CONNECTION_POOLED_DATASOURCE_CLASS = "conn-pooled-datasource-class";
static final String CONNECTION_POOLED_DATASOURCE_CLASS__HELP =
- "This is the fully qualified name of the connection pool implementation to hold XA datasource connections.";
+ "This is the fully qualified name of the connection pool implementation to hold XA datasource connections."
+ + " When used with --type=POOLED this class must implement org.apache.geode.datasource.PooledDataSourceFactory.";
static final String CONNECTION_URL = "connection-url";
static final String URL = "url";
static final String CONNECTION_URL__HELP =
@@ -93,7 +94,9 @@ public class CreateJndiBindingCommand extends SingleGfshCommand {
"Skip the create operation when a jndi binding with the same name already exists. Without specifying this option, this command execution results into an error.";
static final String DATASOURCE_CONFIG_PROPERTIES = "datasource-config-properties";
static final String DATASOURCE_CONFIG_PROPERTIES_HELP =
- "Properties for the custom XADataSource driver. Append json string containing (name, type, value) to set any property. Eg: --datasource-config-properties={'name':'name1','type':'type1','value':'value1'},{'name':'name2','type':'type2','value':'value2'}";
+ "Properties for the data source. When used with the --type==POOLED, these properties will be used to configure the database data source unless the name begins with \"pool.\"."
+ + " If that prefix is used it will be used to configure the pool data source. Append json string containing (name, type, value) to set any property. "
+ + "For example: --datasource-config-properties={'name':'name1','type':'type1','value':'value1'},{'name':'name2','type':'type2','value':'value2'}";
@CliCommand(value = CREATE_JNDIBINDING, help = CREATE_JNDIBINDING__HELP)
@CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION,
diff --git a/geode-core/src/test/java/org/apache/geode/internal/datasource/DataSourceFactoryTest.java b/geode-core/src/test/java/org/apache/geode/internal/datasource/DataSourceFactoryTest.java
index 466c334..0e9f615 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/datasource/DataSourceFactoryTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/datasource/DataSourceFactoryTest.java
@@ -17,27 +17,64 @@
package org.apache.geode.internal.datasource;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
+import javax.sql.DataSource;
+
import org.junit.Test;
+import org.apache.geode.datasource.PooledDataSourceFactory;
+
public class DataSourceFactoryTest {
private final Map<String, String> inputs = new HashMap<>();
+ private final List<ConfigProperty> configProperties = new ArrayList<>();
@Test
public void creatPoolPropertiesWithNullInputReturnsEmptyOutput() {
- Properties output = DataSourceFactory.createPoolProperties(null);
+ Properties output = DataSourceFactory.createPoolProperties(null, null);
+
+ assertThat(output.isEmpty()).isTrue();
+ }
+
+ @Test
+ public void creatPoolPropertiesWithOneConfigDataSourcePropertyReturnsEmptyOutput() {
+ configProperties.add(new ConfigProperty("n1", "v1", null));
+
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.isEmpty()).isTrue();
}
@Test
+ public void creatPoolPropertiesWithOneConfigPoolPropertyReturnsOneOutput() {
+ configProperties.add(new ConfigProperty("pool.n1", "v1", null));
+
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
+
+ assertThat(output.size()).isEqualTo(1);
+ assertThat(output.getProperty("n1")).isEqualTo("v1");
+ }
+
+ @Test
+ public void creatPoolPropertiesWithOneUpperCaseConfigPoolPropertyReturnsOneOutput() {
+ configProperties.add(new ConfigProperty("POOL.N1", "v1", null));
+
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
+
+ assertThat(output.size()).isEqualTo(1);
+ assertThat(output.getProperty("N1")).isEqualTo("v1");
+ }
+
+ @Test
public void creatPoolPropertiesWithEmptyInputReturnsEmptyOutput() {
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.isEmpty()).isTrue();
}
@@ -46,7 +83,7 @@ public class DataSourceFactoryTest {
public void creatPoolPropertiesWithNullValueInputReturnsEmptyOutput() {
inputs.put("name", null);
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.isEmpty()).isTrue();
}
@@ -55,7 +92,7 @@ public class DataSourceFactoryTest {
public void creatPoolPropertiesWithEmptyStringValueInputReturnsEmptyOutput() {
inputs.put("name", "");
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.isEmpty()).isTrue();
}
@@ -64,7 +101,7 @@ public class DataSourceFactoryTest {
public void creatPoolPropertiesWithOneInputReturnsOneOutput() {
inputs.put("name", "value");
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.size()).isEqualTo(1);
assertThat(output.getProperty("name")).isEqualTo("value");
@@ -79,7 +116,7 @@ public class DataSourceFactoryTest {
inputs.put("managed-conn-factory-class", "value");
inputs.put("xa-datasource-class", "value");
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
assertThat(output.isEmpty()).isTrue();
}
@@ -96,12 +133,91 @@ public class DataSourceFactoryTest {
inputs.put("xa-datasource-class", "value");
inputs.put("validname1", "value1");
inputs.put("validname2", "value2");
+ configProperties.add(new ConfigProperty("pool.n1", "v1", null));
+ configProperties.add(new ConfigProperty("dataSourceProp", "dataSourceValue", null));
+ configProperties.add(new ConfigProperty("pool.n2", "v2", null));
- Properties output = DataSourceFactory.createPoolProperties(inputs);
+ Properties output = DataSourceFactory.createPoolProperties(inputs, configProperties);
- assertThat(output.size()).isEqualTo(2);
+ assertThat(output.size()).isEqualTo(4);
assertThat(output.getProperty("validname1")).isEqualTo("value1");
assertThat(output.getProperty("validname2")).isEqualTo("value2");
+ assertThat(output.getProperty("n1")).isEqualTo("v1");
+ assertThat(output.getProperty("n2")).isEqualTo("v2");
+ }
+
+ @Test
+ public void createDataSourcePropertiesWithNullReturnsEmpty() {
+ Properties output = DataSourceFactory.createDataSourceProperties(null);
+
+ assertThat(output).isEmpty();
+ }
+
+ @Test
+ public void createDataSourcePropertiesWithEmptyListReturnsEmpty() {
+ Properties output = DataSourceFactory.createDataSourceProperties(configProperties);
+
+ assertThat(output).isEmpty();
+ }
+
+ @Test
+ public void createDataSourcePropertiesWithPoolPropertyReturnsEmpty() {
+ configProperties.add(new ConfigProperty("pool.n1", "v1", null));
+
+ Properties output = DataSourceFactory.createDataSourceProperties(configProperties);
+
+ assertThat(output).isEmpty();
+ }
+
+ @Test
+ public void createDataSourcePropertiesWithNonPoolPropertyReturnsOne() {
+ configProperties.add(new ConfigProperty("n1", "v1", null));
+
+ Properties output = DataSourceFactory.createDataSourceProperties(configProperties);
+
+ assertThat(output.size()).isEqualTo(1);
+ assertThat(output.getProperty("n1")).isEqualTo("v1");
+ }
+
+ @Test
+ public void createDataSourcePropertiesWithMuliplePropertiesReturnsJustNonPool() {
+ configProperties.add(new ConfigProperty("n1", "v1", null));
+ configProperties.add(new ConfigProperty("pool.n3", "v3", null));
+ configProperties.add(new ConfigProperty("n2", "v2", null));
+
+ Properties output = DataSourceFactory.createDataSourceProperties(configProperties);
+
+ assertThat(output.size()).isEqualTo(2);
+ assertThat(output.getProperty("n1")).isEqualTo("v1");
+ assertThat(output.getProperty("n2")).isEqualTo("v2");
+ }
+
+ public static class TestPooledDataSourceFactory implements PooledDataSourceFactory {
+ @Override
+ public DataSource createDataSource(Properties poolProperties, Properties dataSourceProperties) {
+ return null;
+ }
+ }
+
+ @Test
+ public void getPooledDataSourceUsesConnPooledDataSourceClass() throws DataSourceCreateException {
+ inputs.put("conn-pooled-datasource-class",
+ "org.apache.geode.internal.datasource.DataSourceFactoryTest$TestPooledDataSourceFactory");
+
+ DataSource dataSource = DataSourceFactory.getPooledDataSource(inputs, configProperties);
+
+ assertThat(dataSource).isNull();
+ }
+
+ @Test
+ public void getPooledDataSourceFailsIfClassDoesNotExist() throws DataSourceCreateException {
+ inputs.put("conn-pooled-datasource-class", "doesNotExist");
+
+ Throwable throwable =
+ catchThrowable(() -> DataSourceFactory.getPooledDataSource(inputs, configProperties));
+
+ assertThat(throwable).isInstanceOf(DataSourceCreateException.class).hasMessage(
+ "DataSourceFactory::getPooledDataSource:Exception creating ConnectionPoolDataSource.Exception string=java.lang.ClassNotFoundException: doesNotExist");
}
}