search

AlariCode / nestjs-rmq

A custom library for NestJS microservice. It allows you to use RabbitMQ or AMQP.
https://purpleschool.ru
MIT License
285 stars 38 forks source link

Feature Request: Multi exchange and multi queue binding #44

Open falahati opened 2 years ago

falahati commented 2 years ago

This feature request asks for the possibility of defining more than one exchange or queue and to allow selecting the right queue or exchange using the decorator. This allows for wider advanced usages like for example when a queue is used exclusively by an instance of the service for getting notifications while another is used for load balancing messages or when two exchanges has to be used for a single topic/message. Implementing this feature also greatly decreases the need for non-global feature specific support for the library.

To achieve this and still be compatible with the older version I suggest adding a new queues and exchanges configurable properties with information about one or more exchanges or queues.

A bindingOption optional argument is then can be added to the decorator allowing it to gets binded to a all or one or more queues or exchanges.

channel.consume obviously has to change to account for this change but it is the only heavy part of this feature.

Are you ok with me creating a PR for this?

P.S. Been using this library in production with a dozen microservices happily for the last year, good job, and thank you.

AlariCode commented 2 years ago

Thanks for support! First we need to determine approach to this feature.

  1. The one that you described. Pros: backward compatibility, easy to code. Cons: NestJS module philosophy is not supported.
  2. We create forFeature() method, that declares name, queue, exchange and other parameters. And forRoot() only declare service names, logger and other common features. @RMQRoute gets additional parameter of module name, to bind correctly to the selected module. Pros: logic separation with modules, you can use different connections with different modules. Cons: no backward compatibility (I assume), more hard to implement. Second approach related to https://github.com/AlariCode/nestjs-rmq/issues/20

What do you think, would be better and why?

falahati commented 2 years ago

I think adding the forFeature() is a must with or without this change. forFeature() allows dependency separation which might be quite useful in a modular sort of way, and as you said it, more in line with what NestJS philosophy dictates or is the norm.

However, this FR is not actually a replacement for the nonglobal modules, but rather helps decrease such needs indirectly by adding a new feature that could in the majority of cases used as a workaround. So from the modular point of view, no this feature is not ideal. However, the added functionalities are not going to be provided with the addition of forFeature anyway, so one can argue that this feature is completely separate.

But yeah, I suppose forFeature is more preferable. If we write it the way TypeORM does it we might be able to allow backward compatibility. But I have to double-check. I will take a look into this and keep you in the loop for any finding; might even submit a PR for forFeature. But in any case, if backward compatibility is broken, you can release a new major version, the migration for users would be easy.