NH-2319 - Refactoring to be squashed.

Author: fredericDelaporte

src/NHibernate/Impl/SessionImpl.cs

@@ -550,48 +550,31 @@ public override IQuery CreateQuery(IQueryExpression queryExpression)
 
 		public override void List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results)
 		{
-			using (new SessionIdLoggingContext(SessionId))
-			{
-				CheckAndUpdateSessionStatus();
-				queryParameters.ValidateParameters();
-				var plan = GetHQLQueryPlan(queryExpression, false);
-
-				AutoFlushIfRequired(plan.QuerySpaces);
-
-				bool success = false;
-				using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called
-				{
-					try
-					{
-						plan.PerformList(queryParameters, this, results);
-						success = true;
-					}
-					catch (HibernateException)
-					{
-						// Do not call Convert on HibernateExceptions
-						throw;
-					}
-					catch (Exception e)
-					{
-						throw Convert(e, "Could not execute query");
-					}
-					finally
-					{
-						AfterOperation(success);
-					}
-				}
-			}
+			List(queryExpression, queryParameters, results, null);
 		}
 
 		protected override void ListFilter(object collection, IQueryExpression queryExpression, QueryParameters queryParameters, IList results)
+		{
+			if (collection == null)
+				throw new ArgumentNullException(nameof(collection));
+			List(queryExpression, queryParameters, results, collection);
+		}
+
+		private void List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results, object filterConnection)
 		{
 			using (new SessionIdLoggingContext(SessionId))
 			{
 				CheckAndUpdateSessionStatus();
 				queryParameters.ValidateParameters();
-				var plan = GetFilterQueryPlan(collection, queryExpression, queryParameters, false);
 
-				AutoFlushIfRequired(plan.QuerySpaces);
+				var isFilter = filterConnection != null;
+				var plan = isFilter
+					? GetFilterQueryPlan(filterConnection, queryExpression, queryParameters, false)
+					: GetHQLQueryPlan(queryExpression, false);
+
+				// GetFilterQueryPlan has already auto flushed or fully flush.
+				if (!isFilter)
+					AutoFlushIfRequired(plan.QuerySpaces);
 
 				bool success = false;
 				using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called
@@ -751,69 +734,27 @@ public override IQuery CreateFilter(object collection, IQueryExpression queryExp
 
 		private IQueryExpressionPlan GetFilterQueryPlan(object collection, IQueryExpression queryExpression, QueryParameters parameters, bool shallow)
 		{
-			using (new SessionIdLoggingContext(SessionId))
-			{
-				CollectionEntry entry = persistenceContext.GetCollectionEntryOrNull(collection);
-				ICollectionPersister roleBeforeFlush = (entry == null) ? null : entry.LoadedPersister;
-
-				IQueryExpressionPlan plan;
-				if (roleBeforeFlush == null)
-				{
-					// if it was previously unreferenced, we need to flush in order to
-					// get its state into the database in order to execute query
-					Flush();
-					entry = persistenceContext.GetCollectionEntryOrNull(collection);
-					ICollectionPersister roleAfterFlush = (entry == null) ? null : entry.LoadedPersister;
-					if (roleAfterFlush == null)
-					{
-						throw new QueryException("The collection was unreferenced");
-					}
-					plan = Factory.QueryPlanCache.GetFilterQueryPlan(queryExpression, roleAfterFlush.Role, shallow, EnabledFilters);
-				}
-				else
-				{
-					// otherwise, we only need to flush if there are in-memory changes
-					// to the queried tables
-					plan = Factory.QueryPlanCache.GetFilterQueryPlan(queryExpression, roleBeforeFlush.Role, shallow, EnabledFilters);
-
-					if (AutoFlushIfRequired(plan.QuerySpaces))
-					{
-						// might need to run a different filter entirely after the flush
-						// because the collection role may have changed
-						entry = persistenceContext.GetCollectionEntryOrNull(collection);
-						ICollectionPersister roleAfterFlush = (entry == null) ? null : entry.LoadedPersister;
-						if (roleBeforeFlush != roleAfterFlush)
-						{
-							if (roleAfterFlush == null)
-							{
-								throw new QueryException("The collection was dereferenced");
-							}
-							plan = Factory.QueryPlanCache.GetFilterQueryPlan(queryExpression, roleAfterFlush.Role, shallow, EnabledFilters);
-						}
-					}
-				}
-
-				if (parameters != null)
-				{
-					parameters.PositionalParameterValues[0] = entry.LoadedKey;
-					parameters.PositionalParameterTypes[0] = entry.LoadedPersister.KeyType;
-				}
-
-				return plan;
-			}
+			return GetFilterQueryPlan(collection, parameters,
+				role => Factory.QueryPlanCache.GetFilterQueryPlan(queryExpression, role, shallow, EnabledFilters));
 		}
 
 		private IQueryExpressionPlan GetFilterQueryPlan(object collection, string filter, QueryParameters parameters, bool shallow)
+		{
+			return GetFilterQueryPlan(collection, parameters,
+				role => Factory.QueryPlanCache.GetFilterQueryPlan(filter, role, shallow, EnabledFilters));
+		}
+
+		private IQueryExpressionPlan GetFilterQueryPlan(object collection, QueryParameters parameters, Func<string, IQueryExpressionPlan> getPlanForRole)
 		{
 			using (new SessionIdLoggingContext(SessionId))
 			{
 				if (collection == null)
 				{
-					throw new ArgumentNullException("collection", "null collection passed to filter");
+					throw new ArgumentNullException(nameof(collection), "null collection passed to filter");
 				}
 
-				CollectionEntry entry = persistenceContext.GetCollectionEntryOrNull(collection);
-				ICollectionPersister roleBeforeFlush = (entry == null) ? null : entry.LoadedPersister;
+				var entry = persistenceContext.GetCollectionEntryOrNull(collection);
+				var roleBeforeFlush = entry?.LoadedPersister;
 
 				IQueryExpressionPlan plan;
 				if (roleBeforeFlush == null)
@@ -822,31 +763,31 @@ private IQueryExpressionPlan GetFilterQueryPlan(object collection, string filter
 					// get its state into the database in order to execute query
 					Flush();
 					entry = persistenceContext.GetCollectionEntryOrNull(collection);
-					ICollectionPersister roleAfterFlush = (entry == null) ? null : entry.LoadedPersister;
+					var roleAfterFlush = entry?.LoadedPersister;
 					if (roleAfterFlush == null)
 					{
 						throw new QueryException("The collection was unreferenced");
 					}
-					plan = Factory.QueryPlanCache.GetFilterQueryPlan(filter, roleAfterFlush.Role, shallow, EnabledFilters);
+					plan = getPlanForRole(roleAfterFlush.Role);
 				}
 				else
 				{
 					// otherwise, we only need to flush if there are in-memory changes
 					// to the queried tables
-					plan = Factory.QueryPlanCache.GetFilterQueryPlan(filter, roleBeforeFlush.Role, shallow, EnabledFilters);
+					plan = getPlanForRole(roleBeforeFlush.Role);
 					if (AutoFlushIfRequired(plan.QuerySpaces))
 					{
 						// might need to run a different filter entirely after the flush
 						// because the collection role may have changed
 						entry = persistenceContext.GetCollectionEntryOrNull(collection);
-						ICollectionPersister roleAfterFlush = (entry == null) ? null : entry.LoadedPersister;
+						var roleAfterFlush = entry?.LoadedPersister;
 						if (roleBeforeFlush != roleAfterFlush)
 						{
 							if (roleAfterFlush == null)
 							{
 								throw new QueryException("The collection was dereferenced");
 							}
-							plan = Factory.QueryPlanCache.GetFilterQueryPlan(filter, roleAfterFlush.Role, shallow, EnabledFilters);
+							plan = getPlanForRole(roleAfterFlush.Role);
 						}
 					}
 				}

src/NHibernate/Linq/NhQueryable.cs

@@ -27,7 +27,7 @@ public NhQueryable(ISessionImplementor session)
 
 		// This constructor is called by our users, create a new IQueryExecutor.
 		public NhQueryable(ISessionImplementor session, string entityName)
-			: base(QueryProviderFactory.CreateQueryProvider(session))
+			: base(QueryProviderFactory.CreateQueryProvider(session, null))
 		{
 			EntityName = entityName;
 		}
@@ -47,7 +47,7 @@ public NhQueryable(IQueryProvider provider, Expression expression, string entity
 		}
 
 		public NhQueryable(ISessionImplementor session, object collection)
-			: base(new DefaultQueryProvider(session, collection))
+			: base(QueryProviderFactory.CreateQueryProvider(session, collection))
 		{
 			EntityName = typeof(T).FullName;
 		}
@@ -59,4 +59,4 @@ public override string ToString()
 			return "NHibernate.Linq.NhQueryable`1[" + EntityName + "]";
 		}
 	}
-}
+}

src/NHibernate/Linq/QueryProviderFactory.cs

@@ -9,16 +9,20 @@ static class QueryProviderFactory
 		/// Builds a new query provider.
 		/// </summary>
 		/// <param name="session">A session.</param>
+		/// <param name="collection">If the query is to be filtered as belonging to an entity collection, the collection.</param>
 		/// <returns>The new query provider instance.</returns>
-		public static INhQueryProvider CreateQueryProvider(ISessionImplementor session)
+		public static INhQueryProvider CreateQueryProvider(ISessionImplementor session, object collection)
 		{
 			if (session.Factory.Settings.LinqQueryProviderType == null)
 			{
-				return new DefaultQueryProvider(session);
+				return new DefaultQueryProvider(session, collection);
 			}
 			else
 			{
-				return Activator.CreateInstance(session.Factory.Settings.LinqQueryProviderType, session) as INhQueryProvider;
+				// For backward compatibility, prioritize using the version without collection.
+				return (collection == null
+					? Activator.CreateInstance(session.Factory.Settings.LinqQueryProviderType, session)
+					: Activator.CreateInstance(session.Factory.Settings.LinqQueryProviderType, session, collection)) as INhQueryProvider;
 			}
 		}
 	}