pardahlman / RawRabbit

A modern .NET framework for communication over RabbitMq
MIT License
747 stars 144 forks source link

Added queue assume initialized and tests #163

Closed cemremengu closed 7 years ago

cemremengu commented 7 years ago

Description

Addresses #160

Check List

pardahlman commented 7 years ago

@cemremengu over all it looks great! I like the tear down etc. I've added three comments - let's discuss :)

pardahlman commented 7 years ago

Top of the morning, @cemremengu! I checkout your branch and run your test and I think I understood why you added AssumeInitialized in the full queue name prop.

By default, RawRabbit adds an application specific name suffix to queues. This is so that all consumer of a message, independent of application, is assumed to want to the published message. I've written about it in the documentation of multiple subscribers. Let me know if you need more clarification on the concept 👍

If you would update your test with AddSubscriberId like this:

client.SubscribeAsync<DynamicMessage>((message, context) =>
{
    tcs.TrySetResult(message);
    return Task.FromResult(true);
}, cfg => cfg
    .WithQueue(q => q
        .AssumeInitialized()
        .WithName(queueName))
        .WithSubscriberId(string.Empty)
    .WithExchange(e => e
        .AssumeInitialized()
        .WithName(exchangeName)));

Your are telling RawRabbit to not add any suffix, and your test succeeds without the AssumeInitialized.

While you're at it... would you mind looking over the tabs/spaces again 👼 . PublishAndSubscribeTests.cs is converted to spaces.

pardahlman commented 7 years ago

Perfect 👌