Skip to content

Commit

Permalink
Polish contribution
Browse files Browse the repository at this point in the history
  • Loading branch information
sbrannen committed Sep 26, 2023
1 parent 6d2d8a3 commit d50ec68
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 70 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -61,7 +61,7 @@ public abstract class AbstractRoutingDataSource extends AbstractDataSource imple

/**
* Specify the map of target DataSources, with the lookup key as key.
* The mapped value can either be a corresponding {@link javax.sql.DataSource}
* <p>The mapped value can either be a corresponding {@link javax.sql.DataSource}
* instance or a data source name String (to be resolved via a
* {@link #setDataSourceLookup DataSourceLookup}).
* <p>The key can be of arbitrary type; this class implements the
Expand Down Expand Up @@ -114,15 +114,23 @@ public void setDataSourceLookup(@Nullable DataSourceLookup dataSourceLookup) {
}


/**
* Delegates to {@link #initialize()}.
*/
@Override
public void afterPropertiesSet() {
initialize();
}

/**
* Synchronizes targetDataSources to resolvedDataSources
* and defaultTargetDataSource to resolvedDefaultDataSource.
* @throws IllegalArgumentException in case of targetDataSources is null
* Initialize the internal state of this {@code AbstractRoutingDataSource}
* by resolving the configured target DataSources.
* @throws IllegalArgumentException if the target DataSources have not been configured
* @since 6.1
* @see #setTargetDataSources(Map)
* @see #setDefaultTargetDataSource(Object)
* @see #getResolvedDataSources()
* @see #getResolvedDefaultDataSource()
*/
public void initialize() {
if (this.targetDataSources == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2023 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.
Expand All @@ -17,7 +17,6 @@
package org.springframework.jdbc.datasource.lookup;


import java.util.HashMap;
import java.util.Map;

import javax.sql.DataSource;
Expand All @@ -28,7 +27,6 @@
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;


/**
* Tests for {@link AbstractRoutingDataSource}.
*
Expand All @@ -38,7 +36,7 @@ class AbstractRoutingDataSourceTests {

@Test
void setTargetDataSources() {
final ThreadLocal<String> lookupKey = new ThreadLocal<>();
ThreadLocal<String> lookupKey = new ThreadLocal<>();
AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
@Override
protected Object determineCurrentLookupKey() {
Expand All @@ -50,14 +48,11 @@ protected Object determineCurrentLookupKey() {

MapDataSourceLookup dataSourceLookup = new MapDataSourceLookup();
dataSourceLookup.addDataSource("dataSource2", ds2);
routingDataSource.setDataSourceLookup(dataSourceLookup);

Map<Object, Object> targetDataSources = new HashMap<>();
targetDataSources.put("ds1", ds1);
targetDataSources.put("ds2", "dataSource2");
routingDataSource.setTargetDataSources(targetDataSources);

routingDataSource.setDataSourceLookup(dataSourceLookup);
routingDataSource.setTargetDataSources(Map.of("ds1", ds1, "ds2", "dataSource2"));
routingDataSource.afterPropertiesSet();

lookupKey.set("ds1");
assertThat(routingDataSource.determineTargetDataSource()).isSameAs(ds1);
lookupKey.set("ds2");
Expand All @@ -84,25 +79,23 @@ protected Object determineCurrentLookupKey() {
return null;
}
};
Map<Object, Object> targetDataSources = new HashMap<>();
targetDataSources.put("ds1", 1);
routingDataSource.setTargetDataSources(targetDataSources);
routingDataSource.setTargetDataSources(Map.of("ds1", 1));
assertThatIllegalArgumentException().isThrownBy(routingDataSource::afterPropertiesSet)
.withMessage("Illegal data source value - only [javax.sql.DataSource] and String supported: 1");
}


@Test
void setDefaultTargetDataSource() {
final ThreadLocal<String> lookupKey = new ThreadLocal<>();
ThreadLocal<String> lookupKey = new ThreadLocal<>();
AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
@Override
protected Object determineCurrentLookupKey() {
return lookupKey.get();
}
};
DataSource ds = new StubDataSource();
routingDataSource.setTargetDataSources(new HashMap<>());
routingDataSource.setTargetDataSources(Map.of());
routingDataSource.setDefaultTargetDataSource(ds);
routingDataSource.afterPropertiesSet();
lookupKey.set("foo");
Expand All @@ -111,15 +104,15 @@ protected Object determineCurrentLookupKey() {

@Test
void setDefaultTargetDataSourceFallbackIsFalse() {
final ThreadLocal<String> lookupKey = new ThreadLocal<>();
ThreadLocal<String> lookupKey = new ThreadLocal<>();
AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
@Override
protected Object determineCurrentLookupKey() {
return lookupKey.get();
}
};
DataSource ds = new StubDataSource();
routingDataSource.setTargetDataSources(new HashMap<>());
routingDataSource.setTargetDataSources(Map.of());
routingDataSource.setDefaultTargetDataSource(ds);
routingDataSource.setLenientFallback(false);
routingDataSource.afterPropertiesSet();
Expand All @@ -130,15 +123,15 @@ protected Object determineCurrentLookupKey() {

@Test
void setDefaultTargetDataSourceLookupKeyIsNullWhenFallbackIsFalse() {
final ThreadLocal<String> lookupKey = new ThreadLocal<>();
ThreadLocal<String> lookupKey = new ThreadLocal<>();
AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
@Override
protected Object determineCurrentLookupKey() {
return lookupKey.get();
}
};
DataSource ds = new StubDataSource();
routingDataSource.setTargetDataSources(new HashMap<>());
routingDataSource.setTargetDataSources(Map.of());
routingDataSource.setDefaultTargetDataSource(ds);
routingDataSource.setLenientFallback(false);
routingDataSource.afterPropertiesSet();
Expand All @@ -147,7 +140,7 @@ protected Object determineCurrentLookupKey() {
}

@Test
void testInitialize_synchronizeTargetDataSourcesToResolvedDataSources() {
void initializeSynchronizesTargetDataSourcesToResolvedDataSources() {
AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
@Override
protected Object determineCurrentLookupKey() {
Expand All @@ -158,11 +151,7 @@ protected Object determineCurrentLookupKey() {
DataSource ds1 = new StubDataSource();
DataSource ds2 = new StubDataSource();

Map<Object, Object> targetDataSources = new HashMap<>();
targetDataSources.put("ds1", ds1);
targetDataSources.put("ds2", ds2);
routingDataSource.setTargetDataSources(targetDataSources);

routingDataSource.setTargetDataSources(Map.of("ds1", ds1, "ds2", ds2));
routingDataSource.initialize();

Map<Object, DataSource> resolvedDataSources = routingDataSource.getResolvedDataSources();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@
/**
* Abstract {@link ConnectionFactory} implementation that routes
* {@link #create()} calls to one of various target
* {@link ConnectionFactory factories} based on a lookup key.
* {@linkplain ConnectionFactory factories} based on a lookup key.
* The latter is typically (but not necessarily) determined from some
* subscriber context.
*
* <p> Allows to configure a {@link #setDefaultTargetConnectionFactory(Object)
* default ConnectionFactory} as fallback.
* <p>Allows to configure a default target {@link #setDefaultTargetConnectionFactory(Object)
* ConnectionFactory} as a fallback.
*
* <p> Calls to {@link #getMetadata()} are routed to the
* <p>Calls to {@link #getMetadata()} are routed to the
* {@link #setDefaultTargetConnectionFactory(Object) default ConnectionFactory}
* if configured.
*
Expand Down Expand Up @@ -125,14 +125,22 @@ public void setConnectionFactoryLookup(ConnectionFactoryLookup connectionFactory
}


/**
* Delegates to {@link #initialize()}.
*/
@Override
public void afterPropertiesSet() {
initialize();
}

/**
* Synchronizes targetConnectionFactories to resolvedConnectionFactories
* and defaultTargetConnectionFactory to resolvedDefaultConnectionFactory.
* Initialize the internal state of this {@code AbstractRoutingConnectionFactory}
* by resolving the configured target ConnectionFactories.
* @throws IllegalArgumentException if the target ConnectionFactories have not
* been configured
* @since 6.1
* @see #setTargetConnectionFactories(Map)
* @see #setDefaultTargetConnectionFactory(Object)
*/
public void initialize() {
Assert.notNull(this.targetConnectionFactories, "Property 'targetConnectionFactories' must not be null");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.r2dbc.connection.lookup;

import java.util.Map;

import io.r2dbc.spi.ConnectionFactory;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -26,7 +28,7 @@
import reactor.test.StepVerifier;
import reactor.util.context.Context;

import static java.util.Collections.singletonMap;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

/**
Expand All @@ -36,30 +38,28 @@
* @author Jens Schauder
*/
@ExtendWith(MockitoExtension.class)
public class AbstractRoutingConnectionFactoryUnitTests {
class AbstractRoutingConnectionFactoryUnitTests {

private static final String ROUTING_KEY = "routingKey";

final DummyRoutingConnectionFactory connectionFactory = new DummyRoutingConnectionFactory();

@Mock
ConnectionFactory defaultConnectionFactory;

@Mock
ConnectionFactory routedConnectionFactory;

DummyRoutingConnectionFactory connectionFactory;


@BeforeEach
public void before() {
connectionFactory = new DummyRoutingConnectionFactory();
void before() {
connectionFactory.setDefaultTargetConnectionFactory(defaultConnectionFactory);
}


@Test
public void shouldDetermineRoutedFactory() {
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
void shouldDetermineRoutedFactory() {
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.setConnectionFactoryLookup(new MapConnectionFactoryLookup());
connectionFactory.afterPropertiesSet();

Expand All @@ -71,9 +71,8 @@ public void shouldDetermineRoutedFactory() {
}

@Test
public void shouldFallbackToDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
void shouldFallbackToDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.afterPropertiesSet();

connectionFactory.determineTargetConnectionFactory()
Expand All @@ -83,29 +82,27 @@ public void shouldFallbackToDefaultConnectionFactory() {
}

@Test
public void initializationShouldFailUnsupportedLookupKey() {
connectionFactory.setTargetConnectionFactories(singletonMap("key", new Object()));
void initializationShouldFailUnsupportedLookupKey() {
connectionFactory.setTargetConnectionFactories(Map.of("key", new Object()));

assertThatThrownBy(() -> connectionFactory.afterPropertiesSet())
.isInstanceOf(IllegalArgumentException.class);
assertThatIllegalArgumentException().isThrownBy(connectionFactory::initialize);
}

@Test
public void initializationShouldFailUnresolvableKey() {
connectionFactory.setTargetConnectionFactories(singletonMap("key", "value"));
void initializationShouldFailUnresolvableKey() {
connectionFactory.setTargetConnectionFactories(Map.of("key", "value"));
connectionFactory.setConnectionFactoryLookup(new MapConnectionFactoryLookup());

assertThatThrownBy(() -> connectionFactory.afterPropertiesSet())
assertThatThrownBy(connectionFactory::initialize)
.isInstanceOf(ConnectionFactoryLookupFailureException.class)
.hasMessageContaining("No ConnectionFactory with name 'value' registered");
}

@Test
public void unresolvableConnectionFactoryRetrievalShouldFail() {
void unresolvableConnectionFactoryRetrievalShouldFail() {
connectionFactory.setLenientFallback(false);
connectionFactory.setConnectionFactoryLookup(new MapConnectionFactoryLookup());
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.afterPropertiesSet();

connectionFactory.determineTargetConnectionFactory()
Expand All @@ -115,9 +112,8 @@ public void unresolvableConnectionFactoryRetrievalShouldFail() {
}

@Test
public void connectionFactoryRetrievalWithUnknownLookupKeyShouldReturnDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
void connectionFactoryRetrievalWithUnknownLookupKeyShouldReturnDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.setDefaultTargetConnectionFactory(defaultConnectionFactory);
connectionFactory.afterPropertiesSet();

Expand All @@ -129,9 +125,8 @@ public void connectionFactoryRetrievalWithUnknownLookupKeyShouldReturnDefaultCon
}

@Test
public void connectionFactoryRetrievalWithoutLookupKeyShouldReturnDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
void connectionFactoryRetrievalWithoutLookupKeyShouldReturnDefaultConnectionFactory() {
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.setDefaultTargetConnectionFactory(defaultConnectionFactory);
connectionFactory.setLenientFallback(false);
connectionFactory.afterPropertiesSet();
Expand All @@ -143,12 +138,12 @@ public void connectionFactoryRetrievalWithoutLookupKeyShouldReturnDefaultConnect
}

@Test
public void shouldLookupFromMap() {
void shouldLookupFromMap() {
MapConnectionFactoryLookup lookup =
new MapConnectionFactoryLookup("lookup-key", routedConnectionFactory);

connectionFactory.setConnectionFactoryLookup(lookup);
connectionFactory.setTargetConnectionFactories(singletonMap("my-key", "lookup-key"));
connectionFactory.setTargetConnectionFactories(Map.of("my-key", "lookup-key"));
connectionFactory.afterPropertiesSet();

connectionFactory.determineTargetConnectionFactory()
Expand All @@ -159,7 +154,7 @@ public void shouldLookupFromMap() {
}

@Test
public void shouldAllowModificationsAfterInitialization() {
void shouldAllowModificationsAfterInitialization() {
MapConnectionFactoryLookup lookup = new MapConnectionFactoryLookup();

connectionFactory.setConnectionFactoryLookup(lookup);
Expand All @@ -183,9 +178,8 @@ public void shouldAllowModificationsAfterInitialization() {
}

@Test
void testInitialize_shouldDetermineRoutedFactory() {
connectionFactory.setTargetConnectionFactories(
singletonMap("key", routedConnectionFactory));
void initializeShouldDetermineRoutedFactory() {
connectionFactory.setTargetConnectionFactories(Map.of("key", routedConnectionFactory));
connectionFactory.setConnectionFactoryLookup(new MapConnectionFactoryLookup());
connectionFactory.initialize();

Expand Down

0 comments on commit d50ec68

Please sign in to comment.