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

Microsoft.Extensions.DependencyInjection does not support co- and contravariance #82372

Open
tvardero opened this issue Feb 19, 2023 · 5 comments
Labels
area-Extensions-DependencyInjection enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@tvardero
Copy link

Description

Related: aspnet/DependencyInjection#453 (resolution: wont fix)
Related: jbogard/MediatR#819

Contravariant handler (see reproduction) is not resolved from MS.Ext.DI container.
If would be really usefull for CQRS applications that use mediator pattern.

For example, my project has a ton of inheritence for some models, and each model has CreateXXXCommand and UpdateXXXCommand. Currently, a workaround (see workaround) is required to register handlers for each concrete type of CreateXXXCommand and UpdateXXXCommand.

Reproduction Steps

using Microsoft.Extensions.DependencyInjection;

var serviceCollection = new ServiceCollection();

serviceCollection.AddTransient<ICommandHandler<PaintCarCommand>, PaintCarCommandHandler>();

using var serviceProvider = serviceCollection.BuildServiceProvider();

var anyColorHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarCommand>>(); // Does not throw
var redHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarRedCommand>>(); // Throws
var greenHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarGreenCommand>>(); // Throws

abstract record PaintCarCommand;

record PaintCarRedCommand : PaintCarCommand;

record PaintCarGreenCommand : PaintCarCommand;

interface ICommandHandler<in TCommand>
{
    void Handle(TCommand command);
}

class PaintCarCommandHandler : ICommandHandler<PaintCarCommand>
{
    public void Handle(PaintCarCommand command)
    {
        Console.WriteLine($"Command type: {command.GetType().Name}");
    }
}

Expected behavior

Red paint command is handled by PaintCarCommandHandler, as well as green command.

Actual behavior

Handler was not resolved for both red and green commands.

Regression?

No, it was never working for MS.Ext.DI.

Known Workarounds

Manual handler resolution is required. That means that handler should be resolved for base class of the command, recursively (if for example concrete command is grandchild of the base class, or even further).
Sometimes it might not be possible, if service resolution goes inside of 3rd party library (for example, MediatR) and you don't have a chance to resolute handler manually there.

Either way, for each concrete command, PaintCarCommandHandler should be registered as implementation of ICommandHandler<ConcretePaintCarCommand>. This either requires registering them by hand, or using reflection (Scrutor?) to register them automatically.

Just look at this beauty:

    private static void Workaround(IServiceCollection services)
    {
        RegisterFor<CreateFlowActionCommandHandler>(typeof(CreateDelayFlowActionCommand),
            typeof(CreateActivateWebhookFlowActionCommand),
            typeof(CreateChangeCheckinStatusFlowActionCommand),
            typeof(CreateChangeReservationStatusFlowActionCommand),
            typeof(CreateExecuteFlowFlowActionCommand),
            typeof(CreateSendChatMessageFlowActionCommand));

        RegisterFor<UpdateFlowActionCommandHandler>(typeof(UpdateDelayFlowActionCommand),
            typeof(UpdateActivateWebhookFlowActionCommand),
            typeof(UpdateChangeCheckinStatusFlowActionCommand),
            typeof(UpdateChangeReservationStatusFlowActionCommand),
            typeof(UpdateExecuteFlowFlowActionCommand),
            typeof(UpdateSendChatMessageFlowActionCommand));

        RegisterFor<CreateFlowConditionCommandHandler>(typeof(CreateWithConstantComparisonFlowConditionCommand),
            typeof(CreateWithVariableComparisonFlowConditionCommand),
            typeof(CreateCronExpressionFlowConditionCommand),
            typeof(CreateDateRangeFlowConditionCommand),
            typeof(CreateDayOfWeekFilterFlowConditionCommand),
            typeof(CreateTimeRangeFlowConditionCommand));

        RegisterFor<UpdateFlowConditionCommandHandler>(typeof(UpdateWithConstantComparisonFlowConditionCommand),
            typeof(UpdateWithVariableComparisonFlowConditionCommand),
            typeof(UpdateCronExpressionFlowConditionCommand),
            typeof(UpdateDateRangeFlowConditionCommand),
            typeof(UpdateDayOfWeekFilterFlowConditionCommand),
            typeof(UpdateTimeRangeFlowConditionCommand));

        RegisterFor<CreateFlowTriggerCommandHandler>(typeof(CreateNamedEventFlowTriggerCommand));

        RegisterFor<UpdateFlowTriggerCommandHandler>(typeof(UpdateNamedEventFlowTriggerCommand));

        void RegisterFor<TCommandHandler>(params Type[] commands)
        {
            foreach (var iHandler in commands.Select(command => typeof(INotificationHandler<>).MakeGenericType(command)))
                services.AddScoped(iHandler, typeof(TCommandHandler));
        }
    }

Configuration

.Net 7
MS.Ext.DI 7.0.*

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 19, 2023
@tvardero
Copy link
Author

tvardero commented Feb 19, 2023

Reposted from aspnet/DependencyInjection#453 because if it is a breaking change, then it could happen on .Net 8 release

@tvardero
Copy link
Author

tvardero commented Feb 19, 2023

Current:

Func<ServiceProviderEngineScope, object?> realizedService = _realizedServices.GetOrAdd(serviceType, _createServiceAccessor);

Proposed:

Func<ServiceProviderEngineScope, object?>? realizedService;
if (!_realizedServices.TryGetValue(serviceType, out realizedService)
{
    realizedService = _realizedServices
        .Where(kv => kv.Key.IsAssignableTo(serviceType))
        .Select(kv => kv.Value)
        .FirstOrDefault();

    realizedService ??= _createServiceAccessor;
    _realizedServices.TryAdd(serviceType, realizedService);
}

@ghost
Copy link

ghost commented Feb 19, 2023

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Related: aspnet/DependencyInjection#453 (resolution: wont fix)
Related: jbogard/MediatR#819

Contravariant handler (see reproduction) is not resolved from MS.Ext.DI container.
If would be really usefull for CQRS applications that use mediator pattern.

For example, my project has a ton of inheritence for some models, and each model has CreateXXXCommand and UpdateXXXCommand. Currently, a workaround (see workaround) is required to register handlers for each concrete type of CreateXXXCommand and UpdateXXXCommand.

Reproduction Steps

using Microsoft.Extensions.DependencyInjection;

var serviceCollection = new ServiceCollection();

serviceCollection.AddTransient<ICommandHandler<PaintCarCommand>, PaintCarCommandHandler>();

using var serviceProvider = serviceCollection.BuildServiceProvider();

var anyColorHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarCommand>>(); // Does not throw
var redHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarRedCommand>>(); // Throws
var greenHandler = serviceProvider.GetRequiredService<ICommandHandler<PaintCarGreenCommand>>(); // Throws

abstract record PaintCarCommand;

record PaintCarRedCommand : PaintCarCommand;

record PaintCarGreenCommand : PaintCarCommand;

interface ICommandHandler<in TCommand>
{
    void Handle(TCommand command);
}

class PaintCarCommandHandler : ICommandHandler<PaintCarCommand>
{
    public void Handle(PaintCarCommand command)
    {
        Console.WriteLine($"Command type: {command.GetType().Name}");
    }
}

Expected behavior

Red paint command is handled by PaintCarCommandHandler, as well as green command.

Actual behavior

Handler was not resolved for both red and green commands.

Regression?

No, it was never working for MS.Ext.DI.

Known Workarounds

Manual handler resolution is required. That means that handler should be resolved for base class of the command, recursively (if for example concrete command is grandchild of the base class, or even further).
Sometimes it might not be possible, if service resolution goes inside of 3rd party library (for example, MediatR) and you don't have a chance to resolute handler manually there.

Either way, for each concrete command, PaintCarCommandHandler should be registered as implementation of ICommandHandler<ConcretePaintCarCommand>. This either requires registering them by hand, or using reflection (Scrutor?) to register them automatically.

Just look at this beauty:

    private static void Workaround(IServiceCollection services)
    {
        RegisterFor<CreateFlowActionCommandHandler>(typeof(CreateDelayFlowActionCommand),
            typeof(CreateActivateWebhookFlowActionCommand),
            typeof(CreateChangeCheckinStatusFlowActionCommand),
            typeof(CreateChangeReservationStatusFlowActionCommand),
            typeof(CreateExecuteFlowFlowActionCommand),
            typeof(CreateSendChatMessageFlowActionCommand));

        RegisterFor<UpdateFlowActionCommandHandler>(typeof(UpdateDelayFlowActionCommand),
            typeof(UpdateActivateWebhookFlowActionCommand),
            typeof(UpdateChangeCheckinStatusFlowActionCommand),
            typeof(UpdateChangeReservationStatusFlowActionCommand),
            typeof(UpdateExecuteFlowFlowActionCommand),
            typeof(UpdateSendChatMessageFlowActionCommand));

        RegisterFor<CreateFlowConditionCommandHandler>(typeof(CreateWithConstantComparisonFlowConditionCommand),
            typeof(CreateWithVariableComparisonFlowConditionCommand),
            typeof(CreateCronExpressionFlowConditionCommand),
            typeof(CreateDateRangeFlowConditionCommand),
            typeof(CreateDayOfWeekFilterFlowConditionCommand),
            typeof(CreateTimeRangeFlowConditionCommand));

        RegisterFor<UpdateFlowConditionCommandHandler>(typeof(UpdateWithConstantComparisonFlowConditionCommand),
            typeof(UpdateWithVariableComparisonFlowConditionCommand),
            typeof(UpdateCronExpressionFlowConditionCommand),
            typeof(UpdateDateRangeFlowConditionCommand),
            typeof(UpdateDayOfWeekFilterFlowConditionCommand),
            typeof(UpdateTimeRangeFlowConditionCommand));

        RegisterFor<CreateFlowTriggerCommandHandler>(typeof(CreateNamedEventFlowTriggerCommand));

        RegisterFor<UpdateFlowTriggerCommandHandler>(typeof(UpdateNamedEventFlowTriggerCommand));

        void RegisterFor<TCommandHandler>(params Type[] commands)
        {
            foreach (var iHandler in commands.Select(command => typeof(INotificationHandler<>).MakeGenericType(command)))
                services.AddScoped(iHandler, typeof(TCommandHandler));
        }
    }

Configuration

.Net 7
MS.Ext.DI 7.0.*

Other information

No response

Author: tvardero
Assignees: -
Labels:

untriaged, area-Extensions-DependencyInjection

Milestone: -

@steveharter steveharter added the enhancement Product code improvement that does NOT require public API changes/additions label Feb 22, 2023
@steveharter
Copy link
Member

How is this different than the proposal in aspnet/DependencyInjection#453 that was rejected?

@buyaa-n buyaa-n added this to the Future milestone Feb 22, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 22, 2023
@tvardero
Copy link
Author

tvardero commented Feb 22, 2023

How is this different than the proposal in aspnet/DependencyInjection#453 that was rejected?

Probably not any different.
But it was back 2016, a lot of things changed, probably now it might be possible to implement the feature.

The feature to me seems not so hard to implement, and would probably be nice-to-have to every current CQRS project.

Also, AFAIU, it should not break other containers, since they are build on DependencyInjection.Abstractions nuget nowadays, not the DependencyInjection nuget itself. (Or I don't get what Eilon ment by saying that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-DependencyInjection enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants