awslabs / aws-dotnet-messaging

An AWS-native framework that simplifies the development of .NET message processing applications that use AWS services, such as SQS, SNS, and EventBridge.
https://awslabs.github.io/aws-dotnet-messaging/
Apache License 2.0
77 stars 15 forks source link

Better handling for when AddAWSMessageBus is invoked multiple times? #152

Open brendonparker opened 3 months ago

brendonparker commented 3 months ago

Describe the feature

When AddAWSMessageBus is invoked multiple times, previous IMessageConfiguration configurations are overwritten/lost. Specifically, the subscriber mappings.

https://github.com/awslabs/aws-dotnet-messaging/blob/main/src/AWS.Messaging/Configuration/MessageBusBuilder.cs#L311

Use Case

I'm attempting to register message handlers within various sub modules/assemblies, instead of needing to have the top most dependency responsible for all the registration.

Proposed Solution

Various ways this may be addressed...

Option 1: Merge in existing configuration (subscriber mappings) with new configuration on subsequent calls. Option 2: A separate API for adding message handlers (separate from AddAWSMessageBus). Option 3: Decide this is unsupported and make it more clear. Perhaps a runtime check to know if it has already been configured.

Other Information

No response

Acknowledgements

AWS.Messaging (or related) package versions

AWS.Messaging 0.9.1

Targeted .NET Platform

.NET 8

Operating System and version

AmazonLinux

ashishdhingra commented 3 months ago

Needs review with the team.

Going with the example mentioned at Dependency injection in ASP.NET Core > Service registration methods,

services.AddSingleton<IMyDependency, MyDependency>();
services.AddSingleton<IMyDependency, DifferentDependency>();

public class MyService
{
    public MyService(IMyDependency myDependency, 
       IEnumerable<IMyDependency> myDependencies)
    {
        Trace.Assert(myDependency is DifferentDependency);

        var dependencyArray = myDependencies.ToArray();
        Trace.Assert(dependencyArray[0] is MyDependency);
        Trace.Assert(dependencyArray[1] is DifferentDependency);
    }
}

AddSingleton is called twice with IMyDependency as the service type. The second call to AddSingleton overrides the previous one when resolved as IMyDependency and adds to the previous one when multiple services are resolved via IEnumerable<IMyDependency>.

philasmar commented 2 months ago

Hi @brendonparker, we have discussed this issue internally and we believe that Option 1 is the best option. We appreciate it if you could submit a PR that would implement Option 1 as that will help us prioritize this issue, review it and get it merged in. Thanks in advance!

brendonparker commented 2 months ago

There are a few different ways of accomplishing Option 1.

  1. Store MessageConfiguration in a static variable, so it can be re-used across calls to AddAWSMessageBus.
    • I've created a branch/PR to this end as it is the simplest approach.
  2. Move the MessageConfiguration down into some intermediate state, and then combine it at runtime (when resolving IMessageConfiguration using the pattern that @ashishdhingra outlined above by resolving all registrations and combining them.
  3. Total rehaul of MessageConfiguration to contruct based purely off of what is registered in the service collection. Hopefully I'm articulating that well enough. For example, the code would .AddSingleton<PublisherMapping> multiple times, rather than adding it to the in memory MessageConfiguration. Then at runtime resolve all registered PublisherMappings. So use the DI Container as the pool of registrations, rather than adding them to a list within the MessageConfiguration