Skip to content

Commit

Permalink
Merge pull request #239 from spikymonkey/url_decoding_data_source
Browse files Browse the repository at this point in the history
Handle drivers which don't support URL-encoding
  • Loading branch information
scottfrederick authored Nov 7, 2018
2 parents dbf8dfa + da84aa7 commit 8b0a2f6
Show file tree
Hide file tree
Showing 11 changed files with 410 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

/**
*
Expand All @@ -32,18 +30,18 @@ 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;
this.driverClasses = driverClasses;
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<>());
}
}

Expand All @@ -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);
Expand All @@ -84,7 +82,7 @@ private Collection<PooledDataSourceCreator<SI>> filterPooledDataSourceCreators(S
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()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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> {

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
@@ -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;

/**
*
Expand All @@ -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));
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

/**
*
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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);
}
}
Loading

0 comments on commit 8b0a2f6

Please sign in to comment.