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

Multiple nested decorators when multiple services implement same generic interface #125

Closed
trapezoide opened this issue Aug 24, 2020 · 3 comments · Fixed by #126
Closed

Comments

@trapezoide
Copy link

First of all, thanks for sharing this library with all of us. It's pretty awesome!
I don't know if this is an issue or not, but I'm having unexpected results when I use the same decorator to decorate multiple (n) services that implement the same generic interface. It seems like the decorator gets called (n) times when I call one of the services instead of being called just once.
Let me explain...

I have three interfaces:

public interface IEvent
{
}

public interface IEventHandler<in TEvent> where TEvent : class, IEvent
{
    Task HandleAsync(TEvent @event);
}

// To prevent Decorator registration via Scan
public interface IHandlerDecorator
{
}

I have MyEvent (class) that implements IEvent and 3 MyEventXHandler services that implement IEventHandler to handle the event:

public sealed class MyEvent : IEvent
{}

internal sealed class MyEvent1Handler : IEventHandler<MyEvent>
{
    public Task HandleAsync(MyEvent @event)
    {
        return Task.CompletedTask;
    }
}

internal sealed class MyEvent2Handler : IEventHandler<MyEvent>
{
    public Task HandleAsync(MyEvent @event)
    {
        return Task.CompletedTask;
    }
}

internal sealed class MyEvent3Handler : IEventHandler<MyEvent>
{
    public Task HandleAsync(MyEvent @event)
    {
        return Task.CompletedTask;
    }
}

I have one decorator to be used on all of my services (these 3 and any other service I create in the future). Notice that future services will implement IEventHandler, but TEvent may or may not necessarily be of MyEvent type):

internal sealed class MyEventHandlerDecorator<TEvent> : IEventHandler<TEvent>, IHandlerDecorator where TEvent: class, IEvent
{
    #region Private Readonly Fields
    private readonly IEventHandler<TEvent> _handler;
    #endregion

    #region Constructors
    publicMyEventHandlerDecorator(
        IEventHandler<TEvent> handler
        )
    {
        this._handler = handler;
    }
    #endregion

    #region Public Methods
    public async Task HandleAsync(TEvent @event)
    {
        // Do something

        await _handler.HandleAsync(@event);
    }
    #endregion
}

I add the three services to IServiceCollection by scanning my assemblies:

        Services.Scan(s =>
            s.FromAssemblies(assembliesToScan)
                .AddClasses(c => c.Where(type =>
                {
                    // Add all Handlers that do not implement the IHandlerDecorator interface
                    bool isHandlerDecorator = false;

                    bool isHandler = type.GetInterfaces().Any(i =>
                          i.IsGenericType &&
                          i.GetGenericTypeDefinition() == typeof(IEventHandler<>)
                      );

                    if (isHandler)
                    {
                        isHandlerDecorator = type.GetInterfaces().Any(i => i == typeof(IHandlerDecorator));
                    }

                    return (isHandler && !isHandlerDecorator);
                }))
                //.AddClasses(c => c.AssignableTo(typeof(IEventHandler<>)))
                .AsImplementedInterfaces()
                .WithTransientLifetime());

Then add the decorator:
Services.TryDecorate(typeof(IEventHandler<>), typeof(MyEventHandlerDecorator<>));

When I call any of my services the decorator gets called three times in a row before getting into my service. It doesn't matter which service I call. I am expecting the decorator to be called just once, and then my service.
I went through Scrutor's code and believe I found out why it is behaving this way. Inside ServiceCollectionExtensions.Decoration.cs file there is a method:

    private static bool TryDecorateOpenGeneric(this IServiceCollection services, Type serviceType, Type decoratorType, [NotNullWhen(false)] out Exception? error)
    {
        var arguments = services
            .Where(x => !x.ServiceType.IsGenericTypeDefinition)
            .Where(x => IsSameGenericType(x.ServiceType, serviceType))
            .Select(x => x.ServiceType.GenericTypeArguments)
            .ToArray();

        if (arguments.Length == 0)
        {
            error = new MissingTypeRegistrationException(serviceType);
            return false;
        }

        foreach (var argument in arguments)
        {
            var closedServiceType = serviceType.MakeGenericType(argument);
            var closedDecoratorType = decoratorType.MakeGenericType(argument);

            if (!services.TryDecorateDescriptors(closedServiceType, out error, x => x.Decorate(closedDecoratorType)))
            {
                return false;
            }
        }

        error = default;
        return true;
    }

In the first line of code, where 'arguments' is being set, the following returns 3 items instead of one.
.Select(x => x.ServiceType.GenericTypeArguments)
The 3 items are of the same type, but when compared are not equal. I tried to use .Distinct() right after the select, but 'arguments' still contained the 3 same items. So what I did instead was to create my own DistinctByType() extension method to compare the items returned by GenericTypeArguments and return only the distinct types.

    public static IEnumerable<TSource> DistinctByType<TSource>(this IEnumerable<TSource> source)
    {
        HashSet<string> seenTypes = new HashSet<string>();

        foreach (TSource element in source)
        {
            if (element is null)
                continue;

            if (seenTypes.Add(element.GetType().FullName))
                yield return element;
        }
    }

and

        var arguments = services
            .Where(x => !x.ServiceType.IsGenericTypeDefinition)
            .Where(x => IsSameGenericType(x.ServiceType, serviceType))
            .Select(x => x.ServiceType.GenericTypeArguments)
            .DistinctByType()
            .ToArray();

This worked for me, but I don't know if it'll break anything else.
Please advise.
Thanks

@khellang
Copy link
Owner

Hey @trapezoide! 👋🏻

Thanks for this awesome issue with all the detailed information!

I've managed to reproduce your scenario and it does indeed look like a bug. I've submitted #126 and will merge and put out a new release as soon as the CI finishes.

@khellang
Copy link
Owner

Pushed v3.2.2 to NuGet. Thanks again! Let me know how if it fixes your issue 😄

@trapezoide
Copy link
Author

It worked like a charm!
Thanks for the prompt response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants