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

Inconsistent getTypeForFactoryMethod results for parameterized factory method [SPR-16720] #21261

Closed
spring-projects-issues opened this issue Apr 12, 2018 · 1 comment
Assignees
Labels
in: core status: backported type: regression
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Apr 12, 2018

Sandeep Donthula opened SPR-16720 and commented

Hi,

We were trying to upgrade Spring 4.2 to 4.3 version. And facing below issue while beans getting created.

Class:
@Component
@RequiredArgsConstructor(onConstructor = @__(@Autowired))
public class ClientFactory {
public <T extends Client> T getRetryableServiceClient(final Class<T> clazz, final String qualifier) {
}
}

package com.coral.client;
public interface Client {
}

public class AdaptorFactory {
@Autowired
private NodeExecutionServiceClient nodeExecutionServiceClient;
}

Bean

<bean id="nodeExecutionServiceClient" factory-bean="clientFactory"
      factory-method="getRetryableServiceClient">
    <constructor-arg index="0" value="com.nodeexecutionservice.NodeExecutionServiceClient" />
    <constructor-arg index="1" value="${NodeExecutionService.qualifier}"/>
</bean>

Bean creation of this was failing with error

Exiting with throwable: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'adaptorFactory': Unsatisfied dependency expressed through field 'nodeExecutionServiceClient'; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'com.nodeexecutionservice.NodeExecutionServiceClient' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {@org.springframework.beans.factory.annotation.Autowired(required=true)}

I debugged into source code of spring and found issue

Referring to this particular commit : b9c4f1f#diff-cc936e219ae3ea6fcf88f74629262bbd

getTypeForFactoryMethod

commonType comes as - com.nodeexecutionservice.NodeExecutionServiceClient
uniqueCandidate comes as - com.coral.client.Client

if (commonType != null) {
     // Clear return type found: all factory methods return same type.
     mbd.factoryMethodReturnType = (uniqueCandidate != null ?
               ResolvableType.forMethodReturnType(uniqueCandidate) : ResolvableType.forClass(commonType));
}
return commonType;

from the above code - ResolvableType of 'commonType' returned. But ResolvableType of 'uniqueCandidate' got saved in cache. My Generic question is ideally whatever we are returning, we should save that in cache as we are referring cache in the method call at the starting.

In my case while resolving 'uniqueCandidate', '?' got saved in cache. It returned NodeExecutionServiceClient for the first call but for other subsequent calls, it's returning '?' which is causing issue.

Why this is written like, return one resolvable type but same something else in cache which is ideally wrong ? And when 'commonType' is getting resolved correctly, rather than using why 'uniqueCandidate' is getting saved in cache ?


Affects: 4.3.14

Issue Links:

  • #19578 getBeanNamesForType(ResolvableType) doesn't match generic factory method return type for yet-to-be-created bean

Referenced from: commits 4763154, c6a7732, 6184c4e

Backported to: 4.3.17

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 12, 2018

Juergen Hoeller commented

This turned out to be a rather specific gap where we used a non-resolved factory method declaration instead of the pre-resolved type of that unique method which happened to be stored in commonType. I've revised the algorithm to always go with the resolved type for a parameterized type, and refactored the end of it to enforce resolution of the to-be-cached ResolvableType instead of just assuming it matches the commonType.

As a side note, one reason why we never noticed this is that higher levels of caching often cover it up. Only specific resolution outcomes - or a general temp class loader setup - reveal the mismatch at the actual factoryMethodReturnType cache level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: regression
Projects
None yet
Development

No branches or pull requests

2 participants