Fix regression caused by logging refactoring (#1479)

- Add tests for legacy logger
- Don't split global state for logging.

Fixes #1478

Author: ngbrown

src/NHibernate.Test/Logging/LoggerProviderTest.cs

@@ -1,3 +1,4 @@
+using System;
 using NUnit.Framework;
 
 namespace NHibernate.Test.Logging
@@ -12,17 +13,29 @@ public void LoggerProviderCanCreateLoggers()
 			Assert.That(NHibernateLogger.For(typeof (LoggerProviderTest)), Is.Not.Null);
 		}
 
+		[Test, Obsolete]
+		public void LoggerProviderCanCreateLoggers_Obsolete()
+		{
+			Assert.That(LoggerProvider.LoggerFor("pizza"), Is.Not.Null);
+			Assert.That(LoggerProvider.LoggerFor(typeof (LoggerProviderTest)), Is.Not.Null);
+		}
+
 		[Test]
 		public void WhenNotConfiguredAndLog4NetExistsThenUseLog4NetFactory()
 		{
-#pragma warning disable 618
-			Assert.That(NHibernateLogger.For("pizza"), Is.Not.InstanceOf<NoLoggingInternalLogger>());
-#pragma warning restore 618
-
 			// NoLoggingNHibernateLogger is internal
 			Assert.That(NHibernateLogger.For("pizza").GetType().Name, Is.Not.EqualTo("NoLoggingNHibernateLogger"));
 		}
 
+		[Test, Obsolete]
+		public void WhenNotConfiguredAndLog4NetExistsThenUseLog4NetFactory_Obsolete()
+		{
+			Assert.That(LoggerProvider.LoggerFor("pizza"), Is.Not.InstanceOf<NoLoggingInternalLogger>());
+
+			// works because this is the legacy provider with a legacy logger
+			Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf<Log4NetLogger>());
+		}
+
 		[Test, Explicit("Changes global state.")]
 		public void WhenConfiguredAsNullThenNoLoggingFactoryIsUsed()
 		{
@@ -31,5 +44,30 @@ public void WhenConfiguredAsNullThenNoLoggingFactoryIsUsed()
 			// NoLoggingNHibernateLogger is internal
 			Assert.That(NHibernateLogger.For("pizza").GetType().Name, Is.EqualTo("NoLoggingNHibernateLogger"));
 		}
+
+		[Test, Explicit("Changes global state."), Obsolete]
+		public void WhenConfiguredAsNullThenNoLoggingFactoryIsUsed_Obsolete()
+		{
+			NHibernateLogger.SetLoggersFactory(default(INHibernateLoggerFactory));
+
+			Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf<NoLoggingInternalLogger>());
+		}
+
+		[Test, Explicit("Changes global state."), Obsolete]
+		public void WhenNoLoggingFactoryIsUsedThenNoLoggingInternalLoggerIsReturned()
+		{
+			LoggerProvider.SetLoggersFactory(new NoLoggingLoggerFactory());
+
+			Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf<NoLoggingInternalLogger>());
+		}
+
+		[Test, Explicit("Changes global state."), Obsolete]
+		public void WhenNoLoggingFactoryIsUsedThenNoLoggingNHibernateLoggerIsReturned()
+		{
+			LoggerProvider.SetLoggersFactory(new NoLoggingLoggerFactory());
+
+			// NoLoggingNHibernateLogger is internal
+			Assert.That(NHibernateLogger.For("pizza").GetType().Name, Is.EqualTo("NoLoggingNHibernateLogger"));
+		}
 	}
 }

src/NHibernate/Logging.cs

@@ -52,6 +52,10 @@ public static class NHibernateLogger
 		private const string nhibernateLoggerConfKey = "nhibernate-logger";
 		private static INHibernateLoggerFactory _loggerFactory;
 
+#pragma warning disable 618
+		internal static ILoggerFactory LegacyLoggerFactory { get; private set; }
+#pragma warning restore 618
+
 		static NHibernateLogger()
 		{
 			var nhibernateLoggerClass = GetNhibernateLoggerClass();
@@ -68,9 +72,21 @@ public static void SetLoggersFactory(INHibernateLoggerFactory loggerFactory)
 			_loggerFactory = loggerFactory ?? new NoLoggingNHibernateLoggerFactory();
 
 #pragma warning disable 618
-			if (!(loggerFactory is LoggerProvider.LegacyLoggerFactoryAdaptor))
+			// Also keep global state for obsolete logger
+			if (loggerFactory == null)
+			{
+				LegacyLoggerFactory = new NoLoggingLoggerFactory();
+			}
+			else
 			{
-				LoggerProvider.SetLoggersFactory(new LoggerProvider.ReverseLegacyLoggerFactoryAdaptor(loggerFactory));
+				if (loggerFactory is LoggerProvider.LegacyLoggerFactoryAdaptor legacyAdaptor)
+				{
+					LegacyLoggerFactory = legacyAdaptor.Factory;
+				}
+				else
+				{
+					LegacyLoggerFactory = new LoggerProvider.ReverseLegacyLoggerFactoryAdaptor(loggerFactory);
+				}
 			}
 #pragma warning restore 618
 		}

src/NHibernate/Logging.obsolete.cs

@@ -82,54 +82,47 @@ public interface ILoggerFactory
 	[Obsolete("Use NHibernateLogger instead.")]
 	public class LoggerProvider
 	{
-		private static ILoggerFactory _legacyLoggerFactory;
-
 		[Obsolete("Implement INHibernateLoggerFactory and use NHibernateLogger.SetLoggersFactory() instead")]
 		public static void SetLoggersFactory(ILoggerFactory loggerFactory)
 		{
-			_legacyLoggerFactory = loggerFactory ?? new NoLoggingLoggerFactory();
-
-			if (!(loggerFactory is ReverseLegacyLoggerFactoryAdaptor))
-			{
-				var factory = loggerFactory == null || loggerFactory is NoLoggingLoggerFactory
-					? null
-					: (INHibernateLoggerFactory) new LegacyLoggerFactoryAdaptor(loggerFactory);
+			var factory = (loggerFactory == null || loggerFactory is NoLoggingLoggerFactory)
+				? null
+				: (INHibernateLoggerFactory) new LegacyLoggerFactoryAdaptor(loggerFactory);
 
-				NHibernateLogger.SetLoggersFactory(factory);
-			}
+			NHibernateLogger.SetLoggersFactory(factory);
 		}
 
 		[Obsolete("Use NHibernateLogger.For() instead.")]
 		public static IInternalLogger LoggerFor(string keyName)
 		{
-			return _legacyLoggerFactory.LoggerFor(keyName);
+			return NHibernateLogger.LegacyLoggerFactory.LoggerFor(keyName);
 		}
 
 		[Obsolete("Use NHibernateLogger.For() instead.")]
 		public static IInternalLogger LoggerFor(System.Type type)
 		{
-			return _legacyLoggerFactory.LoggerFor(type);
+			return NHibernateLogger.LegacyLoggerFactory.LoggerFor(type);
 		}
 
 		// Since 5.1
 		[Obsolete("Used only in Obsolete functions to thunk to INHibernateLoggerFactory")]
 		internal class LegacyLoggerFactoryAdaptor : INHibernateLoggerFactory
 		{
-			private readonly ILoggerFactory _factory;
+			internal ILoggerFactory Factory { get; }
 
 			public LegacyLoggerFactoryAdaptor(ILoggerFactory factory)
 			{
-				_factory = factory;
+				Factory = factory;
 			}
 
 			public INHibernateLogger LoggerFor(string keyName)
 			{
-				return new NHibernateLoggerThunk(_factory.LoggerFor(keyName));
+				return new NHibernateLoggerThunk(Factory.LoggerFor(keyName));
 			}
 
 			public INHibernateLogger LoggerFor(System.Type type)
 			{
-				return new NHibernateLoggerThunk(_factory.LoggerFor(type));
+				return new NHibernateLoggerThunk(Factory.LoggerFor(type));
 			}
 		}