MessageListenerAdapter improve

[kc910521]

about closed #972
I must use ->

• the old style listener adapter i.e MessageListenerAdapter (because i need queueName to be a string that concat queue name with the current server name )
• i need manual ACK

so I try to extend MessageListenerAdapter, but that is not so convenient, i must copy all code from method onMessage, if this pull request pass, i could finish former biz by override repackArgument with code ->

if (listenerArguments != null) {
List resList = new LinkedList(Arrays.asList(listenerArguments));
resList.add(channel);
resList.add(message);
return resList.toArray();
}
return listenerArguments;

[garyrussell]

This change is reasonable; I am not keen on the method name "repack...", though. How about "enhanceListenerArguments"?

Also, a test case would be good, together with some documentation on how a user might subclass the adapter to use this.

Finally, I am curious as to why you can't use the new-style adapter.

because i need queueName to be a string that concat queue name with the current server name

doesn't make sense to me.

[kc910521]

@garyrussell @artembilan

thx for your reply and method name suggests, but I don't think MessagingMessageListenerAdapter could meet my needs.
my project has many msg listeners with likely naming rules in a class, but I think there are only carry one method in a MessagingMessageListenerAdapter you declare,(not confirmed)
so i suppose that i must declare 10 MessagingMessageListenerAdapter s for 10 consumers of the queue, that cause 10 SimpleMessageListenerContainer to receive it.

about use case ...

public class YourMessageListenerAdapter extends MessageListenerAdapter {

    @Override
    protected Object[] repackArgument(Object[] listenerArguments, Channel channel, Message message) {
        if (listenerArguments != null) {
            List<Object> resList = new LinkedList<Object>(Arrays.asList(listenerArguments));
            // put anything u want
            resList.add(channel);
            resList.add(message);
            return resList.toArray();
        }
        return listenerArguments;
    }
 }

I think change parameters of buildListenerArguments is not good for the older version, because the method is protected, may in use by extends

[artembilan]

I think there are only carry one method in a MessagingMessageListenerAdapter

That's not correct. See @RabbitHandler approach in the Reference Manual: https://docs.spring.io/spring-amqp/docs/2.1.5.RELEASE/reference/#annotation-method-selection

Re. may in use by extends, I would say that you try to fix a bug for missed arguments to use for building.

This way I would suggest then to deprecate a buildListenerArguments() method and make end-user to use a new one like just listenerArguments() with an appropriate set of parameters to consider.

With two methods from API you are going to confuse end-users what and how to implement/override.

However we still can just break that API in the current version and change a buildListenerArguments() signature. in the end we will just mention it in the Migration Guide.

Let's hear what @garyrussell thinks on the matter and then ,please, also share some test-case and some docs are needed for this any way.

Thanks

[garyrussell]

However we still can just break that API in the current version and change a buildListenerArguments() signature. in the end we will just mention it in the Migration Guide.

I have no real problems with a breaking change to that method in 2.2.

We don't hear much about this old adapter these days anyway.

However, why not simply add

protected Object[] buildListenerArguments(Object extractedMessage, Channel channel, Message message) {
    return buildListenerArguments(extractedMessage);
}

??

[artembilan]

Doh! Of course! We can rely on the polymorphism!
And still deprecate an old method 😄

I just don't see reason in confusing API...

[kc910521]

also share some test-case and some docs are needed for this any way.

OK, I have pushed test case, but I do not know where should i submit documents, or u mean comment?

[kc910521]

However we still can just break that API in the current version and change a buildListenerArguments() signature. in the end we will just mention it in the Migration Guide.

I have no real problems with a breaking change to that method in 2.2.

We don't hear much about this old adapter these days anyway.

However, why not simply add

protected Object[] buildListenerArguments(Object extractedMessage, Channel channel, Message message) {
    return buildListenerArguments(extractedMessage);
}

??

thx, good suggestion

[artembilan]

Re. Doc:

There is an amqp.adoc file in the project and you can find there a message-listener-adapter section.
So, your change should be reflected there with mentioning in what version we have introduced it.

Then there is a whats-new.adoc. You can add a simple paragraph there to mention this change with a link to the target section.

Thanks

[kc910521]

doc uploaded

[artembilan]

Thank you for update.

Please, consider my nit-picks in the review.

[artembilan]

Your name to all the affected classes, please.

[kc910521]

@artembilan OK, I'm trying to make the doc simple and supply sample code

[artembilan]

LGTM.

Merging with minor code style and wording polishing.

[artembilan]

Merged as 8ac8ea1.

@kc910521 ,

thank you for contribution; looking forward for more!

[kc910521]

Merged as 8ac8ea1.

@kc910521 ,

thank you for contribution; looking forward for more!

My pleasure.