golevelup / nestjs

A collection of badass modules and utilities to help you level up your NestJS applications 🚀
MIT License
2.27k stars 261 forks source link

@RabbitSubscribe in controllers? #251

Closed iamonkey closed 2 years ago

iamonkey commented 3 years ago

Hi,

Firstly, many thanks for developing the rabbitmq for NestJS. We are using it for messaging where we have multiple subscribers.

We like the approach of having all our interfaces to the outside world being defined in our controlers (which then choose which service to route to). Is there a way we can get @RabbitSubscribe to work in a controller (@Controller) rather than jus in services (@Service).

Thanks!

Ian

WonderPanda commented 3 years ago

@sz-adminuser Yeah I don't see why this should be an issue, the decision not to discover handlers in Controllers was somewhat arbitrary and wasn't based on any technical limitations

RLSWE commented 3 years ago

We would like to be able to do so, too. Usually in NestJS that's what happens, so it's weird to use the service class for that logic. If now we need to route some logic to other layers, we will have to name those layers with something else than service, because service is now being used for the routing logic (instead of the controller).

What was the decision originally on using only service classes for it?

WonderPanda commented 3 years ago

@RoniLeshes Controllers in NestJS are discovered different and are much more closely tied to the HTTP lifecycle which is separate from RabbitMQ. I find that users are more likely to inject a HTTP Request scoped dependency in Controllers which will break the ability for the Class to be properly instantiated and used for receiving messages from RabbitMQ.

It's easy to have a separate routing layer that separates receiving messages over Rabbit from the business logic using services instead of controllers. However, guard rails aren't always useful and so if people want to put things in their Controller and accept the tradeoffs the library can support that

RLSWE commented 3 years ago

@WonderPanda yea I was imagining it has to be something like that (the coupling of controllers to HTTP).

For us, implementing such feature is great, as our microservices are using only RabbitMQ for inter-service-communication and the only HTTP that is open to the world is our API Gateway.

Maybe it's worth adding the capability to do so and mention the tradeoffs in the README?

iamonkey commented 3 years ago

Thanks for the feedback. We are using this in prod and it's great, so thank you to everyone that supports and develops this!

From our perspective, we have resolvers and controllers managing what comes into a MS, whether that is HTTP, GRPC or GraphQL. We separate all of these routes from the Service layer. In that architecture, that natural place is for external comms from RabbitMQ to also sit in the controllers or resolvers.

Also, I did want to highlight, currently the module doesn't work if we try and put these in controllers/resolvers. The events are simply ignored.

underfisk commented 2 years ago

@iamonkey My proposal #369 will have any rmq decorator being supported within a controller instead of only services.