From cba2b6eaf440b61895d2eea1e008eefe393dc5b3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 3 Aug 2023 18:10:07 +0200 Subject: [PATCH 1/2] Check FactoryBean targetType for generic type as well Closes gh-30987 --- .../AbstractAutowireCapableBeanFactory.java | 5 ++++ .../DefaultListableBeanFactoryTests.java | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 8f03d2f605a..667969ed83d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -908,6 +908,11 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac // static factory method signature or from class inheritance hierarchy... return getTypeForFactoryBeanFromMethod(mbd.getBeanClass(), factoryMethodName); } + + result = getFactoryBeanGeneric(mbd.targetType); + if (result.resolve() != null) { + return result; + } result = getFactoryBeanGeneric(beanType); if (result.resolve() != null) { return result; diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 76962198046..98f69c44d40 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -1980,6 +1980,16 @@ class DefaultListableBeanFactoryTests { assertBeanNamesForType(FactoryBean.class, false, false); } + @Test // gh-30987 + void getBeanNamesForTypeWithFactoryBeanDefinedAsTargetType() { + RootBeanDefinition beanDefinition = new RootBeanDefinition(TestRepositoryFactoryBean.class); + beanDefinition.setTargetType(ResolvableType.forClassWithGenerics(TestRepositoryFactoryBean.class, + CityRepository.class, Object.class, Object.class)); + lbf.registerBeanDefinition("factoryBean", beanDefinition); + assertBeanNamesForType(TestRepositoryFactoryBean.class, true, false, "&factoryBean"); + assertBeanNamesForType(CityRepository.class, true, false, "factoryBean"); + } + /** * Verifies that a dependency on a {@link FactoryBean} can not * be autowired by name, as & is an illegal character in @@ -3068,6 +3078,25 @@ class DefaultListableBeanFactoryTests { } + public static class TestRepositoryFactoryBean, S, ID extends Serializable> + extends RepositoryFactoryBeanSupport { + + @Override + public T getObject() throws Exception { + throw new IllegalArgumentException("Should not be called"); + } + + @Override + public Class getObjectType() { + throw new IllegalArgumentException("Should not be called"); + } + } + + public record City(String name) {} + + public static class CityRepository implements Repository {} + + public static class LazyInitFactory implements FactoryBean { public boolean initialized = false; From 4b6fabbd2fac1cb0a71e4c8bae2e20f6f696854a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 3 Aug 2023 18:10:13 +0200 Subject: [PATCH 2/2] Polishing --- .../beans/TypeConverterDelegate.java | 3 ++- .../factory/support/AbstractBeanFactory.java | 1 - .../annotation/LookupAnnotationTests.java | 10 +------ .../ClassPathXmlApplicationContextTests.java | 5 ++-- .../ConversionServiceFactoryBeanTests.java | 2 +- .../AbstractSockJsIntegrationTests.java | 27 +++++++++++-------- 6 files changed, 22 insertions(+), 26 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java index 1f8fb61cd8b..123bb1d4270 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -166,7 +166,8 @@ class TypeConverterDelegate { } else if (requiredType.isArray()) { // Array required -> apply appropriate conversion of elements. - if (convertedValue instanceof String text && Enum.class.isAssignableFrom(requiredType.getComponentType())) { + if (convertedValue instanceof String text && + Enum.class.isAssignableFrom(requiredType.getComponentType())) { convertedValue = StringUtils.commaDelimitedListToStringArray(text); } return (T) convertToTypedArray(convertedValue, propertyName, requiredType.getComponentType()); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 0d1faf1f559..c0f9bb58689 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -583,7 +583,6 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp Class[] typesToMatch = (FactoryBean.class == classToMatch ? new Class[] {classToMatch} : new Class[] {FactoryBean.class, classToMatch}); - // Attempt to predict the bean type Class predictedType = null; diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/LookupAnnotationTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/LookupAnnotationTests.java index afa61bd9a60..52892821a6d 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/LookupAnnotationTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/LookupAnnotationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,7 +53,6 @@ public class LookupAnnotationTests { @Test public void testWithoutConstructorArg() { AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean"); - assertThat(bean).isNotNull(); Object expected = bean.get(); assertThat(expected.getClass()).isEqualTo(TestBean.class); assertThat(beanFactory.getBean(BeanConsumer.class).abstractBean).isSameAs(bean); @@ -62,7 +61,6 @@ public class LookupAnnotationTests { @Test public void testWithOverloadedArg() { AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean"); - assertThat(bean).isNotNull(); TestBean expected = bean.get("haha"); assertThat(expected.getClass()).isEqualTo(TestBean.class); assertThat(expected.getName()).isEqualTo("haha"); @@ -72,7 +70,6 @@ public class LookupAnnotationTests { @Test public void testWithOneConstructorArg() { AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean"); - assertThat(bean).isNotNull(); TestBean expected = bean.getOneArgument("haha"); assertThat(expected.getClass()).isEqualTo(TestBean.class); assertThat(expected.getName()).isEqualTo("haha"); @@ -82,7 +79,6 @@ public class LookupAnnotationTests { @Test public void testWithTwoConstructorArg() { AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean"); - assertThat(bean).isNotNull(); TestBean expected = bean.getTwoArguments("haha", 72); assertThat(expected.getClass()).isEqualTo(TestBean.class); assertThat(expected.getName()).isEqualTo("haha"); @@ -93,7 +89,6 @@ public class LookupAnnotationTests { @Test public void testWithThreeArgsShouldFail() { AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean"); - assertThat(bean).isNotNull(); assertThatExceptionOfType(AbstractMethodError.class).as("TestBean has no three arg constructor").isThrownBy(() -> bean.getThreeArguments("name", 1, 2)); assertThat(beanFactory.getBean(BeanConsumer.class).abstractBean).isSameAs(bean); @@ -102,7 +97,6 @@ public class LookupAnnotationTests { @Test public void testWithEarlyInjection() { AbstractBean bean = beanFactory.getBean("beanConsumer", BeanConsumer.class).abstractBean; - assertThat(bean).isNotNull(); Object expected = bean.get(); assertThat(expected.getClass()).isEqualTo(TestBean.class); assertThat(beanFactory.getBean(BeanConsumer.class).abstractBean).isSameAs(bean); @@ -115,7 +109,6 @@ public class LookupAnnotationTests { beanFactory.registerBeanDefinition("testBean", tbd); AbstractBean bean = beanFactory.getBean("beanConsumer", BeanConsumer.class).abstractBean; - assertThat(bean).isNotNull(); Object expected = bean.get(); assertThat(expected).isNull(); assertThat(beanFactory.getBean(BeanConsumer.class).abstractBean).isSameAs(bean); @@ -128,7 +121,6 @@ public class LookupAnnotationTests { beanFactory.registerBeanDefinition("floatStore", new RootBeanDefinition(FloatStore.class)); NumberBean bean = (NumberBean) beanFactory.getBean("numberBean"); - assertThat(bean).isNotNull(); assertThat(beanFactory.getBean(DoubleStore.class)).isSameAs(bean.getDoubleStore()); assertThat(beanFactory.getBean(FloatStore.class)).isSameAs(bean.getFloatStore()); } diff --git a/spring-context/src/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java b/spring-context/src/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java index 655c91de039..44e0bcd9e34 100644 --- a/spring-context/src/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/ClassPathXmlApplicationContextTests.java @@ -226,7 +226,7 @@ public class ClassPathXmlApplicationContextTests { } @Test - void childWithProxy() throws Exception { + void childWithProxy() { ClassPathXmlApplicationContext ctx = new ClassPathXmlApplicationContext(CONTEXT_WILDCARD); ClassPathXmlApplicationContext child = new ClassPathXmlApplicationContext( new String[] {CHILD_WITH_PROXY_CONTEXT}, ctx); @@ -337,8 +337,7 @@ public class ClassPathXmlApplicationContextTests { }; ResourceTestBean resource1 = (ResourceTestBean) ctx.getBean("resource1"); ResourceTestBean resource2 = (ResourceTestBean) ctx.getBean("resource2"); - boolean condition = resource1.getResource() instanceof ClassPathResource; - assertThat(condition).isTrue(); + assertThat(resource1.getResource()).isInstanceOf(ClassPathResource.class); StringWriter writer = new StringWriter(); FileCopyUtils.copy(new InputStreamReader(resource1.getResource().getInputStream()), writer); assertThat(writer.toString()).isEqualTo("contexttest"); diff --git a/spring-context/src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java b/spring-context/src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java index 233ecc98ceb..697e4fc834a 100644 --- a/spring-context/src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/ConversionServiceFactoryBeanTests.java @@ -118,7 +118,7 @@ class ConversionServiceFactoryBeanTests { ConfigurableApplicationContext ctx = new ClassPathXmlApplicationContext(fileName, getClass()); ResourceTestBean tb = ctx.getBean("resourceTestBean", ResourceTestBean.class); assertThat(resourceClass.isInstance(tb.getResource())).isTrue(); - assertThat(tb.getResourceArray()).isNotEmpty(); + assertThat(tb.getResourceArray()).hasSize(1); assertThat(resourceClass.isInstance(tb.getResourceArray()[0])).isTrue(); assertThat(tb.getResourceMap()).hasSize(1); assertThat(resourceClass.isInstance(tb.getResourceMap().get("key1"))).isTrue(); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/AbstractSockJsIntegrationTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/AbstractSockJsIntegrationTests.java index daf4e5f8a08..a6bae1556d4 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/AbstractSockJsIntegrationTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/AbstractSockJsIntegrationTests.java @@ -113,8 +113,9 @@ abstract class AbstractSockJsIntegrationTests { this.baseUrl = "http://localhost:" + this.server.getPort(); } + @AfterEach - void teardown() throws Exception { + void teardown() { try { this.sockJsClient.stop(); } @@ -141,6 +142,7 @@ abstract class AbstractSockJsIntegrationTests { } } + protected abstract Class upgradeStrategyConfigClass(); protected abstract WebSocketTestServer createWebSocketTestServer(); @@ -154,6 +156,7 @@ abstract class AbstractSockJsIntegrationTests { this.sockJsClient.start(); } + @Test void echoWebSocket() throws Exception { testEcho(100, createWebSocketTransport(), null); @@ -305,8 +308,8 @@ abstract class AbstractSockJsIntegrationTests { try { Thread.sleep(timeToSleep); } - catch (InterruptedException e) { - throw new IllegalStateException("Interrupted while waiting for " + description, e); + catch (InterruptedException ex) { + throw new IllegalStateException("Interrupted while waiting for " + description, ex); } } throw new IllegalStateException("Timed out waiting for " + description); @@ -333,6 +336,7 @@ abstract class AbstractSockJsIntegrationTests { } } + private static class TestClientHandler extends TextWebSocketHandler { private final BlockingQueue receivedMessages = new LinkedBlockingQueue<>(); @@ -341,7 +345,6 @@ abstract class AbstractSockJsIntegrationTests { private volatile Throwable transportError; - @Override public void afterConnectionEstablished(WebSocketSession session) throws Exception { this.session = session; @@ -376,6 +379,7 @@ abstract class AbstractSockJsIntegrationTests { } } + private static class EchoHandler extends TextWebSocketHandler { @Override @@ -384,21 +388,23 @@ abstract class AbstractSockJsIntegrationTests { } } + private static class TestServerHandler extends TextWebSocketHandler { private WebSocketSession session; @Override - public void afterConnectionEstablished(WebSocketSession session) throws Exception { + public void afterConnectionEstablished(WebSocketSession session) { this.session = session; } - public WebSocketSession awaitSession(long timeToWait) throws InterruptedException { + public WebSocketSession awaitSession(long timeToWait) { awaitEvent(() -> this.session != null, timeToWait, " session"); return this.session; } } + private static class TestFilter implements Filter { private final Map requests = new HashMap<>(); @@ -407,7 +413,6 @@ abstract class AbstractSockJsIntegrationTests { private final Map sendErrorMap = new HashMap<>(); - @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { @@ -418,18 +423,18 @@ abstract class AbstractSockJsIntegrationTests { this.requests.put(uri, headers); for (String suffix : this.sleepDelayMap.keySet()) { - if ((httpRequest).getRequestURI().endsWith(suffix)) { + if (httpRequest.getRequestURI().endsWith(suffix)) { try { Thread.sleep(this.sleepDelayMap.get(suffix)); break; } - catch (InterruptedException e) { - e.printStackTrace(); + catch (InterruptedException ex) { + ex.printStackTrace(); } } } for (String suffix : this.sendErrorMap.keySet()) { - if ((httpRequest).getRequestURI().endsWith(suffix)) { + if (httpRequest.getRequestURI().endsWith(suffix)) { ((HttpServletResponse) response).sendError(this.sendErrorMap.get(suffix)); return; }