corvus-dotnet / corvus-dotnet/Corvus.ContentHandling

If `IServiceCollection.AddContentTypeBasedSerializationSupport` is called prior to `IServiceCollection.AddContent`, the generic factory for content handler dispatchers will not be registered in the service collection

Open
#282 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
3
Forks
0
PR merge metrics
No merged PRs in 30d

Description

There are two means by which the instance of `ContentFactory` can be added to the `IServiceCollection`:
- `ContentHandlingJsonServiceCollectionExtensions.AddContentTypeBasedSerializationSupport`
- `ContentFactoryServiceCollectionExtensions.AddContent`

The first adds our implementation `IJsonSerializationSettingsProvider`, an instance of `ContentFactory` and our custom `JsonConverter` implementations, `ContentTypeConverter` and `ContentEnvelopeConverter`.

The second is used to add content types to the `ContentFactory`. It also registers the generic factory for content handler dispatchers (`IContentHandlerDispatcher<>` implemented by `ContentHandlerDispatcher<>`).

If during service container setup, `AddContentTypeBasedSerializationSupport` is called prior to `AddContent`, the generic factory for content handler dispatchers will not be registered due to the conditional logic in that method.

We should either:
1. Add extra conditional logic to `AddContent` to ensure the factory is registered even if the `ContentFactory` has already been added to the collection, or
2. Move the factory registration into its own method so that it can be explicitly added to the container if needed.

(My preference is for the second option, for two reasons:
- usage of our content type-based serialization is much more common than usage of our content handler dispatcher approach, so it makes sense to make it opt-in rather than automatically registering it with the serialization support.
- it is not at all obvious that you need to call `AddContent` in order to register the factory)

Note that whilst this is clearly a breaking change, I have not yet found anywhere we're actually using the `IContentHandlerDispatcher<>` so this isn't a huge issue.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at IServiceCollection.AddContentTypeBasedSerializationSupport and AddContent, then trace the conditional registration of IContentHandlerDispatcher<> implemented by ContentHandlerDispatcher<>. Resolve the registration approach and verify both service-registration call orders; done means the generic factory is available when the supported setup is used.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.