antonyvorontsov / RabbitMQ.Client.Core.DependencyInjection

.Net Core library-wrapper of RabbitMQ.Client for Dependency Injection.
MIT License
111 stars 36 forks source link

Cannot create dead letter exchanges with a type different than "direct" #81

Closed sblasetti closed 3 years ago

sblasetti commented 3 years ago

Problem When configuring an exchange with a DeadLetterExchange defined in options, the library creates a dead letter exchange with type = "direct". In the example below, the exchange sample.dlx.exchange gets created as direct.

{
  "exchangeName": "sample.exchange",
  "isConsuming": false,
  "exchangeOptions": {
    "Type": "topic",
    "Durable": true,
    "AutoDelete": false,
    "DeadLetterExchange": "sample.dlx.exchange",
    "Queues": [ ]
  }
},

Expected Being able to define/parameterize the type of exchange for the dead letter exchange.

In master https://github.com/AntonyVorontsov/RabbitMQ.Client.Core.DependencyInjection/blob/a83f428e4ad4081da824d2d02c348ad71696fa77/src/RabbitMQ.Client.Core.DependencyInjection/Services/ChannelDeclarationService.cs#L107

In v4.3.0 https://github.com/AntonyVorontsov/RabbitMQ.Client.Core.DependencyInjection/blob/v4.3.0/src/RabbitMQ.Client.Core.DependencyInjection/Services/QueueService.cs#L388

sblasetti commented 3 years ago

Thinking on how this could be approached:

An initial strategy could be to add a parameter for the exchange type and then use it if defined instead of the default "direct":

{
  "exchangeName": "sample.exchange",
  "isConsuming": false,
  "exchangeOptions": {
    "Type": "topic",
    "Durable": true,
    "AutoDelete": false,
    "DeadLetterExchange": "sample.dlx.exchange",
    "DeadLetterExchangeType": "topic", <----
    "Queues": [ ]
  }
},

And then something like:

private static void StartDeadLetterExchange(IModel channel, string exchangeName, string exchangeType)
{
    channel.ExchangeDeclare(
        exchange: exchangeName,
        type: !string.IsNullOrWhiteSpace(exchangeType) ? exchangeType : "direct",
        durable: true,
        autoDelete: false,
        arguments: null);
}

But perhaps a better approach could be to check if the dead letter exchange is defined in the list of exchanges too and if so either use its configuration to create it or skip its creation in StartDeadLetterExchange(channel, exchangeName); and let the exchange to be created when iterating through the list of exchanges instead.

I can help coding this if you have a recommended approach. Please let me know.

antonyvorontsov commented 3 years ago

Hello Sebastian

Thank you for the suggestion, I appreciate it. I've just made a pull request, you can take a look at it. I am thinking right now about making validation for values provided via DeadLetterExchangeType and Type properties. Exchange declaration will just fail with an invalid value in the current implementation.

I am also trying to find free time to finish documentation for the newest version of this library (upcoming v.5.0.0). I can publish another pre-release library with this functionality in case you do not wanna wait for the documentation completion. Let me know what to do here.

Best regards, Antony

sblasetti commented 3 years ago

Thanks a lot for taking care of this, this will work for sure. It makes sense to validate the DeadLetterExchangeType to be one of the valid exchange types. I can wait until the new version comes out. Thanks again, this issue can be closed!

antonyvorontsov commented 3 years ago

This issue has been auto closed right after the linked pull request has been merged. I've just added simple validation for exchange types.