This is an automated email from the ASF dual-hosted git repository. dschneider pushed a commit to branch feature/GEODE-5861 in repository https://gitbox.apache.org/repos/asf/geode.git
commit 8fbbc370f8d6612ef90775c3b726f82afe96fab7 Author: Darrel Schneider <[email protected]> AuthorDate: Wed Oct 17 16:59:15 2018 -0700 the jndi-binding --type=POOLED now creates an instance of PooledDataSourceFactory and calls createDataSource on it. The jdbc end-to-end test now uses the POOLED type instead of HIKARI. The jdbc-driver-class is no longer required on jndi-binding. --- .../jdbc/CreatePooledJndiBindingDUnitTest.java | 120 +++++++++++++++++++++ .../geode/connectors/jdbc/JdbcDistributedTest.java | 4 +- .../jdbc/JdbcPooledDataSourceFactory.java | 80 ++++++++++++++ .../jdbc/JdbcPooledDataSourceFactoryTest.java | 102 ++++++++++++++++++ .../geode/datasource/PooledDataSourceFactory.java | 45 ++++++++ .../internal/datasource/DataSourceFactory.java | 64 +++++++++-- .../cli/commands/CreateJndiBindingCommand.java | 2 +- .../cli/commands/CreateJndiBindingCommandTest.java | 10 +- 8 files changed, 412 insertions(+), 15 deletions(-) diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/CreatePooledJndiBindingDUnitTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/CreatePooledJndiBindingDUnitTest.java new file mode 100644 index 0000000..a85493c --- /dev/null +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/CreatePooledJndiBindingDUnitTest.java @@ -0,0 +1,120 @@ +/* + * 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.geode.connectors.jdbc; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +import java.net.URL; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; + +import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService; +import org.apache.geode.internal.jndi.JNDIInvoker; +import org.apache.geode.management.internal.configuration.domain.Configuration; +import org.apache.geode.management.internal.configuration.utils.XmlUtils; +import org.apache.geode.test.dunit.rules.ClusterStartupRule; +import org.apache.geode.test.dunit.rules.MemberVM; +import org.apache.geode.test.junit.categories.JDBCConnectorTest; +import org.apache.geode.test.junit.rules.DatabaseConnectionRule; +import org.apache.geode.test.junit.rules.GfshCommandRule; +import org.apache.geode.test.junit.rules.MySqlConnectionRule; +import org.apache.geode.test.junit.rules.VMProvider; + +@Category({JDBCConnectorTest.class}) +public class CreatePooledJndiBindingDUnitTest { + + private static final URL COMPOSE_RESOURCE_PATH = + MySqlJdbcAsyncWriterIntegrationTest.class.getResource("mysql.yml"); + + @ClassRule + public static DatabaseConnectionRule dbRule = new MySqlConnectionRule.Builder() + .file(COMPOSE_RESOURCE_PATH.getPath()).serviceName("db").port(3306).database("test").build(); + + private static MemberVM locator, server1, server2; + + @ClassRule + public static ClusterStartupRule cluster = new ClusterStartupRule(); + + @ClassRule + public static GfshCommandRule gfsh = new GfshCommandRule(); + + + @BeforeClass + public static void before() throws Exception { + locator = cluster.startLocatorVM(0); + server1 = cluster.startServerVM(1, "group1", locator.getPort()); + server2 = cluster.startServerVM(2, "group1", locator.getPort()); + + gfsh.connectAndVerify(locator); + } + + @Test + public void testCreateJndiBinding() throws Exception { + // assert that is no datasource + VMProvider.invokeInEveryMember( + () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0)); + + // create the binding + gfsh.executeAndAssertThat( + "create jndi-binding --name=jndi1 --username=myuser --password=mypass --type=POOLED --connection-url=\"jdbc:mysql://127.0.0.1:32782/test\"") + .statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1", "server-2"); + + // verify cluster config is updated + locator.invoke(() -> { + InternalConfigurationPersistenceService ccService = + ClusterStartupRule.getLocator().getConfigurationPersistenceService(); + Configuration configuration = ccService.getConfiguration("cluster"); + String xmlContent = configuration.getCacheXmlContent(); + + Document document = XmlUtils.createDocumentFromXml(xmlContent); + NodeList jndiBindings = document.getElementsByTagName("jndi-binding"); + + assertThat(jndiBindings.getLength()).isEqualTo(1); + assertThat(xmlContent).contains("user-name=\"myuser\""); + assertThat(xmlContent).contains("password=\"mypass\""); + + boolean found = false; + for (int i = 0; i < jndiBindings.getLength(); i++) { + Element eachBinding = (Element) jndiBindings.item(i); + if (eachBinding.getAttribute("jndi-name").equals("jndi1")) { + found = true; + break; + } + } + assertThat(found).isTrue(); + }); + + // verify datasource exists + VMProvider.invokeInEveryMember( + () -> assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(1)); + + // bounce server1 + server1.stop(false); + server1 = cluster.startServerVM(1, locator.getPort()); + + // verify it has recreated the datasource from cluster config + server1.invoke(() -> { + assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(1); + }); + } +} diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java index 400d82f..15e58c6 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java @@ -608,8 +608,8 @@ public abstract class JdbcDistributedTest implements Serializable { private void createJdbcConnection() { final String commandStr = - "create jndi-binding --type=HIKARI --name=" + CONNECTION_NAME + " --jdbc-driver-class=none" - + " --connection-url=" + connectionUrl; + "create jndi-binding --type=POOLED --name=" + CONNECTION_NAME + " --connection-url=" + + connectionUrl; gfsh.executeAndAssertThat(commandStr).statusIsSuccess(); } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactory.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactory.java new file mode 100644 index 0000000..552e7ca --- /dev/null +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactory.java @@ -0,0 +1,80 @@ +/* + * 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.geode.connectors.jdbc; + +import static com.google.common.base.CaseFormat.LOWER_CAMEL; +import static com.google.common.base.CaseFormat.LOWER_HYPHEN; + +import java.util.Properties; + +import javax.sql.DataSource; + +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; + +import org.apache.geode.datasource.PooledDataSourceFactory; + +/** + * This class implements PooledDataSourceFactory for the JDBC Connector extension. + * It will be used by default for a jndi-binding of type "POOLED". + * For more information see "gfsh create jndi-binding". + */ +public class JdbcPooledDataSourceFactory implements PooledDataSourceFactory { + + public JdbcPooledDataSourceFactory() {} + + @Override + public DataSource createDataSource(Properties poolProperties, Properties dataSourceProperties) { + Properties hikariProperties = convertToHikari(poolProperties); + HikariConfig config = new HikariConfig(hikariProperties); + config.setDataSourceProperties(dataSourceProperties); + return new HikariDataSource(config); + } + + Properties convertToHikari(Properties poolProperties) { + final int MILLIS_PER_SECOND = 1000; + Properties result = new Properties(); + for (String name : poolProperties.stringPropertyNames()) { + String hikariName = convertToCamelCase(name); + String hikariValue = poolProperties.getProperty(name); + if (name.equals("connection-url")) { + hikariName = "jdbcUrl"; + } else if (name.equals("jdbc-driver-class")) { + hikariName = "driverClassName"; + } else if (name.equals("user-name")) { + hikariName = "username"; + } else if (name.equals("max-pool-size")) { + hikariName = "maximumPoolSize"; + } else if (name.equals("idle-timeout-seconds")) { + hikariName = "idleTimeout"; + hikariValue = String.valueOf(Integer.valueOf(hikariValue) * MILLIS_PER_SECOND); + } + result.setProperty(hikariName, hikariValue); + // TODO: we are setting initializationFailTimeout to -1 + // to prevent a failure of create jndi-binding when the + // database is not running. This causes the hikari pool + // to keep attempting to connect in the background. + } + result.setProperty("initializationFailTimeout", "-1"); + return result; + } + + private String convertToCamelCase(String name) { + return LOWER_HYPHEN.to(LOWER_CAMEL, name); + } + +} diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactoryTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactoryTest.java new file mode 100644 index 0000000..48a6137 --- /dev/null +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcPooledDataSourceFactoryTest.java @@ -0,0 +1,102 @@ +/* + * 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.geode.connectors.jdbc; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Properties; + +import org.junit.Test; + +public class JdbcPooledDataSourceFactoryTest { + + private JdbcPooledDataSourceFactory instance = new JdbcPooledDataSourceFactory(); + + @Test + public void validateThatConnectionUrlConvertedToJdbcUrl() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("connection-url", "foo"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).contains("jdbcUrl"); + } + + @Test + public void validateThatUserNameConvertedToUsername() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("user-name", "foo"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).contains("username"); + } + + @Test + public void validateThatJdbcDriverClassConvertedToDriverClassName() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("jdbc-driver-class", "foo"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).contains("driverClassName"); + } + + @Test + public void validateThatMaxPoolSizeConvertedToMaximumPoolSize() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("max-pool-size", "foo"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).contains("maximumPoolSize"); + } + + @Test + public void validateThatIdleTimeoutSecondsConvertedToIdleTimeout() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("idle-timeout-seconds", "20"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).contains("idleTimeout"); + } + + @Test + public void validateThatIdleTimeoutSecondsConvertedToMilliseconds() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("idle-timeout-seconds", "20"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.getProperty("idleTimeout")).isEqualTo("20000"); + } + + @Test + public void validateThatTnitializationFailTimeoutSetToMinusOne() throws Exception { + Properties poolProperties = new Properties(); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.getProperty("initializationFailTimeout")).isEqualTo("-1"); + } + + @Test + public void validateThatHyphensConvertedToCamelCase() throws Exception { + Properties poolProperties = new Properties(); + poolProperties.setProperty("foo-bar-zoo", "value"); + poolProperties.setProperty("foo", "value"); + poolProperties.setProperty("-bar", "value"); + poolProperties.setProperty("zoo-", "value"); + Properties hikariProperties = instance.convertToHikari(poolProperties); + + assertThat(hikariProperties.stringPropertyNames()).containsExactlyInAnyOrder("foo", "Bar", + "zoo", "fooBarZoo", "initializationFailTimeout"); + } + +} 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 new file mode 100644 index 0000000..d3bd80a --- /dev/null +++ b/geode-core/src/main/java/org/apache/geode/datasource/PooledDataSourceFactory.java @@ -0,0 +1,45 @@ +/* + * 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.geode.datasource; + +import java.util.Properties; + +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". + */ +public interface PooledDataSourceFactory { + /** + * Create and return a data source configured with the given properties. + * + * @param poolProperties properties to use to initialize the pool part of the data source + * The poolProperties names can be any of the following: connection-url, user-name, + * password, + * jdbc-driver-class, max-pool-size, init-pool-size, idle-timeout-seconds, + * login-timeout-seconds, + * or blocking-timeout-seconds. + * + * @param dataSourceProperties properties to use to initialize the data source the pool will use + * to create connections + * @return the created data source + */ + public DataSource createDataSource(Properties poolProperties, Properties dataSourceProperties); + +} 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 b471f52..f49717a 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 @@ -33,10 +33,10 @@ import java.lang.reflect.Method; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Properties; import javax.resource.spi.ConnectionManager; import javax.resource.spi.ManagedConnectionFactory; -import javax.sql.ConnectionPoolDataSource; import javax.sql.DataSource; import javax.sql.XADataSource; @@ -163,27 +163,27 @@ public class DataSourceFactory { return new ClientConnectionFactoryWrapper(cf, cm); } + private static final String DEFAULT_CONNECTION_POOL_DS_CLASS = + "org.apache.geode.connectors.jdbc.JdbcPooledDataSourceFactory"; + /** * This function returns the datasource with connection pooling. - * - * @param configMap a map containing configurations required for datasource. - * @return ??? */ public static DataSource getPooledDataSource(Map configMap, List<ConfigProperty> props) throws DataSourceCreateException { ConfiguredDataSourceProperties configs = createDataSourceProperties(configMap); String connpoolClassName = configs.getConnectionPoolDSClass(); if (connpoolClassName == null) { - logger.error( - "DataSourceFactory::getPooledDataSource:ConnectionPoolDataSource class name for the ResourceManager is not available"); - throw new DataSourceCreateException( - "DataSourceFactory::getPooledDataSource:ConnectionPoolDataSource class name for the ResourceManager is not available"); + connpoolClassName = DEFAULT_CONNECTION_POOL_DS_CLASS; } try { + Properties poolProperties = createPoolProperties(configMap); + Properties dataSourceProperties = createDataSourceProperties(props); Class cl = ClassPathLoader.getLatest().forName(connpoolClassName); - Object Obj = cl.newInstance(); - invokeAllMethods(cl, Obj, props); - return new GemFireConnPooledDataSource((ConnectionPoolDataSource) Obj, configs); + Object obj = cl.newInstance(); + Method createDataSourceMethod = + cl.getMethod("createDataSource", Properties.class, Properties.class); + return (DataSource) createDataSourceMethod.invoke(obj, poolProperties, dataSourceProperties); } catch (Exception ex) { String exception = String.format( @@ -197,6 +197,48 @@ public class DataSourceFactory { } } + private static Properties createPoolProperties(Map<String, String> configMap) { + Properties result = new Properties(); + if (configMap != null) { + for (Map.Entry<String, String> entry : configMap.entrySet()) { + if (entry.getValue() == null || entry.getValue().equals("")) { + continue; + } + if (entry.getKey().equals("type")) { + continue; + } + if (entry.getKey().equals("jndi-name")) { + continue; + } + if (entry.getKey().equals("transaction-type")) { + continue; + } + if (entry.getKey().equals("conn-pooled-datasource-class")) { + continue; + } + if (entry.getKey().equals("managed-conn-factory-class")) { + continue; + } + if (entry.getKey().equals("xa-datasource-class")) { + continue; + } + result.setProperty(entry.getKey(), entry.getValue()); + } + } + return result; + } + + private static Properties createDataSourceProperties(List<ConfigProperty> props) { + Properties result = new Properties(); + if (props != null) { + for (ConfigProperty prop : props) { + result.setProperty(prop.getName(), prop.getValue()); + } + } + return result; + } + + /** * This function returns the datasource with connection pooling and transaction participation * capabilities. 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 bebb226..91b1045 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 @@ -108,7 +108,7 @@ public class CreateJndiBindingCommand extends SingleGfshCommand { help = CONNECTION_URL__HELP) String connectionUrl, @CliOption(key = IDLE_TIMEOUT_SECONDS, help = IDLE_TIMEOUT_SECONDS__HELP) Integer idleTimeout, @CliOption(key = INIT_POOL_SIZE, help = INIT_POOL_SIZE__HELP) Integer initPoolSize, - @CliOption(key = JDBC_DRIVER_CLASS, mandatory = true, + @CliOption(key = JDBC_DRIVER_CLASS, help = JDBC_DRIVER_CLASS__HELP) String jdbcDriver, @CliOption(key = JNDI_NAME, mandatory = true, help = JNDI_NAME__HELP) String jndiName, @CliOption(key = LOGIN_TIMEOUT_SECONDS, diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandTest.java index a65e4b3..4359aae 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommandTest.java @@ -44,6 +44,7 @@ import org.xml.sax.SAXException; import org.apache.geode.cache.configuration.CacheConfig; import org.apache.geode.cache.configuration.JndiBindingsType; import org.apache.geode.distributed.DistributedMember; +import org.apache.geode.distributed.internal.DistributionManager; import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.management.internal.cli.GfshParseResult; @@ -66,8 +67,9 @@ public class CreateJndiBindingCommandTest { @Before public void setUp() throws Exception { cache = mock(InternalCache.class); + when(cache.getDistributionManager()).thenReturn(mock(DistributionManager.class)); command = spy(CreateJndiBindingCommand.class); - doReturn(cache).when(command).getCache(); + command.setCache(cache); binding = new JndiBindingsType.JndiBinding(); binding.setJndiName("name"); @@ -102,6 +104,12 @@ public class CreateJndiBindingCommandTest { } @Test + public void verifyJdbcDriverClassNotRequired() { + gfsh.executeAndAssertThat(command, COMMAND + " --type=SIMPLE --name=name --connection-url=url") + .statusIsSuccess(); + } + + @Test public void returnsErrorIfBindingAlreadyExistsAndIfUnspecified() throws ParserConfigurationException, SAXException, IOException { InternalConfigurationPersistenceService clusterConfigService =
