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

Add RabbitTemplate getBeforePublishPostProcessors method #2839

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

leshalv
Copy link

@leshalv leshalv commented Sep 26, 2024

No description provided.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot approve this change without any reasoning.
Share with us, please, your experience why would one get access to this property?

@leshalv
Copy link
Author

leshalv commented Sep 27, 2024

The RabbitTemplate class currently provides a addBeforePublishPostProcessors method to configure MessagePostProcessor instances that can modify messages before they are sent. However, there is no corresponding getBeforePublishPostProcessors method to retrieve the currently configured processors. This PR introduces the getBeforePublishPostProcessors method, enhancing the flexibility and configurability of RabbitTemplate.

Justification:

  1. Increased Flexibility:

    • While addBeforePublishPostProcessors allows users to configure message processors, there is currently no way to retrieve or modify the existing MessagePostProcessor list. Adding getBeforePublishPostProcessors gives users the ability to access and modify these processors dynamically, allowing for greater flexibility in scenarios where message processing needs to be altered based on context.
  2. Support for Multiple RabbitTemplate Instances:

    • In more complex applications, it's common to create multiple RabbitTemplate instances to handle different business logic. For instance, the ConfirmCallback mechanism is globally applied, but by manually creating different RabbitTemplate instances, users can configure distinct callbacks for each instance. Introducing the getBeforePublishPostProcessors method will allow users to retrieve and reuse message processors between different RabbitTemplate instances, enhancing flexibility when handling different message routing and confirmation scenarios.
  3. Improved Consistency:

    • Spring generally follows a "getter-setter" pattern for its components, and many classes offer both methods for configuring and retrieving values. By providing getBeforePublishPostProcessors, the API will become more consistent, following Spring’s design principles and making it easier for developers to interact with the RabbitTemplate class.
  4. Better Support for Advanced Use Cases:

    • In advanced scenarios where users need to dynamically add or remove message processors based on business needs, the ability to retrieve the current processors simplifies the code. Without this getter method, developers need to manually track and manage MessagePostProcessor lists, which adds unnecessary complexity and can lead to duplication of logic.
  5. Enhanced Developer Experience:

    • Having the ability to get the current MessagePostProcessor list helps developers debug and analyze the message pipeline more easily. This makes it simpler to understand how messages are being transformed before being sent, improving both the developer experience and troubleshooting process.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. All those arguments still don't convince me in the importance of this API since all the objects you are going to inject/retrieve from there are already somewhere in your configuration.
Apparently what you are going to be is to get them, modify and set back. So, why just not have a set of those post-processors somewhere outside of RabbitTemplate and set them (or override) respectively whenever you need?

Nevertheless since this new API is totally not harmful, I'll accept the change and merge it shortly.


/**
* Return configured before post {@link MessagePostProcessor}s or {@code null}.
* @return configured before post {@link MessagePostProcessor}s or {@code null}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add @since 3.2 and your name to the @author list.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also see Checkstyle violations:

Error: eckstyle] [ERROR] /home/runner/work/spring-amqp/spring-amqp/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitTemplate.java:635: Line matches the illegal pattern 'Trailing whitespace'. [Regexp]

Run ./gradlew clean check locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants