Skip to content

Commit

Permalink
DATAJDBC-293 - Applying feedback from review.
Browse files Browse the repository at this point in the history 
The logic is now split in two parts:
If a bean name is given, this is configured on the BeanDefinition level in the JdbcRepositoryConfigExtension.
If no name is give a bean is looked up by type in the JdbcRepositoryFactoryBean.

This makes the code even simpler and uses standard Spring features instead of reimplementing them.
  • Loading branch information
@schauder
schauder committed Feb 6, 2019
1 parent 9224b5a commit f099967
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 76 deletions.
24 changes: 7 additions & 17 deletions ...n/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java
View file
Expand Up @@ -15,27 +15,14 @@
*/
package org.springframework.data.jdbc.repository.config;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.function.Supplier;

import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
import org.springframework.data.jdbc.core.DataAccessStrategy;
import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactoryBean;
import org.springframework.data.repository.config.RepositoryConfigurationExtensionSupport;
import org.springframework.data.repository.config.RepositoryConfigurationSource;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -94,10 +81,13 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf
@Override
public void postProcess(BeanDefinitionBuilder builder, RepositoryConfigurationSource source) {

builder.addPropertyValue("jdbcOperationsRef", source.getAttribute("jdbcOperationsRef").orElse(null));
builder.addPropertyValue("dataAccessStrategyRef", source.getAttribute("dataAccessStrategyRef").orElse(null));
}

source.getAttribute("jdbcOperationsRef") //
.filter(s -> !StringUtils.isEmpty(s)) //
.ifPresent(s -> builder.addPropertyReference("jdbcOperations", s));

source.getAttribute("dataAccessStrategyRef") //
.filter(s -> !StringUtils.isEmpty(s)) //
.ifPresent(s -> builder.addPropertyReference("dataAccessStrategy", s));
}

}
64 changes: 7 additions & 57 deletions ...main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java
View file
Expand Up @@ -16,12 +16,9 @@
package org.springframework.data.jdbc.repository.support;

import java.io.Serializable;
import java.util.Map;

import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.ApplicationEventPublisherAware;
import org.springframework.data.jdbc.core.DataAccessStrategy;
Expand All @@ -35,9 +32,7 @@
import org.springframework.data.repository.core.support.RepositoryFactorySupport;
import org.springframework.data.repository.core.support.TransactionalRepositoryFactoryBeanSupport;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;

/**
* Special adapter for Springs {@link org.springframework.beans.factory.FactoryBean} interface to allow easy setup of
Expand All @@ -53,14 +48,12 @@ public class JdbcRepositoryFactoryBean<T extends Repository<S, ID>, S, ID extend
extends TransactionalRepositoryFactoryBeanSupport<T, S, ID> implements ApplicationEventPublisherAware {

private ApplicationEventPublisher publisher;
private ConfigurableListableBeanFactory beanFactory;
private BeanFactory beanFactory;
private RelationalMappingContext mappingContext;
private RelationalConverter converter;
private DataAccessStrategy dataAccessStrategy;
private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY;
private NamedParameterJdbcOperations operations;
private String dataAccessStrategyRef;
private String jdbcOperationsRef;

/**
* Creates a new {@link JdbcRepositoryFactoryBean} for the given repository interface.
Expand Down Expand Up @@ -110,10 +103,6 @@ public void setDataAccessStrategy(DataAccessStrategy dataAccessStrategy) {
this.dataAccessStrategy = dataAccessStrategy;
}

public void setDataAccessStrategyRef(String dataAccessStrategyRef) {
this.dataAccessStrategyRef = dataAccessStrategyRef;
}

/**
* @param queryMappingConfiguration can be {@literal null}. {@link #afterPropertiesSet()} defaults to
* {@link QueryMappingConfiguration#EMPTY} if {@literal null}.
Expand All @@ -126,7 +115,6 @@ public void setQueryMappingConfiguration(QueryMappingConfiguration queryMappingC
/**
* @param rowMapperMap can be {@literal null}. {@link #afterPropertiesSet()} defaults to {@link RowMapperMap#EMPTY} if
* {@literal null}.
*
* @deprecated use {@link #setQueryMappingConfiguration(QueryMappingConfiguration)} instead.
*/
@Deprecated
Expand All @@ -139,10 +127,6 @@ public void setJdbcOperations(NamedParameterJdbcOperations operations) {
this.operations = operations;
}

public void setJdbcOperationsRef(String jdbcOperationsRef) {
this.jdbcOperationsRef = jdbcOperationsRef;
}

@Autowired
public void setConverter(RelationalConverter converter) {
this.converter = converter;
Expand All @@ -153,7 +137,7 @@ public void setBeanFactory(BeanFactory beanFactory) {

super.setBeanFactory(beanFactory);

this.beanFactory = (ConfigurableListableBeanFactory) beanFactory;
this.beanFactory = beanFactory;
}

/*
Expand Down Expand Up @@ -182,8 +166,7 @@ private void ensureJdbcOperationsIsInitialized() {
return;
}

operations = findBeanByNameOrType(NamedParameterJdbcOperations.class, jdbcOperationsRef);

operations = beanFactory.getBean(NamedParameterJdbcOperations.class);
}

private void ensureDataAccessStrategyIsInitialized() {
Expand All @@ -192,44 +175,11 @@ private void ensureDataAccessStrategyIsInitialized() {
return;
}

dataAccessStrategy = findBeanByNameOrType(DataAccessStrategy.class, dataAccessStrategyRef);
dataAccessStrategy = beanFactory.getBeanProvider(DataAccessStrategy.class).getIfAvailable(() -> {

if (dataAccessStrategy != null) {
return;
}

SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext);
this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter,
operations);
SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext);
return new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter, operations);
});
}

@Nullable
private <B> B findBeanByNameOrType(Class<B> beanType, String beanName) {

if (beanFactory == null) {
return null;
}

if (!StringUtils.isEmpty(beanName)) {
return beanFactory.getBean(beanName, beanType);
}

Map<String, B> beansOfType = beanFactory.getBeansOfType(beanType);

if (beansOfType.size() == 0) {
return null;
}

if (beansOfType.size() == 1) {
return beansOfType.values().iterator().next();
}

for (Map.Entry<String, B> entry : beansOfType.entrySet()) {
if (beanFactory.getBeanDefinition(entry.getKey()).isPrimary()) {
return entry.getValue();
}
}

throw new NoUniqueBeanDefinitionException(beanType, beansOfType.keySet());
}
}
20 changes: 18 additions & 2 deletions .../org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java
View file
Expand Up @@ -21,9 +21,13 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.data.annotation.Id;
import org.springframework.data.jdbc.core.DataAccessStrategy;
Expand All @@ -33,8 +37,11 @@
import org.springframework.data.relational.core.conversion.BasicRelationalConverter;
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
import org.springframework.data.repository.CrudRepository;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
import org.springframework.test.util.ReflectionTestUtils;

import java.util.function.Supplier;

/**
* Tests the dependency injection for {@link JdbcRepositoryFactoryBean}.
*
Expand All @@ -44,7 +51,6 @@
* @author Oliver Gierke
* @author Mark Paluch
* @author Evgeni Dimitrov
*
*/
@RunWith(MockitoJUnitRunner.class)
public class JdbcRepositoryFactoryBeanUnitTests {
Expand All @@ -53,6 +59,7 @@ public class JdbcRepositoryFactoryBeanUnitTests {

@Mock DataAccessStrategy dataAccessStrategy;
@Mock ApplicationEventPublisher publisher;
@Mock(answer = Answers.RETURNS_DEEP_STUBS) ListableBeanFactory beanFactory;

RelationalMappingContext mappingContext;

Expand All @@ -63,6 +70,11 @@ public void setUp() {

// Setup standard configuration
factoryBean = new JdbcRepositoryFactoryBean<>(DummyEntityRepository.class);


when(beanFactory.getBean(NamedParameterJdbcOperations.class)).thenReturn(mock(NamedParameterJdbcOperations.class));
when(beanFactory.getBeanProvider(DataAccessStrategy.class).getIfAvailable(any()))
.then((Answer) invocation -> ((Supplier)invocation.getArgument(0)).get());
}

@Test
Expand All @@ -72,6 +84,7 @@ public void setsUpBasicInstanceCorrectly() {
factoryBean.setMappingContext(mappingContext);
factoryBean.setConverter(new BasicRelationalConverter(mappingContext));
factoryBean.setApplicationEventPublisher(publisher);
factoryBean.setBeanFactory(beanFactory);
factoryBean.afterPropertiesSet();

assertThat(factoryBean.getObject()).isNotNull();
Expand All @@ -88,6 +101,7 @@ public void afterPropertiesThowsExceptionWhenNoMappingContextSet() {

factoryBean.setMappingContext(null);
factoryBean.setApplicationEventPublisher(publisher);
factoryBean.setBeanFactory(beanFactory);
factoryBean.afterPropertiesSet();
}

Expand All @@ -97,12 +111,14 @@ public void afterPropertiesSetDefaultsNullablePropertiesCorrectly() {
factoryBean.setMappingContext(mappingContext);
factoryBean.setConverter(new BasicRelationalConverter(mappingContext));
factoryBean.setApplicationEventPublisher(publisher);
factoryBean.setBeanFactory(beanFactory);
factoryBean.afterPropertiesSet();

assertThat(factoryBean.getObject()).isNotNull();
assertThat(ReflectionTestUtils.getField(factoryBean, "dataAccessStrategy"))
.isInstanceOf(DefaultDataAccessStrategy.class);
assertThat(ReflectionTestUtils.getField(factoryBean, "queryMappingConfiguration")).isEqualTo(QueryMappingConfiguration.EMPTY);
assertThat(ReflectionTestUtils.getField(factoryBean, "queryMappingConfiguration"))
.isEqualTo(QueryMappingConfiguration.EMPTY);
}

private static class DummyEntity {
Expand Down

0 comments on commit f099967

Please sign in to comment.