search

cloudamqp / lavinmq

Lightweight and fast AMQP (0-9-1) server
https://lavinmq.com
Apache License 2.0
582 stars 33 forks source link

Publish to exchange #786

Closed snichme closed 2 weeks ago

snichme commented 1 month ago

WHAT is this pull request doing?

Move the logic of publishing a message to an exchange from Vhost to the exchange.

The idea came because of the need for the exchange to handle the messages differently for MQTT support but it has additional benefits like @queue_bindings and @exchange_bindings can have different types and as you may see I've used only one "bindings" for both queue and exchange. for example for FanoutExchange it's just a @bindings : Set(Destination)

There is probably some more things that should move down to each subclass to make the code clearer and to support the different types that @bindings has now.

HOW can this pull request be tested?

specs

snichme commented 1 month ago

@spuun @carlhoerberg any thoughts about always including "default" exchange when getting bindings for a queue heere: https://github.com/cloudamqp/lavinmq/blob/publish-to-exchange/src/lavinmq/vhost.cr#L286 instead of adding the default in the http controllers

carlhoerberg commented 1 month ago

Im fine with that

snichme commented 1 month ago

@spuun agree, early return is nicer. Fixed all now I thinkg

snichme commented 3 weeks ago

Specs fails because of this: https://github.com/cloudamqp/amqp-client.cr/pull/48