Skip to content

Commit

Permalink
Fix Issue#963: Partially decorating service implementing multiple int…
Browse files Browse the repository at this point in the history 
…erfaces throws (#1072)

* update test case for issue #963

* Add target component to be decorated when applying decorating process

* Refactor and remove unused method.
  • Loading branch information
@RaymondHuy
RaymondHuy committed Feb 6, 2020
1 parent cf3674a commit d485f39
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 44 deletions.
13 changes: 10 additions & 3 deletions src/Autofac/Core/ISharingLifetimeScope.cs
View file
Expand Up @@ -43,12 +43,19 @@ public interface ISharingLifetimeScope : ILifetimeScope
ISharingLifetimeScope? ParentLifetimeScope { get; }

/// <summary>
/// Try to retrieve an instance based on a GUID key. If the instance
/// does not exist, invoke <paramref name="creator"/> to create it.
/// Try to retrieve an instance based on a GUID key.
/// </summary>
/// <param name="id">Key to look up.</param>
/// <param name="value">The instance that has the specified key.</param>
/// <returns>true if the key was found; otherwise, false.</returns>
bool TryGetSharedInstance(Guid id, out object value);

/// <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 GetOrCreateAndShare(Guid id, Func<object> creator);
object CreateSharedInstance(Guid id, Func<object> creator);
}
}
24 changes: 16 additions & 8 deletions src/Autofac/Core/Lifetime/LifetimeScope.cs
View file
Expand Up @@ -290,30 +290,38 @@ public object ResolveComponent(ResolveRequest request)
public ISharingLifetimeScope RootLifetimeScope { get; }

/// <summary>
/// Try to retrieve an instance based on a GUID key. If the instance
/// does not exist, invoke <paramref name="creator"/> to create it.
/// Try to create an instance with a GUID key.
/// </summary>
/// <param name="id">Key to look up.</param>
/// <param name="id">Key.</param>
/// <param name="creator">Creation function.</param>
/// <returns>An instance.</returns>
public object GetOrCreateAndShare(Guid id, Func<object> creator)
public object CreateSharedInstance(Guid id, Func<object> creator)
{
if (creator == null) throw new ArgumentNullException(nameof(creator));

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

lock (_synchRoot)
{
if (_sharedInstances.TryGetValue(id, out result)) return result;
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;
}
}

return result;
/// <summary>
/// Try to retrieve an instance based on a GUID key.
/// </summary>
/// <param name="id">Key to look up.</param>
/// <param name="value">The instance that has the specified key.</param>
/// <returns>true if the key was found; otherwise, false.</returns>
public bool TryGetSharedInstance(Guid id, out object value)
{
return _sharedInstances.TryGetValue(id, out value);
}

/// <summary>
Expand Down
54 changes: 33 additions & 21 deletions src/Autofac/Core/Resolving/InstanceLookup.cs
View file
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,41 @@ 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));

var sharing = _decoratorTargetComponent != null
? _decoratorTargetComponent.Sharing
: ComponentRegistration.Sharing;

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

if (!_activationScope.TryGetSharedInstance(ComponentRegistration.Id, out _newInstance))
{
_newInstance = sharing == InstanceSharing.Shared
? _activationScope.CreateSharedInstance(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)
{
if (instance is IStartable startable
&& ComponentRegistration.Services.Any(s => (s is TypedService typed) && typed.ServiceType == typeof(IStartable))
Expand All @@ -111,24 +135,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 +162,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 +199,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
View file
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
View file
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
View file
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);
}

[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
12 changes: 6 additions & 6 deletions test/Autofac.Test/Features/Decorators/OpenGenericDecoratorTests.cs
View file
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

0 comments on commit d485f39

Please sign in to comment.