Registry-level changes prevention

alex-kulakov · alex-kulakov · commit f91b6c5f2f7a · 2022-06-06T11:34:35.000+05:00
diff --git a/Orm/Xtensive.Orm/Orm/Internals/EntityChangeRegistry.cs b/Orm/Xtensive.Orm/Orm/Internals/EntityChangeRegistry.cs
@@ -21,6 +21,8 @@ public sealed class EntityChangeRegistry : SessionBound
     private readonly HashSet<EntityState> removed = new();
     private int count;
 
+    private bool changesDisabled;
+
     /// <summary>
     /// Gets the number of registered entities.
     /// </summary>
@@ -34,7 +36,7 @@ internal void Register(EntityState item)
     {
       // Remove-create sequences fix for Issue 690
       if (item.PersistenceState == PersistenceState.New && removed.Contains(item)) {
-        EnsureChangesAreNotPersisting();
+        EnsureRegistrationsAllowed();
         _ = removed.Remove(item);
         count--;
         if (item.DifferentialTuple.Difference == null) {
@@ -44,19 +46,19 @@ internal void Register(EntityState item)
         item.SetPersistenceState(PersistenceState.Modified);
       }
       else if (item.PersistenceState == PersistenceState.Removed && @new.Contains(item)) {
-        EnsureChangesAreNotPersisting();
+        EnsureRegistrationsAllowed();
         _ = @new.Remove(item);
         count--;
         return;
       }
       else if (item.PersistenceState == PersistenceState.Removed && modified.Contains(item)) {
-        EnsureChangesAreNotPersisting();
+        EnsureRegistrationsAllowed();
         _ = modified.Remove(item);
         count--;
       }
 
       var container = GetContainer(item.PersistenceState);
-      EnsureChangesAreNotPersisting();
+      EnsureRegistrationsAllowed();
       if (container.Add(item)) {
         count++;
       }
@@ -84,18 +86,32 @@ public void Clear()
       removed.Clear();
     }
 
+    internal Core.Disposable PreventChanges()
+    {
+      changesDisabled = true;
+      return new Core.Disposable((a) => changesDisabled = false);
+    }
+
     /// <exception cref="ArgumentOutOfRangeException"><paramref name="state"/> is out of range.</exception>
     private HashSet<EntityState> GetContainer(PersistenceState state)
     {
       switch (state) {
-      case PersistenceState.New:
-        return @new;
-      case PersistenceState.Modified:
-        return modified;
-      case PersistenceState.Removed:
-        return removed;
-      default:
-        throw new ArgumentOutOfRangeException("state");
+        case PersistenceState.New:
+          return @new;
+        case PersistenceState.Modified:
+          return modified;
+        case PersistenceState.Removed:
+          return removed;
+        default:
+          throw new ArgumentOutOfRangeException(nameof(state));
+      }
+    }
+
+    private void EnsureRegistrationsAllowed()
+    {
+      if (changesDisabled) {
+        throw new InvalidOperationException(
+          string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));
       }
     }
 
diff --git a/Orm/Xtensive.Orm/Orm/Internals/EntitySetChangeRegistry.cs b/Orm/Xtensive.Orm/Orm/Internals/EntitySetChangeRegistry.cs
@@ -4,6 +4,7 @@
 // Created by: Alexey Kulakov
 // Created:    2014.03.27
 
+using System;
 using System.Collections.Generic;
 
 namespace Xtensive.Orm.Internals
@@ -15,6 +16,8 @@ public sealed class EntitySetChangeRegistry : SessionBound
   {
     private readonly HashSet<EntitySetState> modifiedEntitySets = new();
 
+    private bool changesDisabled;
+
     /// <summary>
     /// Count of registered <see cref="EntitySetState"/>.
     /// </summary>
@@ -26,7 +29,7 @@ public sealed class EntitySetChangeRegistry : SessionBound
     /// <param name="entitySetState"><see cref="EntitySetState"/> to bound.</param>
     public void Register(EntitySetState entitySetState)
     {
-      EnsureChangesAreNotPersisting();
+      EnsureRegistrationsAllowed();
       _ = modifiedEntitySets.Add(entitySetState);
     }
 
@@ -41,11 +44,26 @@ public void Register(EntitySetState entitySetState)
     /// </summary>
     public void Clear() => modifiedEntitySets.Clear();
 
+    internal Core.Disposable PreventChanges()
+    {
+      changesDisabled = true;
+      return new Core.Disposable((a) => changesDisabled = false);
+    }
+
+    private void EnsureRegistrationsAllowed()
+    {
+      if (changesDisabled) {
+        throw new InvalidOperationException(
+          string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));
+      }
+    }
+
     /// <summary>
     /// Initializes a new instance of this class.
     /// </summary>
     /// <param name="session"><see cref="Session"/>, to which current instance 
     /// is bound.</param>
+    /// 
     public EntitySetChangeRegistry(Session session)
       : base(session)
     {
diff --git a/Orm/Xtensive.Orm/Orm/Internals/NonPairedReferenceChangesRegistry.cs b/Orm/Xtensive.Orm/Orm/Internals/NonPairedReferenceChangesRegistry.cs
@@ -54,6 +54,8 @@ public Identifier(EntityState entityState, AssociationInfo association)
     private readonly IDictionary<Identifier, HashSet<EntityState>> addedReferences = new Dictionary<Identifier, HashSet<EntityState>>();
     private readonly object accessGuard = new();
 
+    private bool changesDisabled;
+
     public int RemovedReferencesCount => removedReferences.Values.Sum(el => el.Count);
 
     public int AddedReferencesCount => addedReferences.Values.Sum(el => el.Count);
@@ -98,6 +100,12 @@ public void Clear()
       }
     }
 
+    internal Core.Disposable PreventChanges()
+    {
+      changesDisabled = true;
+      return new Core.Disposable((a) => changesDisabled = false);
+    }
+
     private void RegisterChange(EntityState referencedState, EntityState referencingState, EntityState noLongerReferencedState, AssociationInfo association)
     {
       ArgumentValidator.EnsureArgumentNotNull(association, "association");
@@ -129,7 +137,7 @@ private void RegisterChange(EntityState referencedState, EntityState referencing
     private void RegisterRemoveInternal(Identifier oldKey, EntityState referencingState)
     {
       if (addedReferences.TryGetValue(oldKey, out var addedRefs)) {
-        EnsureChangesAreNotPersisting();
+        EnsureRegistrationsAllowed();
         if (addedRefs.Remove(referencingState)) {
           if (addedRefs.Count == 0) {
             _ = addedReferences.Remove(oldKey);
@@ -143,14 +151,14 @@ private void RegisterRemoveInternal(Identifier oldKey, EntityState referencingSt
         }
         return;
       }
-      EnsureChangesAreNotPersisting();
+      EnsureRegistrationsAllowed();
       removedReferences.Add(oldKey, new HashSet<EntityState>{referencingState});
     }
 
     private void RegisterAddInternal(Identifier newKey, EntityState referencingState)
     {
       if (removedReferences.TryGetValue(newKey, out var removedRefs)) {
-        EnsureChangesAreNotPersisting();
+        EnsureRegistrationsAllowed();
         if (removedRefs.Remove(referencingState)) {
           if (removedRefs.Count == 0) {
             _ = removedReferences.Remove(newKey);
@@ -165,7 +173,7 @@ private void RegisterAddInternal(Identifier newKey, EntityState referencingState
         }
         return;
       }
-      EnsureChangesAreNotPersisting();
+      EnsureRegistrationsAllowed();
       addedReferences.Add(newKey, new HashSet<EntityState>{referencingState});
     }
 
@@ -276,6 +284,14 @@ private EntityState GetStructureFieldValue(FieldInfo fieldOfStructure, Structure
     private string BuildNameOfEntityField(FieldInfo fieldOfOwner, FieldInfo referenceFieldOfStructure) =>
       $"{fieldOfOwner.Name}.{referenceFieldOfStructure.Name}";
 
+    private void EnsureRegistrationsAllowed()
+    {
+      if (changesDisabled) {
+        throw new InvalidOperationException(
+          string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));
+      }
+    }
+
     internal NonPairedReferenceChangesRegistry(Session session)
       : base(session)
     {
diff --git a/Orm/Xtensive.Orm/Orm/Internals/ReferenceFieldsChangesRegistry.cs b/Orm/Xtensive.Orm/Orm/Internals/ReferenceFieldsChangesRegistry.cs
@@ -4,6 +4,7 @@
 // Created by: Alexey Kulakov
 // Created:    2014.04.07
 
+using System;
 using System.Collections.Generic;
 using Xtensive.Core;
 using Xtensive.Orm.Model;
@@ -17,6 +18,8 @@ internal sealed class ReferenceFieldsChangesRegistry : SessionBound
   {
     private readonly HashSet<ReferenceFieldChangeInfo> changes = new();
 
+    private bool changesDisabled;
+
     /// <summary>
     /// Registrates information about field which value was set.
     /// </summary>
@@ -63,13 +66,27 @@ public void Clear()
     {
       changes.Clear();
     }
-    
+
+    internal Core.Disposable PreventChanges()
+    {
+      changesDisabled = true;
+      return new Core.Disposable((a) => changesDisabled = false);
+    }
+
     private void Register(ReferenceFieldChangeInfo fieldChangeInfo)
     {
-      EnsureChangesAreNotPersisting();
+      EnsureRegistrationsAllowed();
       _ = changes.Add(fieldChangeInfo);
     }
 
+    private void EnsureRegistrationsAllowed()
+    {
+      if (changesDisabled) {
+        throw new System.InvalidOperationException(
+          string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));
+      }
+    }
+
     public ReferenceFieldsChangesRegistry(Session session)
       : base(session)
     {
diff --git a/Orm/Xtensive.Orm/Orm/Session.Persist.cs b/Orm/Xtensive.Orm/Orm/Session.Persist.cs
@@ -134,12 +134,15 @@ private async ValueTask Persist(PersistReason reason, bool isAsync, Cancellation
 
       var ts = await InnerOpenTransaction(
         TransactionOpenMode.Default, IsolationLevel.Unspecified, false, isAsync, token);
+
+      IDisposable changesGuard = null;
       try {
         IsPersisting = true;
         persistingIsFailed = false;
         SystemEvents.NotifyPersisting();
         Events.NotifyPersisting();
-        ChangesInProcessing = true;
+
+        changesGuard = PreventRegistryChanges();
         using (OpenSystemLogicOnlyRegion()) {
           DemandTransaction();
           if (IsDebugEventLoggingEnabled) {
@@ -173,11 +176,16 @@ private async ValueTask Persist(PersistReason reason, bool isAsync, Cancellation
           }
           catch (Exception) {
             persistingIsFailed = true;
+            changesGuard.Dispose();
+            changesGuard = null;
+
             RollbackChangesOfEntitySets();
             RestoreEntityChangesAfterPersistFailed();
             throw;
           }
           finally {
+            changesGuard.DisposeSafely();
+            changesGuard = null;
             if (persistIsSuccessful || !Configuration.Supports(SessionOptions.NonTransactionalEntityStates)) {
               DropDifferenceBackup();
               foreach (var item in itemsToPersist.GetItems(PersistenceState.New)) {
@@ -210,12 +218,12 @@ private async ValueTask Persist(PersistReason reason, bool isAsync, Cancellation
           }
         }
 
-        ChangesInProcessing = false;
         SystemEvents.NotifyPersisted();
         Events.NotifyPersisted();
       }
       finally {
         IsPersisting = false;
+        changesGuard.DisposeSafely();
         if (isAsync) {
           await ts.DisposeAsync().ConfigureAwait(false);
         }
@@ -363,5 +371,13 @@ private void ProcessChangesOfEntitySets(Action<EntitySetState> action)
       foreach (var entitySet in itemsToProcess)
         action.Invoke(entitySet);
     }
+
+    private IDisposable PreventRegistryChanges()
+    {
+      return EntityChangeRegistry.PreventChanges()
+        & EntitySetChangeRegistry.PreventChanges()
+        & NonPairedReferencesRegistry.PreventChanges()
+        & ReferenceFieldsChangesRegistry.PreventChanges();
+    }
   }
 }
diff --git a/Orm/Xtensive.Orm/Orm/Session.cs b/Orm/Xtensive.Orm/Orm/Session.cs
@@ -113,12 +113,6 @@ private static readonly Type
     /// </summary>
     internal bool IsPersisting { get; private set; }
 
-    /// <summary>
-    /// Gets a value indicating that saving changes in that stage when any other persistent
-    /// object cannot be changed within session.
-    /// </summary>
-    internal bool ChangesInProcessing { get; private set; }
-
     /// <summary>
     /// Gets a value indicating whether session is disconnected:
     /// session supports non-transactional entity states and does not support autosaving of changes.
diff --git a/Orm/Xtensive.Orm/Orm/SessionBound.cs b/Orm/Xtensive.Orm/Orm/SessionBound.cs
@@ -33,18 +33,6 @@ protected void EnsureTheSameSession(SessionBound other)
         throw new ArgumentException(Strings.ExSessionOfAnotherSessionBoundMustBeTheSame, nameof(other));
     }
 
-    /// <summary>
-    /// Ensures that <see cref="Session"/> is not on stage when changes are persisting
-    /// and any changes are forbidden.
-    /// </summary>
-    protected void EnsureChangesAreNotPersisting()
-    {
-      if (Session.ChangesInProcessing) {
-        throw new InvalidOperationException(
-          string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));
-      }
-    }
-
     #region IContextBound<Session> Members
 
     Session IContextBound<Session>.Context {

Original file line number	Diff line number	Diff line change
`@@ -54,6 +54,8 @@ public Identifier(EntityState entityState, AssociationInfo association)`
`54`	`54`	`private readonly IDictionary<Identifier, HashSet<EntityState>> addedReferences = new Dictionary<Identifier, HashSet<EntityState>>();`
`55`	`55`	`private readonly object accessGuard = new();`
`56`	`56`
	`57`	`+ private bool changesDisabled;`
	`58`	`+`
`57`	`59`	`public int RemovedReferencesCount => removedReferences.Values.Sum(el => el.Count);`
`58`	`60`
`59`	`61`	`public int AddedReferencesCount => addedReferences.Values.Sum(el => el.Count);`
`@@ -98,6 +100,12 @@ public void Clear()`
`98`	`100`	`}`
`99`	`101`	`}`
`100`	`102`
	`103`	`+ internal Core.Disposable PreventChanges()`
	`104`	`+ {`
	`105`	`+ changesDisabled = true;`
	`106`	`+ return new Core.Disposable((a) => changesDisabled = false);`
	`107`	`+ }`
	`108`	`+`
`101`	`109`	`private void RegisterChange(EntityState referencedState, EntityState referencingState, EntityState noLongerReferencedState, AssociationInfo association)`
`102`	`110`	`{`
`103`	`111`	`ArgumentValidator.EnsureArgumentNotNull(association, "association");`
`@@ -129,7 +137,7 @@ private void RegisterChange(EntityState referencedState, EntityState referencing`
`129`	`137`	`private void RegisterRemoveInternal(Identifier oldKey, EntityState referencingState)`
`130`	`138`	`{`
`131`	`139`	`if (addedReferences.TryGetValue(oldKey, out var addedRefs)) {`
`132`		`- EnsureChangesAreNotPersisting();`
	`140`	`+ EnsureRegistrationsAllowed();`
`133`	`141`	`if (addedRefs.Remove(referencingState)) {`
`134`	`142`	`if (addedRefs.Count == 0) {`
`135`	`143`	`_ = addedReferences.Remove(oldKey);`
`@@ -143,14 +151,14 @@ private void RegisterRemoveInternal(Identifier oldKey, EntityState referencingSt`
`143`	`151`	`}`
`144`	`152`	`return;`
`145`	`153`	`}`
`146`		`- EnsureChangesAreNotPersisting();`
	`154`	`+ EnsureRegistrationsAllowed();`
`147`	`155`	`removedReferences.Add(oldKey, new HashSet<EntityState>{referencingState});`
`148`	`156`	`}`
`149`	`157`
`150`	`158`	`private void RegisterAddInternal(Identifier newKey, EntityState referencingState)`
`151`	`159`	`{`
`152`	`160`	`if (removedReferences.TryGetValue(newKey, out var removedRefs)) {`
`153`		`- EnsureChangesAreNotPersisting();`
	`161`	`+ EnsureRegistrationsAllowed();`
`154`	`162`	`if (removedRefs.Remove(referencingState)) {`
`155`	`163`	`if (removedRefs.Count == 0) {`
`156`	`164`	`_ = removedReferences.Remove(newKey);`
`@@ -165,7 +173,7 @@ private void RegisterAddInternal(Identifier newKey, EntityState referencingState`
`165`	`173`	`}`
`166`	`174`	`return;`
`167`	`175`	`}`
`168`		`- EnsureChangesAreNotPersisting();`
	`176`	`+ EnsureRegistrationsAllowed();`
`169`	`177`	`addedReferences.Add(newKey, new HashSet<EntityState>{referencingState});`
`170`	`178`	`}`
`171`	`179`
`@@ -276,6 +284,14 @@ private EntityState GetStructureFieldValue(FieldInfo fieldOfStructure, Structure`
`276`	`284`	`private string BuildNameOfEntityField(FieldInfo fieldOfOwner, FieldInfo referenceFieldOfStructure) =>`
`277`	`285`	`$"{fieldOfOwner.Name}.{referenceFieldOfStructure.Name}";`
`278`	`286`
	`287`	`+ private void EnsureRegistrationsAllowed()`
	`288`	`+ {`
	`289`	`+ if (changesDisabled) {`
	`290`	`+ throw new InvalidOperationException(`
	`291`	`+ string.Format(Strings.ExSessionXIsActivelyPersistingChangesNoPersistentChangesAllowed, Session.Guid));`
	`292`	`+ }`
	`293`	`+ }`
	`294`	`+`
`279`	`295`	`internal NonPairedReferenceChangesRegistry(Session session)`
`280`	`296`	`: base(session)`
`281`	`297`	`{`