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

Fix Issue#963: Partially decorating service implementing multiple interfaces throws #1072

Merged
merged 7 commits into from
Feb 6, 2020
15 changes: 15 additions & 0 deletions src/Autofac/Core/ISharingLifetimeScope.cs
Expand Up @@ -50,5 +50,20 @@ public interface ISharingLifetimeScope : ILifetimeScope
/// <param name="creator">Creation function.</param>
/// <returns>An instance.</returns>
object GetOrCreateAndShare(Guid id, Func<object> creator);

/// <summary>
/// Try to retrieve an instance based on a GUID key.
/// </summary>
/// <param name="id">Key to look up.</param>
/// <returns>An instance.</returns>
object Get(Guid id);
RaymondHuy marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Try to create an instance with a GUID key.
/// </summary>
/// <param name="id">Key.</param>
/// <param name="creator">Creation function.</param>
/// <returns>An instance.</returns>
object Create(Guid id, Func<object> creator);
}
}
35 changes: 35 additions & 0 deletions src/Autofac/Core/Lifetime/LifetimeScope.cs
Expand Up @@ -316,6 +316,41 @@ public object GetOrCreateAndShare(Guid id, Func<object> creator)
return result;
}

/// <summary>
/// Try to create an instance with a GUID key.
/// </summary>
/// <param name="id">Key.</param>
/// <param name="creator">Creation function.</param>
/// <returns>An instance.</returns>
public object Create(Guid id, Func<object> creator)
RaymondHuy marked this conversation as resolved.
Show resolved Hide resolved
{
if (creator == null) throw new ArgumentNullException(nameof(creator));

lock (_synchRoot)
{
if (_sharedInstances.TryGetValue(id, out var result)) return result;

result = creator();
if (_sharedInstances.ContainsKey(id))
throw new DependencyResolutionException(string.Format(CultureInfo.CurrentCulture, LifetimeScopeResources.SelfConstructingDependencyDetected, result.GetType().FullName));

_sharedInstances.TryAdd(id, result);

return result;
}
}

/// <summary>
/// Try to retrieve an instance based on a GUID key.
/// </summary>
/// <param name="id">Key to look up.</param>
/// <returns>An instance.</returns>
public object Get(Guid id)
{
_sharedInstances.TryGetValue(id, out var result);
return result;
}

/// <summary>
/// Gets the disposer associated with this container. Instances can be associated
/// with it manually if required.
Expand Down
56 changes: 35 additions & 21 deletions src/Autofac/Core/Resolving/InstanceLookup.cs
Expand Up @@ -40,6 +40,7 @@ internal class InstanceLookup : IComponentContext, IInstanceLookup
{
private readonly IResolveOperation _context;
private readonly ISharingLifetimeScope _activationScope;
private readonly IComponentRegistration? _decoratorTargetComponent;
private readonly Service _service;
private object? _newInstance;
private bool _executed;
Expand All @@ -52,6 +53,7 @@ internal class InstanceLookup : IComponentContext, IInstanceLookup
{
_context = context;
_service = request.Service;
_decoratorTargetComponent = request.DecoratorTarget;
ComponentRegistration = request.Registration;
Parameters = request.Parameters;

Expand Down Expand Up @@ -81,19 +83,43 @@ public object Execute()
_executed = true;

object? decoratorTarget = null;
object instance = ComponentRegistration.Sharing == InstanceSharing.None
? Activate(Parameters, out decoratorTarget)
: _activationScope.GetOrCreateAndShare(ComponentRegistration.Id, () => Activate(Parameters, out decoratorTarget));

alexmg marked this conversation as resolved.
Show resolved Hide resolved
var sharing = _decoratorTargetComponent != null
? _decoratorTargetComponent.Sharing
: ComponentRegistration.Sharing;

var resolveParameters = Parameters as Parameter[] ?? Parameters.ToArray();

_newInstance = _activationScope.Get(ComponentRegistration.Id);

if (_newInstance == null)
{
_newInstance = sharing == InstanceSharing.Shared
? _activationScope.Create(ComponentRegistration.Id, () => CreateInstance(Parameters))
: CreateInstance(Parameters);
}

decoratorTarget = _newInstance;

_newInstance = InstanceDecorator.TryDecorateRegistration(
_service,
ComponentRegistration,
_newInstance,
_activationScope,
resolveParameters);

if (_newInstance != decoratorTarget)
ComponentRegistration.RaiseActivating(this, resolveParameters, ref _newInstance);

var handler = InstanceLookupEnding;
handler?.Invoke(this, new InstanceLookupEndingEventArgs(this, NewInstanceActivated));

StartStartableComponent(decoratorTarget);

return instance;
return _newInstance;
}

private void StartStartableComponent(object? instance)
private void StartStartableComponent(object instance)
tillig marked this conversation as resolved.
Show resolved Hide resolved
{
if (instance is IStartable startable
&& ComponentRegistration.Services.Any(s => (s is TypedService typed) && typed.ServiceType == typeof(IStartable))
Expand All @@ -111,24 +137,17 @@ private void StartStartableComponent(object? instance)
private bool NewInstanceActivated => _newInstance != null;

[SuppressMessage("CA1031", "CA1031", Justification = "General exception gets rethrown in a PropagateActivationException.")]
private object Activate(IEnumerable<Parameter> parameters, out object decoratorTarget)
private object CreateInstance(IEnumerable<Parameter> parameters)
{
ComponentRegistration.RaisePreparing(this, ref parameters);

var resolveParameters = parameters as Parameter[] ?? parameters.ToArray();

try
{
decoratorTarget = _newInstance = ComponentRegistration.Activator.ActivateInstance(this, resolveParameters);
_newInstance = ComponentRegistration.Activator.ActivateInstance(this, resolveParameters);

ComponentRegistration.RaiseActivating(this, resolveParameters, ref _newInstance);

_newInstance = InstanceDecorator.TryDecorateRegistration(
_service,
ComponentRegistration,
_newInstance,
_activationScope,
resolveParameters);
}
catch (ObjectDisposedException)
{
Expand All @@ -145,19 +164,16 @@ private object Activate(IEnumerable<Parameter> parameters, out object decoratorT
// important. The ProvidedInstanceActivator will NOT dispose of the provided
// instance once the instance has been activated - assuming that it will be
// done during the lifetime scope's Disposer executing.
if (decoratorTarget is IDisposable instanceAsDisposable)
if (_newInstance is IDisposable instanceAsDisposable)
{
_activationScope.Disposer.AddInstanceForDisposal(instanceAsDisposable);
}
else if (decoratorTarget is IAsyncDisposable asyncDisposableInstance)
else if (_newInstance is IAsyncDisposable asyncDisposableInstance)
{
_activationScope.Disposer.AddInstanceForAsyncDisposal(asyncDisposableInstance);
}
}

if (_newInstance != decoratorTarget)
ComponentRegistration.RaiseActivating(this, resolveParameters, ref _newInstance);

return _newInstance;
}

Expand Down Expand Up @@ -185,8 +201,6 @@ public void Complete()
var beginningHandler = CompletionBeginning;
beginningHandler?.Invoke(this, new InstanceLookupCompletionBeginningEventArgs(this));

// _newInstance will definitely have been instantiated by the time
// this is called.
ComponentRegistration.RaiseActivated(this, Parameters, _newInstance!);

var endingHandler = CompletionEnding;
Expand Down
2 changes: 1 addition & 1 deletion src/Autofac/Features/Decorators/InstanceDecorator.cs
Expand Up @@ -69,7 +69,7 @@ internal static class InstanceDecorator
invokeParameters[invokeParameters.Length - 2] = serviceParameter;
invokeParameters[invokeParameters.Length - 1] = contextParameter;

var resolveRequest = new ResolveRequest(decoratorService, decoratorRegistration, invokeParameters);
var resolveRequest = new ResolveRequest(decoratorService, decoratorRegistration, invokeParameters, registration);
instance = context.ResolveComponent(resolveRequest);

if (index < decoratorCount - 1)
Expand Down
15 changes: 15 additions & 0 deletions src/Autofac/ResolveRequest.cs
Expand Up @@ -15,10 +15,23 @@ public class ResolveRequest
/// <param name="registration">The component registration for the service.</param>
/// <param name="parameters">The parameters used when resolving the service.</param>
public ResolveRequest(Service service, IComponentRegistration registration, IEnumerable<Parameter> parameters)
: this(service, registration, parameters, null)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="ResolveRequest"/> class.
/// </summary>
/// <param name="service">The service being resolved.</param>
/// <param name="registration">The component registration for the service.</param>
/// <param name="parameters">The parameters used when resolving the service.</param>
/// <param name="decoratorTarget">The target component to be decorated.</param>
public ResolveRequest(Service service, IComponentRegistration registration, IEnumerable<Parameter> parameters, IComponentRegistration? decoratorTarget = null)
{
Service = service;
Registration = registration;
Parameters = parameters;
DecoratorTarget = decoratorTarget;
}

/// <summary>
Expand All @@ -35,5 +48,7 @@ public ResolveRequest(Service service, IComponentRegistration registration, IEnu
/// Gets the parameters used when resolving the service.
/// </summary>
public IEnumerable<Parameter> Parameters { get; }

public IComponentRegistration? DecoratorTarget { get; }
}
}
15 changes: 10 additions & 5 deletions test/Autofac.Test/Features/Decorators/DecoratorTests.cs
Expand Up @@ -104,7 +104,7 @@ public void DecoratedInstancePerDependencyRegistrationCanIncludeOtherServices()
Assert.NotSame(serviceInstance, decoratedServiceInstance);
}

[Fact(Skip = "Issue #963")]
[Fact]
public void DecoratedInstancePerLifetimeScopeRegistrationCanIncludeOtherServices()
{
var builder = new ContainerBuilder();
Expand All @@ -128,10 +128,10 @@ public void DecoratedInstancePerLifetimeScopeRegistrationCanIncludeOtherServices
var decoratedServiceInstance = container.Resolve<IDecoratedService>();
Assert.IsType<DecoratorA>(decoratedServiceInstance);

Assert.Same(serviceInstance, decoratedServiceInstance);
Assert.Same(serviceInstance, decoratedServiceInstance.Decorated);
tillig marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact(Skip = "Issue #963")]
[Fact]
public void DecoratedSingleInstanceRegistrationCanIncludeOtherServices()
{
var builder = new ContainerBuilder();
Expand All @@ -149,8 +149,13 @@ public void DecoratedSingleInstanceRegistrationCanIncludeOtherServices()
Assert.NotNull(decoratedServiceRegistration);
Assert.Same(serviceRegistration, decoratedServiceRegistration);

Assert.IsType<ImplementorA>(container.Resolve<IService>());
Assert.IsType<DecoratorA>(container.Resolve<IDecoratedService>());
var serviceInstance = container.Resolve<IService>();
Assert.IsType<ImplementorA>(serviceInstance);

var decoratedServiceInstance = container.Resolve<IDecoratedService>();
Assert.IsType<DecoratorA>(decoratedServiceInstance);

Assert.Same(serviceInstance, decoratedServiceInstance.Decorated);
}

[Fact]
Expand Down
Expand Up @@ -222,7 +222,7 @@ public void DecoratedInstancePerDependencyRegistrationCanIncludeOtherServices()
Assert.NotSame(serviceInstance, decoratedServiceInstance);
}

[Fact(Skip = "Issue #963")]
[Fact]
public void DecoratedInstancePerLifetimeScopeRegistrationCanIncludeOtherServices()
{
var builder = new ContainerBuilder();
Expand All @@ -241,15 +241,15 @@ public void DecoratedInstancePerLifetimeScopeRegistrationCanIncludeOtherServices
Assert.Same(serviceRegistration, decoratedServiceRegistration);

var serviceInstance = container.Resolve<IService<int>>();
Assert.IsType<DecoratorA<int>>(serviceInstance);
Assert.IsType<ImplementorA<int>>(serviceInstance);

var decoratedServiceInstance = container.Resolve<IDecoratedService<int>>();
Assert.IsType<DecoratorA<int>>(decoratedServiceInstance);

Assert.Same(serviceInstance, decoratedServiceInstance);
Assert.Same(serviceInstance, decoratedServiceInstance.Decorated);
}

[Fact(Skip = "Issue #963")]
[Fact]
public void DecoratedSingleInstanceRegistrationCanIncludeOtherServices()
{
var builder = new ContainerBuilder();
Expand All @@ -268,12 +268,12 @@ public void DecoratedSingleInstanceRegistrationCanIncludeOtherServices()
Assert.Same(serviceRegistration, decoratedServiceRegistration);

var serviceInstance = container.Resolve<IService<int>>();
Assert.IsType<DecoratorA<int>>(serviceInstance);
Assert.IsType<ImplementorA<int>>(serviceInstance);

var decoratedServiceInstance = container.Resolve<IDecoratedService<int>>();
Assert.IsType<DecoratorA<int>>(decoratedServiceInstance);

Assert.Same(serviceInstance, decoratedServiceInstance);
Assert.Same(serviceInstance, decoratedServiceInstance.Decorated);
}

[Fact]
Expand Down