From 39a28aa283a284d02fb47a0b0f5f54ebe87a997d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Tue, 26 Jan 2021 21:41:09 +0100 Subject: [PATCH] Fix #2608 - persisting a one-to-one with delayed insert fails (#2609) Co-authored-by: Roman Artiukhin --- .../Async/NHSpecificTest/GH2608/Fixture.cs | 57 +++++++++++++++++++ .../Async/Operations/MergeFixture.cs | 39 +++++++++++++ .../NHSpecificTest/GH2608/Fixture.cs | 46 +++++++++++++++ .../NHSpecificTest/GH2608/Mappings.hbm.xml | 24 ++++++++ .../NHSpecificTest/GH2608/Person.cs | 9 +++ .../NHSpecificTest/GH2608/PersonalDetails.cs | 10 ++++ .../Operations/MergeFixture.cs | 4 +- .../Operations/OneToOne.hbm.xml | 6 +- .../Action/DelayedPostInsertIdentifier.cs | 5 ++ src/NHibernate/Action/EntityInsertAction.cs | 13 +++++ .../Async/Action/EntityInsertAction.cs | 13 +++++ .../Engine/StatefulPersistenceContext.cs | 2 +- .../Default/AbstractSaveEventListener.cs | 3 +- .../Engine/StatefulPersistenceContext.cs | 4 +- .../Default/AbstractSaveEventListener.cs | 3 +- 15 files changed, 231 insertions(+), 7 deletions(-) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/GH2608/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH2608/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH2608/Mappings.hbm.xml create mode 100644 src/NHibernate.Test/NHSpecificTest/GH2608/Person.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/GH2608/PersonalDetails.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH2608/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH2608/Fixture.cs new file mode 100644 index 00000000000..c30caa64a25 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH2608/Fixture.cs @@ -0,0 +1,57 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH2608 +{ + using System.Threading.Tasks; + [TestFixture] + public class FixtureAsync : BugTestCase + { + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.CreateQuery("delete from PersonalDetails").ExecuteUpdate(); + session.CreateQuery("delete from System.Object").ExecuteUpdate(); + + transaction.Commit(); + } + } + + [Test] + public async Task MergeBidiPrimaryKeyOneToOneAsync() + { + using (var s = OpenSession()) + using (var tx = s.BeginTransaction()) + { + var p = new Person { Name = "steve" }; + p.Details = new PersonalDetails { SomePersonalDetail = "I have big feet", Person = p }; + await (s.MergeAsync(p)); + await (tx.CommitAsync()); + } + } + + [Test] + public async Task PersistBidiPrimaryKeyOneToOneAsync() + { + using (var s = OpenSession()) + using (var tx = s.BeginTransaction()) + { + var p = new Person { Name = "steve" }; + p.Details = new PersonalDetails { SomePersonalDetail = "I have big feet", Person = p }; + await (s.PersistAsync(p)); + await (tx.CommitAsync()); + } + } + } +} diff --git a/src/NHibernate.Test/Async/Operations/MergeFixture.cs b/src/NHibernate.Test/Async/Operations/MergeFixture.cs index 9f41851d87e..ecaf09463c8 100644 --- a/src/NHibernate.Test/Async/Operations/MergeFixture.cs +++ b/src/NHibernate.Test/Async/Operations/MergeFixture.cs @@ -50,6 +50,8 @@ protected override void OnTearDown() await (s.DeleteAsync("from Competition", cancellationToken)); await (s.DeleteAsync("from Employer", cancellationToken)); + await (s.DeleteAsync("from Address", cancellationToken)); + await (s.DeleteAsync("from Person", cancellationToken)); await (tx.CommitAsync(cancellationToken)); } @@ -76,6 +78,8 @@ private void Cleanup() s.Delete("from Competition"); s.Delete("from Employer"); + s.Delete("from Address"); + s.Delete("from Person"); tx.Commit(); } @@ -156,6 +160,41 @@ public async Task MergeBidiForeignKeyOneToOneAsync() } } + [Test] + public async Task MergeBidiPrimayKeyOneToOneAsync() + { + Person p; + using (ISession s = OpenSession()) + using (ITransaction tx = s.BeginTransaction()) + { + p = new Person {Name = "steve"}; + new PersonalDetails {SomePersonalDetail = "I have big feet", Person = p}; + await (s.PersistAsync(p)); + await (tx.CommitAsync()); + } + + ClearCounts(); + + p.Details.SomePersonalDetail = p.Details.SomePersonalDetail + " and big hands too"; + using (ISession s = OpenSession()) + using (ITransaction tx = s.BeginTransaction()) + { + p = (Person) await (s.MergeAsync(p)); + await (tx.CommitAsync()); + } + + AssertInsertCount(0); + AssertUpdateCount(1); + AssertDeleteCount(0); + + using (ISession s = OpenSession()) + using (ITransaction tx = s.BeginTransaction()) + { + await (s.DeleteAsync(p)); + await (tx.CommitAsync()); + } + } + [Test] public async Task MergeDeepTreeAsync() { diff --git a/src/NHibernate.Test/NHSpecificTest/GH2608/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH2608/Fixture.cs new file mode 100644 index 00000000000..38d7ead063b --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH2608/Fixture.cs @@ -0,0 +1,46 @@ +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH2608 +{ + [TestFixture] + public class Fixture : BugTestCase + { + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.CreateQuery("delete from PersonalDetails").ExecuteUpdate(); + session.CreateQuery("delete from System.Object").ExecuteUpdate(); + + transaction.Commit(); + } + } + + [Test] + public void MergeBidiPrimaryKeyOneToOne() + { + using (var s = OpenSession()) + using (var tx = s.BeginTransaction()) + { + var p = new Person { Name = "steve" }; + p.Details = new PersonalDetails { SomePersonalDetail = "I have big feet", Person = p }; + s.Merge(p); + tx.Commit(); + } + } + + [Test] + public void PersistBidiPrimaryKeyOneToOne() + { + using (var s = OpenSession()) + using (var tx = s.BeginTransaction()) + { + var p = new Person { Name = "steve" }; + p.Details = new PersonalDetails { SomePersonalDetail = "I have big feet", Person = p }; + s.Persist(p); + tx.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH2608/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH2608/Mappings.hbm.xml new file mode 100644 index 00000000000..4ec65706c90 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH2608/Mappings.hbm.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + Person + + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/GH2608/Person.cs b/src/NHibernate.Test/NHSpecificTest/GH2608/Person.cs new file mode 100644 index 00000000000..923120dbb48 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH2608/Person.cs @@ -0,0 +1,9 @@ +namespace NHibernate.Test.NHSpecificTest.GH2608 +{ + public class Person + { + public virtual long Id { get; set; } + public virtual string Name { get; set; } + public virtual PersonalDetails Details { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH2608/PersonalDetails.cs b/src/NHibernate.Test/NHSpecificTest/GH2608/PersonalDetails.cs new file mode 100644 index 00000000000..40d0c554eae --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH2608/PersonalDetails.cs @@ -0,0 +1,10 @@ +namespace NHibernate.Test.NHSpecificTest.GH2608 +{ + public class PersonalDetails + { + public virtual long Id { get; set; } + public virtual string SomePersonalDetail { get; set; } + + public virtual Person Person { get; set; } + } +} diff --git a/src/NHibernate.Test/Operations/MergeFixture.cs b/src/NHibernate.Test/Operations/MergeFixture.cs index 44526c27ef9..e3e362b2a24 100644 --- a/src/NHibernate.Test/Operations/MergeFixture.cs +++ b/src/NHibernate.Test/Operations/MergeFixture.cs @@ -38,6 +38,8 @@ private void Cleanup() s.Delete("from Competition"); s.Delete("from Employer"); + s.Delete("from Address"); + s.Delete("from Person"); tx.Commit(); } @@ -118,7 +120,7 @@ public void MergeBidiForeignKeyOneToOne() } } - [Test, Ignore("Need some more investigation about id sync.")] + [Test] public void MergeBidiPrimayKeyOneToOne() { Person p; diff --git a/src/NHibernate.Test/Operations/OneToOne.hbm.xml b/src/NHibernate.Test/Operations/OneToOne.hbm.xml index ab672c0ce04..b6a08e09729 100644 --- a/src/NHibernate.Test/Operations/OneToOne.hbm.xml +++ b/src/NHibernate.Test/Operations/OneToOne.hbm.xml @@ -32,10 +32,12 @@ - + + Person + - \ No newline at end of file + diff --git a/src/NHibernate/Action/DelayedPostInsertIdentifier.cs b/src/NHibernate/Action/DelayedPostInsertIdentifier.cs index 083e7cd14b2..7d271038ffc 100644 --- a/src/NHibernate/Action/DelayedPostInsertIdentifier.cs +++ b/src/NHibernate/Action/DelayedPostInsertIdentifier.cs @@ -45,5 +45,10 @@ public override string ToString() { return string.Format("", sequence); } + + /// + /// The actual identifier value that has been generated. + /// + public object ActualId { get; set; } } } diff --git a/src/NHibernate/Action/EntityInsertAction.cs b/src/NHibernate/Action/EntityInsertAction.cs index 0f8e7e6ac24..62b459f7242 100644 --- a/src/NHibernate/Action/EntityInsertAction.cs +++ b/src/NHibernate/Action/EntityInsertAction.cs @@ -44,10 +44,19 @@ public override void Execute() bool veto = PreInsert(); + var wasDelayed = false; // Don't need to lock the cache here, since if someone // else inserted the same pk first, the insert would fail if (!veto) { + // The identifier may be a foreign delayed identifier, which at this point should have been resolved. + if (id is DelayedPostInsertIdentifier delayed) + { + wasDelayed = true; + id = delayed.ActualId ?? + throw new InvalidOperationException( + $"The delayed foreign identifier {delayed} has not been resolved before insertion of a {instance}"); + } persister.Insert(id, State, instance, Session); EntityEntry entry = Session.PersistenceContext.GetEntry(instance); @@ -57,6 +66,10 @@ public override void Execute() } entry.PostInsert(); + if (wasDelayed) + { + Session.PersistenceContext.ReplaceDelayedEntityIdentityInsertKeys(entry.EntityKey, id); + } if (persister.HasInsertGeneratedProperties) { diff --git a/src/NHibernate/Async/Action/EntityInsertAction.cs b/src/NHibernate/Async/Action/EntityInsertAction.cs index 17f5c4d0bfb..13ada50e720 100644 --- a/src/NHibernate/Async/Action/EntityInsertAction.cs +++ b/src/NHibernate/Async/Action/EntityInsertAction.cs @@ -40,10 +40,19 @@ public override async Task ExecuteAsync(CancellationToken cancellationToken) bool veto = await (PreInsertAsync(cancellationToken)).ConfigureAwait(false); + var wasDelayed = false; // Don't need to lock the cache here, since if someone // else inserted the same pk first, the insert would fail if (!veto) { + // The identifier may be a foreign delayed identifier, which at this point should have been resolved. + if (id is DelayedPostInsertIdentifier delayed) + { + wasDelayed = true; + id = delayed.ActualId ?? + throw new InvalidOperationException( + $"The delayed foreign identifier {delayed} has not been resolved before insertion of a {instance}"); + } await (persister.InsertAsync(id, State, instance, Session, cancellationToken)).ConfigureAwait(false); EntityEntry entry = Session.PersistenceContext.GetEntry(instance); @@ -53,6 +62,10 @@ public override async Task ExecuteAsync(CancellationToken cancellationToken) } entry.PostInsert(); + if (wasDelayed) + { + Session.PersistenceContext.ReplaceDelayedEntityIdentityInsertKeys(entry.EntityKey, id); + } if (persister.HasInsertGeneratedProperties) { diff --git a/src/NHibernate/Async/Engine/StatefulPersistenceContext.cs b/src/NHibernate/Async/Engine/StatefulPersistenceContext.cs index e15d0623f32..4fe2714f641 100644 --- a/src/NHibernate/Async/Engine/StatefulPersistenceContext.cs +++ b/src/NHibernate/Async/Engine/StatefulPersistenceContext.cs @@ -13,8 +13,8 @@ using System.Collections.Generic; using System.Runtime.Serialization; using System.Security; -using System.Security.Permissions; using System.Text; +using NHibernate.Action; using NHibernate.Collection; using NHibernate.Engine.Loading; using NHibernate.Impl; diff --git a/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs b/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs index 985ead87deb..f86e9a6c158 100644 --- a/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs +++ b/src/NHibernate/Async/Event/Default/AbstractSaveEventListener.cs @@ -146,7 +146,8 @@ protected virtual async Task PerformSaveAsync(object entity, object id, throw new NonUniqueObjectException(id, persister.EntityName); } } - persister.SetIdentifier(entity, id); + if (!(id is DelayedPostInsertIdentifier)) + persister.SetIdentifier(entity, id); } else { diff --git a/src/NHibernate/Engine/StatefulPersistenceContext.cs b/src/NHibernate/Engine/StatefulPersistenceContext.cs index 7ca2ce0338c..7819a59989b 100644 --- a/src/NHibernate/Engine/StatefulPersistenceContext.cs +++ b/src/NHibernate/Engine/StatefulPersistenceContext.cs @@ -3,8 +3,8 @@ using System.Collections.Generic; using System.Runtime.Serialization; using System.Security; -using System.Security.Permissions; using System.Text; +using NHibernate.Action; using NHibernate.Collection; using NHibernate.Engine.Loading; using NHibernate.Impl; @@ -1392,6 +1392,8 @@ public void ReplaceDelayedEntityIdentityInsertKeys(EntityKey oldKey, object gene AddEntity(newKey, entity); AddEntry(entity, oldEntry.Status, oldEntry.LoadedState, oldEntry.RowId, generatedId, oldEntry.Version, oldEntry.LockMode, oldEntry.ExistsInDatabase, oldEntry.Persister, oldEntry.IsBeingReplicated); + if (oldKey.Identifier is DelayedPostInsertIdentifier delayed) + delayed.ActualId = generatedId; } public bool IsLoadFinished diff --git a/src/NHibernate/Event/Default/AbstractSaveEventListener.cs b/src/NHibernate/Event/Default/AbstractSaveEventListener.cs index 802ed4eeb6b..8c9f356304f 100644 --- a/src/NHibernate/Event/Default/AbstractSaveEventListener.cs +++ b/src/NHibernate/Event/Default/AbstractSaveEventListener.cs @@ -173,7 +173,8 @@ protected virtual object PerformSave(object entity, object id, IEntityPersister throw new NonUniqueObjectException(id, persister.EntityName); } } - persister.SetIdentifier(entity, id); + if (!(id is DelayedPostInsertIdentifier)) + persister.SetIdentifier(entity, id); } else {