Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, RepositoryFactoryInformation.class, false, false) doesn't find beans that are found in 4.1 [SPR-12846] #17444

Closed
spring-issuemaster opened this issue Mar 24, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 24, 2015

Andy Wilkinson opened SPR-12846 and commented

A Spring Data JPA-based test is failing in Spring Boot when I upgrade to Spring Framework 4.2. The failure occurs because Spring Data's Repositories class fails to find the JpaRepositoryFactoryBean in the application context. It looks for it by calling BeanFactoryUtils.beanNamesForTypeIncludingAncestors(factory, RepositoryFactoryInformation.class, false, false)). RepositoryFactoryInformation is an interface that is implemented by JpaRepositoryFactoryInformation.

The problem looks like it may be due to the use of ResolvableType in 4.2. When run against Spring Framework 4.1.x, it boils down to AbstractBeanFactory.isTypeMatch calling org.springframework.data.repository.core.support.RepositoryFactoryInformation.class.isAssignableFrom(org.springframework.data.jpa.repository.support.JpaRepositoryFactoryBean.class). When run against Spring Framework 4.2 it's the ResolvableType for RepositoryFactoryInformation that's used. In 4.1.x the isAssignableFrom call returns true, in 4.2 it returns false. This is summarised in the following test:

assertTrue(RepositoryFactoryInformation.class
        .isAssignableFrom(JpaRepositoryFactoryBean.class));
assertTrue(ResolvableType.forClass(RepositoryFactoryInformation.class)
        .isAssignableFrom(JpaRepositoryFactoryBean.class));

It fails on the second assertion.


Affects: 4.2 RC1

Referenced from: commits 09027f7

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2015

Juergen Hoeller commented

Thanks for setting up a JIRA issue for this, Andy.

Ollie raised a related test failure in the Spring Data tests end of last week but your report here nails it: For some reason, ResolvableType.isAssignableFrom doesn't reliably behave like Class.isAssignableFrom even if just created as a raw Class wrapper.

I couldn't reproduce this in BeanFactory-based tests with various FactoryBean resolution scenarios yet but will now focus on ResolvableType-based unit tests instead! I'm pretty sure it's the rather wild use of generics in RepositoryFactoryInformation...

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2015

Juergen Hoeller commented

Hmm, it seems to be a misconception on my side that ResolvableType.isAssignableFrom can be a replacement for Class.isAssignableFrom to begin with. The latter always does raw assignability checks, after all, whereas ResolvableType.forClass fully introspects generic information within the given Class. Clearly we don't want that sort of change for getBeanNamesForType(Class) and isTypeMatch(name, Class), so I'll replace this with some immediate Class.isAssignableFrom wrapper instead. In the worst case, we could always duplicate the algorithm, keeping them separate between isTypeMatch(name, ResolvableType) and isTypeMatch(name, Class). It was just a code reuse exercise, after all.

I'll have this fixed by tonight.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2015

Juergen Hoeller commented

A unit test modeled along the lines of Spring Data's specific type variable arrangement, including the hierarchy for the repository factory bean impls, finally reproduces this in the core test suite.

Fixed now through using the new ResolvableType.forRawClass variant, guaranteed to be a straight wrapper for Class.isAssignableFrom. We do not want to mess with the semantics of Class-based assignability, after all; the sole goal is to reuse AbstractBeanFactory's type matching algorithm with ResolvableType.isAssignableFrom as common delegate.

To be pushed to master within a few minutes.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 25, 2015

Andy Wilkinson commented

Thanks for the quick turnaround, Juergen. The affected Spring Boot tests are now passing against the latest 4.2 snapshots.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 25, 2015

Juergen Hoeller commented

Great to hear, Andy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.