From 770713b249719303b926ae2fd2348b75589aa7e6 Mon Sep 17 00:00:00 2001
From: CSharper2010 <csharper2010@googlemail.com>
Date: Tue, 10 Dec 2024 14:46:54 +0100
Subject: [PATCH 1/2] Fixes #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.

---
 .../VersionTest/Db/DbVersionFixture.cs        |  1 +
 src/NHibernate.Test/VersionTest/Db/User.cs    |  4 +++-
 .../VersionTest/Db/User.hbm.xml               |  5 +++--
 .../DefaultFlushEntityEventListener.cs        | 20 +++++++++++--------
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/NHibernate.Test/VersionTest/Db/DbVersionFixture.cs b/src/NHibernate.Test/VersionTest/Db/DbVersionFixture.cs
index c5c50b96ab8..7e22a859145 100644
--- a/src/NHibernate.Test/VersionTest/Db/DbVersionFixture.cs
+++ b/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();
 
diff --git a/src/NHibernate.Test/VersionTest/Db/User.cs b/src/NHibernate.Test/VersionTest/Db/User.cs
index 6dc9368c55f..ad0aafb833b 100644
--- a/src/NHibernate.Test/VersionTest/Db/User.cs
+++ b/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; }
 	}
-}
\ No newline at end of file
+}
diff --git a/src/NHibernate.Test/VersionTest/Db/User.hbm.xml b/src/NHibernate.Test/VersionTest/Db/User.hbm.xml
index b809ccc8c72..1dfc4080a07 100644
--- a/src/NHibernate.Test/VersionTest/Db/User.hbm.xml
+++ b/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>
\ No newline at end of file
+</hibernate-mapping>
diff --git a/src/NHibernate/Event/Default/DefaultFlushEntityEventListener.cs b/src/NHibernate/Event/Default/DefaultFlushEntityEventListener.cs
index 7848f25cdaa..749273d9107 100644
--- a/src/NHibernate/Event/Default/DefaultFlushEntityEventListener.cs
+++ b/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))

From 83fd64a61ad9ac60f6a2796e7d8e7581f8520b58 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Tue, 10 Dec 2024 14:22:12 +0000
Subject: [PATCH 2/2] Generate async files

---
 .../Async/VersionTest/Db/DbVersionFixture.cs  |  1 +
 .../DefaultFlushEntityEventListener.cs        | 30 ++++++++++---------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/NHibernate.Test/Async/VersionTest/Db/DbVersionFixture.cs b/src/NHibernate.Test/Async/VersionTest/Db/DbVersionFixture.cs
index fe5a770c4c9..b3ec27add5f 100644
--- a/src/NHibernate.Test/Async/VersionTest/Db/DbVersionFixture.cs
+++ b/src/NHibernate.Test/Async/VersionTest/Db/DbVersionFixture.cs
@@ -52,6 +52,7 @@ public async System.Threading.Tasks.Task CollectionVersionAsync()
 			admin = await (s.GetAsync<Group>(admin.Id));
 			guy.Groups.Add(admin);
 			admin.Users.Add(guy);
+			guy.NoOptimisticLock = "changed";
 			await (t.CommitAsync());
 			s.Close();
 
diff --git a/src/NHibernate/Async/Event/Default/DefaultFlushEntityEventListener.cs b/src/NHibernate/Async/Event/Default/DefaultFlushEntityEventListener.cs
index d70bbb4b562..19b418755dd 100644
--- a/src/NHibernate/Async/Event/Default/DefaultFlushEntityEventListener.cs
+++ b/src/NHibernate/Async/Event/Default/DefaultFlushEntityEventListener.cs
@@ -336,33 +336,35 @@ private async Task<object> GetNextVersionAsync(FlushEntityEvent @event, Cancella
 		/// to synchronize its state to the database. Modifies the event by side-effect!
 		/// Note: this method is quite slow, avoid calling if possible!
 		/// </summary>
-		protected Task<bool> IsUpdateNecessaryAsync(FlushEntityEvent @event, CancellationToken cancellationToken)
+		protected async Task<bool> IsUpdateNecessaryAsync(FlushEntityEvent @event, CancellationToken cancellationToken)
 		{
-			if (cancellationToken.IsCancellationRequested)
-			{
-				return Task.FromCanceled<bool>(cancellationToken);
-			}
+			cancellationToken.ThrowIfCancellationRequested();
 			IEntityPersister persister = @event.EntityEntry.Persister;
 			Status status = @event.EntityEntry.Status;
 
 			if (!@event.DirtyCheckPossible)
 			{
-				return Task.FromResult<bool>(true);
+				return true;
 			}
 			else
 			{
+				// call to HasDirtyCollections must not be optimized away because of its side effect
+				bool hasDirtyCollections = await (HasDirtyCollectionsAsync(@event, persister, status, cancellationToken)).ConfigureAwait(false);
+
 				int[] dirtyProperties = @event.DirtyProperties;
-				if (dirtyProperties != null && dirtyProperties.Length != 0)
-				{
-					return Task.FromResult<bool>(true); //TODO: suck into event class
-				}
-				else
-				{
-					return HasDirtyCollectionsAsync(@event, persister, status, cancellationToken);
-				}
+				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>
+		/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
+		/// <returns></returns>
 		private async Task<bool> HasDirtyCollectionsAsync(FlushEntityEvent @event, IEntityPersister persister, Status status, CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();