Skip to content

Fix: type matching for request-scope generic beans #34537

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

import org.springframework.beans.BeanUtils;
Expand Down Expand Up @@ -583,6 +584,27 @@ else if (typeToMatch.hasGenerics() && containsBeanDefinition(beanName)) {
// Generics potentially only match on the target class, not on the proxy...
RootBeanDefinition mbd = getMergedLocalBeanDefinition(beanName);
Class<?> targetType = mbd.getTargetType();

String scope = mbd.getScope();
if (targetType == null && scope != null && !scope.isEmpty()) {
String targetBeanName = "scopedTarget." + beanName;
if (containsBeanDefinition(targetBeanName)) {
RootBeanDefinition targetMbd = getMergedLocalBeanDefinition(targetBeanName);

ResolvableType targetResolvableType = targetMbd.targetType;
if (targetResolvableType == null) {
targetResolvableType = targetMbd.factoryMethodReturnType;
if (targetResolvableType == null) {
targetResolvableType = ResolvableType.forClass(targetMbd.getBeanClass());
}
}

if (typeToMatch.isAssignableFrom(targetResolvableType)) {
Comment on lines +594 to +602
Copy link

@Pankraz76 Pankraz76 Mar 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering

https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/ObjectUtils.html#defaultIfNull(T,T)

#34587

we should be able to reduce all redundancy by using the internal ternary operator hidden inside ObjectUtils.defaultIfNull

Suggested change
ResolvableType targetResolvableType = targetMbd.targetType;
if (targetResolvableType == null) {
targetResolvableType = targetMbd.factoryMethodReturnType;
if (targetResolvableType == null) {
targetResolvableType = ResolvableType.forClass(targetMbd.getBeanClass());
}
}
if (typeToMatch.isAssignableFrom(targetResolvableType)) {
if (typeToMatch.isAssignableFrom(ObjectUtils.getIfNull(targetResolvableType, ObjectUtils.getIfNull(targetMbd.factoryMethodReturnType, ResolvableType.forClass(targetMbd.getBeanClass()))))) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but of course this can be done afterward or not at all, as its working an acceptable anyways.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion and for pointing me to the ObjectUtils.defaultIfNull from Apache Commons Lang!

I initially wasn't able to use any defaultIfNull method because I was working with org.springframework.util.ObjectUtils, which doesn't have this method in our current version.

Your approach using nested ternary operators looks more concise and elegant. I agree this would be a great improvement, but I think we should leave it for a separate refactoring PR in the future. This way, we can keep the current PR focused on fixing the original issue with clean, minimal changes.

Thanks again for your approval and the valuable feedback throughout this PR!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I just noticed you've added the getIfNull method to ObjectUtils.java - this is fantastic!

Could you guide me on how I can best incorporate this change into my implementation? Should I update my PR to use this new method with the nested ternary approach you suggested, or would you prefer to handle that separately after merging?

I'm excited to use this more elegant solution if that's the right approach at this stage.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest merging it as it is and incorporating the refactoring into the Util PR (#34587) so we don’t add empty code and can deliver a practical example right away.

If it’s merged before, then you can, of course, try it here. I would not suggest mixing the PRs now.

Thanks for your heartwarming feedback—teamwork makes the dream work!

return true;
}
}
}

if (targetType != null && targetType != ClassUtils.getUserClass(beanInstance)) {
// Check raw class match as well, making sure it's exposed on the proxy.
Class<?> classToMatch = typeToMatch.resolve();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.springframework.beans.factory.support;

import org.junit.jupiter.api.Test;
import org.springframework.core.ResolvableType;

import java.util.UUID;
import java.util.function.Supplier;

import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests for {@link AbstractBeanFactory#isTypeMatch} with scoped proxy beans that use generic types.
*/
class ScopedProxyGenericTypeMatchTests {

@Test
void scopedProxyBeanTypeMatching() {
DefaultListableBeanFactory factory = new DefaultListableBeanFactory();

String proxyBeanName = "wordBean-" + UUID.randomUUID();
String targetBeanName = "scopedTarget." + proxyBeanName;

RootBeanDefinition targetDef = new RootBeanDefinition(SomeGenericSupplier.class);
targetDef.setScope("request");
factory.registerBeanDefinition(targetBeanName, targetDef);

RootBeanDefinition proxyDef = new RootBeanDefinition();
proxyDef.setScope("singleton");
proxyDef.setTargetType(ResolvableType.forClassWithGenerics(Supplier.class, String.class));
proxyDef.setAttribute("targetBeanName", targetBeanName);
factory.registerBeanDefinition(proxyBeanName, proxyDef);

ResolvableType supplierType = ResolvableType.forClassWithGenerics(Supplier.class, String.class);

assertThat(factory.isTypeMatch(proxyBeanName, supplierType)).isTrue();

assertThat(factory.getBeanNamesForType(supplierType)).contains(proxyBeanName);
}

static class SomeGenericSupplier implements Supplier<String> {
@Override
public String get() {
return "value";
}
}
}