Merge pull request #1484 from autofac/feature/issue-1459

Fix decorator parameter selection for derived-service dependencies (#1459)

Author: tillig

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ repos:
       - id: end-of-file-fixer
       - id: trailing-whitespace
   - repo: https://github.com/igorshubovych/markdownlint-cli
-    rev: "e72a3ca1632f0b11a07d171449fe447a7ff6795e"  # frozen: v0.48.0
+    rev: "c7c1c7640e610068e8e4754e9f1bf109bd987dc7"  # post-v0.48.0 with patches
     hooks:
       - id: markdownlint
         args:

default.proj

@@ -2,7 +2,7 @@
 <Project DefaultTargets="All" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="Current">
   <PropertyGroup>
     <!-- Increment the overall semantic version here. -->
-    <Version>9.1.0</Version>
+    <Version>9.1.1</Version>
     <SolutionName>Autofac</SolutionName>
     <Configuration Condition="'$(Configuration)'==''">Release</Configuration>
     <ArtifactDirectory>$([System.IO.Path]::Combine($(MSBuildProjectDirectory),"artifacts"))</ArtifactDirectory>

src/Autofac/Features/Decorators/DecoratorMiddleware.cs

@@ -85,10 +85,21 @@ public void Execute(ResolveRequestContext context, Action<ResolveRequestContext>
         // decorators assume an exact typed parameter but the multi-service
         // registered decorators need the resolved version that can determine
         // compatibility by casting.
-        var typedServiceParameter = new TypedParameter(serviceType, context.DecoratorContext.CurrentInstance);
+        //
+        // Issue 1459: The compatible parameter must only supply the decorated
+        // instance to constructor parameters that the instance can actually be
+        // assigned to. A parameter typed as a more derived service than the one
+        // being decorated (for example, a sub-interface) satisfies
+        // IsAssignableFrom on the parameter type, but the decorated instance may
+        // not actually be of that more derived type. In that case the parameter
+        // should fall through to normal autowiring rather than receiving the
+        // decorated instance (which would otherwise throw an InvalidCastException).
+        var currentInstance = context.DecoratorContext.CurrentInstance;
+        var typedServiceParameter = new TypedParameter(serviceType, currentInstance);
         var compatibleServiceParameter = new ResolvedParameter(
-            (pi, ctx) => serviceType.IsAssignableFrom(pi.ParameterType),
-            (pi, ctx) => context.DecoratorContext.CurrentInstance);
+            (pi, ctx) => serviceType.IsAssignableFrom(pi.ParameterType)
+                && pi.ParameterType.IsInstanceOfType(currentInstance),
+            (pi, ctx) => currentInstance);
         var contextParameter = new TypedParameter(typeof(IDecoratorContext), context.DecoratorContext);
 
         Parameter[] resolveParameters;

test/Autofac.Test/Features/Decorators/DecoratorTests.cs

@@ -225,6 +225,125 @@ public void DecorateProvidedInstanceActivatorWithPropertyInjection()
         Assert.True(service.NestedServiceIsNotNull());
     }
 
+    // Issue 1459: A decorator constructor may take a dependency typed as a more
+    // derived service than the one being decorated. The decorated instance must
+    // not be force-injected into that parameter; it should be resolved normally.
+    private interface IBase
+    {
+    }
+
+    private interface IDerived : IBase
+    {
+    }
+
+    private class BaseImpl : IBase
+    {
+    }
+
+    private class DerivedImpl : IDerived
+    {
+    }
+
+    private class DerivedDependencyDecorator : IBase
+    {
+        public DerivedDependencyDecorator(IDerived derived, IBase decorated)
+        {
+            Derived = derived;
+            Decorated = decorated;
+        }
+
+        public IDerived Derived
+        {
+            get;
+        }
+
+        public IBase Decorated
+        {
+            get;
+        }
+    }
+
+    private class BaseDecorator : IBase
+    {
+        public BaseDecorator(IBase decorated)
+        {
+            Decorated = decorated;
+        }
+
+        public IBase Decorated
+        {
+            get;
+        }
+    }
+
+    [Fact]
+    public void DecoratorWithMoreDerivedServiceDependencyResolvesDependencyNormally()
+    {
+        // Issue 1459: The decorated service (IBase) should be supplied to the
+        // "Decorated" parameter, while the more-derived "Derived" (IDerived)
+        // parameter must be resolved from the container rather than receiving
+        // the decorated IBase instance (which is not an IDerived).
+        var builder = new ContainerBuilder();
+        builder.RegisterType<BaseImpl>().As<IBase>();
+        builder.RegisterType<DerivedImpl>().As<IDerived>();
+        builder.RegisterDecorator<DerivedDependencyDecorator, IBase>();
+
+        var container = builder.Build();
+
+        var resolved = container.Resolve<IBase>();
+
+        var decorator = Assert.IsType<DerivedDependencyDecorator>(resolved);
+        Assert.IsType<BaseImpl>(decorator.Decorated);
+        Assert.IsType<DerivedImpl>(decorator.Derived);
+    }
+
+    [Fact]
+    public void DecoratorWithMoreDerivedServiceDependencyResolvesDependencyNormallyInChain()
+    {
+        // Issue 1459: When decorators are chained, the decorated instance seen by
+        // the outer decorator is the inner decorator's output (via
+        // DecoratorContext.UpdateContext), which is an IBase but not an IDerived.
+        // The outer decorator's more-derived "Derived" (IDerived) parameter must
+        // still be resolved from the container rather than receiving that chained
+        // instance, while "Decorated" receives the inner decorator.
+        var builder = new ContainerBuilder();
+        builder.RegisterType<BaseImpl>().As<IBase>();
+        builder.RegisterType<DerivedImpl>().As<IDerived>();
+
+        // Registered first => innermost decorator.
+        builder.RegisterDecorator<BaseDecorator, IBase>();
+        builder.RegisterDecorator<DerivedDependencyDecorator, IBase>();
+
+        var container = builder.Build();
+
+        var resolved = container.Resolve<IBase>();
+
+        var outer = Assert.IsType<DerivedDependencyDecorator>(resolved);
+        Assert.IsType<DerivedImpl>(outer.Derived);
+
+        var inner = Assert.IsType<BaseDecorator>(outer.Decorated);
+        Assert.IsType<BaseImpl>(inner.Decorated);
+    }
+
+    [Fact]
+    public void DecoratorWithUnregisteredMoreDerivedServiceDependencyThrowsResolutionException()
+    {
+        // Issue 1459: When the more-derived "Derived" (IDerived) parameter is not
+        // registered, the parameter falls through to normal autowiring, which
+        // cannot satisfy it. The result should be a clean DependencyResolutionException
+        // rather than the previous InvalidCastException from force-injecting the
+        // decorated instance.
+        var builder = new ContainerBuilder();
+        builder.RegisterType<BaseImpl>().As<IBase>();
+
+        // IDerived is intentionally not registered.
+        builder.RegisterDecorator<DerivedDependencyDecorator, IBase>();
+
+        var container = builder.Build();
+
+        Assert.Throws<DependencyResolutionException>(() => container.Resolve<IBase>());
+    }
+
     private abstract class Decorator : IDecoratedService
     {
         protected Decorator(IDecoratedService decorated)

test/Autofac.Test/Features/Decorators/OpenGenericDecoratorTests.cs

@@ -923,6 +923,62 @@ public void ResolvesMultipleDecoratedServicesWhenResolvedByOtherServices()
             });
     }
 
+    // Issue 1459: A decorator constructor may take a dependency typed as a more
+    // derived service than the one being decorated. The decorated instance must
+    // not be force-injected into that parameter; it should be resolved normally.
+    // ReSharper disable once UnusedTypeParameter
+    private interface IDerivedService<T> : IService<T>
+    {
+    }
+
+    private class PlainService<T> : IService<T>
+    {
+    }
+
+    private class DerivedServiceImpl<T> : IDerivedService<T>
+    {
+    }
+
+    private class DerivedDependencyDecorator<T> : IService<T>
+    {
+        public DerivedDependencyDecorator(IDerivedService<T> derived, IService<T> decorated)
+        {
+            Derived = derived;
+            Decorated = decorated;
+        }
+
+        public IDerivedService<T> Derived
+        {
+            get;
+        }
+
+        public IService<T> Decorated
+        {
+            get;
+        }
+    }
+
+    [Fact]
+    public void DecoratorWithMoreDerivedServiceDependencyResolvesDependencyNormally()
+    {
+        // Issue 1459: The decorated service (IService<T>) should be supplied to
+        // the "Decorated" parameter, while the more-derived "Derived"
+        // (IDerivedService<T>) parameter must be resolved from the container
+        // rather than receiving the decorated instance (which is only an
+        // IService<T> here, not an IDerivedService<T>).
+        var builder = new ContainerBuilder();
+        builder.RegisterGeneric(typeof(PlainService<>)).As(typeof(IService<>));
+        builder.RegisterGeneric(typeof(DerivedServiceImpl<>)).As(typeof(IDerivedService<>));
+        builder.RegisterGenericDecorator(typeof(DerivedDependencyDecorator<>), typeof(IService<>));
+        var container = builder.Build();
+
+        var resolved = container.Resolve<IService<int>>();
+
+        var decorator = Assert.IsType<DerivedDependencyDecorator<int>>(resolved);
+        Assert.IsType<PlainService<int>>(decorator.Decorated);
+        Assert.IsType<DerivedServiceImpl<int>>(decorator.Derived);
+    }
+
     private interface ICommandHandler<T>
     {
         void Handle(T command);