Springwolf AMQP - more than one @RabbitHandler per @RabbitListener not permitted

[unbearables]

Describe the bug
Springwolf-amqp does not permit more than one @RabbitHandler per @RabbitListener. Having 2 or more methods annotated with @RabbitHandler causes springwolf to fail at https://github.com/springwolf/springwolf-core/blob/master/springwolf-plugins/springwolf-amqp-plugin/src/main/java/io/github/stavshamir/springwolf/asyncapi/scanners/channels/annotation/MethodLevelRabbitListenerScanner.java#L45 - where the .collect(Collectors.toMap(Binding::getDestination, Function.identity())); throws, as there are more bindings with same destination

Dependencies and versions used
springwolf-amqp version 0.8.1
springwolf-ui version 0.8.0

Code example

@Component
@RabbitListener(...)
public class NotificationListener {

    @RabbitHandler
    public void notificationSend(NotificationSend dto) { // routing key = notification.send
        // do something
    }

    @RabbitHandler
    public void notificationQuery(NotificationQuery dto) { // routing key = notification.query
        // do something else
    }
}

Stack trace and error logs

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [io.github.stavshamir.springwolf.asyncapi.scanners.channels.annotation.MethodLevelRabbitListenerScanner]: Constructor threw exception; nested exception is java.lang.IllegalStateException: Duplicate key notification (attempted merging values Binding [destination=notification, exchange=notification, routingKey=notification.send, arguments={}] and Binding [destination=notification, exchange=notification, routingKey=notification.query, arguments={}])
	at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:224)
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:117)
	at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:311)
	... 19 common frames omitted
Caused by: java.lang.IllegalStateException: Duplicate key notification (attempted merging values Binding [destination=notification, exchange=notification, routingKey=notification.send, arguments={}] and Binding [destination=notification, exchange=notification, routingKey=notification.query, arguments={}])
	at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:135)
	at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:182)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at io.github.stavshamir.springwolf.asyncapi.scanners.channels.annotation.MethodLevelRabbitListenerScanner.<init>(MethodLevelRabbitListenerScanner.java:44)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:211)
	... 21 common frames omitted

Suggestion
It seems that only destination is not enough of uniqueness for binding, as it should be identified by destination + exchange + routingKey + arguments (arguments for headers exchange). Going deeper, the method MethodLevelRabbitListenerScanner.getExchangeName counts with only 1 exchange per whole @RabbitListener, but property bindings can be mapped to multiple exchanges. Is this a known/intended limitation?

[github-actions]

Welcome to Springwolf. Thanks a lot for reporting your first issue. Please check out our contributors guide and feel free to join us on discord.

[timonback]

Hi @unbearables,

I used the latest springwolf version, and I was not able to reproduce your issue. Code added to the amqp example project to verify:

package io.github.stavshamir.springwolf.example.amqp.consumers;

import io.github.stavshamir.springwolf.example.amqp.dtos.AnotherPayloadDto;
import io.github.stavshamir.springwolf.example.amqp.dtos.ExamplePayloadDto;
import org.springframework.amqp.rabbit.annotation.RabbitHandler;
import org.springframework.amqp.rabbit.annotation.RabbitListener;
import org.springframework.stereotype.Component;

@Component
@RabbitListener(queues = "my-queue")
public class TestConsumer {

    @RabbitHandler
    public void notificationSend(AnotherPayloadDto dto) { // routing key = notification.send
        // do something
    }

    @RabbitHandler
    public void notificationQuery(ExamplePayloadDto dto) { // routing key = notification.query
        // do something else
    }
}

I noticed that we didn't support @RabbitListener on the class level at all, which will be added as part of #287

With the change, one channel my-queue will be detected having two (anyOf) message. One of type AnotherPayloadDto, one of type ExamplePayloadDto

[timonback]

The change #287 has been merged and will be part of the next release.

If you want to try and verify it in your application, use the current SNAPSHOT build as described in our README.md

[timonback]

Thank your for the report, the issue has been addressed in the new release.

Feel free to reopen this issue if there is still something missing.

[unbearables]

Hi, I tested your version but the problem still persists. We have some custom annotations, which are deserializing based on routing keys - we put them directly on the dto class, therefore it's harder to pair this with binding.

But mainly, I discovered that springwolf genereates wrong AsyncAPI doc. The problem is that springwolf uses queues as channels, but from what I found in AsyncAPI docs the routing keys are supposed to be channels. See example from AsyncAPI doc and AsyncAPI issue . Is this something you are aware of or do I misunderstand something?

[timonback]

Hi @unbearables,
feel free to open the issue directly, then it becomes visible to us :)

Thank you for the feedback, I am not very familiar with amqp. I think you are right and at the same time, Springwolf is spec conform as long as no exchanges nor routing keys are used. In those cases, the queue name becomes the exchange name.
[0]

But lets add support for amqp routing keys to support your use-case.
How do you specify the routing key? As part of the annotation, like:

    @RabbitListener(
            bindings = {
                @QueueBinding(
                        exchange = @Exchange(name = "name", type = ExchangeTypes.TOPIC),
                        value = @Queue(name = "example-bindings-queue", durable = "false"),
                        key = "example-topic-routing-key")
            })

[0] https://www.cloudamqp.com/blog/part4-rabbitmq-for-beginners-exchanges-routing-keys-bindings.html

[timonback]

A new version of Springwolf has been released, hopefully resolving your issue.
If not, feel free to re-open. Any help, code examples, test cases are welcome to improve support for amqp.