Consider Removing 'Dangling' Handler Bean Definitions [INT-3009]

[spring-operator]

Gary Russell opened INT-3009 and commented

When a consumer endpoint is re-defined, the ConsumerEndpointFactoryBean definition is correctly overridden; however, the handler bean definition is left as a 'hanging' bean definition that has no references.

Consider removing such definitions with something like...

diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java
index 805a90b..6fdf0d4 100644
--- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java
+++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractConsumerEndpointParser.java
@@ -19,14 +19,19 @@ package org.springframework.integration.config.xml;
 import java.util.Collection;
 import java.util.List;
 
+import org.w3c.dom.Element;
+
+import org.springframework.beans.PropertyValue;
 import org.springframework.beans.factory.BeanDefinitionStoreException;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConstructorArgumentValues;
 import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder;
+import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.parsing.BeanComponentDefinition;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionReaderUtils;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.ManagedSet;
 import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
@@ -34,7 +39,6 @@ import org.springframework.integration.config.ConsumerEndpointFactoryBean;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
-import org.w3c.dom.Element;
 
 /**
  * Base class parser for elements that create Message Endpoints.
@@ -55,10 +59,7 @@ public abstract class AbstractConsumerEndpointParser extends AbstractBeanDefinit
 	@Override
 	protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext)
 			throws BeanDefinitionStoreException {
-		String id = element.getAttribute(ID_ATTRIBUTE);
-		if (!StringUtils.hasText(id)) {
-			id = element.getAttribute("name");
-		}
+		String id = obtainId(element);
 		if (!StringUtils.hasText(id)) {
 			id = BeanDefinitionReaderUtils.generateBeanName(definition, parserContext.getRegistry(), parserContext.isNested());
 		}
@@ -74,8 +75,28 @@ public abstract class AbstractConsumerEndpointParser extends AbstractBeanDefinit
 		return "input-channel";
 	}
 
+	private String obtainId(Element element) {
+		String id = element.getAttribute(ID_ATTRIBUTE);
+		if (!StringUtils.hasText(id)) {
+			id = element.getAttribute("name");
+		}
+		return id;
+	}
+
 	@Override
 	protected final AbstractBeanDefinition parseInternal(Element element, ParserContext parserContext) {
+		String id = obtainId(element);
+		BeanDefinitionRegistry registry = parserContext.getRegistry();
+		if (StringUtils.hasText(id)) {
+			if (registry.containsBeanDefinition(id)){
+				BeanDefinition oldBeanDefinition = registry.getBeanDefinition(id);
+				PropertyValue handlerProperty = oldBeanDefinition.getPropertyValues().getPropertyValue("handler");
+				if (handlerProperty.getValue() instanceof RuntimeBeanReference) {
+					String handlerBeanName = ((RuntimeBeanReference) handlerProperty.getValue()).getBeanName();
+					registry.removeBeanDefinition(handlerBeanName);
+				}
+			}
+		}
 		BeanDefinitionBuilder handlerBuilder = this.parseHandler(element, parserContext);
 		IntegrationNamespaceUtils.setReferenceIfAttributeDefined(handlerBuilder, element, "output-channel");
 		IntegrationNamespaceUtils.setValueIfAttributeDefined(handlerBuilder, element, "order");

However, see #6731 (chain), where the chain parser needs the definition to remain so it can remove any registered chain beans. So, the removal of the stale handler definition needs to be deferred until after the handler has been parsed.

But, that adds a further complication in that the generated bean names will out of whack.

Perhaps this should be combined with doing away with generic bean name generation for handlers, and use the "consumer.handler" name as the child name instead of adding it as an alias.

---

Affects: 3.0 M1

Issue Links:

• #6568 Spring Integration Components should not use generated inner-bean names

• #6731 Core XSD Schema - Allow the "id" attribute for elements within Chains

1 votes, 2 watchers

[spring-operator]

Artem Bilan commented

In addition we have to think about some support beans: StoredProcExecutor, JpaExecutor etc. Maybe they should be registered as BeanDefinitionHolder as it is for chain handlers now... I mean that

registry.removeBeanDefinition(handlerBeanName)

isn't enough to get rid 'dangling' beans.

[spring-operator]

Gary Russell commented

Yeah - I am thinking we'd need to do something like add an overridable method removeChildBeanDefinitionsFor(String id) - then chains etc can add an implementation to clean up their danglers when the parent bean is overridden.

[spring-operator]

Artem Bilan commented

Of course, this method can contain registry.removeBeanDefinition(handlerBeanName) and can be overrided in the ChainParser to get deal with its handlerList. I mean it is up to concrete parser what to do on removeBeanDefinitionsFor event.

[spring-operator]

Artem Bilan commented

I think this is the fact of revising parser for this extra option:

/**
 * Set the name of the parent definition of this bean definition.
 */
public BeanDefinitionBuilder setParentName(String parentName) {

Which will lead to cascade bean definition removals when we deal with endpoint bean respectively.

[artembilan]

Having this a very old issue and modern tendency to move away from XML configuration, plus complexity of the possible solution, I treating this as Won't Fix.