fixup! Fix shortcomings of QueryBatcher initial implementation

Additional cleanup/doc originating from #1788 but which should be here

Author: fredericDelaporte

src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs

@@ -384,6 +384,7 @@ public async Task UsingHqlToFutureWithCacheAndTransformerDoesntThrowAsync()
 						.ToList();
 
 				Assert.That(results.Count, Is.EqualTo(2));
+				await (t.CommitAsync());
 			}
 		}
 

src/NHibernate/Async/Multi/IQueryBatchItem.cs

@@ -23,9 +23,10 @@ public partial interface IQueryBatchItem
 	{
 
 		/// <summary>
-		/// Returns commands generated by query
+		/// Get the commands to execute for getting the not-already cached results of this query.
 		/// </summary>
 		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
+		/// <returns>The commands for obtaining the results not already cached.</returns>
 		Task<IEnumerable<ISqlCommand>> GetCommandsAsync(CancellationToken cancellationToken);
 
 		/// <summary>
@@ -36,13 +37,16 @@ public partial interface IQueryBatchItem
 		Task<int> ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken);
 
 		/// <summary>
-		/// Executed after all commands in batch are processed
+		/// Process the results of the query, including cached results.
 		/// </summary>
 		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
+		/// <remarks>Any result from the database must have been previously processed
+		/// through <see cref="ProcessResultsSet"/>.</remarks>
 		Task ProcessResultsAsync(CancellationToken cancellationToken);
 
 		/// <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);

src/NHibernate/Async/Multi/QueryBatch.cs

@@ -119,7 +119,7 @@ protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
 			// calling CombineQueries.
 			await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false);
 
-			bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
+			var statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
 			Stopwatch stopWatch = null;
 			if (statsEnabled)
 			{
@@ -132,7 +132,7 @@ protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
 				Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql);
 			}
 
-			int rowCount = 0;
+			var rowCount = 0;
 			try
 			{
 				if (resultSetsCommand.HasQueries)

src/NHibernate/Async/Multi/QueryBatchItemBase.cs

@@ -25,12 +25,7 @@ namespace NHibernate.Multi
 	public abstract partial class QueryBatchItemBase<TResult> : IQueryBatchItem<TResult>
 	{
 
-		/// <summary>
-		/// Gets the commands to execute for getting the not-already cached results of this query. Does retrieves
-		/// already cached results by side-effect.
-		/// </summary>
-		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
-		/// <returns>The commands for obtaining the results not already cached.</returns>
+		/// <inheritdoc />
 		public async Task<IEnumerable<ISqlCommand>> GetCommandsAsync(CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
@@ -60,6 +55,7 @@ public async Task<IEnumerable<ISqlCommand>> GetCommandsAsync(CancellationToken c
 			return yields;
 		}
 
+		/// <inheritdoc />
 		public async Task<int> ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
@@ -140,10 +136,11 @@ public async Task<int> ProcessResultsSetAsync(DbDataReader reader, CancellationT
 			return rowCount;
 		}
 
+		/// <inheritdoc />
 		public async Task ProcessResultsAsync(CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
-			for (int i = 0; i < _queryInfos.Count; i++)
+			for (var i = 0; i < _queryInfos.Count; i++)
 			{
 				var queryInfo = _queryInfos[i];
 				if (_subselectResultKeys[i] != null)
@@ -172,6 +169,7 @@ public async Task ProcessResultsAsync(CancellationToken cancellationToken)
 			AfterLoadCallback?.Invoke(GetResults());
 		}
 
+		/// <inheritdoc />
 		public async Task ExecuteNonBatchedAsync(CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();

src/NHibernate/Multi/IQueryBatchItem.cs

@@ -7,38 +7,49 @@
 namespace NHibernate.Multi
 {
 	/// <summary>
-	/// Interface for wrapping query to be batched by <see cref="IQueryBatch"/>
+	/// Interface for wrapping query to be batched by <see cref="IQueryBatch"/>.
 	/// </summary>
 	public interface IQueryBatchItem<TResult> : IQueryBatchItem
 	{
 		/// <summary>
-		/// Returns loaded typed results by query.
+		/// Return loaded typed results by query.
 		/// Must be called only after <see cref="IQueryBatch.Execute"/>.
 		/// </summary>
 		IList<TResult> GetResults();
 
 		/// <summary>
-		/// Callback is executed after results are loaded by batch.
-		/// Loaded results are provided in action parameter.
+		/// A callback, executed after results are loaded by the batch.
+		/// Loaded results are provided as the action parameter.
 		/// </summary>
 		Action<IList<TResult>> AfterLoadCallback { get; set; }
 	}
 
 	/// <summary>
-	/// Interface for wrapping query to be batched by <see cref="IQueryBatch"/>
+	/// Interface for wrapping query to be batched by <see cref="IQueryBatch"/>.
 	/// </summary>
 	public partial interface IQueryBatchItem
 	{
 		/// <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>
-		/// Returns commands generated by query
+		/// Get the commands to execute for getting the not-already cached results of this query.
 		/// </summary>
+		/// <returns>The commands for obtaining the results not already cached.</returns>
 		IEnumerable<ISqlCommand> GetCommands();
 
 		/// <summary>
@@ -49,18 +60,16 @@ public partial interface IQueryBatchItem
 		int ProcessResultsSet(DbDataReader reader);
 
 		/// <summary>
-		/// Executed after all commands in batch are processed
+		/// Process the results of the query, including cached results.
 		/// </summary>
+		/// <remarks>Any result from the database must have been previously processed
+		/// through <see cref="ProcessResultsSet"/>.</remarks>
 		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>
-		/// Get cache query spaces
-		/// </summary>
-		IEnumerable<string> GetQuerySpaces();
 	}
 }

src/NHibernate/Multi/QueryBatch.cs

@@ -144,7 +144,7 @@ protected void ExecuteBatched()
 			// calling CombineQueries.
 			CombineQueries(resultSetsCommand);
 
-			bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
+			var statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
 			Stopwatch stopWatch = null;
 			if (statsEnabled)
 			{
@@ -157,7 +157,7 @@ protected void ExecuteBatched()
 				Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql);
 			}
 
-			int rowCount = 0;
+			var rowCount = 0;
 			try
 			{
 				if (resultSetsCommand.HasQueries)

src/NHibernate/Multi/QueryBatchItemBase.cs

@@ -37,6 +37,7 @@ protected class QueryLoadInfo
 
 		protected abstract List<QueryLoadInfo> GetQueryLoadInfo();
 
+		/// <inheritdoc />
 		public virtual void Init(ISessionImplementor session)
 		{
 			Session = session;
@@ -50,11 +51,13 @@ public virtual void Init(ISessionImplementor session)
 			_finalResults = null;
 		}
 
-		/// <summary>
-		/// Gets the commands to execute for getting the not-already cached results of this query. Does retrieves
-		/// already cached results by side-effect.
-		/// </summary>
-		/// <returns>The commands for obtaining the results not already cached.</returns>
+		/// <inheritdoc />
+		public IEnumerable<string> GetQuerySpaces()
+		{
+			return _queryInfos.SelectMany(q => q.QuerySpaces);
+		}
+
+		/// <inheritdoc />
 		public IEnumerable<ISqlCommand> GetCommands()
 		{
 			for (var index = 0; index < _queryInfos.Count; index++)
@@ -82,6 +85,7 @@ public IEnumerable<ISqlCommand> GetCommands()
 			}
 		}
 
+		/// <inheritdoc />
 		public int ProcessResultsSet(DbDataReader reader)
 		{
 			var dialect = Session.Factory.Dialect;
@@ -161,9 +165,10 @@ public int ProcessResultsSet(DbDataReader reader)
 			return rowCount;
 		}
 
+		/// <inheritdoc />
 		public void ProcessResults()
 		{
-			for (int i = 0; i < _queryInfos.Count; i++)
+			for (var i = 0; i < _queryInfos.Count; i++)
 			{
 				var queryInfo = _queryInfos[i];
 				if (_subselectResultKeys[i] != null)
@@ -192,17 +197,13 @@ public void ProcessResults()
 			AfterLoadCallback?.Invoke(GetResults());
 		}
 
+		/// <inheritdoc />
 		public void ExecuteNonBatched()
 		{
 			_finalResults = GetResultsNonBatched();
 			AfterLoadCallback?.Invoke(_finalResults);
 		}
 
-		public IEnumerable<string> GetQuerySpaces()
-		{
-			return _queryInfos.SelectMany(q => q.QuerySpaces);
-		}
-
 		protected abstract IList<TResult> GetResultsNonBatched();
 
 		protected List<T> GetTypedResults<T>()
@@ -211,8 +212,8 @@ protected List<T> GetTypedResults<T>()
 			{
 				throw new HibernateException("Batch wasn't executed. You must call IQueryBatch.Execute() before accessing results.");
 			}
-			List<T> results = new List<T>(_loaderResults.Sum(tr => tr.Count));
-			for (int i = 0; i < _queryInfos.Count; i++)
+			var results = new List<T>(_loaderResults.Sum(tr => tr.Count));
+			for (var i = 0; i < _queryInfos.Count; i++)
 			{
 				var list = _queryInfos[i].Loader.GetResultList(
 					_loaderResults[i],
@@ -223,11 +224,13 @@ protected List<T> GetTypedResults<T>()
 			return results;
 		}
 
+		/// <inheritdoc />
 		public IList<TResult> GetResults()
 		{
 			return _finalResults ?? (_finalResults = DoGetResults());
 		}
 
+		/// <inheritdoc />
 		public Action<IList<TResult>> AfterLoadCallback { get; set; }
 
 		protected abstract List<TResult> DoGetResults();