Fixes nhibernate#3631

Incrementing Version on parent entity for a collection change works correctly
even with optimistic-lock="false" on another property of the same entity.
Call to HasDirtyCollections has a side effect thus must be called independently
from the existence of other dirty properties.

Author: csharper2010

src/NHibernate.Test/VersionTest/Db/DbVersionFixture.cs

@@ -42,6 +42,7 @@ public void CollectionVersion()
 			admin = s.Get<Group>(admin.Id);
 			guy.Groups.Add(admin);
 			admin.Users.Add(guy);
+			guy.NoOptimisticLock = "changed";
 			t.Commit();
 			s.Close();
 

src/NHibernate.Test/VersionTest/Db/User.cs

@@ -11,8 +11,10 @@ public class User
 
 		public virtual string Username { get; set; }
 
+		public virtual string NoOptimisticLock { get; set; }
+
 		public virtual ISet<Group> Groups { get; set; }
 
 		public virtual ISet<Permission> Permissions { get; set; }
 	}
-}
+}

src/NHibernate.Test/VersionTest/Db/User.hbm.xml

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8" ?>
+<?xml version="1.0" encoding="utf-8" ?>
 <!--
     Demonstrates how to control the optimistic locking behavior
     of a collection (do changes to the collection result in
@@ -14,6 +14,7 @@
 		</id>
 		<timestamp name="Timestamp" column="ts" source="db"/>
 		<property name="Username" column="user_name" type="string" unique="true"/>
+		<property name="NoOptimisticLock" column="no_optimistic_lock" type="string" optimistic-lock="false"/>
 		<set name="Groups" table="db_vers_user_group" batch-size="9" inverse="true" optimistic-lock="true" lazy="true" cascade="none" >
 			<key column="user_id"/>
 			<many-to-many column="group_id" class="Group" lazy="false" fetch="join" />
@@ -45,4 +46,4 @@
 		<property name="Context" column="ctx" type="string"/>
 		<property name="Access" column="priv" type="string"/>
 	</class>
-</hibernate-mapping>
+</hibernate-mapping>

src/NHibernate/Event/Default/DefaultFlushEntityEventListener.cs

@@ -389,18 +389,22 @@ protected bool IsUpdateNecessary(FlushEntityEvent @event)
 			}
 			else
 			{
+				// call to HasDirtyCollections must not be optimized away because of its side effect
+				bool hasDirtyCollections = HasDirtyCollections(@event, persister, status);
+
 				int[] dirtyProperties = @event.DirtyProperties;
-				if (dirtyProperties != null && dirtyProperties.Length != 0)
-				{
-					return true; //TODO: suck into event class
-				}
-				else
-				{
-					return HasDirtyCollections(@event, persister, status);
-				}
+				return dirtyProperties != null && dirtyProperties.Length != 0 || hasDirtyCollections;
 			}
 		}
 
+		/// <summary>
+		/// Check if there are any dirty collections.
+		/// Has a side effect of setting the HasDirtyCollection property of the event.
+		/// </summary>
+		/// <param name="event"></param>
+		/// <param name="persister"></param>
+		/// <param name="status"></param>
+		/// <returns></returns>
 		private bool HasDirtyCollections(FlushEntityEvent @event, IEntityPersister persister, Status status)
 		{
 			if (IsCollectionDirtyCheckNecessary(persister, status))