nblumhardt#34: providing strategy in addition to a bool flag

src/AutofacSerilogIntegration/ActivatorExtensions.cs

@@ -0,0 +1,55 @@
+using System;
+using System.Linq;
+using System.Reflection;
+using Autofac.Core;
+using Autofac.Core.Activators.Reflection;
+using Serilog;
+
+namespace AutofacSerilogIntegration
+{
+    internal static class ActivatorExtensions
+    {
+        internal static bool TryFindLoggerDependencies(this IInstanceActivator activator, bool inspectProperties, out bool injectParameter, out PropertyInfo[] targetProperties)
+        {
+            injectParameter = false;
+            targetProperties = null;
+            switch (activator)
+            {
+                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 (Exception ex) when (ex.GetType().Name == "NoConstructorsFoundException"
+                    ) // Avoid needing to upgrade our Autofac reference to 4.7.0
+                    {
+                        ctors = new ConstructorInfo[0];
+                    }
+
+                    injectParameter = ctors.SelectMany(ctor => ctor.GetParameters())
+                        .Any(pi => pi.ParameterType == typeof(ILogger));
+
+                    if (inspectProperties)
+                    {
+                        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;
+                        }
+                    }
+
+                    return true;
+                default:
+                    return false;
+            }
+        }
+    }
+}

src/AutofacSerilogIntegration/ContextualLoggingModule.cs

@@ -1,12 +1,9 @@
 using System;
 using System.Linq;
-using System.Reflection;
 using Autofac;
 using Autofac.Core;
-using Autofac.Core.Activators.Reflection;
 using Autofac.Core.Registration;
 using Serilog;
-using Module = Autofac.Module;
 
 namespace AutofacSerilogIntegration
 {
@@ -15,10 +12,9 @@ internal class ContextualLoggingModule : Module
         const string TargetTypeParameterName = "Autofac.AutowiringPropertyInjector.InstanceType";
 
         readonly ILogger _logger;
-        readonly bool _autowireProperties;
+        readonly IRegistrationProcessor _registrationProcessor;
         readonly bool _skipRegistration;
         readonly bool _dispose;
-        readonly bool _onlyKnownConsumers;
 
         [Obsolete("Do not use this constructor. This is required by the Autofac assembly scanning")]
         public ContextualLoggingModule()
@@ -27,12 +23,11 @@ public ContextualLoggingModule()
             _skipRegistration = true;
         }
 
-        internal ContextualLoggingModule(ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool onlyKnownConsumers = false)
+        internal ContextualLoggingModule(IRegistrationProcessor registrationProcessor, ILogger logger, bool dispose)
         {
             _logger = logger;
-            _autowireProperties = autowireProperties;
+            _registrationProcessor = registrationProcessor;
             _dispose = dispose;
-            _onlyKnownConsumers = onlyKnownConsumers;
             _skipRegistration = false;
         }
 
@@ -94,60 +89,24 @@ protected override void AttachToComponentRegistration(IComponentRegistryBuilder
             if (registration.Services.OfType<TypedService>().Any(ts => ts.ServiceType == typeof(ILogger) || ts.ServiceType == typeof(LoggerProvider)))
                 return;
 
-            PropertyInfo[] targetProperties = null;
+            var source = _registrationProcessor.Process(registration, out var injectParameter, out var targetProperties);
+            if (source == null)
+                return;
 
-            if (registration.Activator is ReflectionActivator ra)
+            if (injectParameter)
             {
-                // 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));
-
-                if (_autowireProperties)
+                registration.Preparing += (sender, args) =>
                 {
-                    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)
-                    return;
-            }
-            else
-            {
-                if (_onlyKnownConsumers)
-                    return;
+                    var log = args.Context.Resolve<ILogger>().ForContext(source);
+                    args.Parameters = new[] {TypedParameter.From(log)}.Concat(args.Parameters);
+                };
             }
 
-            registration.Preparing += (sender, args) =>
-            {
-                var log = args.Context.Resolve<ILogger>().ForContext(registration.Activator.LimitType);
-                args.Parameters = new[] {TypedParameter.From(log)}.Concat(args.Parameters);
-            };
-
-            if (targetProperties != null)
+            if (targetProperties != null && targetProperties.Length > 0)
             {
                 registration.Activating += (sender, args) =>
                 {
-                    var log = args.Context.Resolve<ILogger>().ForContext(registration.Activator.LimitType);
+                    var log = args.Context.Resolve<ILogger>().ForContext(source);
                     foreach (var targetProperty in targetProperties)
                     {
                         targetProperty.SetValue(args.Instance, log);

src/AutofacSerilogIntegration/DefaultRegistrationProcessor.cs

@@ -0,0 +1,26 @@
+using System;
+using System.Reflection;
+using Autofac.Core;
+
+namespace AutofacSerilogIntegration
+{
+    internal class DefaultRegistrationProcessor : IRegistrationProcessor
+    {
+        readonly bool _autowireProperties;
+
+        public DefaultRegistrationProcessor(bool autowireProperties)
+        {
+            _autowireProperties = autowireProperties;
+        }
+
+        public Type Process(IComponentRegistration registration, out bool injectParameter, out PropertyInfo[] targetProperties)
+        {
+            if (!registration.Activator.TryFindLoggerDependencies(_autowireProperties, out injectParameter, out targetProperties))
+            {
+                //this is the legacy behavior: we skipped injection only if the answer was a definitive "no", not an "I don't know"
+                injectParameter = true;
+            }
+            return registration.Activator.LimitType;
+        }
+    }
+}

src/AutofacSerilogIntegration/IRegistrationProcessor.cs

@@ -0,0 +1,11 @@
+using System;
+using System.Reflection;
+using Autofac.Core;
+
+namespace AutofacSerilogIntegration
+{
+    public interface IRegistrationProcessor
+    {
+        Type Process(IComponentRegistration registration, out bool injectParameter, out PropertyInfo[] targetProperties);
+    }
+}

src/AutofacSerilogIntegration/OnlyKnownCustomersRegistrationProcessor.cs

@@ -0,0 +1,24 @@
+using System;
+using System.Reflection;
+using Autofac.Core;
+
+namespace AutofacSerilogIntegration
+{
+    internal class OnlyKnownCustomersRegistrationProcessor : IRegistrationProcessor
+    {
+        readonly bool _autowireProperties;
+
+        public OnlyKnownCustomersRegistrationProcessor(bool autowireProperties)
+        {
+            _autowireProperties = autowireProperties;
+        }
+
+        public Type Process(IComponentRegistration registration, out bool injectParameter, out PropertyInfo[] targetProperties)
+        {
+            if (registration.Activator.TryFindLoggerDependencies(_autowireProperties, out injectParameter, out targetProperties))
+                return registration.Activator.LimitType;
+            else
+                return null;
+        }
+    }
+}

src/AutofacSerilogIntegration/SerilogContainerBuilderExtensions.cs

@@ -1,5 +1,6 @@
 using System;
 using Autofac;
+using Autofac.Core;
 using Autofac.Core.Registration;
 using Serilog;
 
@@ -26,8 +27,24 @@ public static class SerilogContainerBuilderExtensions
         /// <returns>An object supporting method chaining.</returns>
         public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, ILogger logger = null, bool autowireProperties = false, bool dispose = false, bool onlyKnownConsumers = false)
         {
-            if (builder == null) throw new ArgumentNullException(nameof(builder));
-            return builder.RegisterModule(new ContextualLoggingModule(logger, autowireProperties, dispose, onlyKnownConsumers));
+            var registrationProcessor = onlyKnownConsumers
+                ? (IRegistrationProcessor) new OnlyKnownCustomersRegistrationProcessor(autowireProperties)
+                : new DefaultRegistrationProcessor(autowireProperties);
+            return builder.RegisterLogger(registrationProcessor, logger, dispose);
+        }
+
+        /// <summary>
+        /// Register the <see cref="ILogger"/> with the <see cref="ContainerBuilder"/>. Where possible, the logger will
+        /// be resolved using the target type as a tagged property.
+        /// </summary>
+        /// <param name="builder">The container builder.</param>
+        /// <param name="registrationProcessor">A strategy to process <see cref="IComponentRegistration"/> for logging injection.</param>
+        /// <param name="logger">The logger. If null, the static <see cref="Log.Logger"/> will be used.</param>
+        /// <param name="dispose"></param>
+        /// <returns>An object supporting method chaining.</returns>
+        public static IModuleRegistrar RegisterLogger(this ContainerBuilder builder, IRegistrationProcessor registrationProcessor, ILogger logger = null, bool dispose = false)
+        {
+            return builder.RegisterModule(new ContextualLoggingModule(registrationProcessor, logger, dispose));
         }
     }
 }

test/AutofacSerilogIntegration.Tests/ActivatorTests.cs

@@ -1,5 +1,9 @@
 using System;
+using System.Linq;
+using System.Reflection;
 using Autofac;
+using Autofac.Core;
+using Autofac.Core.Activators.Reflection;
 using Moq;
 using Serilog;
 using Xunit;
@@ -16,14 +20,21 @@ public ActivatorTests()
             _logger.SetReturnsDefault(_logger.Object);
         }
 
-        private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureContainer, bool? onlyKnownConsumers)
+        private void ResolveInstance<TDependency>(Action<ContainerBuilder> configureContainer, bool? onlyKnownConsumers, IRegistrationProcessor registrationProcessor = null)
         {
             var containerBuilder = new ContainerBuilder();
 
-            if (onlyKnownConsumers == null)
+            if (onlyKnownConsumers == null && registrationProcessor == null)
                 containerBuilder.RegisterLogger(_logger.Object);
-            else
+            else if (onlyKnownConsumers != null)
+            {
+                Assert.Null(registrationProcessor);
                 containerBuilder.RegisterLogger(_logger.Object, onlyKnownConsumers: onlyKnownConsumers.Value);
+            }
+            else
+            {
+                containerBuilder.RegisterLogger(registrationProcessor, _logger.Object);
+            }
 
             containerBuilder.RegisterType<Component<TDependency>>();
             configureContainer(containerBuilder);
@@ -90,6 +101,56 @@ public void OnlyKnownCustomers_DelegateActivator_DependencyWithoutLogger_ShouldN
             VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
         }
 
+        [Fact]
+        public void CustomStrategy_DependencyWithLogger_ShouldCreateLogger()
+        {
+            ResolveInstance<DependencyWithLogger>(containerBuilder => 
+                containerBuilder.RegisterType<DependencyWithLogger>(), null, new Strategy());
+            VerifyLoggerCreation<DependencyWithLogger>(Times.AtLeastOnce);
+        }
+
+        [Fact]
+        public void CustomStrategy_DependencyWithoutLogger_ShouldNotCreateLogger()
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => 
+                containerBuilder.RegisterType<DependencyWithoutLogger>(), null, new Strategy());
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        [Fact]
+        public void CustomStrategy_ProvidedInstanceActivator_DependencyWithoutLogger_ShouldNotCreateLogger()
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => 
+                containerBuilder.RegisterInstance(new DependencyWithoutLogger()), null, new Strategy());
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        [Fact]
+        public void CustomStrategy_DelegateActivator_DependencyWithoutLogger_ShouldNotCreateLogger()
+        {
+            ResolveInstance<DependencyWithoutLogger>(containerBuilder => 
+                containerBuilder.Register(_ => new DependencyWithoutLogger()), null, new Strategy());
+            VerifyLoggerCreation<DependencyWithoutLogger>(Times.Never);
+        }
+
+        private class Strategy : IRegistrationProcessor
+        {
+            public Type Process(IComponentRegistration registration, out bool injectParameter, out PropertyInfo[] targetProperties)
+            {
+                injectParameter = false;
+                targetProperties = null;
+                if (!(registration.Activator is ReflectionActivator ra))
+                    return null;
+
+                injectParameter = ra.ConstructorFinder
+                    .FindConstructors(ra.LimitType)
+                    .SelectMany(c => c.GetParameters())
+                    .Any(p => p.ParameterType == typeof(ILogger));
+
+                return ra.LimitType;
+            }
+        }
+
         private class Component<TDependency>
         {
             public Component(TDependency dependency)