-
Notifications
You must be signed in to change notification settings - Fork 14.9k
MINOR: Refactor auto topic creation to separate envelope logic #21272
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
base: trunk
Are you sure you want to change the base?
Conversation
This commit introduces a TopicCreator interface to abstract topic creation logic, making the code more modular and testable. The main changes include: - Created TopicCreator interface for topic creation abstraction - Implemented KRaftTopicCreator for KRaft-based topic creation - Refactored DefaultAutoTopicCreationManager to use TopicCreator instead of directly managing NodeToControllerChannelManager - Simplified envelope handling by moving it into TopicCreator implementation - Updated AutoTopicCreationManagerTest to use mocked TopicCreator - Removed obsolete envelope-related test methods - Use a mock instance of TopicCreator in AutoTopicCreationManagerTest to better test without Mockito's argumentcaptor complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors auto topic creation logic by extracting envelope handling into a separate TopicCreator interface and KRaftTopicCreator implementation. The main motivation is to improve modularity and testability by separating concerns between topic creation coordination (in DefaultAutoTopicCreationManager) and the protocol-level details of forwarding requests to the controller.
Key changes:
- Introduced a new
TopicCreatorinterface with two methods:createTopicWithPrincipalandcreateTopicWithoutPrincipalthat returnCompletableFuture<CreateTopicsResponse> - Implemented
KRaftTopicCreatorin Java that encapsulates envelope request building and response parsing logic previously embedded inDefaultAutoTopicCreationManager - Refactored
DefaultAutoTopicCreationManagerto use theTopicCreatorinterface and simplified response handling usingCompletableFuture.whenComplete - Replaced complex Mockito-based tests with a custom
TestTopicCreatorimplementation for better control and clarity in testing
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/kafka/server/TopicCreator.java | New interface defining the contract for topic creation operations |
| core/src/main/java/kafka/server/KRaftTopicCreator.java | KRaft implementation handling envelope requests and response parsing |
| core/src/test/java/kafka/server/KRaftTopicCreatorTest.java | Comprehensive test suite for KRaftTopicCreator covering success, error, and edge cases |
| core/src/main/scala/kafka/server/AutoTopicCreationManager.scala | Refactored to delegate topic creation to TopicCreator and simplified callback handling |
| core/src/main/scala/kafka/server/BrokerServer.scala | Updated to instantiate KRaftTopicCreator and inject it into DefaultAutoTopicCreationManager |
| core/src/test/scala/unit/kafka/server/AutoTopicCreationManagerTest.scala | Replaced Mockito ArgumentCaptor usage with TestTopicCreator for cleaner, more maintainable tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit introduces a TopicCreator interface to abstract topic
creation logic, making the code more modular and testable. The main
changes include:
and removed the corresponding logic from DefaultAutoTopicCreationManager
better test without Mockito's ArgumentCaptor complexity.