Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
brendonparker opened this issue Jul 26, 2024 · 3 comments
Open
2 tasks done
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue queued

Comments

@brendonparker
Copy link
Contributor

brendonparker commented Jul 26, 2024

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS.Messaging (or related) package versions

AWS.Messaging 0.9.1

Targeted .NET Platform

.NET 8

Operating System and version

AmazonLinux

@brendonparker brendonparker added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 26, 2024
@ashishdhingra ashishdhingra added needs-review and removed needs-triage This issue or PR still needs to be triaged. labels Jul 26, 2024
@ashishdhingra
Copy link
Contributor

ashishdhingra commented Jul 26, 2024

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>.

@ashishdhingra ashishdhingra added p2 This is a standard priority issue queued and removed needs-review labels Jul 26, 2024
@philasmar
Copy link
Contributor

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue queued
Projects
None yet
Development

No branches or pull requests

3 participants