From da84aa7008c7a0ff10ea327c632d7dd9b0bb3c9c Mon Sep 17 00:00:00 2001 From: Gareth Clay Date: Fri, 2 Nov 2018 12:30:05 +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 | 2 +- .../service/relational/DataSourceCreator.java | 16 +- .../HikariCpPooledDataSourceCreator.java | 9 +- .../TomcatDbcpPooledDataSourceCreator.java | 6 +- .../TomcatJdbcPooledDataSourceCreator.java | 6 +- .../relational/UrlDecodingDataSource.java | 118 ++++++++++ .../DataSourceCloudConfigTestHelper.java | 28 ++- .../config/xml/DataSourceXmlConfigTest.java | 11 +- .../AbstractDataSourceCreatorTest.java | 20 +- .../PooledDataSourceCreatorsTest.java | 28 ++- .../relational/UrlDecodingDataSourceTest.java | 204 ++++++++++++++++++ 11 files changed, 410 insertions(+), 38 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 5158b54d..34fcdf5c 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 @@ -21,6 +21,6 @@ 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"); } } 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..f4cf81a6 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; /** * @@ -32,7 +30,7 @@ public abstract class DataSourceCreator extend private String validationQuery; private Map> pooledDataSourceCreators = - new LinkedHashMap>(); + new LinkedHashMap<>(); public DataSourceCreator(String driverSystemPropKey, String[] driverClasses, String validationQuery) { this.driverSystemPropKey = driverSystemPropKey; @@ -40,10 +38,10 @@ public DataSourceCreator(String driverSystemPropKey, String[] driverClasses, Str this.validationQuery = validationQuery; if (pooledDataSourceCreators.size() == 0) { - putPooledDataSourceCreator(new TomcatJdbcPooledDataSourceCreator()); - putPooledDataSourceCreator(new HikariCpPooledDataSourceCreator()); - putPooledDataSourceCreator(new TomcatDbcpPooledDataSourceCreator()); - putPooledDataSourceCreator(new BasicDbcpPooledDataSourceCreator()); + putPooledDataSourceCreator(new TomcatJdbcPooledDataSourceCreator<>()); + putPooledDataSourceCreator(new HikariCpPooledDataSourceCreator<>()); + putPooledDataSourceCreator(new TomcatDbcpPooledDataSourceCreator<>()); + putPooledDataSourceCreator(new BasicDbcpPooledDataSourceCreator<>()); } } @@ -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); @@ -84,7 +82,7 @@ private Collection> filterPooledDataSourceCreators(S if (serviceConnectorConfig != null) { List pooledDataSourceNames = ((DataSourceConfig) serviceConnectorConfig).getPooledDataSourceNames(); if (pooledDataSourceNames != null) { - List> filtered = new ArrayList>(); + List> filtered = new ArrayList<>(); for (String name : pooledDataSourceNames) { for (String key : pooledDataSourceCreators.keySet()) { 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..b2f1a2eb --- /dev/null +++ b/spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/UrlDecodingDataSource.java @@ -0,0 +1,118 @@ +/* + * 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.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.function.Function; +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 { + + private static final Logger logger = Logger.getLogger(DelegatingDataSource.class.getName()); + + private final String urlPropertyName; + private final Function connectionTestDataSourceFactory; + + private volatile boolean successfullyConnected = false; + + UrlDecodingDataSource(String jdbcUrl) { + this(newSimpleDriverDataSource(jdbcUrl), "url"); + } + + UrlDecodingDataSource(DataSource targetDataSource, String urlPropertyName) { + this(targetDataSource, urlPropertyName, UrlDecodingDataSource::newSimpleDriverDataSource); + } + + UrlDecodingDataSource(DataSource targetDataSource, String urlPropertyName, Function 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 String decode(String string) { + try { + return URLDecoder.decode(string, StandardCharsets.UTF_8.name()); + } 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 35a24c18..c3353066 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; @@ -108,6 +108,10 @@ public void pooledDataSourceCreationHikariCP() throws Exception { @Test public void pooledDataSourceCreationInvalid() { DataSource ds = createMysqlDataSourceWithPooledName("Dummy"); + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(org.springframework.jdbc.datasource.SimpleDriverDataSource.class)); } @@ -125,6 +129,10 @@ private DataSource createMysqlDataSource(ServiceConnectorConfig config) { } private void assertBasicDbcpDataSource(DataSource ds) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(Class.forName(DBCP2_BASIC_DATASOURCE))); assertEquals(MIN_POOL_SIZE, getIntValue(ds, "minIdle")); @@ -134,6 +142,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)) { @@ -154,6 +166,10 @@ private void assertTomcatDbcpDataSource(DataSource ds) throws ClassNotFoundExcep } private void assertTomcatJdbcDataSource(DataSource ds, boolean overrideConfig) throws ClassNotFoundException { + assertThat(ds, instanceOf(UrlDecodingDataSource.class)); + + ds = ((UrlDecodingDataSource) ds).getTargetDataSource(); + assertThat(ds, instanceOf(Class.forName(TOMCAT_JDBC_DATASOURCE))); if (overrideConfig) { @@ -171,6 +187,10 @@ private void assertTomcatJdbcDataSource(DataSource ds, boolean overrideConfig) t } 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..3300b2ec --- /dev/null +++ b/spring-cloud-spring-service-connector/src/test/java/org/springframework/cloud/service/relational/UrlDecodingDataSourceTest.java @@ -0,0 +1,204 @@ +/* + * 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.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Objects; +import java.util.function.BiFunction; +import javax.sql.DataSource; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.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, + UrlDecodingDataSourceTest::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, + UrlDecodingDataSourceTest::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, + url -> 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, + url -> 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; + } + } + + @FunctionalInterface + private interface ConnectionSupplier { + Connection getConnection(String url) throws SQLException; + } + + private static Connection withPooling(ConnectionSupplier supplier, String url) throws SQLException { + Connection connection = supplier.getConnection(url); + when(connection.getClientInfo(IS_POOLED)).thenReturn(TRUE); + return connection; + } + + private static MockDataSource pooledDataSourceUnableToConnectToAnyUrl() { + return new MockDataSource(ENCODED_URL, u -> withPooling(UrlDecodingDataSourceTest::neverConnect, u)); + } + + private static MockDataSource pooledDataSourceRequiringDecodedUrl() { + return new MockDataSource(ENCODED_URL, u -> withPooling(UrlDecodingDataSourceTest::onlyConnectToDecodedUrls, u)); + } + + private static MockDataSource pooledDataSourceRequiringEncodedUrl() { + return new MockDataSource(ENCODED_URL, u -> withPooling(UrlDecodingDataSourceTest::onlyConnectToEncodedUrls, u)); + } + + private static MockDataSource dataSourceUnableToConnectToAnyUrl(String url) { + return new MockDataSource(url, UrlDecodingDataSourceTest::neverConnect); + } + + private static MockDataSource dataSourceRequiringDecodedUrl(String url) { + return new MockDataSource(url, UrlDecodingDataSourceTest::onlyConnectToDecodedUrls); + } + + private static Connection neverConnect(String url) throws SQLException { + throw new SQLException("unable to connect to " + url); + } + + private static Connection onlyConnectToDecodedUrls(String url) throws SQLException { + return onlyConnectToUrlsPassingDecodingTest(url, Objects::equals); + } + + private static Connection onlyConnectToEncodedUrls(String url) throws SQLException { + return onlyConnectToUrlsPassingDecodingTest(url, (original, decodedUrl) -> !Objects.equals(original, decodedUrl)); + } + + private static Connection onlyConnectToUrlsPassingDecodingTest(String url, BiFunction urlTest) throws SQLException { + try { + String decodedUrl = URLDecoder.decode(url, StandardCharsets.UTF_8.name()); + 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); + } + } +}