From a422ad5850a338a4b1f171a8ec515c0599f98923 Mon Sep 17 00:00:00 2001 From: Nathan Brown Date: Fri, 8 Dec 2017 11:55:44 -0700 Subject: [PATCH 1/2] Add tests for legacy logger For issue #1478 --- .../Logging/LoggerProviderTest.cs | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Logging/LoggerProviderTest.cs b/src/NHibernate.Test/Logging/LoggerProviderTest.cs index 75c01b6148d..ae3b852f9d4 100644 --- a/src/NHibernate.Test/Logging/LoggerProviderTest.cs +++ b/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()); -#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()); + + // works because this is the legacy provider with a legacy logger + Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf()); + } + [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()); + } + + [Test, Explicit("Changes global state."), Obsolete] + public void WhenNoLoggingFactoryIsUsedThenNoLoggingInternalLoggerIsReturned() + { + LoggerProvider.SetLoggersFactory(new NoLoggingLoggerFactory()); + + Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf()); + } + + [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")); + } } } From de93c69a2a55f95755b1d6f54ca6728612c20142 Mon Sep 17 00:00:00 2001 From: Nathan Brown Date: Fri, 8 Dec 2017 11:50:26 -0700 Subject: [PATCH 2/2] Don't split global state for logging. Fixes #1478 --- src/NHibernate/Logging.cs | 20 ++++++++++++++++++-- src/NHibernate/Logging.obsolete.cs | 27 ++++++++++----------------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/NHibernate/Logging.cs b/src/NHibernate/Logging.cs index 5d7cae37150..d0248bd57a3 100644 --- a/src/NHibernate/Logging.cs +++ b/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 } diff --git a/src/NHibernate/Logging.obsolete.cs b/src/NHibernate/Logging.obsolete.cs index ff8f5f5be30..fa70325e399 100644 --- a/src/NHibernate/Logging.obsolete.cs +++ b/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)); } }