Revert #378 - NH-3480

The NH-3453 part is not reverted. The NH-3480 part was wrecking the
many-to-many mapping with property ref.

Author: fredericDelaporte

src/NHibernate/Async/Engine/Collections.cs

@@ -145,51 +145,41 @@ private static Task ProcessNeverReferencedCollectionAsync(IPersistentCollection
 		/// <param name="entity">The owner of the collection. </param>
 		/// <param name="session">The session.</param>
 		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
-		public static Task ProcessReachableCollectionAsync(IPersistentCollection collection, CollectionType type, object entity, ISessionImplementor session, CancellationToken cancellationToken)
+		public static async Task ProcessReachableCollectionAsync(IPersistentCollection collection, CollectionType type, object entity, ISessionImplementor session, CancellationToken cancellationToken)
 		{
-			if (cancellationToken.IsCancellationRequested)
+			cancellationToken.ThrowIfCancellationRequested();
+			collection.Owner = entity;
+			CollectionEntry ce = session.PersistenceContext.GetCollectionEntry(collection);
+
+			if (ce == null)
 			{
-				return Task.FromCanceled<object>(cancellationToken);
+				// refer to comment in StatefulPersistenceContext.addCollection()
+				throw new HibernateException(string.Format("Found two representations of same collection: {0}", type.Role));
 			}
-			try
-			{
-				collection.Owner = entity;
-				CollectionEntry ce = session.PersistenceContext.GetCollectionEntry(collection);
 
-				if (ce == null)
-				{
-					// refer to comment in StatefulPersistenceContext.addCollection()
-					return Task.FromException<object>(new HibernateException(string.Format("Found two representations of same collection: {0}", type.Role)));
-				}
-
-				// The CollectionEntry.isReached() stuff is just to detect any silly users  
-				// who set up circular or shared references between/to collections.
-				if (ce.IsReached)
-				{
-					// We've been here before
-					return Task.FromException<object>(new HibernateException(string.Format("Found shared references to a collection: {0}", type.Role)));
-				}
-				ce.IsReached = true;
+			// The CollectionEntry.isReached() stuff is just to detect any silly users  
+			// who set up circular or shared references between/to collections.
+			if (ce.IsReached)
+			{
+				// We've been here before
+				throw new HibernateException(string.Format("Found shared references to a collection: {0}", type.Role));
+			}
+			ce.IsReached = true;
 
-				ISessionFactoryImplementor factory = session.Factory;
-				ICollectionPersister persister = factory.GetCollectionPersister(type.Role);
-				ce.CurrentPersister = persister;
-				ce.CurrentKey = type.GetKeyOfOwner(entity, session); //TODO: better to pass the id in as an argument?
+			ISessionFactoryImplementor factory = session.Factory;
+			ICollectionPersister persister = factory.GetCollectionPersister(type.Role);
+			ce.CurrentPersister = persister;
+			ce.CurrentKey = await (type.GetKeyOfOwnerAsync(entity, session, cancellationToken)).ConfigureAwait(false); //TODO: better to pass the id in as an argument?
 
-				if (log.IsDebugEnabled())
-				{
-					log.Debug("Collection found: {0}, was: {1}{2}",
+			if (log.IsDebugEnabled())
+			{
+				log.Debug("Collection found: {0}, was: {1}{2}",
 				          MessageHelper.CollectionInfoString(persister, collection, ce.CurrentKey, session),
 				          MessageHelper.CollectionInfoString(ce.LoadedPersister, collection, ce.LoadedKey, session),
 				          (collection.WasInitialized ? " (initialized)" : " (uninitialized)"));
-				}
-
-				return PrepareCollectionForUpdateAsync(collection, ce, factory, cancellationToken);
-			}
-			catch (System.Exception ex)
-			{
-				return Task.FromException<object>(ex);
 			}
+
+			await (PrepareCollectionForUpdateAsync(collection, ce, factory, cancellationToken)).ConfigureAwait(false);
 		}
 
 		private static Task PrepareCollectionForUpdateAsync(IPersistentCollection collection, CollectionEntry entry, ISessionFactoryImplementor factory, CancellationToken cancellationToken)

src/NHibernate/Async/Loader/Loader.cs

@@ -238,7 +238,7 @@ private async Task ReadCollectionElementsAsync(object[] row, DbDataReader result
 					}
 					else
 					{
-						key = collectionPersister.CollectionType.GetKeyOfOwner(owner, session);
+						key = await (collectionPersister.CollectionType.GetKeyOfOwnerAsync(owner, session, cancellationToken)).ConfigureAwait(false);
 						//TODO: old version did not require hashmap lookup:
 						//keys[collectionOwner].getIdentifier()
 					}

src/NHibernate/Async/Type/CollectionType.cs

@@ -81,33 +81,23 @@ public override Task NullSafeSetAsync(DbCommand cmd, object value, int index, IS
 			}
 		}
 
-		public override Task<object> DisassembleAsync(object value, ISessionImplementor session, object owner, CancellationToken cancellationToken)
+		public override async Task<object> DisassembleAsync(object value, ISessionImplementor session, object owner, CancellationToken cancellationToken)
 		{
-			if (cancellationToken.IsCancellationRequested)
-			{
-				return Task.FromCanceled<object>(cancellationToken);
-			}
-			try
-			{
-				//remember the uk value
+			cancellationToken.ThrowIfCancellationRequested();
+			//remember the uk value
 
-				//This solution would allow us to eliminate the owner arg to disassemble(), but
-				//what if the collection was null, and then later had elements added? seems unsafe
-				//session.getPersistenceContext().getCollectionEntry( (PersistentCollection) value ).getKey();
+			//This solution would allow us to eliminate the owner arg to disassemble(), but
+			//what if the collection was null, and then later had elements added? seems unsafe
+			//session.getPersistenceContext().getCollectionEntry( (PersistentCollection) value ).getKey();
 
-				object key = GetKeyOfOwner(owner, session);
-				if (key == null)
-				{
-					return Task.FromResult<object>(null);
-				}
-				else
-				{
-					return GetPersister(session).KeyType.DisassembleAsync(key, session, owner, cancellationToken);
-				}
+			object key = await (GetKeyOfOwnerAsync(owner, session, cancellationToken)).ConfigureAwait(false);
+			if (key == null)
+			{
+				return null;
 			}
-			catch (Exception ex)
+			else
 			{
-				return Task.FromException<object>(ex);
+				return await (GetPersister(session).KeyType.DisassembleAsync(key, session, owner, cancellationToken)).ConfigureAwait(false);
 			}
 		}
 
@@ -152,20 +142,10 @@ public override Task<object> HydrateAsync(DbDataReader rs, string[] name, ISessi
 			}
 		}
 
-		public override Task<object> ResolveIdentifierAsync(object key, ISessionImplementor session, object owner, CancellationToken cancellationToken)
+		public override async Task<object> ResolveIdentifierAsync(object key, ISessionImplementor session, object owner, CancellationToken cancellationToken)
 		{
-			if (cancellationToken.IsCancellationRequested)
-			{
-				return Task.FromCanceled<object>(cancellationToken);
-			}
-			try
-			{
-				return ResolveKeyAsync(GetKeyOfOwner(owner, session), session, owner, cancellationToken);
-			}
-			catch (Exception ex)
-			{
-				return Task.FromException<object>(ex);
-			}
+			cancellationToken.ThrowIfCancellationRequested();
+			return await (ResolveKeyAsync(await (GetKeyOfOwnerAsync(owner, session, cancellationToken)).ConfigureAwait(false), session, owner, cancellationToken)).ConfigureAwait(false);
 		}
 
 		private Task<object> ResolveKeyAsync(object key, ISessionImplementor session, object owner, CancellationToken cancellationToken)
@@ -310,5 +290,42 @@ public override Task<bool> IsModifiedAsync(object oldHydratedState, object curre
 				return Task.FromException<bool>(ex);
 			}
 		}
+
+		/// <summary>
+		/// Get the key value from the owning entity instance. It is usually the identifier, but it might be some
+		/// other unique key, in the case of a property-ref.
+		/// </summary>
+		public async Task<object> GetKeyOfOwnerAsync(object owner, ISessionImplementor session, CancellationToken cancellationToken)
+		{
+			cancellationToken.ThrowIfCancellationRequested();
+			var entityEntry = session.PersistenceContext.GetEntry(owner);
+			if (entityEntry == null)
+			{
+				// This just handles a particular case of component
+				// projection, perhaps get rid of it and throw an exception
+				return null;
+			}
+
+			if (foreignKeyPropertyName == null)
+			{
+				return entityEntry.Id;
+			}
+
+			// TODO: at the point where we are resolving collection references, we don't
+			// know if the uk value has been resolved (depends if it was earlier or
+			// later in the mapping document) - now, we could try and use e.getStatus()
+			// to decide to semiResolve(), trouble is that initializeEntity() reuses
+			// the same array for resolved and hydrated values
+			var id = entityEntry.LoadedState != null
+				? entityEntry.GetLoadedValue(foreignKeyPropertyName)
+				: entityEntry.Persister.GetPropertyValue(owner, foreignKeyPropertyName);
+			// NOTE VERY HACKISH WORKAROUND!!
+			var keyType = GetPersister(session).KeyType;
+			if (!keyType.ReturnedClass.IsInstanceOfType(id))
+			{
+				id = await (keyType.SemiResolveAsync(entityEntry.GetLoadedValue(foreignKeyPropertyName), session, owner, cancellationToken)).ConfigureAwait(false);
+			}
+			return id;
+		}
 	}
 }

src/NHibernate/Engine/JoinHelper.cs

@@ -27,7 +27,7 @@ public static string[] GetRHSColumnNames(IAssociationType type, ISessionFactoryI
 			IJoinable joinable = type.GetAssociatedJoinable(factory);
 			if (uniqueKeyPropertyName == null)
 			{
-				return joinable.JoinColumnNames;
+				return joinable.KeyColumnNames;
 			}
 			else
 			{
@@ -208,4 +208,4 @@ public override string GetTableName(IAssociationType type)
 		#endregion
 	}
 
-}
+}

src/NHibernate/Loader/Collection/OneToManyJoinWalker.cs

@@ -58,7 +58,7 @@ private void InitStatementString(IOuterJoinLoadable elementPersister, string ali
 			int collectionJoins = CountCollectionPersisters(associations) + 1;
 			CollectionSuffixes = BasicLoader.GenerateSuffixes(joins + 1, collectionJoins);
 
-			SqlStringBuilder whereString = WhereString(oneToManyPersister.GenerateTableAliasForKeyColumns(alias), oneToManyPersister.KeyColumnNames, subquery, batchSize);
+			SqlStringBuilder whereString = WhereString(alias, oneToManyPersister.KeyColumnNames, subquery, batchSize);
 			string filter = oneToManyPersister.FilterFragment(alias, EnabledFilters);
 			whereString.Insert(0, StringHelper.MoveAndToBeginning(filter));
 
@@ -71,7 +71,7 @@ private void InitStatementString(IOuterJoinLoadable elementPersister, string ali
 #pragma warning restore 618
 						SelectString(associations))
 					.SetFromClause(
-						elementPersister.FromTableFragment(alias) + oneToManyPersister.FromJoinFragment(alias, true, true))
+						elementPersister.FromTableFragment(alias) + elementPersister.FromJoinFragment(alias, true, true))
 					.SetWhereClause(whereString.ToSqlString())
 					.SetOuterJoins(ojf.ToFromFragmentString,
 					               ojf.ToWhereFragmentString + elementPersister.WhereJoinFragment(alias, true, true));

src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs

@@ -86,7 +86,6 @@ public abstract partial class AbstractCollectionPersister : ICollectionMetadata,
 		private readonly string[] keyColumnAliases;
 		private readonly string identifierColumnName;
 		private readonly string identifierColumnAlias;
-		private readonly string[] joinColumnNames;
 
 		#endregion
 
@@ -226,45 +225,16 @@ public AbstractCollectionPersister(Mapping.Collection collection, ICacheConcurre
 
 			isVersioned = collection.IsOptimisticLocked;
 
-			if (collection.CollectionType.UseLHSPrimaryKey)
+			keyType = collection.Key.Type;
+			int keySpan = collection.Key.ColumnSpan;
+			keyColumnNames = new string[keySpan];
+			keyColumnAliases = new string[keySpan];
+			int k = 0;
+			foreach (Column col in collection.Key.ColumnIterator)
 			{
-				keyType = collection.Key.Type;
-				int keySpan = collection.Key.ColumnSpan;
-				keyColumnNames = new string[keySpan];
-				keyColumnAliases = new string[keySpan];
-				int k = 0;
-				foreach (Column col in collection.Key.ColumnIterator)
-				{
-					keyColumnNames[k] = col.GetQuotedName(dialect);
-					keyColumnAliases[k] = col.GetAlias(dialect, table);
-					k++;
-				}
-				joinColumnNames = keyColumnNames;
-			}
-			else
-			{
-				keyType = collection.Owner.Key.Type;
-				int keySpan = collection.Owner.Key.ColumnSpan;
-				keyColumnNames = new string[keySpan];
-				keyColumnAliases = new string[keySpan];
-				int k = 0;
-				foreach (Column col in collection.Owner.Key.ColumnIterator)
-				{
-					keyColumnNames[k] = col.GetQuotedName(dialect);
-					// Force the alias to be unique in case it conflicts with an alias in the entity
-					// As per Column.GetAlias, we have 3 characters left for SelectFragment suffix and one for here.
-					// Since suffixes are composed of digits and '_', and GetAlias is already suffixed, adding any other
-					// letter will avoid collision.
-					keyColumnAliases[k] = col.GetAlias(dialect) + "o";
-					k++;
-				}
-				joinColumnNames = new string[collection.Key.ColumnSpan];
-				k = 0;
-				foreach (Column col in collection.Key.ColumnIterator)
-				{
-					joinColumnNames[k] = col.GetQuotedName(dialect);
-					k++;
-				}
+				keyColumnNames[k] = col.GetQuotedName(dialect);
+				keyColumnAliases[k] = col.GetAlias(dialect, table);
+				k++;
 			}
 
 			HashSet<string> distinctColumns = new HashSet<string>();
@@ -1889,9 +1859,11 @@ public string[] KeyColumnNames
 			get { return keyColumnNames; }
 		}
 
+		// Since v5.2
+		[Obsolete("Use KeyColumnNames instead")]
 		public string[] JoinColumnNames
 		{
-			get { return joinColumnNames; }
+			get { return keyColumnNames; }
 		}
 
 		protected string[] KeyColumnAliases
@@ -2071,6 +2043,8 @@ protected Dialect.Dialect Dialect
 		public abstract bool CascadeDeleteEnabled { get; }
 		public abstract bool IsOneToMany { get; }
 
+		// Since v5.2
+		[Obsolete("Use directly the alias parameter value instead")]
 		public virtual string GenerateTableAliasForKeyColumns(string alias)
 		{
 			return alias;

src/NHibernate/Persister/Collection/IQueryableCollection.cs

@@ -1,3 +1,4 @@
+using System;
 using NHibernate.Persister.Entity;
 
 namespace NHibernate.Persister.Collection
@@ -81,11 +82,13 @@ public interface IQueryableCollection : IPropertyMapping, IJoinable, ICollection
 		/// <returns>Appropriate order-by fragment or empty string.</returns>
 		string GetManyToManyOrderByString(string alias);
 
+		// Obsolete since v5.2
 		/// <summary>
 		/// Generate the table alias to use for the collection's key columns
 		/// </summary>
 		/// <param name="alias">The alias for the target table</param>
 		/// <returns>Appropriate table alias.</returns>
+		[Obsolete("Use directly the alias parameter value instead")]
 		string GenerateTableAliasForKeyColumns(string alias);
 	}
-}
+}