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 4e0e124d6eef1da8d2abecc8f1c51756b9b78194 Author: Darrel Schneider <[email protected]> AuthorDate: Tue Oct 16 15:03:22 2018 -0700 removed the DataSourceManager --- .../jdbc/JdbcAsyncWriterIntegrationTest.java | 3 +- .../connectors/jdbc/JdbcLoaderIntegrationTest.java | 3 +- .../connectors/jdbc/JdbcWriterIntegrationTest.java | 3 +- .../jdbc/internal/TestableConnectionManager.java | 22 ----- .../jdbc/internal/AbstractJdbcCallback.java | 8 +- .../jdbc/internal/DataSourceManager.java | 48 ----------- .../jdbc/internal/JdbcConnectorService.java | 2 - .../jdbc/internal/JdbcConnectorServiceImpl.java | 7 -- .../geode/connectors/jdbc/internal/SqlHandler.java | 15 +--- .../jdbc/internal/AbstractJdbcCallbackTest.java | 16 ---- .../jdbc/internal/DataSourceManagerUnitTest.java | 98 ---------------------- .../jdbc/internal/JdbcConnectorServiceTest.java | 6 -- .../connectors/jdbc/internal/SqlHandlerTest.java | 11 +-- 13 files changed, 10 insertions(+), 232 deletions(-) diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java index 2ec35d2..7c5fe94 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java @@ -37,7 +37,6 @@ import org.apache.geode.connectors.jdbc.internal.RegionMappingExistsException; import org.apache.geode.connectors.jdbc.internal.SqlHandler; import org.apache.geode.connectors.jdbc.internal.TableMetaDataManager; import org.apache.geode.connectors.jdbc.internal.TestConfigService; -import org.apache.geode.connectors.jdbc.internal.TestableConnectionManager; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.pdx.PdxInstance; @@ -243,7 +242,7 @@ public abstract class JdbcAsyncWriterIntegrationTest { private SqlHandler createSqlHandler() throws ConnectionConfigExistsException, RegionMappingExistsException { - return new SqlHandler(new TestableConnectionManager(), new TableMetaDataManager(), + return new SqlHandler(new TableMetaDataManager(), TestConfigService.getTestConfigService(getConnectionUrl()), new TestDataSourceFactory(getConnectionUrl())); } diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java index 3726f01..a95d914 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java @@ -38,7 +38,6 @@ import org.apache.geode.connectors.jdbc.internal.RegionMappingExistsException; import org.apache.geode.connectors.jdbc.internal.SqlHandler; import org.apache.geode.connectors.jdbc.internal.TableMetaDataManager; import org.apache.geode.connectors.jdbc.internal.TestConfigService; -import org.apache.geode.connectors.jdbc.internal.TestableConnectionManager; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.util.BlobHelper; import org.apache.geode.pdx.PdxInstance; @@ -176,7 +175,7 @@ public abstract class JdbcLoaderIntegrationTest { private SqlHandler createSqlHandler(String pdxClassName, boolean primaryKeyInValue) throws ConnectionConfigExistsException, RegionMappingExistsException { - return new SqlHandler(new TestableConnectionManager(), new TableMetaDataManager(), + return new SqlHandler(new TableMetaDataManager(), TestConfigService.getTestConfigService((InternalCache) cache, pdxClassName, primaryKeyInValue, getConnectionUrl()), new TestDataSourceFactory(getConnectionUrl())); diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java index 14fd872..a0028a0 100644 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java +++ b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcWriterIntegrationTest.java @@ -37,7 +37,6 @@ import org.apache.geode.connectors.jdbc.internal.RegionMappingExistsException; import org.apache.geode.connectors.jdbc.internal.SqlHandler; import org.apache.geode.connectors.jdbc.internal.TableMetaDataManager; import org.apache.geode.connectors.jdbc.internal.TestConfigService; -import org.apache.geode.connectors.jdbc.internal.TestableConnectionManager; import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.pdx.PdxInstance; @@ -227,7 +226,7 @@ public abstract class JdbcWriterIntegrationTest { private SqlHandler createSqlHandler() throws ConnectionConfigExistsException, RegionMappingExistsException { - return new SqlHandler(new TestableConnectionManager(), new TableMetaDataManager(), + return new SqlHandler(new TableMetaDataManager(), TestConfigService.getTestConfigService(getConnectionUrl()), new TestDataSourceFactory(getConnectionUrl())); } diff --git a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestableConnectionManager.java b/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestableConnectionManager.java deleted file mode 100644 index 4436f41..0000000 --- a/geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/internal/TestableConnectionManager.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * 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.internal; - -public class TestableConnectionManager extends DataSourceManager { - - public TestableConnectionManager() { - super(new HikariJdbcDataSourceFactory()); - } -} diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java index dc6b265..3c85f3b 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallback.java @@ -35,11 +35,7 @@ public abstract class AbstractJdbcCallback implements CacheCallback { } @Override - public void close() { - if (sqlHandler != null) { - sqlHandler.close(); - } - } + public void close() {} protected SqlHandler getSqlHandler() { return sqlHandler; @@ -60,7 +56,7 @@ public abstract class AbstractJdbcCallback implements CacheCallback { this.cache = cache; JdbcConnectorService service = cache.getService(JdbcConnectorService.class); TableMetaDataManager tableMetaDataManager = new TableMetaDataManager(); - sqlHandler = new SqlHandler(service.getDataSourceManager(), tableMetaDataManager, service); + sqlHandler = new SqlHandler(tableMetaDataManager, service); } } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java deleted file mode 100644 index e994585..0000000 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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.internal; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import org.apache.geode.connectors.jdbc.internal.configuration.ConnectorService; - -class DataSourceManager { - - private final JdbcDataSourceFactory jdbcDataSourceFactory; - private final Map<String, JdbcDataSource> dataSourceMap = new ConcurrentHashMap<>(); - - DataSourceManager(JdbcDataSourceFactory jdbcDataSourceFactory) { - this.jdbcDataSourceFactory = jdbcDataSourceFactory; - } - - JdbcDataSource getOrCreateDataSource(ConnectorService.Connection config) { - return dataSourceMap.computeIfAbsent(config.getName(), k -> { - return this.jdbcDataSourceFactory.create(config); - }); - } - - synchronized void close() { - dataSourceMap.values().forEach(this::close); - dataSourceMap.clear(); - } - - private void close(JdbcDataSource dataSource) { - try { - dataSource.close(); - } catch (Exception ignore) { - } - } -} diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java index 2b8aacd..10bb2b8 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java @@ -46,6 +46,4 @@ public interface JdbcConnectorService extends CacheService { ConnectorService.RegionMapping getMappingForRegion(String regionName); Set<ConnectorService.RegionMapping> getRegionMappings(); - - DataSourceManager getDataSourceManager(); } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceImpl.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceImpl.java index 72da9dc..82217e1 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceImpl.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceImpl.java @@ -33,8 +33,6 @@ public class JdbcConnectorServiceImpl implements JdbcConnectorService { new ConcurrentHashMap<>(); private final Map<String, ConnectorService.RegionMapping> mappingsByRegion = new ConcurrentHashMap<>(); - private final DataSourceManager manager = - new DataSourceManager(new HikariJdbcDataSourceFactory()); private volatile InternalCache cache; private boolean registered; @@ -84,11 +82,6 @@ public class JdbcConnectorServiceImpl implements JdbcConnectorService { } @Override - public DataSourceManager getDataSourceManager() { - return manager; - } - - @Override public void createRegionMapping(ConnectorService.RegionMapping mapping) throws RegionMappingExistsException { ConnectorService.RegionMapping existing = diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java index 5a444d3..924d3b3 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java @@ -38,21 +38,18 @@ import org.apache.geode.pdx.PdxInstance; @Experimental public class SqlHandler { private final JdbcConnectorService configService; - private final DataSourceManager manager; private final TableMetaDataManager tableMetaDataManager; private final DataSourceFactory dataSourceFactory; - public SqlHandler(DataSourceManager manager, TableMetaDataManager tableMetaDataManager, - JdbcConnectorService configService, DataSourceFactory dataSourceFactory) { - this.manager = manager; + public SqlHandler(TableMetaDataManager tableMetaDataManager, JdbcConnectorService configService, + DataSourceFactory dataSourceFactory) { this.tableMetaDataManager = tableMetaDataManager; this.configService = configService; this.dataSourceFactory = dataSourceFactory; } - public SqlHandler(DataSourceManager manager, TableMetaDataManager tableMetaDataManager, - JdbcConnectorService configService) { - this(manager, tableMetaDataManager, configService, + public SqlHandler(TableMetaDataManager tableMetaDataManager, JdbcConnectorService configService) { + this(tableMetaDataManager, configService, dataSourceName -> JNDIInvoker.getDataSource(dataSourceName)); } @@ -60,10 +57,6 @@ public class SqlHandler { public DataSource getDataSource(String dataSourceName); } - public void close() { - manager.close(); - } - Connection getConnection(String connectionName) throws SQLException { return getDataSource(connectionName).getConnection(); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java index eb2ee54..7a54c4e 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/AbstractJdbcCallbackTest.java @@ -18,8 +18,6 @@ package org.apache.geode.connectors.jdbc.internal; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.junit.Before; @@ -43,20 +41,6 @@ public class AbstractJdbcCallbackTest { } @Test - public void closesSqlHandler() { - jdbcCallback.close(); - verify(sqlHandler, times(1)).close(); - } - - @Test - public void closeDoesNothingIfSqlHandlerNull() { - jdbcCallback = new AbstractJdbcCallback(null, cache) {}; - jdbcCallback.close(); - verify(sqlHandler, times(0)).close(); - } - - - @Test public void returnsCorrectSqlHander() { assertThat(jdbcCallback.getSqlHandler()).isSameAs(sqlHandler); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/DataSourceManagerUnitTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/DataSourceManagerUnitTest.java deleted file mode 100644 index a470c90..0000000 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/DataSourceManagerUnitTest.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * 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.internal; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import org.junit.Before; -import org.junit.Test; - -import org.apache.geode.connectors.jdbc.internal.configuration.ConnectorService; - -public class DataSourceManagerUnitTest { - - private DataSourceManager manager; - - private JdbcDataSource dataSource; - private JdbcDataSource dataSource2; - - private ConnectorService.Connection connectionConfig; - private ConnectorService.Connection connectionConfig2; - - @Before - public void setup() throws Exception { - dataSource = mock(JdbcDataSource.class); - JdbcDataSourceFactory dataSourceFactory = mock(JdbcDataSourceFactory.class); - when(dataSourceFactory.create(any())).thenReturn(dataSource); - manager = new DataSourceManager(dataSourceFactory); - connectionConfig = - new ConnectorService.Connection("dataSource1", "url", null, null, (String[]) null); - connectionConfig2 = - new ConnectorService.Connection("dataSource2", "url", null, null, (String[]) null); - - dataSource2 = mock(JdbcDataSource.class); - doThrow(new Exception()).when(dataSource2).close(); - } - - @Test - public void retrievesANewDataSource() throws Exception { - JdbcDataSource returnedDataSource = manager.getOrCreateDataSource(connectionConfig); - - assertThat(returnedDataSource).isNotNull().isSameAs(dataSource); - } - - @Test - public void retrievesSameDataSourceForSameConnectionConfig() throws Exception { - JdbcDataSource returnedDataSource = manager.getOrCreateDataSource(connectionConfig); - JdbcDataSource secondReturnedDataSource = manager.getOrCreateDataSource(connectionConfig); - - assertThat(returnedDataSource).isNotNull().isSameAs(dataSource); - assertThat(secondReturnedDataSource).isNotNull().isSameAs(dataSource); - } - - private void registerTwoDataSourceFactory() { - JdbcDataSourceFactory dataSourceFactory = mock(JdbcDataSourceFactory.class); - when(dataSourceFactory.create(connectionConfig)).thenReturn(dataSource); - when(dataSourceFactory.create(connectionConfig2)).thenReturn(dataSource2); - manager = new DataSourceManager(dataSourceFactory); - } - - @Test - public void retrievesDifferentDataSourceForEachConfig() throws Exception { - registerTwoDataSourceFactory(); - JdbcDataSource returnedDataSource = manager.getOrCreateDataSource(connectionConfig); - JdbcDataSource secondReturnedDataSource = manager.getOrCreateDataSource(connectionConfig2); - - assertThat(returnedDataSource).isNotNull().isSameAs(dataSource); - assertThat(secondReturnedDataSource).isNotNull().isSameAs(dataSource2); - assertThat(returnedDataSource).isNotSameAs(secondReturnedDataSource); - } - - @Test - public void closesAllDataSources() throws Exception { - registerTwoDataSourceFactory(); - manager.getOrCreateDataSource(connectionConfig); - manager.getOrCreateDataSource(connectionConfig2); - manager.close(); - - verify(dataSource).close(); - verify(dataSource2).close(); - } -} diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java index 1913930..1449750 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java @@ -104,10 +104,4 @@ public class JdbcConnectorServiceTest { assertThatThrownBy(() -> service.createConnectionConfig(config2)) .isInstanceOf(ConnectionConfigExistsException.class).hasMessageContaining(TEST_CONFIG_NAME); } - - @Test - public void hasDataSourceManagerOnCreation() { - assertThat(service.getDataSourceManager()).isNotNull(); - } - } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java index 73ff53e..070dc8a 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java @@ -64,7 +64,6 @@ public class SqlHandlerTest { @Rule public ExpectedException thrown = ExpectedException.none(); - private DataSourceManager manager; private DataSource dataSource; private JdbcConnectorService connectorService; private DataSourceFactory dataSourceFactory; @@ -82,7 +81,6 @@ public class SqlHandlerTest { @SuppressWarnings("unchecked") @Before public void setup() throws Exception { - manager = mock(DataSourceManager.class); dataSource = mock(DataSource.class); region = mock(Region.class); when(region.getName()).thenReturn(REGION_NAME); @@ -98,7 +96,7 @@ public class SqlHandlerTest { connectorService = mock(JdbcConnectorService.class); dataSourceFactory = mock(DataSourceFactory.class); when(dataSourceFactory.getDataSource(DATA_SOURCE_NAME)).thenReturn(dataSource); - handler = new SqlHandler(manager, tableMetaDataManager, connectorService, dataSourceFactory); + handler = new SqlHandler(tableMetaDataManager, connectorService, dataSourceFactory); key = "key"; value = mock(PdxInstanceImpl.class); when(value.getPdxType()).thenReturn(mock(PdxType.class)); @@ -117,13 +115,6 @@ public class SqlHandlerTest { } @Test - public void verifyCloseCallsManagerClose() { - handler.close(); - - verify(manager).close(); - } - - @Test public void readThrowsIfNoKeyProvided() throws Exception { thrown.expect(IllegalArgumentException.class); handler.read(region, null);
