Skip to content
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

Docs say defaultRetryTopicKafkaTemplate is the default template bean name. @RetryableTopic javadoc says retryTopicDefaultKafkaTemplate #3514

Closed
ndwhelan opened this issue Sep 23, 2024 · 5 comments · Fixed by #3543
Labels
Milestone

Comments

@ndwhelan
Copy link

ndwhelan commented Sep 23, 2024

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

Between (at least) 2.9.13 and 3.2.4

Describe the bug

The documentation states that the default KafkaTemplate bean for publishing retries / DLT through @RetryableTopic is defaultRetryTopicKafkaTemplate. The annotation states, and at least some tests use this, that it should be retryTopicDefaultKafkaTemplate. On the other hand, some tests and code refer to defaultRetryTopicKafkaTemplate through RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME

I don't know which one to use at this point. Though I'm working around this by specifying a specific values for the kafkaTemplate parameter on the @RetryableTopic annotation.

To Reproduce

  • Create listener annotated with @RetryableTopic
  • Create a bean named retryTopicDefaultKafkaTemplate
  • See that it's not used by the @RetryableTopic publishing.

To be honest, I'm having trouble determining which bean its using.

Expected behavior

I would expect consistency in the javadoc and the docs on the Spring Kafka website to be consistent. At this point, I don't know which one is actually used in the code.

Sample

If necessary, I will work on an isolated example in a new GitHub repository. Though, i'm hoping this issue/concern is straightforward enough, with the links above, that it's not necessary. Still, I understand if it is.

@artembilan
Copy link
Member

That JavaDoc has to be fixed.
The logic there in the RetryableTopicAnnotationProcessor is like this:

	private KafkaOperations<?, ?> getKafkaTemplate(@Nullable String kafkaTemplateName, String[] topics) {
		if (StringUtils.hasText(kafkaTemplateName)) {
			Assert.state(this.beanFactory != null, "BeanFactory must be set to obtain kafka template by bean name");
			try {
				return this.beanFactory.getBean(kafkaTemplateName, KafkaOperations.class);
			}
			catch (NoSuchBeanDefinitionException ex) {
				throw new BeanInitializationException("Could not register Kafka listener endpoint for topics "
						+ Arrays.asList(topics) + ", no " + KafkaOperations.class.getSimpleName()
						+ " with id '" + kafkaTemplateName + "' was found in the application context", ex);
			}
		}
		Assert.state(this.beanFactory != null, "BeanFactory must be set to obtain kafka template by default bean name");
		try {
			return this.beanFactory.getBean(RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME,
					KafkaOperations.class);
		}
		catch (NoSuchBeanDefinitionException ex2) {
			KafkaOperations<?, ?> kafkaOps = this.beanFactory.getBeanProvider(KafkaOperations.class).getIfUnique();
			Assert.state(kafkaOps != null, () -> "A single KafkaTemplate bean could not be found in the context; "
					+ " a single instance must exist, or one specifically named "
					+ RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME);
			return kafkaOps;
		}
	}

So, t definitely uses the mentioned RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME constant. There the JavaDoc is misleading and has to refer to this constant as well.

Contribution is welcome!

@artembilan artembilan added the ideal-for-user-contribution An issue that would ideal for a user to get started with contributing. label Sep 23, 2024
@ndwhelan
Copy link
Author

Thanks for quickly triaging and providing this information!

I may be able to contribute the necessary changes this weekend or next week... we'll see. Hopefully.

@sobychacko
Copy link
Contributor

@ndwhelan Are you still planning to contribute these changes? Thanks!

@JeonDaehong
Copy link
Contributor

JeonDaehong commented Oct 8, 2024

@ndwhelan @artembilan

#3543

Hello,
I have made the changes and submitted a PR.
This is my first open-source PR. I would be grateful if you could accept the PR and give me the opportunity to become a contributor.
Thank you!

@ndwhelan
Copy link
Author

ndwhelan commented Oct 8, 2024

Thanks for taking this! I hadn't been able to get to it.

I'm outside the org here, though, and can't merge.

spring-builds pushed a commit that referenced this issue Oct 8, 2024
…fkaTemplate

Fixes: #3514

#3514

* Fix: Replace `retryTopicDefaultKafkaTemplate` with `defaultRetryTopicKafkaTemplate` in javadocs
* Fix: Replace `retryTopicDefaultKafkaTemplate` with `RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME` in Test Code

(cherry picked from commit f1cb003)
spring-builds pushed a commit that referenced this issue Oct 8, 2024
…fkaTemplate

Fixes: #3514

#3514

* Fix: Replace `retryTopicDefaultKafkaTemplate` with `defaultRetryTopicKafkaTemplate` in javadocs
* Fix: Replace `retryTopicDefaultKafkaTemplate` with `RetryTopicBeanNames.DEFAULT_KAFKA_TEMPLATE_BEAN_NAME` in Test Code

(cherry picked from commit f1cb003)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants