BrighterCommand
Bug: DynamoDbOutbox Archiver Missing Topics
Describe the bug
Current implementation depends on DynamoDbOutbox._topicName being populated as messages are added to the outbox. The archiver is trying to find messages through the async outbox but the sync version is used when adding to the outbox.
services
.AddBrighter()
.UseExternalBus(...)
.AutoFromAssemblies()
.UseDynamoDbOutbox() <---- registers 3 instances of the outbox, IAmAnOutbox, IAmAnOutboxSync, IAmAnOutboxAsync
.UseOutboxSweeper() <---- uses non-async outbox
.UseOutboxArchiver(new NullOutboxArchiveProvider()); <---- uses async outbox
Proposed fix
Short term
Make DynamoDbOutbox._topicNames static
Long term Maybe it should be possible to derive topic names from publications so that the archival process can run without needing any messages added first
Further technical details
- Brighter v9
I can't see a good reason for having three instances of the outbox registered, so to me the most sensible option would be to ensure only a single instance of the dynamo outbox is registered for all three interfaces.
One possible issue there is that if you create a DynamoDbOutbox instance inside UseDynamoDbOutbox and add that instance directly, then the method can't be called until IAmazonDynamoDB has been registered, instead of just relying that it's going to be registered at some point. So perhaps, then, BuildDynamoDbOutbox should, instead of automatically creating a new outbox instance, try to retrieve the implementation of IAmAnOutbox and then return that instance if it exists. Otherwise return a new instance.
Maybe it should be possible to derive topic names from publications so that the archival process can run without needing any messages added first
In V10 we have the Topic on the Publication, we could backport this to V9, as it is additive and not a breaking change.
I can't see a good reason for having three instances of the outbox registered, so to me the most sensible option would be to ensure only a single instance of the dynamo outbox is registered for all three interfaces. Agreed, that is an error. It implements all three for us.
BuildDynamoDbOutbox should, instead of automatically creating a new outbox instance, try to retrieve the implementation of IAmAnOutbox and then return that instance if it exists. Otherwise return a new instance. Yeah, it sounds reasonable that we don't rebuild if we have built it once already. Effectively it is a singleton. In fact, ExternalBusService pretty much enforces that.
@iancooper the merged PR was for v9. I had a look at the v10 master branch and I can't find UseDynamoDbOutbox there