java.lang.IllegalStateException: Only one 'RetryTopicConfigurationSupport' is allowed when Spring context is reloaded

[jrenzullo]

In what version(s) of Spring for Apache Kafka are you seeing this issue?
Since 2.9

Describe the bug
RetryTopicConfigurationSupport has a static variable to keep track of whether or not the class has been instantiated more than once. This breaks unit tests whenever the Spring context is refreshed.

public RetryTopicConfigurationSupport() {
	Assert.state(ONLY_ONE_ALLOWED.getAndSet(false), "Only one 'RetryTopicConfigurationSupport' is allowed");
}

To Reproduce
Setup a Kafka listener with a retry topic configuration. Then create a unit test where the Spring context is refreshed. I forced it by using the DirtiesContext annotation, but this can occur anytime the context is reloaded.

Expected behavior
You should be able to reset the Spring context without any unexpected side effects.

Sample
https://github.com/jrenzullo/spring-kafka-test-bug

[garyrussell]

We handle this in the framework tests by subclassing this class in all retry topic test classes:

spring-kafka/spring-kafka/src/test/java/org/springframework/kafka/retrytopic/AbstractRetryTopicIntegrationTests.java

Lines 29 to 41 in 2ad36bd

	public class AbstractRetryTopicIntegrationTests {
	protected AbstractRetryTopicIntegrationTests() {
	}
	@BeforeAll
	static void reset() throws Exception { // NOSONAR
	Field field = RetryTopicConfigurationSupport.class.getDeclaredField("ONLY_ONE_ALLOWED");
	field.setAccessible(true);
	((AtomicBoolean) field.get(null)).set(true);
	}
	}

[garyrussell]

If you have a better suggestion, please submit a PR.

[jrenzullo]

Are you suggesting every consumer of this library should do the same? I would suggest we remove this field entirely since static state causes issues with testing. Do you know the reason for having this field/check in the first place?

[jrenzullo]

Also to be clear this is not specific to unit testing. This can happen any time you restart the Spring context, which could happen on a live production app as well.

[garyrussell]

Good point. We should change it to ensure there is only one instance in each application context.