DATAJDBC-330 - DI logic now runs after Spring Boot. #115
Conversation
So far the lookup of `NamedParameterJdbcOperations` and `DataAccessStrategy` happend before Spring Boot injected its beans, resulting in failure to consider those beans. This logic is now in the `JdbcRepositoryFactoryBean` which does see the beans created by Spring Boot.
|
|
||
| return nullOrThrowException(required, | ||
| () -> new NoSuchBeanDefinitionException(String.format("No bean of name %s found.", propertyName))); | ||
| builder.addPropertyValue("jdbcOperationsRef", source.getAttribute("jdbcOperationsRef").orElse(null)); |
There was a problem hiding this comment.
Let's keep the notion of conditional configuration here. If a bean name is declared, then we should set and configure a property value. Otherwise, if a default applies, then we don't configure that property and let the factory bean do its default.
There was a problem hiding this comment.
What is the benefit of that? As I see it it would split the logic into two places.
I'm not arguing against, just trying to understand.
There was a problem hiding this comment.
The benefit is that we've got a split of responsibilities. The config extension is responsible for configuring a property if the annotation indicates to do so. If the annotation indicates defaulting, then nothing is configured and the issue is handled in a downstream component.
| private RelationalMappingContext mappingContext; | ||
| private RelationalConverter converter; | ||
| private DataAccessStrategy dataAccessStrategy; | ||
| private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY; | ||
| private NamedParameterJdbcOperations operations; | ||
| private String dataAccessStrategyRef; |
There was a problem hiding this comment.
Let's not use string bean names in here. Rather afterPropertiesSet() should try to find a default bean using the type. Calling BeanFactory.getBean(Class<?>) will take care of @Primary selection and will report the appropriate exception if the bean is not unique.
| @@ -47,11 +53,14 @@ | |||
| extends TransactionalRepositoryFactoryBeanSupport<T, S, ID> implements ApplicationEventPublisherAware { | |||
|
|
|||
| private ApplicationEventPublisher publisher; | |||
| private ConfigurableListableBeanFactory beanFactory; | |||
There was a problem hiding this comment.
BeanFactory should be sufficient.
| @Nullable | ||
| private <B> B findBeanByNameOrType(Class<B> beanType, String beanName) { | ||
|
|
||
| if (beanFactory == null) { |
There was a problem hiding this comment.
This must not happen as the bean is BeanFactoryAware.
| return; | ||
| } | ||
|
|
||
| dataAccessStrategy = findBeanByNameOrType(DataAccessStrategy.class, dataAccessStrategyRef); |
There was a problem hiding this comment.
This method is a good candidate for BeanFactory.getBeanProvider(…).getIfAvailable(Supplier)
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.
|
Just pushed a commit with the requested changes. |
This is broken from JDK9 on for generic types. See also: mockito/mockito#357
mp911de
changed the title
…nsion to JdbcRepositoryFactoryBean. So far the lookup of `NamedParameterJdbcOperations` and `DataAccessStrategy` could happend before these beans were registered resulting in failure to consider those beans. 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. Original pull request: #115.
Replace lambdas with method references. Remove extraneous overrides. Inline methods. Formatting. Original pull request: #115.
|
That's merged and polished now. |
Upgraded Spring Data parent, Commons and Relational modules to Moore M4 versions.
So far the lookup of
NamedParameterJdbcOperationsandDataAccessStrategyhappend before Spring Boot injected its beans, resulting in failure to consider those beans.This logic is now in the
JdbcRepositoryFactoryBeanwhich does see the beans created by Spring Boot.