Allow STOMP to declare an exchange before publishing/subscribing

sparkyb commented 4 years ago

Proposed Changes

Currently publishing or subscribing to a /queue/ destination will declare the queue, but publishing/subscribing to a /exchange/ destination will not declare the exchange meaning all exchanges must already exist before being used by the STOMP plugin. This change allows the STOMP plugin to optionally declare the exchange before publishing/subscribing to it. The default is still the same. If you want to declare the exchange, add a declare-exchange:true header to the SEND or SUBSCRIBE frame. Additional headers can be used to specify properties of the exchange (exchange-type, exchange-durable, and exchange-auto-delete). These are prefixed with exchange- to distinguish them from the durable and auto-delete properties of the receive queue created when subscribing. This change requires a related changed to rabbitmq-erlang-client.

Types of Changes

What types of changes does your code introduce to this project?

[ ] Bugfix (non-breaking change which fixes issue #NNNN)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation (correction or otherwise)
[ ] Cosmetics (whitespace, appearance)

Checklist

[x] I have read the CONTRIBUTING.md document
[x] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
[x] All tests pass locally with my changes
[x] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if appropriate)
[ ] Any dependent changes have been merged and published in related repositories

michaelklishin commented 4 years ago

This depends on https://github.com/rabbitmq/rabbitmq-erlang-client/pull/131.

sparkyb commented 4 years ago

I can't help but think that this makes an already hard to reason about mess of STOMP endpoints even more complicated. Most STOMP users probably want to use the /topic and /queue destinations. Please provide a justification for this feature. I don't recall anyone else asking for this.

At a previous job, we had an internal framework that used RabbitMQ to communicate between a number of different components. Backend/headless components were mostly written in Python (a few in Java) and used AMQP, but the front-end/GUI components were all webapps run in Chrome which meant their only option for talking to RabbitMQ was STOMP-over-WebSockets. Yet the framework attempted to be agnostic about that different and have either work in the same way which was to have two high-level functions: publish that took an exchange name, optional routing key, and message, and subscribe that took an exchange name, optional binding key, and callback. All exchanges were transient direct exchanges declared on demand. The Python had no problem always declaring exchanges before using them, but for the webapps using STOMP, they would get declared with a PUT request to the management HTTP API before subscribing or publishing.

On a new project I have similar requirements, and I thought this improvement would be a better way to accomplish it rather than mixing the management API and STOMP-over-WebSockets. In general, I'd like the STOMP plugin to be as close as possible to the full capabilities of AMQP so that I don't have to limit my design to a subset of what RabbitMQ/AMQP are capable of just because I have some components that are forced to use STOMP-over-WebSockets. I have several other ideas for improving the STOMP plugin as well. I'm happy to discuss these changes in case they don't fit within the design goals of the maintainers, I just wanted to prove that I'm willing to do the work and submit pull requests rather than just begging someone else for a bunch of feature requests. I started with this change because I thought it complicated the STOMP endpoints much less than some of the other ideas I have and it didn't take me long to try. But if you aren't interested in these kinds of contributions, it's no problem. I'm happy to stick with my workarounds or create my own private forked plugin.

sparkyb commented 4 years ago

Tests have been added. They are currently failing pending the acceptance of the related change to rabbitmq-erlang-client.

lukebakken commented 4 years ago

FWIW this seems reasonable to me. I'm assuming this doc may need an update as well?

michaelklishin commented 4 years ago

We will consider this for 3.8.4. It has not been forgotten.

lukebakken commented 3 years ago

New pull request: https://github.com/rabbitmq/rabbitmq-server/pull/2637

rabbitmq / rabbitmq-stomp