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

Make sure we don't re-decorate the same service types multiple times #126

Merged
merged 2 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions src/Scrutor/ServiceCollectionExtensions.Decoration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,29 +239,32 @@ private static IServiceCollection DecorateOpenGeneric(this IServiceCollection se
throw error;
}

private static bool IsSameGenericType(Type t1, Type t2)
private static bool HasSameTypeDefinition(Type t1, Type t2)
{
return t1.IsGenericType && t2.IsGenericType && t1.GetGenericTypeDefinition() == t2.GetGenericTypeDefinition();
}

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

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

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

var closedServiceType = serviceType.MakeGenericType(arguments);
var closedDecoratorType = decoratorType.MakeGenericType(arguments);

if (!services.TryDecorateDescriptors(closedServiceType, out error, x => x.Decorate(closedDecoratorType)))
{
Expand Down
114 changes: 114 additions & 0 deletions test/Scrutor.Tests/DecorationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,59 @@ public void DisposableServicesAreDisposed()
Assert.True(decorator.Inner.WasDisposed);
}

[Fact]
public void ServicesWithSameServiceTypeAreOnlyDecoratedOnce()
{
// See issue: https://github.com/khellang/Scrutor/issues/125

bool IsHandlerButNotDecorator(Type type)
{
var isHandlerDecorator = false;

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

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

return isHandler && !isHandlerDecorator;
}

var provider = ConfigureProvider(services =>
{
// This should end up with 3 registrations of type IEventHandler<MyEvent>.
services.Scan(s =>
s.FromAssemblyOf<DecorationTests>()
.AddClasses(c => c.Where(IsHandlerButNotDecorator))
.AsImplementedInterfaces()
.WithTransientLifetime());

// This should not decorate each registration 3 times.
services.Decorate(typeof(IEventHandler<>), typeof(MyEventHandlerDecorator<>));
});

var instances = provider.GetRequiredService<IEnumerable<IEventHandler<MyEvent>>>().ToList();

Assert.Equal(3, instances.Count);

Assert.All(instances, instance =>
{
var decorator = Assert.IsType<MyEventHandlerDecorator<MyEvent>>(instance);

// The inner handler should not be a decorator.
Assert.IsNotType<MyEventHandlerDecorator<MyEvent>>(decorator.Handler);

// The return call count should only be 1, we've only called Handle on one decorator.
// If there were nested decorators, this would return a higher call count as it
// would increment at each level.
Assert.Equal(1, decorator.Handle(new MyEvent()));
});
}

[Fact]
public void DecoratingNonRegisteredServiceThrows()
{
Expand Down Expand Up @@ -227,5 +280,66 @@ public void Dispose()
WasDisposed = true;
}
}

public interface IEvent
{
}

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

public interface IHandlerDecorator
{
}

public sealed class MyEvent : IEvent
{}

internal sealed class MyEvent1Handler : IEventHandler<MyEvent>
{
private int _callCount;

public int Handle(MyEvent @event)
{
return _callCount++;
}
}

internal sealed class MyEvent2Handler : IEventHandler<MyEvent>
{
private int _callCount;

public int Handle(MyEvent @event)
{
return _callCount++;
}
}

internal sealed class MyEvent3Handler : IEventHandler<MyEvent>
{
private int _callCount;

public int Handle(MyEvent @event)
{
return _callCount++;
}
}

internal sealed class MyEventHandlerDecorator<TEvent> : IEventHandler<TEvent>, IHandlerDecorator where TEvent: class, IEvent
{
public readonly IEventHandler<TEvent> Handler;

public MyEventHandlerDecorator(IEventHandler<TEvent> handler)
{
Handler = handler;
}

public int Handle(TEvent @event)
{
return Handler.Handle(@event) + 1;
}
}
}
}