Skip to content

Commit

Permalink
Resoolved #965: Do not apply the decorator if already applied, or if …
Browse files Browse the repository at this point in the history 
…the registration is for an adapter.
  • Loading branch information
@alexmg
alexmg committed Mar 20, 2019
1 parent 641379c commit 7aa556a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
3 changes: 2 additions & 1 deletion Autofac.sln.DotSettings
View file
Expand Up @@ -19,4 +19,5 @@
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Autofac/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Autofac/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Startable/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
8 changes: 7 additions & 1 deletion src/Autofac/Features/Decorators/InstanceDecorator.cs
View file
Expand Up @@ -26,6 +26,7 @@
using System.Collections.Generic;
using System.Linq;
using Autofac.Core;
using Autofac.Core.Registration;

namespace Autofac.Features.Decorators
{
Expand All @@ -37,6 +38,11 @@ internal static class InstanceDecorator
IComponentContext context,
IEnumerable<Parameter> parameters)
{
var instanceType = instance.GetType();

// Issue #965: Do not apply the decorator if already applied, or if the registration is for an adapter.
if (registration.IsAdapting() || registration.Activator.LimitType != instanceType) return instance;

var decoratorRegistrations = context.ComponentRegistry.DecoratorsFor(registration);

// ReSharper disable once PossibleMultipleEnumeration
Expand All @@ -56,7 +62,7 @@ internal static class InstanceDecorator
var serviceType = decorators[0].Service.ServiceType;
var resolveParameters = parameters as Parameter[] ?? parameters.ToArray();

var decoratorContext = DecoratorContext.Create(instance.GetType(), serviceType, instance);
var decoratorContext = DecoratorContext.Create(instanceType, serviceType, instance);

foreach (var decorator in decorators)
{
Expand Down
26 changes: 19 additions & 7 deletions test/Autofac.Specification.Test/Features/DecoratorTests.cs
View file
Expand Up @@ -98,7 +98,9 @@ public void CanResolveDecoratorWithFunc()

var factory = container.Resolve<Func<IDecoratedService>>();

Assert.IsType<DecoratorA>(factory());
var decoratedService = factory();
Assert.IsType<DecoratorA>(decoratedService);
Assert.IsType<ImplementorA>(decoratedService.Decorated);
}

[Fact]
Expand All @@ -111,7 +113,9 @@ public void CanResolveDecoratorWithLazy()

var lazy = container.Resolve<Lazy<IDecoratedService>>();

Assert.IsType<DecoratorA>(lazy.Value);
var decoratedService = lazy.Value;
Assert.IsType<DecoratorA>(decoratedService);
Assert.IsType<ImplementorA>(decoratedService.Decorated);
}

[Fact]
Expand Down Expand Up @@ -259,13 +263,12 @@ public void DecoratorAndDecoratedBothDisposedWhenSingleInstance()
Assert.Equal(1, decorated.DisposeCallCount);
}

[Fact(Skip = "Issue #963: Need to figure out how to track which decorators have already been applied.")]
public void DecoratorAppliedOnlyOnceToComponent()
[Fact]
public void DecoratorAppliedOnlyOnceToComponentWithExternalRegistrySource()
{
// #965: A nested lifetime scope that has a registration lambda
// causes the decorator to be applied twice - once for the container
// level, once for the scope level. This doesn't seem to happen
// if there's no registration lambda.
// level, and once for the scope level.
var builder = new ContainerBuilder();
builder.RegisterType<ImplementorA>().As<IDecoratedService>();
builder.RegisterDecorator<DecoratorA, IDecoratedService>();
Expand All @@ -288,6 +291,7 @@ public void DecoratorCanBeAppliedToServiceRegisteredInChildLifetimeScope()
var instance = scope.Resolve<IDecoratedService>();

Assert.IsType<DecoratorA>(instance);
Assert.IsType<ImplementorA>(instance.Decorated);
}

[Fact]
Expand All @@ -301,6 +305,7 @@ public void DecoratorCanBeRegisteredInChildLifetimeScope()

var scopedInstance = scope.Resolve<IDecoratedService>();
Assert.IsType<DecoratorA>(scopedInstance);
Assert.IsType<ImplementorA>(scopedInstance.Decorated);

var rootInstance = container.Resolve<IDecoratedService>();
Assert.IsType<ImplementorA>(rootInstance);
Expand Down Expand Up @@ -602,7 +607,7 @@ private abstract class Decorator : IDecoratedService
{
protected Decorator(IDecoratedService decorated)
{
this.Decorated = decorated;
Decorated = decorated;
}

public IDecoratedService Decorated { get; }
Expand Down Expand Up @@ -635,6 +640,7 @@ public DecoratorWithContextA(IDecoratedService decorated, IDecoratorContext cont
public IDecoratorContext Context { get; }
}

// ReSharper disable once ClassNeverInstantiated.Local
private class DecoratorWithContextB : Decorator, IDecoratorWithContext
{
public DecoratorWithContextB(IDecoratedService decorated, IDecoratorContext context)
Expand All @@ -657,6 +663,7 @@ public DecoratorWithParameter(IDecoratedService decorated, string parameter)
public string Parameter { get; }
}

// ReSharper disable once ClassNeverInstantiated.Local
private class DisposableDecorator : Decorator, IDisposable
{
public DisposableDecorator(IDecoratedService decorated)
Expand All @@ -672,6 +679,7 @@ public void Dispose()
}
}

// ReSharper disable once ClassNeverInstantiated.Local
private class DisposableImplementor : IDecoratedService, IDisposable
{
public IDecoratedService Decorated => this;
Expand All @@ -689,11 +697,13 @@ private class ImplementorA : IDecoratedService
public IDecoratedService Decorated => this;
}

// ReSharper disable once ClassNeverInstantiated.Local
private class ImplementorB : IDecoratedService
{
public IDecoratedService Decorated => this;
}

// ReSharper disable once ClassNeverInstantiated.Local
private class ImplementorWithParameters : IDecoratedService
{
public ImplementorWithParameters(string parameter)
Expand All @@ -706,11 +716,13 @@ public ImplementorWithParameters(string parameter)
public string Parameter { get; }
}

// ReSharper disable once ClassNeverInstantiated.Local
private class ImplementorWithSomeOtherService : IDecoratedService, ISomeOtherService
{
public IDecoratedService Decorated => this;
}

// ReSharper disable once ClassNeverInstantiated.Local
private class StartableImplementation : IDecoratedService, IStartable
{
public IDecoratedService Decorated => this;
Expand Down
28 changes: 26 additions & 2 deletions test/Autofac.Test/Features/Decorators/OpenGenericDecoratorTests.cs
View file
Expand Up @@ -14,6 +14,7 @@ public interface IService<T>
{
}

// ReSharper disable once UnusedTypeParameter
public interface ISomeOtherService<T>
{
}
Expand Down Expand Up @@ -295,7 +296,9 @@ public void CanResolveDecoratorWithFunc()

var factory = container.Resolve<Func<IDecoratedService<int>>>();

Assert.IsType<DecoratorA<int>>(factory());
var decoratedService = factory();
Assert.IsType<DecoratorA<int>>(decoratedService);
Assert.IsType<ImplementorA<int>>(decoratedService.Decorated);
}

[Fact]
Expand All @@ -308,7 +311,9 @@ public void CanResolveDecoratorWithLazy()

var lazy = container.Resolve<Lazy<IDecoratedService<int>>>();

Assert.IsType<DecoratorA<int>>(lazy.Value);
var decoratedService = lazy.Value;
Assert.IsType<DecoratorA<int>>(decoratedService);
Assert.IsType<ImplementorA<int>>(decoratedService.Decorated);
}

[Fact]
Expand Down Expand Up @@ -528,6 +533,23 @@ public void CanResolveClosedGenericDecoratorOverOpenGeneric()
Assert.IsType<ImplementorA<string>>(instance.Decorated);
}

[Fact]
public void DecoratorAppliedOnlyOnceToComponentWithExternalRegistrySource()
{
// #965: A nested lifetime scope that has a registration lambda
// causes the decorator to be applied twice - once for the container
// level, and once for the scope level.
var builder = new ContainerBuilder();
builder.RegisterGeneric(typeof(ImplementorA<>)).As(typeof(IDecoratedService<>));
builder.RegisterGenericDecorator(typeof(DecoratorA<>), typeof(IDecoratedService<>));
var container = builder.Build();

var scope = container.BeginLifetimeScope(b => { });
var service = scope.Resolve<IDecoratedService<int>>();
Assert.IsType<DecoratorA<int>>(service);
Assert.IsType<ImplementorA<int>>(service.Decorated);
}

[Fact]
public void DecoratorCanBeAppliedToServiceRegisteredInChildLifetimeScope()
{
Expand All @@ -539,6 +561,7 @@ public void DecoratorCanBeAppliedToServiceRegisteredInChildLifetimeScope()
var instance = scope.Resolve<IDecoratedService<int>>();

Assert.IsType<DecoratorA<int>>(instance);
Assert.IsType<ImplementorA<int>>(instance.Decorated);
}

[Fact]
Expand All @@ -552,6 +575,7 @@ public void DecoratorCanBeRegisteredInChildLifetimeScope()

var scopedInstance = scope.Resolve<IDecoratedService<int>>();
Assert.IsType<DecoratorA<int>>(scopedInstance);
Assert.IsType<ImplementorA<int>>(scopedInstance.Decorated);

var rootInstance = container.Resolve<IDecoratedService<int>>();
Assert.IsType<ImplementorA<int>>(rootInstance);
Expand Down

0 comments on commit 7aa556a

Please sign in to comment.