zarusz / SlimMessageBus

Lightweight message bus interface for .NET (pub/sub and request-response) with transport plugins for popular message brokers.
Apache License 2.0
475 stars 78 forks source link

v3: Remove one of DefaultPath / DefaultTopic #289

Open W1zzardTPU opened 3 months ago

W1zzardTPU commented 3 months ago

It's not worth it dragging both along in the code base forever

zarusz commented 3 months ago

So you suggest in v3 we leave the one DefaultPath() ?

The Path is more encompassing than the Topic or Queue name hence my tendency to leave Path terminology.

cc: @EtherZa

W1zzardTPU commented 3 months ago

Yeah I would lean slightly towards "Path", too, as it's more neutral and less bus-specific jargon

EtherZa commented 3 months ago

It may be worthwhile to have deeper look into the configuration as a whole in v3. I suspect a lot of it has come through organic growth (and perhaps operational bias) but there is generally a lot of repetition when configuring producers and consumers. This is exacerbated when a single queue is used for multiple message types resulting in a single consumer being created with multiple ConsumerSettings.

As an example, I use a local method similar to the one below to ease registration of messages/consumers being processed with shared subscriptions on Azure Service Bus.

void AddConsumer<TMessage, TConsumer>(string subscriptionName, int instances = 20, int prefetch = 100)
    where TConsumer : class, IConsumer<TMessage>
{
    bus.Produce<TMessage>(x => x.DefaultTopic(busOptions.Topic));
    bus.Consume<TMessage>(
        cfg =>
        {
            var type = typeof(TMessage);
            var typeName = resolver.ToName(type);
            cfg
                .Topic(busOptions.Topic)
                .SubscriptionName(subscriptionName)
                .Instances(instances)
                .PrefetchCount(prefetch)
                .WithConsumer<TConsumer>()
                .SubscriptionSqlFilter($"MessageType='{typeName}'", Hash(typeName));
        });
}

Each message that is registered in this way results in a new ConsumerBuilder being constructed even though the topic, subscription name, instance and prefetch count will be identical. In fact, if they were to differ, it would cause an issue with topology creation, etc. IMO it would be better to invert the logic here, such that the queue can be configured before configuring Produce and Consume. The same queue configuration should be used to configure all messages that it will be responsible for (or at least allow for a single instance to be referenced by each cfg => ...).

The current implementation is obviously succinct when each message is consumed by its own queue.

There are also other little niggles that make sense when you read through the code, but are not immediately obvious eg. mbb.AutoStartConsumersEnabled(false) being effective before mbb.AddChild(), but not afterwards.