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

Allow access to the errorHandler and messageAckListener values of an AbstractMessageListenerContainer #2978

Closed
tirz opened this issue Feb 16, 2025 · 3 comments

Comments

@tirz
Copy link

tirz commented Feb 16, 2025

Expected Behavior

I was wondering if there were any reasons why the getMessageAckListener() function for the AbstractMessageListenerContainer class would be protected while the setter is public.
This is also true for setErrorHandler() which is public, but the getter does not exist.

Current Behavior

The class org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer :

  • getMessageAckListener() is protected ;
  • does not contain a getErrorHandler() function ;

Context

I'm currently in a situation where I need a decorator pattern for the MessageListenerContainer and MessageListener interfaces, But not having access to getMessageAckListener() and getErrorHandler() makes delegation difficult.

@artembilan
Copy link
Member

Would mind sharing with us what kind of decoration you do?
I wonder why existing proxy functionality is not enough: https://docs.spring.io/spring-amqp/reference/amqp/containerAttributes.html#adviceChain

@tirz
Copy link
Author

tirz commented Feb 16, 2025

Hello, thank you for your fast answer.

In fact, I don't think my use case is standard.

I work at a company where we use a library built on top of spring-boot.
When we have a @QueueListener, it uses our company's bean.
While it doesn't expose any spring-boot objects, it does come with automatic error handling, logs/metrics, and publishing to graphana/elastics (I have to keep this behavior).
Luckily, they use a MessageListenerContainer bean internally, so I can access it.

I needed to handle special cases, so I created a custom MessageListener and MessageListenerContainer and initializing the proxy with like this:

@Bean("my-bean")
public MessageListenerContainer myBen(@Qualifier("company-bean") MessageListenerContainer companyBean) {
 return new MLCProxy(companyBean)); // MLCProxy delegate to MessageListenerContainer interface
}

@Bean("my-bean-feature-x")
public MessageListenerContainer myBenFeatureX(@Qualifier("company-bean") AbstractMessageListenerContainer companyBean) {
 return new MLCFeatureX(companyBean)); // extends MLCProxy but also delegate to AbstractMessageListenerContainerProxy
}

This was fine in most cases, but I have several decorators that extend the Proxy, and one of them requires changing the behavior of start, stop for the MessageListenerContainer and onMessage, onComplete and handleError for the MessageListener.

Basically, I want to be able to do something like this:

// does compile
var simpleMessageListenerProxy = new MLProxy(featureXMLC.getMessageListener());

// does not compile
var featureXMessageListener = new FeatureXMessageListener(
    featureXMLC.getMessageListener() ,   // custom logic, then delegation to our official listener
    featureXMLC.getMessageAckListener(), // here too (but featureXMLC cannot delegate to a protected getter)
    featureXMLC.getErrorHandler()        // and here (getErrorHandler does not exsist in myCompanyBean)
);

featureXMLC.setMessageListener(featureXMessageListener);
featureXMLC.setMessageAckListener(featureXMessageListener);
featureXMLC.setErrorHandler(featureXMessageListener);

@artembilan
Copy link
Member

I think the architectural decision to proxy such a framework heavy component was wrong from day first.
The user API is @RabbitListener. That's where we could proxy the target POJO method and do respective metrics.
And to be honest they are there out-of-the-box by the Framework: https://docs.spring.io/spring-amqp/reference/appendix/micrometer.html.

In other words the listener container was not designed to be proxied. That's why I suggested that adviceChain option instead.
Not sure why would one use proxy to inject error handler...

Either way, that is your project decision and from our perspective it is not too hard to expose requested options.
So, I'll review your PR shortly and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment