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

spring-cloud-spring-service-connector/src/main/java/org/springframework/cloud/service/relational/BasicDbcpPooledDataSourceCreator.java

@@ -21,6 +21,6 @@ public class BasicDbcpPooledDataSourceCreator<SI extends RelationalServiceInfo>
 		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");
 	}
 }

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.AbstractServiceConnectorCreator;
 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<SI extends RelationalServiceInfo> extend
 	private String validationQuery;
 
 	private Map<String, PooledDataSourceCreator<SI>> pooledDataSourceCreators =
-			new LinkedHashMap<String, PooledDataSourceCreator<SI>>();
+			new LinkedHashMap<>();
 
 	public DataSourceCreator(String driverSystemPropKey, String[] driverClasses, String validationQuery) {
 		this.driverSystemPropKey = driverSystemPropKey;
@@ -40,10 +38,10 @@ public abstract class DataSourceCreator<SI extends RelationalServiceInfo> extend
 		this.validationQuery = validationQuery;
 
 		if (pooledDataSourceCreators.size() == 0) {
-			putPooledDataSourceCreator(new TomcatJdbcPooledDataSourceCreator<SI>());
-			putPooledDataSourceCreator(new HikariCpPooledDataSourceCreator<SI>());
-			putPooledDataSourceCreator(new TomcatDbcpPooledDataSourceCreator<SI>());
-			putPooledDataSourceCreator(new BasicDbcpPooledDataSourceCreator<SI>());
+			putPooledDataSourceCreator(new TomcatJdbcPooledDataSourceCreator<>());
+			putPooledDataSourceCreator(new HikariCpPooledDataSourceCreator<>());
+			putPooledDataSourceCreator(new TomcatDbcpPooledDataSourceCreator<>());
+			putPooledDataSourceCreator(new BasicDbcpPooledDataSourceCreator<>());
 		}
 	}
 
@@ -60,7 +58,7 @@ public abstract class DataSourceCreator<SI extends RelationalServiceInfo> extend
 			}
 			// 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 @@ public abstract class DataSourceCreator<SI extends RelationalServiceInfo> extend
 		if (serviceConnectorConfig != null) {
 			List<String> pooledDataSourceNames = ((DataSourceConfig) serviceConnectorConfig).getPooledDataSourceNames();
 			if (pooledDataSourceNames != null) {
-				List<PooledDataSourceCreator<SI>> filtered = new ArrayList<PooledDataSourceCreator<SI>>();
+				List<PooledDataSourceCreator<SI>> filtered = new ArrayList<>();
 
 				for (String name : pooledDataSourceNames) {
 					for (String key : pooledDataSourceCreators.keySet()) {

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<SI extends RelationalServiceInfo> implements PooledDataSourceCreator<SI> {
 
@@ -41,7 +40,7 @@ public class HikariCpPooledDataSourceCreator<SI extends RelationalServiceInfo> i
 			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;
 		}

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 @@ public class TomcatDbcpPooledDataSourceCreator<SI extends RelationalServiceInfo>
 		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);
 		}

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 class TomcatJdbcPooledDataSourceCreator<SI extends RelationalServiceInfo>
 			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;
 		}

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;
+
+/**
+ * <p>
+ * {@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.
+ * </p>
+ *
+ * <p>
+ * 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.
+ * </p>
+ *
+ * @author Gareth Clay
+ */
+class UrlDecodingDataSource extends DelegatingDataSource {
+
+	private static final Logger logger = Logger.getLogger(DelegatingDataSource.class.getName());
+
+	private final String urlPropertyName;
+	private final Function<String, DataSource> 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<String, DataSource> 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;
+		}
+	}
+}

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 @@ import org.springframework.cloud.ReflectionUtils;
 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));
+	}
 }

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.cloud.service.relational.TomcatJdbcPooledDataSourceCr
 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 abstract class DataSourceXmlConfigTest extends AbstractServiceXmlConfigTe
 				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 abstract class DataSourceXmlConfigTest extends AbstractServiceXmlConfigTe
 				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 abstract class DataSourceXmlConfigTest extends AbstractServiceXmlConfigTe
 				createService("my-service"));
 
 		DataSource ds = testContext.getBean("db-pool-invalid", getConnectorType());
-		assertThat(ds, instanceOf(SimpleDriverDataSource.class));
+		assertDataSourceType(ds, SimpleDriverDataSource.class);
 	}
 }

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 abstract class AbstractDataSourceCreatorTest<C extends DataSourceCreator<
 	}
 	
 	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"));
 

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 class PooledDataSourceCreatorsTest {
 	@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 @@ public class PooledDataSourceCreatorsTest {
 	}
 
 	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 @@ public class PooledDataSourceCreatorsTest {
 	}
 
 	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 @@ public class PooledDataSourceCreatorsTest {
 	}
 
 	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 @@ public class PooledDataSourceCreatorsTest {
 	}
 
 	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"));

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<String, String, Boolean> 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);
+		}
+	}
+}