From 8841c47d8f3aa1a5bf724d9996a8326384816a03 Mon Sep 17 00:00:00 2001 From: Gareth Clay Date: Fri, 2 Nov 2018 15:15:09 +0000 Subject: [PATCH] Handle drivers which don't support URL-encoding 1.2.6 introduced a change to URL-encode credentials in JDBC connection strings by default. This was to prevent special characters from interfering with connections, but unfortunately some drivers can't handle the encoding. Also in some clouds, the JDBC URL is passed to us in URL encoded form. Again, in the case where the driver is unable to URL-decode the URL itself, this causes connection failures. This commit introduces `UrlDecodingDataSource`, which is returned in all cases by whichever `DataSourceCreator` is in play. `UrlDecodingDataSource` transparently delegates to an underlying `DataSource`, except in the case where a connection attempt is made and no previous connection attempt has been successful. In this case, if the connection attempt with the configured JDBC URL fails, a test connection is attempted using a URL-decoded version of the JDBC URL. If this is successful, the underlying `DataSource` is updated with the decoded JDBC URL and used to provide a final connection, which is returned to the client. Fixes gh-237 --- .../BasicDbcpPooledDataSourceCreator.java | 8 +- .../service/relational/DataSourceCreator.java | 4 +- .../HikariCpPooledDataSourceCreator.java | 9 +- .../TomcatDbcpPooledDataSourceCreator.java | 6 +- .../TomcatJdbcPooledDataSourceCreator.java | 6 +- .../relational/UrlDecodingDataSource.java | 129 +++++++++ .../DataSourceCloudConfigTestHelper.java | 28 +- .../config/xml/DataSourceXmlConfigTest.java | 11 +- .../AbstractDataSourceCreatorTest.java | 20 +- .../PooledDataSourceCreatorsTest.java | 28 +- .../relational/UrlDecodingDataSourceTest.java | 264 ++++++++++++++++++ 11 files changed, 478 insertions(+), 35 deletions(-) create mode 100644 spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/UrlDecodingDataSource.java create mode 100644 spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/UrlDecodingDataSourceTest.java diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/BasicDbcpPooledDataSourceCreator.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/BasicDbcpPooledDataSourceCreator.java index 2e78dc4b..f3e29a2b 100644 --- a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/BasicDbcpPooledDataSourceCreator.java +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/BasicDbcpPooledDataSourceCreator.java @@ -1,12 +1,12 @@ package org.springframework.cloud.service.relational; -import static org.springframework.cloud.service.Util.hasClass; - import javax.sql.DataSource; import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.common.RelationalServiceInfo; +import static org.springframework.cloud.service.Util.hasClass; + /** * * @author Ramnivas Laddad @@ -24,12 +24,12 @@ public DataSource create(RelationalServiceInfo serviceInfo, ServiceConnectorConf logger.info("Found DBCP2 on the classpath. Using it for DataSource connection pooling."); org.apache.commons.dbcp2.BasicDataSource ds = new org.apache.commons.dbcp2.BasicDataSource(); setBasicDataSourceProperties(ds, serviceInfo, serviceConnectorConfig, driverClassName, validationQuery); - return ds; + return new UrlDecodingDataSource(ds, "url"); } else if (hasClass(DBCP_BASIC_DATASOURCE)) { logger.info("Found DBCP on the classpath. Using it for DataSource connection pooling."); org.apache.commons.dbcp.BasicDataSource ds = new org.apache.commons.dbcp.BasicDataSource(); setBasicDataSourceProperties(ds, serviceInfo, serviceConnectorConfig, driverClassName, validationQuery); - return ds; + return new UrlDecodingDataSource(ds, "url"); } else { return null; } diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/DataSourceCreator.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/DataSourceCreator.java index 4a36040f..bf1465f8 100644 --- a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/DataSourceCreator.java +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/DataSourceCreator.java @@ -1,6 +1,5 @@ package org.springframework.cloud.service.relational; -import java.sql.DriverManager; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; @@ -15,7 +14,6 @@ import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.ServiceConnectorCreationException; import org.springframework.cloud.service.common.RelationalServiceInfo; -import org.springframework.jdbc.datasource.SimpleDriverDataSource; /** * @@ -60,7 +58,7 @@ public DataSource create(SI serviceInfo, ServiceConnectorConfig serviceConnector } // Only for testing outside Tomcat/CloudFoundry logger.warning("No connection pooling DataSource implementation found on the classpath - no pooling is in effect."); - return new SimpleDriverDataSource(DriverManager.getDriver(serviceInfo.getJdbcUrl()), serviceInfo.getJdbcUrl()); + return new UrlDecodingDataSource(serviceInfo.getJdbcUrl()); } catch (Exception e) { throw new ServiceConnectorCreationException( "Failed to created cloud datasource for " + serviceInfo.getId() + " service", e); diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/HikariCpPooledDataSourceCreator.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/HikariCpPooledDataSourceCreator.java index cc951888..f29dfbbc 100644 --- a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/HikariCpPooledDataSourceCreator.java +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/HikariCpPooledDataSourceCreator.java @@ -1,17 +1,16 @@ package org.springframework.cloud.service.relational; -import static org.springframework.cloud.service.Util.*; - import java.util.logging.Logger; - import javax.sql.DataSource; +import com.zaxxer.hikari.HikariDataSource; + import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.common.RelationalServiceInfo; -import com.zaxxer.hikari.HikariDataSource; +import static org.springframework.cloud.service.Util.hasClass; public class HikariCpPooledDataSourceCreator implements PooledDataSourceCreator { @@ -41,7 +40,7 @@ public DataSource create(RelationalServiceInfo serviceInfo, ServiceConnectorConf logger.info("Found HikariCP on the classpath. Using it for DataSource connection pooling."); HikariDataSource ds = new HikariDataSource(); setBasicDataSourceProperties(ds, serviceInfo, serviceConnectorConfig, driverClassName, validationQuery); - return ds; + return new UrlDecodingDataSource(ds, "jdbcUrl"); } else { return null; } diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatDbcpPooledDataSourceCreator.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatDbcpPooledDataSourceCreator.java index 90951b30..05749693 100644 --- a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatDbcpPooledDataSourceCreator.java +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatDbcpPooledDataSourceCreator.java @@ -1,13 +1,13 @@ package org.springframework.cloud.service.relational; -import static org.springframework.cloud.service.Util.hasClass; - import javax.sql.DataSource; import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.ServiceConnectorCreationException; import org.springframework.cloud.service.common.RelationalServiceInfo; +import static org.springframework.cloud.service.Util.hasClass; + /** * * @author Ramnivas Laddad @@ -39,7 +39,7 @@ private DataSource createDataSource(String className, RelationalServiceInfo serv try { DataSource dataSource = (DataSource) Class.forName(className).newInstance(); setBasicDataSourceProperties(dataSource, serviceInfo, serviceConnectorConfig, driverClassName, validationQuery); - return dataSource; + return new UrlDecodingDataSource(dataSource, "url"); } catch (Throwable e) { throw new ServiceConnectorCreationException("Error instantiating Tomcat DBCP connection pool", e); } diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatJdbcPooledDataSourceCreator.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatJdbcPooledDataSourceCreator.java index a0066107..9c3feb88 100644 --- a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatJdbcPooledDataSourceCreator.java +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/TomcatJdbcPooledDataSourceCreator.java @@ -1,12 +1,12 @@ package org.springframework.cloud.service.relational; -import static org.springframework.cloud.service.Util.hasClass; - import javax.sql.DataSource; import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.common.RelationalServiceInfo; +import static org.springframework.cloud.service.Util.hasClass; + /** * * @author Ramnivas Laddad @@ -23,7 +23,7 @@ public DataSource create(RelationalServiceInfo serviceInfo, ServiceConnectorConf logger.info("Found Tomcat JDBC connection pool on the classpath. Using it for DataSource connection pooling."); org.apache.tomcat.jdbc.pool.DataSource ds = new org.apache.tomcat.jdbc.pool.DataSource(); setBasicDataSourceProperties(ds, serviceInfo, serviceConnectorConfig, driverClassName, validationQuery); - return ds; + return new UrlDecodingDataSource(ds, "url"); } else { return null; } diff --git a/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/UrlDecodingDataSource.java b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/UrlDecodingDataSource.java new file mode 100644 index 00000000..a8ce4787 --- /dev/null +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/UrlDecodingDataSource.java @@ -0,0 +1,129 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed 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.springframework.cloud.service.relational; + +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.logging.Logger; +import javax.sql.DataSource; + +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeanWrapperImpl; +import org.springframework.jdbc.datasource.DelegatingDataSource; +import org.springframework.jdbc.datasource.SimpleDriverDataSource; + +/** + *

+ * {@link UrlDecodingDataSource} is used to transparently handle combinations of Clouds and database + * drivers which may or may not provide or accept URL-encoded connection strings. + *

+ * + *

+ * This {@link DataSource} implementation transparently delegates to an underlying {@link DataSource}, + * except in the case where a connection attempt is made and no previous connection attempt has been + * successful. In this case, if the underlying {@link DataSource} fails to connect, + * {@link UrlDecodingDataSource} makes a test connection using a URL-decoded version of the configured + * JDBC URL. If this is successful, the underlying {@link DataSource} is updated with the decoded JDBC + * URL, and used to establish the final connection which is returned to the client. + *

+ * + * @author Gareth Clay + */ +class UrlDecodingDataSource extends DelegatingDataSource { + + interface DataSourceFactory { + DataSource apply(String jdbcUrl); + } + + private static final Logger logger = Logger.getLogger(DelegatingDataSource.class.getName()); + + private final String urlPropertyName; + private final DataSourceFactory connectionTestDataSourceFactory; + + private volatile boolean successfullyConnected = false; + + UrlDecodingDataSource(String jdbcUrl) { + this(newSimpleDriverDataSource(jdbcUrl), "url"); + } + + UrlDecodingDataSource(DataSource targetDataSource, String urlPropertyName) { + this(targetDataSource, urlPropertyName, newSimpleDriverDataSourceFactory()); + } + + UrlDecodingDataSource(DataSource targetDataSource, String urlPropertyName, DataSourceFactory connectionTestDataSourceFactory) { + super(targetDataSource); + this.urlPropertyName = urlPropertyName; + this.connectionTestDataSourceFactory = connectionTestDataSourceFactory; + } + + @Override + public Connection getConnection() throws SQLException { + if (successfullyConnected) return super.getConnection(); + + synchronized (this) { + Connection connection; + + try { + connection = super.getConnection(); + successfullyConnected = true; + } catch (SQLException e) { + logger.info("Database connection failed. Trying again with url-decoded jdbc url"); + DataSource targetDataSource = getTargetDataSource(); + if (targetDataSource == null) throw new IllegalStateException("target DataSource should never be null"); + BeanWrapper dataSourceWrapper = new BeanWrapperImpl(targetDataSource); + String decodedJdbcUrl = decode((String) dataSourceWrapper.getPropertyValue(urlPropertyName)); + DataSource urlDecodedConnectionTestDataSource = connectionTestDataSourceFactory.apply(decodedJdbcUrl); + urlDecodedConnectionTestDataSource.getConnection(); + + logger.info("Connection test successful. Continuing with url-decoded jdbc url"); + dataSourceWrapper.setPropertyValue(urlPropertyName, decodedJdbcUrl); + connection = super.getConnection(); + successfullyConnected = true; + } + + return connection; + } + } + + private static DataSource newSimpleDriverDataSource(String jdbcUrl) { + try { + return new SimpleDriverDataSource(DriverManager.getDriver(jdbcUrl), jdbcUrl); + } catch (SQLException e) { + throw new RuntimeException("Unable to construct DataSource", e); + } + } + + private static DataSourceFactory newSimpleDriverDataSourceFactory() { + return new DataSourceFactory() { + @Override + public DataSource apply(String jdbcUrl) { + return newSimpleDriverDataSource(jdbcUrl); + } + }; + } + + private static String decode(String string) { + try { + return URLDecoder.decode(string, "UTF-8"); + } catch (UnsupportedEncodingException e) { + return string; + } + } +} diff --git a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/DataSourceCloudConfigTestHelper.java b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/DataSourceCloudConfigTestHelper.java index bb509cd3..9f0adca5 100644 --- a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/DataSourceCloudConfigTestHelper.java +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/DataSourceCloudConfigTestHelper.java @@ -1,12 +1,15 @@ package org.springframework.cloud.config; -import static org.junit.Assert.assertEquals; - import java.util.Properties; import javax.sql.DataSource; import org.springframework.cloud.ReflectionUtils; +import org.springframework.jdbc.datasource.DelegatingDataSource; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; /** * @@ -16,14 +19,35 @@ public class DataSourceCloudConfigTestHelper extends CommonPoolCloudConfigTestHelper { public static void assertPoolProperties(DataSource dataSource, int maxActive, int minIdle, long maxWait) { + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } assertCommonsPoolProperties(dataSource, maxActive, minIdle, maxWait); + } public static void assertConnectionProperties(DataSource dataSource, Properties connectionProp) { + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } assertEquals(connectionProp, ReflectionUtils.getValue(dataSource, "connectionProperties")); } public static void assertConnectionProperty(DataSource dataSource, String key, Object value) { + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } assertEquals(value, ReflectionUtils.getValue(dataSource, key)); } + + public static void assertDataSourceType(DataSource dataSource, String className) throws ClassNotFoundException { + assertDataSourceType(dataSource, Class.forName(className)); + } + + public static void assertDataSourceType(DataSource dataSource, Class clazz) throws ClassNotFoundException { + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } + assertThat(dataSource, instanceOf(clazz)); + } } diff --git a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/xml/DataSourceXmlConfigTest.java b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/xml/DataSourceXmlConfigTest.java index 32703ba3..0b204c60 100644 --- a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/xml/DataSourceXmlConfigTest.java +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/config/xml/DataSourceXmlConfigTest.java @@ -1,10 +1,10 @@ package org.springframework.cloud.config.xml; import java.util.Properties; - import javax.sql.DataSource; import org.junit.Test; + import org.springframework.beans.factory.BeanCreationException; import org.springframework.cloud.config.DataSourceCloudConfigTestHelper; import org.springframework.cloud.service.ServiceInfo; @@ -13,10 +13,9 @@ import org.springframework.context.ApplicationContext; import org.springframework.jdbc.datasource.SimpleDriverDataSource; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.junit.Assert.assertThat; import static org.springframework.cloud.config.DataSourceCloudConfigTestHelper.assertConnectionProperties; import static org.springframework.cloud.config.DataSourceCloudConfigTestHelper.assertConnectionProperty; +import static org.springframework.cloud.config.DataSourceCloudConfigTestHelper.assertDataSourceType; /** * @@ -109,7 +108,7 @@ public void cloudDataSourceWithTomcatJdbcDataSource() throws Exception { createService("my-service")); DataSource ds = testContext.getBean("db-pool-tomcat-jdbc", getConnectorType()); - assertThat(ds, instanceOf(Class.forName(TomcatJdbcPooledDataSourceCreator.TOMCAT_JDBC_DATASOURCE))); + assertDataSourceType(ds, TomcatJdbcPooledDataSourceCreator.TOMCAT_JDBC_DATASOURCE); } @Test @@ -118,7 +117,7 @@ public void cloudDataSourceWithHikariCpDataSource() throws Exception { createService("my-service")); DataSource ds = testContext.getBean("db-pool-hikari", getConnectorType()); - assertThat(ds, instanceOf(Class.forName(HikariCpPooledDataSourceCreator.HIKARI_DATASOURCE))); + assertDataSourceType(ds, HikariCpPooledDataSourceCreator.HIKARI_DATASOURCE); } @Test @@ -127,6 +126,6 @@ public void cloudDataSourceWithInvalidDataSource() throws Exception { createService("my-service")); DataSource ds = testContext.getBean("db-pool-invalid", getConnectorType()); - assertThat(ds, instanceOf(SimpleDriverDataSource.class)); + assertDataSourceType(ds, SimpleDriverDataSource.class); } } diff --git a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/AbstractDataSourceCreatorTest.java b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/AbstractDataSourceCreatorTest.java index 51bf5e7d..79aaa939 100644 --- a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/AbstractDataSourceCreatorTest.java +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/AbstractDataSourceCreatorTest.java @@ -1,23 +1,24 @@ package org.springframework.cloud.service.relational; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - import java.util.Collections; import java.util.List; import java.util.Properties; - import javax.sql.DataSource; import org.junit.Test; + import org.springframework.cloud.ReflectionUtils; import org.springframework.cloud.config.DataSourceCloudConfigTestHelper; import org.springframework.cloud.service.PooledServiceConnectorConfig.PoolConfig; import org.springframework.cloud.service.common.RelationalServiceInfo; import org.springframework.cloud.service.relational.DataSourceConfig.ConnectionConfig; +import org.springframework.jdbc.datasource.DelegatingDataSource; import org.springframework.test.util.ReflectionTestUtils; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + /** * * @author Ramnivas Laddad @@ -59,12 +60,21 @@ public void cloudDataSourceCreationWithConfig() throws Exception { } private void assertConnectionProperties(DataSource dataSource, Properties connectionProp) { + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } assertEquals(connectionProp, ReflectionTestUtils.getField(dataSource, "connectionProperties")); } private void assertDataSourceProperties(RelationalServiceInfo relationalServiceInfo, DataSource dataSource) { assertNotNull(dataSource); + if (dataSource instanceof DelegatingDataSource) { + dataSource = ((DelegatingDataSource) dataSource).getTargetDataSource(); + } + + assertNotNull(dataSource); + assertEquals(getDriverName(), ReflectionUtils.getValue(dataSource, "driverClassName")); assertEquals(relationalServiceInfo.getJdbcUrl(), ReflectionUtils.getValue(dataSource, "url")); diff --git a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/PooledDataSourceCreatorsTest.java b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/PooledDataSourceCreatorsTest.java index 0ea45fe0..ab2931d9 100644 --- a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/PooledDataSourceCreatorsTest.java +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/PooledDataSourceCreatorsTest.java @@ -1,21 +1,21 @@ package org.springframework.cloud.service.relational; +import java.util.Collections; +import java.util.List; +import java.util.Properties; import javax.sql.DataSource; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; + import org.springframework.cloud.ReflectionUtils; import org.springframework.cloud.service.PooledServiceConnectorConfig.PoolConfig; import org.springframework.cloud.service.ServiceConnectorConfig; import org.springframework.cloud.service.common.MysqlServiceInfo; import org.springframework.cloud.service.relational.DataSourceConfig.ConnectionConfig; -import java.util.Collections; -import java.util.List; -import java.util.Properties; - import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -99,6 +99,10 @@ public void pooledDataSourceCreationHikariCP() throws Exception { @Test public void pooledDataSourceCreationInvalid() throws Exception { DataSource ds = createMysqlDataSourceWithPooledName("Dummy"); + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(org.springframework.jdbc.datasource.SimpleDriverDataSource.class)); } @@ -116,6 +120,10 @@ private DataSource createMysqlDataSource(ServiceConnectorConfig config) { } private void assertBasicDbcpDataSource(DataSource ds) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertTrue(hasClass(DBCP2_BASIC_DATASOURCE) || hasClass(DBCP_BASIC_DATASOURCE)); if (hasClass(DBCP2_BASIC_DATASOURCE)) { @@ -137,6 +145,10 @@ private void assertBasicDbcpDataSource(DataSource ds) throws ClassNotFoundExcept } private void assertTomcatDbcpDataSource(DataSource ds) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertTrue(hasClass(TOMCAT_7_DBCP) || hasClass(TOMCAT_8_DBCP)); if (hasClass(TOMCAT_7_DBCP)) { @@ -157,6 +169,10 @@ private void assertTomcatDbcpDataSource(DataSource ds) throws ClassNotFoundExcep } private void assertTomcatJdbcDataSource(DataSource ds) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(Class.forName(TOMCAT_JDBC_DATASOURCE))); assertEquals(MIN_POOL_SIZE, getIntValue(ds, "minIdle")); @@ -166,6 +182,10 @@ private void assertTomcatJdbcDataSource(DataSource ds) throws ClassNotFoundExcep } private void assertHikariDataSource(DataSource ds) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(Class.forName(HIKARI_DATASOURCE))); assertEquals(MIN_POOL_SIZE, getIntValue(ds, "minimumIdle")); diff --git a/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/UrlDecodingDataSourceTest.java b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/UrlDecodingDataSourceTest.java new file mode 100644 index 00000000..c5ca0546 --- /dev/null +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/UrlDecodingDataSourceTest.java @@ -0,0 +1,264 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed 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.springframework.cloud.service.relational; + +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.sql.Connection; +import java.sql.SQLException; +import javax.sql.DataSource; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; + +import org.springframework.jdbc.datasource.AbstractDataSource; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class UrlDecodingDataSourceTest { + + private static final String ENCODED_URL = "jdbc:mysql://host.example.com:3306/db?user=user%40host&password=password%21"; + private static final String DECODED_URL = "jdbc:mysql://host.example.com:3306/db?user=user@host&password=password!"; + private static final String URL_PROPERTY_NAME = "url"; + private static final String IS_POOLED = "is pooled"; + private static final String TRUE = "true"; + + @Test + public void whenConnectionToEncodedUrlIsSuccessful() throws SQLException { + MockDataSource delegateDataSource = pooledDataSourceRequiringEncodedUrl(); + UrlDecodingDataSource urlDecodingDataSource = new UrlDecodingDataSource(delegateDataSource, "url"); + + Connection connection = urlDecodingDataSource.getConnection(); + + assertNotNull("Returned connection must not be null", connection); + assertEquals(ENCODED_URL, delegateDataSource.url); + assertEquals("Connection must be made using the pooled (delegate) data source", TRUE, connection.getClientInfo(IS_POOLED)); + } + + @Test + public void whenConnectionToDecodedUrlIsSuccessful() throws SQLException { + MockDataSource delegateDataSource = pooledDataSourceRequiringDecodedUrl(); + UrlDecodingDataSource urlDecodingDataSource = new UrlDecodingDataSource(delegateDataSource, URL_PROPERTY_NAME, + dataSourceRequiringDecodedUrl()); + + Connection connection = urlDecodingDataSource.getConnection(); + + assertNotNull("Returned connection must not be null", connection); + assertEquals(DECODED_URL, delegateDataSource.url); + assertEquals("Connection must be made using the pooled (delegate) data source", TRUE, connection.getClientInfo(IS_POOLED)); + } + + @Test(expected = SQLException.class) + public void whenNoConnectionIsSuccessful() throws SQLException { + MockDataSource delegateDataSource = pooledDataSourceUnableToConnectToAnyUrl(); + UrlDecodingDataSource urlDecodingDataSource = new UrlDecodingDataSource(delegateDataSource, URL_PROPERTY_NAME, + dataSourceUnableToConnectToAnyUrl()); + + try { + urlDecodingDataSource.getConnection(); + } catch (SQLException e) { + assertEquals(ENCODED_URL, delegateDataSource.url); + throw e; + } + } + + @Test + public void successfulUrlDecodedConnectionTestIsOnlyPerformedOnce() throws SQLException { + MockDataSource delegateDataSource = pooledDataSourceRequiringDecodedUrl(); + DataSource decodedUrlConnectionTestDataSource = mock(DataSource.class); + when(decodedUrlConnectionTestDataSource.getConnection()).thenReturn(mock(Connection.class)); + UrlDecodingDataSource urlDecodingDataSource = new UrlDecodingDataSource(delegateDataSource, URL_PROPERTY_NAME, + factoryFor(decodedUrlConnectionTestDataSource)); + + urlDecodingDataSource.getConnection(); + urlDecodingDataSource.getConnection(); + + verify(decodedUrlConnectionTestDataSource, times(1)).getConnection(); + } + + @Test + public void unsuccessfulUrlDecodedConnectionTestIsTriedAgain() throws SQLException { + MockDataSource delegateDataSource = pooledDataSourceRequiringDecodedUrl(); + DataSource decodedUrlConnectionTestDataSource = mock(DataSource.class); + when(decodedUrlConnectionTestDataSource.getConnection()).thenThrow(new SQLException("unable to connect")); + UrlDecodingDataSource urlDecodingDataSource = new UrlDecodingDataSource(delegateDataSource, URL_PROPERTY_NAME, + factoryFor(decodedUrlConnectionTestDataSource)); + + try { + urlDecodingDataSource.getConnection(); + } catch (SQLException e) {} + try { + urlDecodingDataSource.getConnection(); + } catch (SQLException e) {} + + verify(decodedUrlConnectionTestDataSource, times(2)).getConnection(); + } + + private static class MockDataSource extends AbstractDataSource { + private final ConnectionSupplier connectionSupplier; + private String url; + + MockDataSource(String url, ConnectionSupplier connectionSupplier) { + this.url = url; + this.connectionSupplier = connectionSupplier; + } + + @Override + public Connection getConnection() throws SQLException { + return connectionSupplier.getConnection(url); + } + + @Override + public Connection getConnection(String username, String password) { + throw new UnsupportedOperationException(); + } + + public String getUrl() { + return url; + } + + public void setUrl(String url) { + this.url = url; + } + } + + private interface ConnectionSupplier { + Connection getConnection(String url) throws SQLException; + } + + private static MockDataSource pooledDataSourceUnableToConnectToAnyUrl() { + return new MockDataSource(ENCODED_URL, withPooling(neverConnect())); + } + + private static MockDataSource pooledDataSourceRequiringDecodedUrl() { + return new MockDataSource(ENCODED_URL, withPooling(onlyConnectToDecodedUrls())); + } + + private static MockDataSource pooledDataSourceRequiringEncodedUrl() { + return new MockDataSource(ENCODED_URL, withPooling(onlyConnectToEncodedUrls())); + } + + private static UrlDecodingDataSource.DataSourceFactory factoryFor(final DataSource dataSource) { + return new UrlDecodingDataSource.DataSourceFactory() { + @Override + public DataSource apply(String jdbcUrl) { + return dataSource; + } + }; + } + + private static UrlDecodingDataSource.DataSourceFactory dataSourceUnableToConnectToAnyUrl() { + return new UrlDecodingDataSource.DataSourceFactory() { + @Override + public DataSource apply(String jdbcUrl) { + return new MockDataSource(jdbcUrl, neverConnect()); + } + }; + } + + private static UrlDecodingDataSource.DataSourceFactory dataSourceRequiringDecodedUrl() { + return new UrlDecodingDataSource.DataSourceFactory() { + @Override + public DataSource apply(String jdbcUrl) { + return new MockDataSource(jdbcUrl, onlyConnectToDecodedUrls()); + } + }; + } + + private static ConnectionSupplier neverConnect() { + return new ConnectionSupplier() { + @Override + public Connection getConnection(String url) throws SQLException { + throw new SQLException("unable to connect to " + url); + } + }; + } + + private interface ConnectionTest { + boolean apply(String url, String decodedUrl); + } + + private static ConnectionSupplier withPooling(final ConnectionSupplier supplier) { + return new ConnectionSupplier() { + @Override + public Connection getConnection(String url) throws SQLException { + Connection connection = supplier.getConnection(url); + when(connection.getClientInfo(IS_POOLED)).thenReturn(TRUE); + return connection; + } + }; + } + + private static ConnectionSupplier onlyConnectToDecodedUrls() { + return new ConnectionSupplier() { + @Override + public Connection getConnection(String url) throws SQLException { + return onlyConnectToUrlsPassingDecodingTest(url, new ConnectionTest() { + @Override + public boolean apply(String url, String decodedUrl) { + if (url == null || decodedUrl == null) return false; + return url.equals(decodedUrl); + } + }); + } + }; + } + + private static ConnectionSupplier onlyConnectToEncodedUrls() { + return new ConnectionSupplier() { + @Override + public Connection getConnection(String url) throws SQLException { + return onlyConnectToUrlsPassingDecodingTest(url, new ConnectionTest() { + @Override + public boolean apply(String url, String decodedUrl) { + if (url == null || decodedUrl == null) return false; + return !url.equals(decodedUrl); + } + }); + } + }; + } + + private static Connection onlyConnectToUrlsPassingDecodingTest(String url, ConnectionTest urlTest) throws SQLException { + try { + String decodedUrl = URLDecoder.decode(url, "UTF-8"); + if (urlTest.apply(url, decodedUrl)) { + return mock(Connection.class); + } else { + throw new SQLException("unable to connect to " + url); + } + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + + private static UrlDecodingDataSource.DataSourceFactory dataSourceFactory(final DataSource dataSource) { + return new UrlDecodingDataSource.DataSourceFactory() { + @Override + public DataSource apply(String jdbcUrl) { + return dataSource; + } + }; + } +}