Refine .Net Core distributed cache factory model

And provide by default the MemoryDistributedCacheFactory.

To be squashed.

Author: fredericDelaporte

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/App.config

@@ -6,7 +6,11 @@
 				 type="NHibernate.Cfg.ConfigurationSectionHandler, NHibernate" />
 	</configSections>
 
-	<coredistributedcache factory-class="NHibernate.Caches.CoreDistributedCache.Tests.MemoryDistributedCacheFactory,NHibernate.Caches.CoreDistributedCache.Tests">
+	<coredistributedcache factory-class="NHibernate.Caches.CoreDistributedCache.MemoryDistributedCacheFactory,NHibernate.Caches.CoreDistributedCache">
+		<properties>
+			<property name="expiration-scan-frequency">00:10:00</property>
+			<property name="size-limit">1048576</property>
+		</properties>
 		<cache region="foo" expiration="500" sliding="true" />
 		<cache region="noExplicitExpiration" sliding="true" />
 	</coredistributedcache>

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/CoreDistributedCacheProviderFixture.cs

@@ -22,6 +22,9 @@
 
 using System;
 using System.Collections.Generic;
+using System.Reflection;
+using Microsoft.Extensions.Caching.Distributed;
+using Microsoft.Extensions.Caching.Memory;
 using NHibernate.Cache;
 using NHibernate.Caches.Common.Tests;
 using NUnit.Framework;
@@ -34,6 +37,36 @@ public class CoreDistributedCacheProviderFixture : CacheProviderFixture
 		protected override Func<ICacheProvider> ProviderBuilder =>
 			() => new CoreDistributedCacheProvider();
 
+		private static readonly FieldInfo MemoryCacheField =
+			typeof(MemoryDistributedCache).GetField("_memCache", BindingFlags.Instance | BindingFlags.NonPublic);
+
+		private static readonly FieldInfo CacheOptionsField =
+			typeof(MemoryCache).GetField("_options", BindingFlags.Instance | BindingFlags.NonPublic);
+
+		[Test]
+		public void ConfiguredCacheFactory()
+		{
+			var factory = CoreDistributedCacheProvider.CacheFactory;
+			Assert.That(factory, Is.Not.Null, "Factory not found");
+			Assert.That(factory, Is.InstanceOf<MemoryDistributedCacheFactory>(), "Unexpected factory");
+			var cache1 = factory.BuildCache();
+			Assert.That(cache1, Is.Not.Null, "Factory has yielded null");
+			Assert.That(cache1, Is.InstanceOf<MemoryDistributedCache>(), "Unexpected cache");
+			var cache2 = factory.BuildCache();
+			Assert.That(cache2, Is.EqualTo(cache1),
+				"The distributed cache factory is supposed to always yield the same instance");
+
+			var memCache = MemoryCacheField.GetValue(cache1);
+			Assert.That(memCache, Is.Not.Null, "Underlying memory cache not found");
+			Assert.That(memCache, Is.InstanceOf<MemoryCache>(), "Unexpected memory cache");
+			var options = CacheOptionsField.GetValue(memCache);
+			Assert.That(options, Is.Not.Null, "Memory cache options not found");
+			Assert.That(options, Is.InstanceOf<MemoryCacheOptions>(), "Unexpected options type");
+			var memOptions = (MemoryCacheOptions) options;
+			Assert.That(memOptions.ExpirationScanFrequency, Is.EqualTo(TimeSpan.FromMinutes(10)));
+			Assert.That(memOptions.SizeLimit, Is.EqualTo(1048576));
+		}
+
 		[Test]
 		public void TestBuildCacheFromConfig()
 		{
@@ -48,18 +81,19 @@ public void TestExpiration()
 			Assert.That(cache, Is.Not.Null, "pre-configured foo cache not found");
 			Assert.That(cache.Expiration, Is.EqualTo(TimeSpan.FromSeconds(500)), "Unexpected expiration value for foo region");
 
-			cache = (CoreDistributedCache)DefaultProvider.BuildCache("noExplicitExpiration", null);
+			cache = (CoreDistributedCache) DefaultProvider.BuildCache("noExplicitExpiration", null);
 			Assert.That(cache.Expiration, Is.EqualTo(TimeSpan.FromSeconds(300)),
 				"Unexpected expiration value for noExplicitExpiration region");
 			Assert.That(cache.UseSlidingExpiration, Is.True, "Unexpected sliding value for noExplicitExpiration region");
 
-			cache = (CoreDistributedCache)DefaultProvider
+			cache = (CoreDistributedCache) DefaultProvider
 				.BuildCache("noExplicitExpiration", new Dictionary<string, string> { { "expiration", "100" } });
 			Assert.That(cache.Expiration, Is.EqualTo(TimeSpan.FromSeconds(100)),
 				"Unexpected expiration value for noExplicitExpiration region with default expiration");
 
-			cache = (CoreDistributedCache)DefaultProvider
-				.BuildCache("noExplicitExpiration", new Dictionary<string, string> { { Cfg.Environment.CacheDefaultExpiration, "50" } });
+			cache = (CoreDistributedCache) DefaultProvider
+				.BuildCache("noExplicitExpiration",
+					new Dictionary<string, string> { { Cfg.Environment.CacheDefaultExpiration, "50" } });
 			Assert.That(cache.Expiration, Is.EqualTo(TimeSpan.FromSeconds(50)),
 				"Unexpected expiration value for noExplicitExpiration region with cache.default_expiration");
 		}

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/CoreDistributedCacheSectionHandlerFixture.cs

@@ -41,25 +41,41 @@ public void TestGetConfigNullSection()
 			var handler = new CoreDistributedCacheSectionHandler();
 			var section = new XmlDocument();
 			var result = handler.Create(null, null, section);
-			Assert.That(result, Is.Not.Null);
-			Assert.IsTrue(result is CacheConfig[]);
-			var caches = result as CacheConfig[];
-			Assert.That(caches.Length, Is.EqualTo(0));
+			Assert.That(result, Is.Not.Null, "result");
+			Assert.That(result, Is.InstanceOf<CacheConfig>());
+			var config = (CacheConfig) result;
+			Assert.That(config.Properties, Is.Not.Null, "Properties");
+			Assert.That(config.Properties.Count, Is.EqualTo(0), "Properties count");
+			Assert.That(config.Regions, Is.Not.Null, "Regions");
+			Assert.That(config.Regions.Length, Is.EqualTo(0));
 		}
 
 		[Test]
 		public void TestGetConfigFromFile()
 		{
-			const string xmlSimple = "<coredistributedcache><cache region=\"foo\" expiration=\"500\" sliding=\"true\" /></coredistributedcache>";
+			const string xmlSimple = "<coredistributedcache factory-class=\"factory1\"><properties><property name=\"prop1\">Value1</property></properties><cache region=\"foo\" expiration=\"500\" sliding=\"true\" /></coredistributedcache>";
 
 			var handler = new CoreDistributedCacheSectionHandler();
 			var section = GetConfigurationSection(xmlSimple);
 			var result = handler.Create(null, null, section);
-			Assert.That(result, Is.Not.Null);
-			Assert.IsTrue(result is CacheConfig[]);
-			var caches = result as CacheConfig[];
-			Assert.That(caches.Length, Is.EqualTo(1));
-			Assert.That(caches[0].Properties, Does.ContainKey("cache.use_sliding_expiration"));
+			Assert.That(result, Is.Not.Null, "result");
+			Assert.That(result, Is.InstanceOf<CacheConfig>());
+			var config = (CacheConfig) result;
+
+			Assert.That(config.FactoryClass, Is.EqualTo("factory1"));
+
+			Assert.That(config.Properties, Is.Not.Null, "Properties");
+			Assert.That(config.Properties.Count, Is.EqualTo(1), "Properties count");
+			Assert.That(config.Properties, Does.ContainKey("prop1"));
+			Assert.That(config.Properties["prop1"], Is.EqualTo("Value1"));
+
+			Assert.That(config.Regions, Is.Not.Null, "Regions");
+			Assert.That(config.Regions.Length, Is.EqualTo(1), "Regions count");
+			Assert.That(config.Regions[0].Region, Is.EqualTo("foo"));
+			Assert.That(config.Regions[0].Properties, Does.ContainKey("cache.use_sliding_expiration"));
+			Assert.That(config.Regions[0].Properties["cache.use_sliding_expiration"], Is.EqualTo("true"));
+			Assert.That(config.Regions[0].Properties, Does.ContainKey("expiration"));
+			Assert.That(config.Regions[0].Properties["expiration"], Is.EqualTo("500"));
 		}
 	}
 }

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache.Tests/MemoryDistributedCacheFactory.cs

@@ -4,28 +4,47 @@
 namespace NHibernate.Caches.CoreDistributedCache
 {
 	/// <summary>
-	/// Cache config properties.
+	/// Cache configuration properties.
 	/// </summary>
 	public class CacheConfig
 	{
 		/// <summary>
-		/// Build a cache global configuration.
+		/// Build a cache configuration.
 		/// </summary>
 		/// <param name="factoryClass">The <see cref="IDistributedCacheFactory"/> factory class name to use for getting
 		/// <see cref="IDistributedCache"/> instances.</param>
-		public CacheConfig(string factoryClass)
+		/// <param name="properties">The cache configuration properties.</param>
+		/// <param name="regions">The configured cache regions.</param>
+		public CacheConfig(string factoryClass, IDictionary<string, string> properties, RegionConfig[] regions)
 		{
 			FactoryClass = factoryClass;
-			Global = true;
+			Regions = regions;
+			Properties = properties;
 		}
 
+		/// <summary>The <see cref="IDistributedCacheFactory"/> factory class name to use for getting
+		/// <see cref="IDistributedCache"/> instances.</summary>
+		public string FactoryClass { get; }
+
+		/// <summary>The configured cache regions.</summary>
+		public RegionConfig[] Regions { get; }
+
+		/// <summary>The cache configuration properties.</summary>
+		public IDictionary<string, string> Properties { get; }
+	}
+
+	/// <summary>
+	/// Region configuration properties.
+	/// </summary>
+	public class RegionConfig
+	{
 		/// <summary>
 		/// Build a cache region configuration.
 		/// </summary>
 		/// <param name="region">The configured cache region.</param>
 		/// <param name="expiration">The expiration for the region.</param>
 		/// <param name="sliding">Whether the expiration should be sliding or not.</param>
-		public CacheConfig(string region, string expiration, string sliding)
+		public RegionConfig(string region, string expiration, string sliding)
 		{
 			Region = region;
 			Properties = new Dictionary<string, string>();
@@ -35,17 +54,10 @@ public CacheConfig(string region, string expiration, string sliding)
 				Properties["cache.use_sliding_expiration"] = sliding;
 		}
 
-		/// <summary>Whether this configuration is global or is a cache region configuration.</summary>
-		public bool Global { get; }
-
 		/// <summary>The region name.</summary>
 		public string Region { get; }
 
-		/// <summary>The <see cref="IDistributedCacheFactory"/> factory class name to use for getting
-		/// <see cref="IDistributedCache"/> instances.</summary>
-		public string FactoryClass { get; }
-
-		/// <summary>The configuration properties.</summary>
-		public IDictionary<string,string> Properties { get; }
+		/// <summary>The region configuration properties.</summary>
+		public IDictionary<string, string> Properties { get; }
 	}
 }

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CacheConfig.cs

@@ -37,6 +37,7 @@ public class CoreDistributedCacheProvider : ICacheProvider
 	{
 		private static readonly Dictionary<string, IDictionary<string, string>> ConfiguredCachesProperties;
 		private static readonly INHibernateLogger Log;
+		private static readonly System.Type[] CacheFactoryCtorWithPropertiesSignature = { typeof(IDictionary<string, string>) };
 
 		/// <summary>
 		/// The <see cref="IDistributedCacheFactory"/> factory to use for getting <see cref="IDistributedCache"/>
@@ -53,31 +54,32 @@ static CoreDistributedCacheProvider()
 			Log = NHibernateLogger.For(typeof(CoreDistributedCacheProvider));
 			ConfiguredCachesProperties = new Dictionary<string, IDictionary<string, string>>();
 
-			if (!(ConfigurationManager.GetSection("coredistributedcache") is CacheConfig[] list))
+			if (!(ConfigurationManager.GetSection("coredistributedcache") is CacheConfig config))
 				return;
 
-			foreach (var cache in list)
+			if (!string.IsNullOrEmpty(config.FactoryClass))
 			{
-				if (cache.Global)
+				try
 				{
-					if (!string.IsNullOrEmpty(cache.FactoryClass))
-					{
-						try
-						{
-							CacheFactory =
-								(IDistributedCacheFactory) Cfg.Environment.BytecodeProvider.ObjectsFactory
-									.CreateInstance(ReflectHelper.ClassForName(cache.FactoryClass));
-						}
-						catch (Exception e)
-						{
-							throw new HibernateException(
-								$"Could not create the {nameof(IDistributedCacheFactory)} factory from '{cache.FactoryClass}'.",
-								e);
-						}
-					}
-					continue;
+					var factoryClass = ReflectHelper.ClassForName(config.FactoryClass);
+					var ctorWithProperties = factoryClass.GetConstructor(CacheFactoryCtorWithPropertiesSignature);
+
+					CacheFactory = (IDistributedCacheFactory) (ctorWithProperties != null ?
+						ctorWithProperties.Invoke(new object[] { config.Properties }):
+						Cfg.Environment.BytecodeProvider.ObjectsFactory.CreateInstance(factoryClass));
 				}
+				catch (Exception e)
+				{
+					throw new HibernateException(
+						$"Could not create the {nameof(IDistributedCacheFactory)} factory from '{config.FactoryClass}'. " +
+						$"(It must implement {nameof(IDistributedCacheFactory)} and have a constructor accepting a " +
+						$"{nameof(IDictionary<string, string>)} or have a parameterless constructor.)",
+						e);
+				}
+			}
 
+			foreach (var cache in config.Regions)
+			{
 				ConfiguredCachesProperties.Add(cache.Region, cache.Properties);
 			}
 		}

CoreDistributedCache/NHibernate.Caches.CoreDistributedCache/CoreDistributedCacheProvider.cs

@@ -19,49 +19,46 @@ public class CoreDistributedCacheSectionHandler : IConfigurationSectionHandler
 		/// <param name="parent"></param>
 		/// <param name="configContext"></param>
 		/// <param name="section"></param>
-		/// <returns>An array of CacheConfig objects.</returns>
+		/// <returns>A <see cref="CacheConfig" /> object.</returns>
 		public object Create(object parent, object configContext, XmlNode section)
 		{
-			var caches = new List<CacheConfig>();
+			var caches = new List<RegionConfig>();
 
-			var fc = section.Attributes?["factory-class"];
-			if (fc != null)
-			{
-				caches.Add(new CacheConfig(fc.Value));
-			}
-			
 			var nodes = section.SelectNodes("cache");
 			foreach (XmlNode node in nodes)
 			{
-				string region = null;
-				string expiration = null;
-				string sliding = null;
-				var r = node.Attributes["region"];
-				var e = node.Attributes["expiration"];
-				var s = node.Attributes["sliding"];
-				if (r != null)
-				{
-					region = r.Value;
-				}
-				if (e != null)
+				var region = node.Attributes["region"]?.Value;
+				var expiration = node.Attributes["expiration"]?.Value;
+				var sliding = node.Attributes["sliding"]?.Value;
+				if (region != null)
 				{
-					expiration = e.Value;
+					caches.Add(new RegionConfig(region, expiration, sliding));
 				}
-				if (s != null)
+				else
 				{
-					sliding = s.Value;
+					Log.Warn("Found a cache region node lacking a region name: ignored. Node: {0}",
+						node.OuterXml);
 				}
-				if (region != null)
+			}
+
+			var factoryClass = section.Attributes?["factory-class"]?.Value;
+			var properties = new Dictionary<string, string>();
+			nodes = section.SelectNodes("properties/property");
+			foreach (XmlNode node in nodes)
+			{
+				var name = node.Attributes["name"]?.Value;
+				if (name != null)
 				{
-					caches.Add(new CacheConfig(region, expiration, sliding));
+					properties.Add(name, node.InnerText);
 				}
 				else
 				{
-					Log.Warn("Found a cache node lacking a region name: ignored. Node: {0}",
+					Log.Warn("Found a cache property node lacking a name: ignored. Node: {0}",
 						node.OuterXml);
 				}
 			}
-			return caches.ToArray();
+
+			return new CacheConfig(factoryClass, properties, caches.ToArray());
 		}
 
 		#endregion