GH1565 Refine code as per reviewer comments:

- Restore and mark SetLockMode(LockMode) obsolete
- Only check for outer join if the dialect is not support locking entire outer join
- Rename test class to reflect that it not limited to PostgreSQL

Author: Doan Van Tuan

src/NHibernate.Test/NHSpecificTest/GH1565/PostgresOuterJoinLockTest.cs renamed to src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs

@@ -1,11 +1,9 @@
-using NHibernate.Cfg;
-using NHibernate.Dialect;
-using NUnit.Framework;
+using NUnit.Framework;
 
 namespace NHibernate.Test.NHSpecificTest.GH1565
 {
 	[TestFixture]
-	public class PostgresOuterJoinLockTest : BugTestCase
+	public class LockEntityWithOuterJoinTest : BugTestCase
 	{
 		[Test]
 		public void LockWithOuterJoin_ShouldBePossible()
@@ -16,7 +14,7 @@ public void LockWithOuterJoin_ShouldBePossible()
 				{
 					var entity = session.Get<MainEntity>(id, LockMode.Upgrade);
 					Assert.That(entity.Id, Is.EqualTo(id));
-					transaction.Rollback();
+					transaction.Commit();
 				}
 			}
 		}
@@ -46,11 +44,6 @@ protected override void OnTearDown()
 				session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate();
 			}
 		}
-
-		protected override bool AppliesTo(Dialect.Dialect dialect)
-		{
-			return dialect is PostgreSQLDialect;
-		}
 	}
 
 	public class MainEntity

src/NHibernate/SqlCommand/SqlSelectBuilder.cs

@@ -1,4 +1,5 @@
 
+using System;
 using NHibernate.Engine;
 using NHibernate.Type;
 using NHibernate.Util;
@@ -21,10 +22,9 @@ public class SqlSelectBuilder : SqlBaseBuilder, ISqlStringBuilder
 		private SqlString groupByClause;
 		private SqlString havingClause;
 		private LockMode lockMode;
-		private string aliasToLock;
+		private string mainTableAlias;
 		private string comment;
-		private bool hasOuterJoin = false;
-
+		
 		public SqlSelectBuilder(ISessionFactoryImplementor factory)
 			: base(factory.Dialect, factory) {}
 
@@ -42,7 +42,6 @@ public SqlSelectBuilder SetComment(string comment)
 		public SqlSelectBuilder SetFromClause(string fromClause)
 		{
 			this.fromClause = fromClause;
-			hasOuterJoin = hasOuterJoin || fromClause.ToLowerInvariant().Contains("left outer join");
 			return this;
 		}
 
@@ -65,8 +64,7 @@ public SqlSelectBuilder SetFromClause(string tableName, string alias)
 		/// <returns>The SqlSelectBuilder</returns>
 		public SqlSelectBuilder SetFromClause(SqlString fromClause)
 		{
-			// it is safe to do this because a fromClause will have no
-			// parameters
+			// it is safe to do this because a fromClause will have no parameters
 			return SetFromClause(fromClause.ToString());
 		}
 
@@ -113,9 +111,6 @@ public SqlSelectBuilder SetOuterJoins(SqlString outerJoinsAfterFrom, SqlString o
 			}
 
 			this.outerJoinsAfterWhere = tmpOuterJoinsAfterWhere;
-
-			hasOuterJoin = hasOuterJoin || 
-			               outerJoinsAfterFrom != null && outerJoinsAfterFrom.ToString().ToLowerInvariant().Contains("outer join");
 			return this;
 		}
 
@@ -187,10 +182,18 @@ public SqlSelectBuilder SetHavingClause(SqlString havingSqlString)
 			return this;
 		}
 
-		public SqlSelectBuilder SetLockMode(LockMode lockMode, string alias)
+		[Obsolete("For some DBMS's such as PostgreSQL, a lock on query with OUTER JOIN is not possible without specifying the not-null side. " +
+				  "Use the new method SetLockMode(LockMode, mainTableAlias) instead.")]
+		public SqlSelectBuilder SetLockMode(LockMode lockMode)
+		{
+			this.lockMode = lockMode;
+			return this;
+		}
+
+		public SqlSelectBuilder SetLockMode(LockMode lockMode, string mainTableAlias)
 		{
 			this.lockMode = lockMode;
-			aliasToLock = alias;
+			this.mainTableAlias = mainTableAlias;
 			return this;
 		}
 
@@ -300,17 +303,23 @@ public SqlString ToSqlString()
 
 		private string GetForUpdateString()
 		{
-			if (!Dialect.SupportsOuterJoinForUpdate && hasOuterJoin)
-			{
-				if (Equals(lockMode, LockMode.Upgrade))
-					return Dialect.GetForUpdateString(aliasToLock);
-				if (Equals(lockMode, LockMode.UpgradeNoWait))
-					return Dialect.GetForUpdateNowaitString(aliasToLock);
-			}
+			if (Dialect.SupportsOuterJoinForUpdate || !HasOuterJoin())
+				return Dialect.GetForUpdateString(lockMode);
+
+			if (Equals(lockMode, LockMode.Upgrade))
+				return Dialect.GetForUpdateString(mainTableAlias);
+			if (Equals(lockMode, LockMode.UpgradeNoWait))
+				return Dialect.GetForUpdateNowaitString(mainTableAlias);
 
 			return Dialect.GetForUpdateString(lockMode);
-		}
 
+			bool HasOuterJoin()
+			{
+				return
+					StringHelper.ContainsCaseInsensitive(fromClause, "outer join") ||
+					!string.IsNullOrWhiteSpace(outerJoinsAfterFrom?.ToString());
+			}
+		}
 		#endregion
 	}
 }