From 39861b763369c9d0773fd92fb41808de132eea2c Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Mon, 22 Nov 2021 12:11:32 -0500 Subject: [PATCH] Remove `Disposables` abstraction * Remove `Disposables` since it is a package protected therefore could not be used outside * Remove the logic relevant ot the `Disposables` in the `MessagingAnnotationPostProcessor` logic * Replace a `registerSingleton()` and `initializeBean()` with the proper `registerBeanDefinition()` and `getBean()` usage. This way bean are going to be destroyed properly * Remove `MessagingAnnotationPostProcessorChannelCreationTests` since its mocking logic is too vague and really covered with many other real tests. --- ...AbstractMethodAnnotationPostProcessor.java | 41 +++----- .../config/annotation/Disposables.java | 57 ------------ ...ChannelAdapterAnnotationPostProcessor.java | 10 +- .../MessagingAnnotationPostProcessor.java | 18 ++-- ...tionPostProcessorChannelCreationTests.java | 93 ------------------- 5 files changed, 26 insertions(+), 193 deletions(-) delete mode 100644 spring-integration-core/src/main/java/org/springframework/integration/config/annotation/Disposables.java delete mode 100644 spring-integration-core/src/test/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessorChannelCreationTests.java diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/AbstractMethodAnnotationPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/AbstractMethodAnnotationPostProcessor.java index fd75c286753..45195b9be89 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/AbstractMethodAnnotationPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/AbstractMethodAnnotationPostProcessor.java @@ -36,10 +36,11 @@ import org.springframework.aop.support.DefaultBeanFactoryPointcutAdvisor; import org.springframework.aop.support.NameMatchMethodPointcut; import org.springframework.aop.support.NameMatchMethodPointcutAdvisor; -import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionValidationException; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.annotation.Bean; import org.springframework.core.GenericTypeResolver; import org.springframework.core.ResolvableType; @@ -120,19 +121,20 @@ public abstract class AbstractMethodAnnotationPostProcessor channelResolver; // NOSONAR protected final Class annotationType; // NOSONAR - protected final Disposables disposables; // NOSONAR - @SuppressWarnings(UNCHECKED) public AbstractMethodAnnotationPostProcessor(ConfigurableListableBeanFactory beanFactory) { Assert.notNull(beanFactory, "'beanFactory' must not be null"); this.messageHandlerAttributes.add(SEND_TIMEOUT_ATTRIBUTE); this.beanFactory = beanFactory; + this.definitionRegistry = (BeanDefinitionRegistry) beanFactory; this.conversionService = this.beanFactory.getConversionService() != null ? this.beanFactory.getConversionService() : DefaultConversionService.getSharedInstance(); @@ -140,14 +142,6 @@ public AbstractMethodAnnotationPostProcessor(ConfigurableListableBeanFactory bea this.annotationType = (Class) GenericTypeResolver.resolveTypeArgument(this.getClass(), MethodAnnotationPostProcessor.class); - Disposables disposablesBean = null; - try { - disposablesBean = beanFactory.getBean(Disposables.class); - } - catch (@SuppressWarnings("unused") Exception e) { - // NOSONAR - only for test cases - } - this.disposables = disposablesBean; } @Override @@ -187,23 +181,19 @@ public Object postProcess(Object bean, String beanName, Method method, List handler)); + return this.beanFactory.getBean(handlerBeanName, MessageHandler.class); } private void orderable(Method method, MessageHandler handler) { @@ -355,12 +345,9 @@ protected AbstractEndpoint createEndpoint(MessageHandler handler, @SuppressWarni } catch (DestinationResolutionException e) { if (e.getCause() instanceof NoSuchBeanDefinitionException) { - inputChannel = new DirectChannel(); - this.beanFactory.registerSingleton(inputChannelName, inputChannel); - inputChannel = (MessageChannel) this.beanFactory.initializeBean(inputChannel, inputChannelName); - if (this.disposables != null) { - this.disposables.add((DisposableBean) inputChannel); - } + this.definitionRegistry.registerBeanDefinition(inputChannelName, + new RootBeanDefinition(DirectChannel.class, DirectChannel::new)); + inputChannel = this.beanFactory.getBean(inputChannelName, MessageChannel.class); } else { throw e; diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/Disposables.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/Disposables.java deleted file mode 100644 index 7a39f30aa34..00000000000 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/Disposables.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2018-2019 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.integration.config.annotation; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import org.springframework.beans.factory.DisposableBean; - -/** - * A container for a collection of {@link DisposableBean} it is, itself a - * {@link DisposableBean} and will dispose of its contained beans when it is destroyed. - * Intended for any {@link DisposableBean} that is registered as a singleton in which - * case, the container does not automatically dispose of them. - * - * @author Gary Russell - * @author Artem Bilan - * - * @since 5.1 - * - */ -class Disposables implements DisposableBean { - - private final List disposables = new ArrayList<>(); - - public final void add(DisposableBean... disposablesToAdd) { - this.disposables.addAll(Arrays.asList(disposablesToAdd)); - } - - @Override - public void destroy() { - this.disposables.forEach((disposable) -> { - try { - disposable.destroy(); - } - catch (@SuppressWarnings("unused") Exception ex) { - // NOSONAR - } - }); - } - -} diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/InboundChannelAdapterAnnotationPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/InboundChannelAdapterAnnotationPostProcessor.java index 31bcc5671c5..278c67c7ee8 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/InboundChannelAdapterAnnotationPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/InboundChannelAdapterAnnotationPostProcessor.java @@ -23,6 +23,7 @@ import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.annotation.Bean; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; @@ -122,12 +123,9 @@ else if (ClassUtils.KOTLIN_FUNCTION_0_INVOKE_METHOD != null) { methodInvokingMessageSource.setObject(bean); methodInvokingMessageSource.setMethod(method); String messageSourceBeanName = generateHandlerBeanName(beanName, method); - this.beanFactory.registerSingleton(messageSourceBeanName, methodInvokingMessageSource); - messageSource = (MessageSource) this.beanFactory - .initializeBean(methodInvokingMessageSource, messageSourceBeanName); - if (this.disposables != null) { - this.disposables.add(methodInvokingMessageSource); - } + this.definitionRegistry.registerBeanDefinition(messageSourceBeanName, + new RootBeanDefinition(MethodInvokingMessageSource.class, () -> methodInvokingMessageSource)); + messageSource = this.beanFactory.getBean(messageSourceBeanName, MessageSource.class); } return messageSource; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessor.java index d74ae88fcf9..414bc0ba86b 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessor.java @@ -56,7 +56,6 @@ import org.springframework.integration.annotation.ServiceActivator; import org.springframework.integration.annotation.Splitter; import org.springframework.integration.annotation.Transformer; -import org.springframework.integration.context.IntegrationContextUtils; import org.springframework.integration.endpoint.AbstractEndpoint; import org.springframework.integration.util.MessagingAnnotationUtils; import org.springframework.util.Assert; @@ -104,11 +103,6 @@ protected ConfigurableListableBeanFactory getBeanFactory() { @Override public void afterPropertiesSet() { Assert.notNull(this.beanFactory, "BeanFactory must not be null"); - if (!this.beanFactory.containsBeanDefinition(IntegrationContextUtils.DISPOSABLES_BEAN_NAME)) { - ((BeanDefinitionRegistry) this.beanFactory) - .registerBeanDefinition(IntegrationContextUtils.DISPOSABLES_BEAN_NAME, - new RootBeanDefinition(Disposables.class, Disposables::new)); - } this.postProcessors.put(Filter.class, new FilterAnnotationPostProcessor(this.beanFactory)); this.postProcessors.put(Router.class, new RouterAnnotationPostProcessor(this.beanFactory)); this.postProcessors.put(Transformer.class, new TransformerAnnotationPostProcessor(this.beanFactory)); @@ -224,17 +218,20 @@ protected void processAnnotationTypeOnMethod(Object bean, String beanName, Metho } } + @SuppressWarnings("unchecked") private void postProcessMethodAndRegisterEndpointIfAny(Object bean, String beanName, Method method, Class annotationType, List annotations, MethodAnnotationPostProcessor postProcessor, Method targetMethod) { Object result = postProcessor.postProcess(bean, beanName, targetMethod, annotations); + ConfigurableListableBeanFactory beanFactory = getBeanFactory(); + BeanDefinitionRegistry definitionRegistry = (BeanDefinitionRegistry) beanFactory; if (result instanceof AbstractEndpoint) { AbstractEndpoint endpoint = (AbstractEndpoint) result; String autoStartup = MessagingAnnotationUtils.resolveAttribute(annotations, "autoStartup", String.class); if (StringUtils.hasText(autoStartup)) { - autoStartup = getBeanFactory().resolveEmbeddedValue(autoStartup); + autoStartup = beanFactory.resolveEmbeddedValue(autoStartup); if (StringUtils.hasText(autoStartup)) { endpoint.setAutoStartup(Boolean.parseBoolean(autoStartup)); } @@ -242,7 +239,7 @@ private void postProcessMethodAndRegisterEndpointIfAny(Object bean, String beanN String phase = MessagingAnnotationUtils.resolveAttribute(annotations, "phase", String.class); if (StringUtils.hasText(phase)) { - phase = getBeanFactory().resolveEmbeddedValue(phase); + phase = beanFactory.resolveEmbeddedValue(phase); if (StringUtils.hasText(phase)) { endpoint.setPhase(Integer.parseInt(phase)); } @@ -255,8 +252,9 @@ private void postProcessMethodAndRegisterEndpointIfAny(Object bean, String beanN String endpointBeanName = generateBeanName(beanName, method, annotationType); endpoint.setBeanName(endpointBeanName); - getBeanFactory().registerSingleton(endpointBeanName, endpoint); - getBeanFactory().initializeBean(endpoint, endpointBeanName); + definitionRegistry.registerBeanDefinition(endpointBeanName, + new RootBeanDefinition((Class) endpoint.getClass(), () -> endpoint)); + beanFactory.getBean(endpointBeanName); } } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessorChannelCreationTests.java b/spring-integration-core/src/test/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessorChannelCreationTests.java deleted file mode 100644 index 184c9b19385..00000000000 --- a/spring-integration-core/src/test/java/org/springframework/integration/config/annotation/MessagingAnnotationPostProcessorChannelCreationTests.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright 2017-2019 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.integration.config.annotation; - -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.BDDMockito.given; -import static org.mockito.BDDMockito.willAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.withSettings; - -import org.junit.Test; - -import org.springframework.beans.factory.BeanCreationException; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.integration.annotation.ServiceActivator; -import org.springframework.integration.channel.DirectChannel; -import org.springframework.messaging.MessageChannel; -import org.springframework.messaging.MessageHandler; -import org.springframework.messaging.core.DestinationResolutionException; - -/** - * @author Gary Russell - * @author Artem Bilan - * - * @since 4.3.8 - * - */ -public class MessagingAnnotationPostProcessorChannelCreationTests { - - @Test - public void testAutoCreateChannel() { - ConfigurableListableBeanFactory beanFactory = mock(ConfigurableListableBeanFactory.class, - withSettings().extraInterfaces(BeanDefinitionRegistry.class)); - given(beanFactory.getBean("channel", MessageChannel.class)).willThrow(NoSuchBeanDefinitionException.class); - willAnswer(invocation -> invocation.getArgument(0)) - .given(beanFactory).initializeBean(any(DirectChannel.class), eq("channel")); - willAnswer(invocation -> invocation.getArgument(0)) - .given(beanFactory).initializeBean(any(MessageHandler.class), eq("foo.foo.serviceActivator.handler")); - MessagingAnnotationPostProcessor mapp = new MessagingAnnotationPostProcessor(); - mapp.setBeanFactory(beanFactory); - mapp.afterPropertiesSet(); - mapp.afterSingletonsInstantiated(); - mapp.postProcessAfterInitialization(new Foo(), "foo"); - verify(beanFactory).registerSingleton(eq("channel"), any(DirectChannel.class)); - } - - @Test - public void testDontCreateChannelWhenChannelHasBadDefinition() { - ConfigurableListableBeanFactory beanFactory = mock(ConfigurableListableBeanFactory.class, - withSettings().extraInterfaces(BeanDefinitionRegistry.class)); - given(beanFactory.getBean("channel", MessageChannel.class)).willThrow(BeanCreationException.class); - willAnswer(invocation -> invocation.getArgument(0)) - .given(beanFactory).initializeBean(any(DirectChannel.class), eq("channel")); - willAnswer(invocation -> invocation.getArgument(0)) - .given(beanFactory).initializeBean(any(MessageHandler.class), eq("foo.foo.serviceActivator.handler")); - MessagingAnnotationPostProcessor mapp = new MessagingAnnotationPostProcessor(); - mapp.setBeanFactory(beanFactory); - mapp.afterPropertiesSet(); - mapp.afterSingletonsInstantiated(); - assertThatExceptionOfType(DestinationResolutionException.class) - .isThrownBy(() -> mapp.postProcessAfterInitialization(new Foo(), "foo")) - .withMessageContaining("A bean definition with name 'channel' exists, but failed to be created"); - } - - public static class Foo { - - @ServiceActivator(inputChannel = "channel") - public void foo(String in) { - // empty - } - - } - -}