fixup! Implement multiple get and put for query cache and query batch

Refactor that nasty QueryInformation thing

Author: fredericDelaporte

src/NHibernate/Async/Cache/StandardQueryCache.cs

@@ -351,7 +351,9 @@ private async Task<IList> GetResultFromCacheableAsync(
 					//      the UnresolvableObjectException could occur while resolving
 					//      associations, leaving the PC in an inconsistent state
 					Log.Debug(ex, "could not reassemble cached result set");
-					// TODO: add and handle a RemoveMany?
+					// Handling a RemoveMany here does not look worth it, as this case short-circuits
+					// the result-set. So a Many could only benefit batched queries, and only if many
+					// of them are natural key lookup with an unresolvable object case.
 					await (_queryCache.RemoveAsync(key, cancellationToken)).ConfigureAwait(false);
 					return null;
 				}

src/NHibernate/Async/Multi/CriteriaBatchItem.cs

@@ -10,6 +10,7 @@
 
 using System;
 using System.Collections.Generic;
+using NHibernate.Engine;
 using NHibernate.Impl;
 using NHibernate.Loader.Criteria;
 using NHibernate.Persister.Entity;

src/NHibernate/Async/Multi/IQueryBatchItem.cs

@@ -25,13 +25,14 @@ public partial interface IQueryBatchItem
 	{
 
 		/// <summary>
-		/// Immediate query execution in case the dialect does not support batches
+		/// Execute immediately the query as a single standalone query. Used in case the data-provider
+		/// does not support batches.
 		/// </summary>
 		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
 		Task ExecuteNonBatchedAsync(CancellationToken cancellationToken);
 
 		/// <summary>
-		/// Initialize loaded entities and collections.
+		/// Initialize entities and collections loaded from database.
 		/// </summary>
 		/// <param name="reader">The reader having loaded the query.</param>
 		/// <param name="cacheBatcher">A cache batcher for batching cache puts of entities and collections.</param>

src/NHibernate/Async/Multi/QueryBatch.cs

@@ -106,8 +106,7 @@ private async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
 				stopWatch.Start();
 			}
 
-			var querySpaces = new HashSet<string>(
-				_queries.SelectMany(q => q.QueryInformation).SelectMany(qi => qi.QuerySpaces));
+			var querySpaces = new HashSet<string>(_queries.SelectMany(q => q.GetQuerySpaces()));
 			if (querySpaces.Count > 0)
 			{
 				// The auto-flush must be handled before obtaining the commands, because an auto-flush
@@ -169,22 +168,16 @@ private async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
 			}
 		}
 
-		private async Task<IDictionary<IQueryInformation, IList>> GetCachedResultsAsync(CancellationToken cancellationToken)
+		private async Task<IDictionary<ICachingInformation, IList>> GetCachedResultsAsync(CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
-			var resultByQueryInfo = new Dictionary<IQueryInformation, IList>();
+			var resultByQueryInfo = new Dictionary<ICachingInformation, IList>();
 			var statisticsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
-			var queriesByCaches =
-				_queries
-					.SelectMany(q => q.QueryInformation)
-					.Where(q => q.Loader.IsCacheable(q.Parameters) && q.Loader.CanGetFromCache(Session, q.Parameters))
-					.GroupBy(
-						q => Session.Factory.GetQueryCache(q.Parameters.CacheRegion),
-						(cache, q) => new { cache, queryInfos = q });
-
+			var queriesByCaches = GetQueriesByCaches(ci => ci.CanGetFromCache);
 			foreach (var queriesByCache in queriesByCaches)
 			{
-				var queryInfos = queriesByCache.queryInfos.ToArray();
+				var queryInfos = queriesByCache.ToArray();
+				var cache = queriesByCache.Key;
 				var keys = new QueryKey[queryInfos.Length];
 				var parameters = new QueryParameters[queryInfos.Length];
 				var returnTypes = new ICacheAssembler[queryInfos.Length][];
@@ -196,26 +189,26 @@ private async Task<IDictionary<IQueryInformation, IList>> GetCachedResultsAsync(
 					parameters[i] = queryInfo.Parameters;
 					returnTypes[i] = queryInfo.Parameters.HasAutoDiscoverScalarTypes
 						? null
-						: queryInfo.CacheKey.ResultTransformer.GetCachedResultTypes(queryInfo.Loader.ResultTypes);
+						: queryInfo.CacheKey.ResultTransformer.GetCachedResultTypes(queryInfo.ResultTypes);
 					spaces[i] = queryInfo.QuerySpaces;
 				}
 
-				var results = await (queriesByCache.cache.GetManyAsync(keys, parameters, returnTypes, spaces, Session, cancellationToken)).ConfigureAwait(false);
+				var results = await (cache.GetManyAsync(keys, parameters, returnTypes, spaces, Session, cancellationToken)).ConfigureAwait(false);
 
 				for (var i = 0; i < queryInfos.Length; i++)
 				{
 					resultByQueryInfo.Add(queryInfos[i], results[i]);
 
 					if (statisticsEnabled)
 					{
-						var queryIdentifier = queryInfos[i].Loader.QueryIdentifier;
+						var queryIdentifier = queryInfos[i].QueryIdentifier;
 						if (results[i] == null)
 						{
-							Session.Factory.StatisticsImplementor.QueryCacheMiss(queryIdentifier, queriesByCache.cache.RegionName);
+							Session.Factory.StatisticsImplementor.QueryCacheMiss(queryIdentifier, cache.RegionName);
 						}
 						else
 						{
-							Session.Factory.StatisticsImplementor.QueryCacheHit(queryIdentifier, queriesByCache.cache.RegionName);
+							Session.Factory.StatisticsImplementor.QueryCacheHit(queryIdentifier, cache.RegionName);
 						}
 					}
 				}
@@ -231,17 +224,11 @@ private async Task PutCacheableResultsAsync(CancellationToken cancellationToken)
 				return;
 
 			var statisticsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
-			var queriesByCaches =
-				_queries
-					.SelectMany(q => q.QueryInformation)
-					.Where(q => q.CachePutRequired)
-					.GroupBy(
-						q => Session.Factory.GetQueryCache(q.Parameters.CacheRegion),
-						(cache, q) => new { cache, queryInfos = q });
-
+			var queriesByCaches = GetQueriesByCaches(ci => ci.CachePutRequired);
 			foreach (var queriesByCache in queriesByCaches)
 			{
-				var queryInfos = queriesByCache.queryInfos.ToArray();
+				var queryInfos = queriesByCache.ToArray();
+				var cache = queriesByCache.Key;
 				var keys = new QueryKey[queryInfos.Length];
 				var parameters = new QueryParameters[queryInfos.Length];
 				var returnTypes = new ICacheAssembler[queryInfos.Length][];
@@ -251,11 +238,11 @@ private async Task PutCacheableResultsAsync(CancellationToken cancellationToken)
 					var queryInfo = queryInfos[i];
 					keys[i] = queryInfo.CacheKey;
 					parameters[i] = queryInfo.Parameters;
-					returnTypes[i] = queryInfo.CacheKey.ResultTransformer.GetCachedResultTypes(queryInfo.Loader.ResultTypes);
+					returnTypes[i] = queryInfo.CacheKey.ResultTransformer.GetCachedResultTypes(queryInfo.ResultTypes);
 					results[i] = queryInfo.Result;
 				}
 
-				var putted = await (queriesByCache.cache.PutManyAsync(keys, parameters, returnTypes, results, Session, cancellationToken)).ConfigureAwait(false);
+				var putted = await (cache.PutManyAsync(keys, parameters, returnTypes, results, Session, cancellationToken)).ConfigureAwait(false);
 
 				if (!statisticsEnabled)
 					continue;
@@ -265,7 +252,7 @@ private async Task PutCacheableResultsAsync(CancellationToken cancellationToken)
 					if (putted[i])
 					{
 						Session.Factory.StatisticsImplementor.QueryCachePut(
-							queryInfos[i].Loader.QueryIdentifier, queriesByCache.cache.RegionName);
+							queryInfos[i].QueryIdentifier, cache.RegionName);
 					}
 				}
 			}

src/NHibernate/Async/Multi/QueryBatchItemBase.cs

@@ -16,6 +16,7 @@
 using NHibernate.Cache;
 using NHibernate.Engine;
 using NHibernate.SqlCommand;
+using NHibernate.Type;
 using NHibernate.Util;
 
 namespace NHibernate.Multi

src/NHibernate/Cache/StandardQueryCache.cs

@@ -376,7 +376,9 @@ private IList GetResultFromCacheable(
 					//      the UnresolvableObjectException could occur while resolving
 					//      associations, leaving the PC in an inconsistent state
 					Log.Debug(ex, "could not reassemble cached result set");
-					// TODO: add and handle a RemoveMany?
+					// Handling a RemoveMany here does not look worth it, as this case short-circuits
+					// the result-set. So a Many could only benefit batched queries, and only if many
+					// of them are natural key lookup with an unresolvable object case.
 					_queryCache.Remove(key);
 					return null;
 				}

src/NHibernate/Multi/CriteriaBatchItem.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using NHibernate.Engine;
 using NHibernate.Impl;
 using NHibernate.Loader.Criteria;
 using NHibernate.Persister.Entity;
@@ -15,7 +16,7 @@ public CriteriaBatchItem(ICriteria query)
 			_criteria = (CriteriaImpl) query ?? throw new ArgumentNullException(nameof(query));
 		}
 
-		protected override List<QueryInfo> GetQueryInformation()
+		protected override List<QueryInfo> GetQueryInformation(ISessionImplementor session)
 		{
 			var factory = Session.Factory;
 			//for detached criteria
@@ -35,13 +36,7 @@ protected override List<QueryInfo> GetQueryInformation()
 					Session.EnabledFilters
 				);
 
-				list.Add(
-					new QueryInfo()
-					{
-						Loader = loader,
-						Parameters = loader.Translator.GetQueryParameters(),
-						QuerySpaces = loader.QuerySpaces,
-					});
+				list.Add(new QueryInfo(loader.Translator.GetQueryParameters(), loader, loader.QuerySpaces, session));
 			}
 
 			return list;

src/NHibernate/Multi/ICachingInformation.cs

@@ -0,0 +1,64 @@
+using System.Collections;
+using System.Collections.Generic;
+using NHibernate.Cache;
+using NHibernate.Engine;
+using NHibernate.Type;
+
+namespace NHibernate.Multi
+{
+	/// <summary>
+	/// Querying information.
+	/// </summary>
+	public interface ICachingInformation
+	{
+		/// <summary>
+		/// The query parameters.
+		/// </summary>
+		QueryParameters Parameters { get; }
+		
+		/// <summary>
+		/// The query result.
+		/// </summary>
+		IList Result { get; }
+
+		/// <summary>
+		/// The query spaces.
+		/// </summary>
+		/// <remarks>
+		/// Query spaces indicates which entity classes are used by the query and need to be flushed
+		/// when auto-flush is enabled. It also indicates which cache update timestamps needs to be
+		/// checked for up-to-date-ness.
+		/// </remarks>
+		ISet<string> QuerySpaces { get; }
+
+		/// <summary>
+		/// Is the query cacheable?
+		/// </summary>
+		bool IsCacheable { get; }
+
+		/// <summary>
+		/// Can the query be obtained from cache?
+		/// </summary>
+		bool CanGetFromCache { get; }
+
+		/// <summary>
+		/// Should the query results be put in cache?
+		/// </summary>
+		bool CachePutRequired { get; }
+
+		/// <summary>
+		/// The query result types.
+		/// </summary>
+		IType[] ResultTypes { get; }
+
+		/// <summary>
+		/// The query identifier, for statistics purpose.
+		/// </summary>
+		string QueryIdentifier { get; }
+
+		/// <summary>
+		/// The query cache key.
+		/// </summary>
+		QueryKey CacheKey { get; }
+	}
+}

src/NHibernate/Multi/IQueryBatchItem.cs

@@ -32,43 +32,57 @@ public interface IQueryBatchItem<TResult> : IQueryBatchItem
 	public partial interface IQueryBatchItem
 	{
 		/// <summary>
-		/// The query loading information.
+		/// Optionally, the query caching information list, for batching. Each element matches
+		/// a SQL-Query resulting from the query translation, in the order they are translated.
+		/// It should be <see langword="null" /> if no batching of caching is handled for this
+		/// query.
 		/// </summary>
-		IEnumerable<IQueryInformation> QueryInformation { get; }
+		IEnumerable<ICachingInformation> CachingInformation { get; }
 
 		/// <summary>
-		/// Method is called right before batch execution.
+		/// Initialize the query. Method is called right before batch execution.
 		/// Can be used for various delayed initialization logic.
 		/// </summary>
 		/// <param name="session"></param>
 		void Init(ISessionImplementor session);
+
+		/// <summary>
+		/// Get the query spaces.
+		/// </summary>
+		/// <remarks>
+		/// Query spaces indicates which entity classes are used by the query and need to be flushed
+		/// when auto-flush is enabled. It also indicates which cache update timestamps needs to be
+		/// checked for up-to-date-ness.
+		/// </remarks>
+		IEnumerable<string> GetQuerySpaces();
 
 		/// <summary>
-		/// Gets the commands to execute for getting the not-already cached results of this query.
+		/// Get the commands to execute for getting the not-already cached results of this query.
 		/// </summary>
 		/// <param name="cachedResults">The cached results.</param>
 		/// <returns>The commands for obtaining the results not already cached.</returns>
-		IEnumerable<ISqlCommand> GetCommands(IDictionary<IQueryInformation, IList> cachedResults);
+		IEnumerable<ISqlCommand> GetCommands(IDictionary<ICachingInformation, IList> cachedResults);
 
 		/// <summary>
-		/// Returns delegates for processing result sets generated by <see cref="GetCommands"/>.
+		/// Return delegates for processing result sets generated by <see cref="GetCommands"/>.
 		/// Delegate should return the number of rows loaded by command.
 		/// </summary>
 		/// <returns></returns>
 		IEnumerable<Func<DbDataReader, int>> GetResultSetHandler();
 
 		/// <summary>
-		/// Executed after all commands in batch are processed
+		/// Process the results of commands executed for the query.
 		/// </summary>
 		void ProcessResults();
 
 		/// <summary>
-		/// Immediate query execution in case the dialect does not support batches
+		/// Execute immediately the query as a single standalone query. Used in case the data-provider
+		/// does not support batches.
 		/// </summary>
 		void ExecuteNonBatched();
 
 		/// <summary>
-		/// Initialize loaded entities and collections.
+		/// Initialize entities and collections loaded from database.
 		/// </summary>
 		/// <param name="reader">The reader having loaded the query.</param>
 		/// <param name="cacheBatcher">A cache batcher for batching cache puts of entities and collections.</param>