Adding a simple switch to exclude unnecessary activations

src/AutofacSerilogIntegration/ContextualLoggingModule.cs

@@ -3,6 +3,7 @@
 using System.Reflection;
 using Autofac;
 using Autofac.Core;
+using Autofac.Core.Activators.ProvidedInstance;
 using Autofac.Core.Activators.Reflection;
 using Autofac.Core.Registration;
 using Serilog;
@@ -18,6 +19,7 @@ internal class ContextualLoggingModule : Module
         readonly bool _autowireProperties;
         readonly bool _skipRegistration;
         readonly bool _dispose;
+        readonly bool _alwaysSupplyParameter;
 
         [Obsolete("Do not use this constructor. This is required by the Autofac assembly scanning")]
         public ContextualLoggingModule()
@@ -26,11 +28,12 @@ public ContextualLoggingModule()
             _skipRegistration = true;
         }
 
-        internal ContextualLoggingModule(ILogger logger = null, bool autowireProperties = false, bool dispose = false)
+        internal ContextualLoggingModule(ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool alwaysSupplyParameter = false)
         {
             _logger = logger;
             _autowireProperties = autowireProperties;
             _dispose = dispose;
+            _alwaysSupplyParameter = alwaysSupplyParameter;
             _skipRegistration = false;
         }
 
@@ -94,41 +97,51 @@ protected override void AttachToComponentRegistration(IComponentRegistryBuilder
 
             PropertyInfo[] targetProperties = null;
 
-            var ra = registration.Activator as ReflectionActivator;
-            if (ra != null)
+            switch (registration.Activator)
             {
-                // As of Autofac v4.7.0 "FindConstructors" will throw "NoConstructorsFoundException" instead of returning an empty array
-                // See: https://github.com/autofac/Autofac/pull/895 & https://github.com/autofac/Autofac/issues/733
-                ConstructorInfo[] ctors;
-                try
-                {
-                    ctors = ra.ConstructorFinder.FindConstructors(ra.LimitType);
-                }
-                catch (Exception ex) when (ex.GetType().Name == "NoConstructorsFoundException") // Avoid needing to upgrade our Autofac reference to 4.7.0
-                {
-                    ctors = new ConstructorInfo[0];
-                }
-
-                var usesLogger =
-                    ctors.SelectMany(ctor => ctor.GetParameters()).Any(pi => pi.ParameterType == typeof (ILogger));
+                case ReflectionActivator ra:
+                    // As of Autofac v4.7.0 "FindConstructors" will throw "NoConstructorsFoundException" instead of returning an empty array
+                    // See: https://github.com/autofac/Autofac/pull/895 & https://github.com/autofac/Autofac/issues/733
+                    ConstructorInfo[] ctors;
+                    try
+                    {
+                        ctors = ra.ConstructorFinder.FindConstructors(ra.LimitType);
+                    }
+                    catch (NoConstructorsFoundException)
+                    {
+                        ctors = new ConstructorInfo[0];
+                    }
 
-                if (_autowireProperties)
-                {
-                    var logProperties = ra.LimitType
-                                            .GetRuntimeProperties()
-                                            .Where(c => c.CanWrite && c.PropertyType == typeof(ILogger) && c.SetMethod.IsPublic && !c.SetMethod.IsStatic)
-                                            .ToArray();
+                    var usesLogger =
+                        ctors.SelectMany(ctor => ctor.GetParameters()).Any(pi => pi.ParameterType == typeof(ILogger));
 
-                    if (logProperties.Any())
+                    if (_autowireProperties)
                     {
-                        targetProperties = logProperties;
-                        usesLogger = true;
+                        var logProperties = ra.LimitType
+                            .GetRuntimeProperties()
+                            .Where(c => c.CanWrite && c.PropertyType == typeof(ILogger) && c.SetMethod.IsPublic && !c.SetMethod.IsStatic)
+                            .ToArray();
+
+                        if (logProperties.Any())
+                        {
+                            targetProperties = logProperties;
+                            usesLogger = true;
+                        }
                     }
-                }
 
-                // Ignore components known to be without logger dependencies
-                if (!usesLogger)
+                    // Ignore components known to be without logger dependencies
+                    if (!usesLogger)
+                        return;
+                    break;
+                case ProvidedInstanceActivator _:
+                    //we cannot and should not affect provided instances
                     return;
+                default:
+                    //most likely a DelegateActivator - or a custom one
+                    if (_alwaysSupplyParameter)
+                        break;
+                    else
+                        return;
             }
 
             registration.Preparing += (sender, args) =>

src/AutofacSerilogIntegration/SerilogContainerBuilderExtensions.cs

@@ -19,11 +19,15 @@ public static class SerilogContainerBuilderExtensions
         /// <param name="autowireProperties">If true, properties on reflection-based components of type <see cref="ILogger"/> will
         /// be injected.</param>
         /// <param name="dispose"></param>
+        /// <param name="alwaysSupplyParameter">
+        /// If true, the parameter containing <see cref="ILogger"/> will be injected even when registration cannot be verified to use it,
+        /// such as <see cref="Autofac.Core.Activators.Delegate.DelegateActivator"/>.
+        /// </param>
         /// <returns>An object supporting method chaining.</returns>
-        public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, ILogger logger = null, bool autowireProperties = false, bool dispose = false)
+        public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool alwaysSupplyParameter = false)
         {
-            if (builder == null) throw new ArgumentNullException("builder");
-            return builder.RegisterModule(new ContextualLoggingModule(logger, autowireProperties, dispose));
+            if (builder == null) throw new ArgumentNullException(nameof(builder));
+            return builder.RegisterModule(new ContextualLoggingModule(logger, autowireProperties, dispose, alwaysSupplyParameter));
         }
     }
 }

test/AutofacSerilogIntegration.Tests/ActivatorTests.cs

@@ -0,0 +1,129 @@
+using System;
+using System.Linq;
+using Autofac;
+using Autofac.Core;
+using Moq;
+using Serilog;
+using Xunit;
+
+namespace AutofacSerilogIntegration.Tests
+{
+    public class ActivatorTests
+    {
+        private readonly Mock<ILogger> _logger;
+
+        public ActivatorTests()
+        {
+            _logger = new Mock<ILogger>();
+            _logger.SetReturnsDefault(_logger.Object);
+        }
+
+        private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureContainer, bool? alwaysSupplyParameter)
+        {
+            var containerBuilder = new ContainerBuilder();
+
+            if (alwaysSupplyParameter == null)
+                containerBuilder.RegisterLogger(_logger.Object);
+            else
+                containerBuilder.RegisterLogger(_logger.Object, alwaysSupplyParameter: alwaysSupplyParameter.Value);
+
+            containerBuilder.RegisterType<Component<TDependency>>();
+            configureContainer(containerBuilder);
+            using (var container = containerBuilder.Build())
+            {
+                container.Resolve<Component<TDependency>>();
+            }
+        }
+
+        private void VerifyLoggerCreation<TContext>(Func<Times> times)
+            => _logger.Verify(logger => logger.ForContext(typeof(TContext)), times);
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void ReflectionActivator_DependencyWithLogger_ShouldCreateLogger(bool? alwaysSupplyParameter)
+        {
+            ResolveInstance<DependencyWithLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithLogger>(), alwaysSupplyParameter);
+            VerifyLoggerCreation<DependencyWithLogger>(Times.AtLeastOnce);
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void ReflectionActivator_DependencyWithoutLogger_ShouldNotCreateLogger(bool? alwaysSupplyParameter)
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterType<DependencyWithoutLogger>(), alwaysSupplyParameter);
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void ProvidedInstanceActivator_ShouldNotCreateLogger(bool? alwaysSupplyParameter)
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.RegisterInstance(new DependencyWithoutLogger()), alwaysSupplyParameter);
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData(false)]
+        public void DelegateActivator_ShouldNotCreateLogger(bool? alwaysSupplyParameter)
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register(_ => new DependencyWithoutLogger()), alwaysSupplyParameter);
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        [Theory]
+        [InlineData(null)]
+        [InlineData(false)]
+        public void DelegateActivator_ShouldNotPassParameter(bool? alwaysSupplyParameter)
+        {
+            Parameter[] parameters = null;
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register((_, pp) =>
+            {
+                parameters = pp.ToArray();
+                return new DependencyWithoutLogger();
+            }), alwaysSupplyParameter);
+            Assert.NotNull(parameters);
+            Assert.Empty(parameters);
+        }
+
+        [Theory]
+        [InlineData(true)]
+        public void DelegateActivator_WhenForced_ShouldPassParameter(bool? alwaysSupplyParameter)
+        {
+            Parameter[] parameters = null;
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => containerBuilder.Register((_, pp) =>
+            {
+                parameters = pp.ToArray();
+                return new DependencyWithoutLogger();
+            }), alwaysSupplyParameter);
+            Assert.NotNull(parameters);
+            var value = Assert.Single(parameters
+                .OfType<TypedParameter>()
+                .Where(p => p.Type == typeof(ILogger))
+            )?.Value;
+            Assert.IsAssignableFrom<ILogger>(value);
+        }
+
+        private class Component<TDependency>
+        {
+            public Component(TDependency dependency)
+            {
+            }
+        }
+
+        private class DependencyWithLogger
+        {
+            public DependencyWithLogger(ILogger logger)
+            {
+            }
+        }
+
+        private class DependencyWithoutLogger {}
+    }
+}