New approach to detect data cleanup caused by table movement

Author: alex-kulakov

Orm/Xtensive.Orm/Modelling/Comparison/Hints/DeleteDataHint.cs

@@ -25,15 +25,23 @@ public class DeleteDataHint : DataHint
     /// </summary>
     public bool PostCopy { get; private set; }
 
+    /// <summary>
+    /// Gets value indicating whether cause of data deletion was moving table to another hierarchy.
+    /// This flag is used to ideitify and prevent unsafe data clean-up even if database structure
+    /// remain identical but logically table serves completely different entity.
+    /// </summary>
+    public bool TableChangedOwner { get; private set; }
+
     /// <inheritdoc/>
     public override string ToString()
     {
       return string.Format(
-        "Delete from '{0}' where ({1}){2}",
+        "Delete from '{0}' where ({1}){2}{3}",
         SourceTablePath,
         string.Join(" and ",
           Identities.Select(pair => pair.ToString()).ToArray()),
-          PostCopy ? " (after data copying)" : string.Empty);
+        PostCopy ? " (after data copying)" : string.Empty,
+        TableChangedOwner ? " due to table owner changed" : string.Empty);
     }
 
 
@@ -58,5 +66,19 @@ public DeleteDataHint(string sourceTablePath,  IList<IdentityPair> identities, b
     {
       PostCopy = postCopy;
     }
+
+    /// <summary>
+    /// Initializes new instance of this type.
+    /// </summary>
+    /// <param name="sourceTablePath">Source table path.</param>
+    /// <param name="identities">Identities for data operation.</param>
+    /// <param name="postCopy"><see cref="PostCopy"/> property value.</param>
+    /// <param name="tableChangedOwner"><see cref="TableChangedOwner"/> property value.</param>
+    public DeleteDataHint(string sourceTablePath, IList<IdentityPair> identities, bool postCopy, bool tableChangedOwner)
+      : base(sourceTablePath, identities)
+    {
+      PostCopy = postCopy;
+      TableChangedOwner = tableChangedOwner;
+    }
   }
 }

Orm/Xtensive.Orm/Orm/Upgrade/Internals/HintGenerator.cs

@@ -51,11 +51,12 @@ public HintGenerationResult Run()
       GenerateCopyColumnHints(copyFieldHints);
 
       var removedTypes = GetRemovedTypes();
-      GenerateRecreateHints(removedTypes);
-      GenerateRecordCleanupHints(removedTypes, false);
+      var conflictsByTable = GetRecreatedTypes(removedTypes);
+      //GenerateRecreateHints(removedTypes);
+      GenerateRecordCleanupHints(removedTypes, conflictsByTable, false);
 
       var movedTypes = GetMovedTypes();
-      GenerateRecordCleanupHints(movedTypes, true);
+      GenerateRecordCleanupHints(movedTypes, conflictsByTable, true);
 
       // Adding useful info
       CalculateAffectedTablesAndColumns(hints);
@@ -218,64 +219,73 @@ private void GenerateCopyColumnHint(CopyFieldHint hint)
       }
     }
 
-    private void GenerateRecreateHints(IList<StoredTypeInfo> removedTypes)
-    {
-      var capacity = currentModel.Types.Length - typeMapping.Count;
-      var newTypes = new Dictionary<string, StoredTypeInfo>(capacity, StringComparer.Ordinal);
-      currentNonConnectorTypes
-        .Where(t => !reverseTypeMapping.ContainsKey(t))
-        .ForEach(t => newTypes.Add($"{t.MappingDatabase}.{t.MappingSchema}.{t.MappingName}", t));
-
-      for (var i = removedTypes.Count - 1; i >= 0; i--) {
-        var rType = removedTypes[i];
-        var rTypeIdentifier = $"{rType.MappingDatabase}.{rType.MappingSchema}.{rType.MappingName}";
-        if (suspiciousTypes.Contains(rType) && newTypes.ContainsKey(rTypeIdentifier)) {
-          schemaHints.Add(new DeleteDataHint(GetTablePath(rType), Array.Empty<IdentityPair>(), true));
-          _ = removedTypes.Remove(rType);
-        }
-      }
-    }
-
-    private void GenerateRecordCleanupHints(List<StoredTypeInfo> removedTypes, bool isMovedToAnotherHierarchy)
+    private void GenerateRecordCleanupHints(List<StoredTypeInfo> removedTypes,
+      Dictionary<StoredTypeInfo, StoredTypeInfo> conflictsByTable, bool isMovedToAnotherHierarchy)
     {
       if (!isMovedToAnotherHierarchy) {
         removedTypes.ForEach(GenerateCleanupByForeignKeyHints);
       }
-      removedTypes.ForEach(type => GenerateCleanupByPrimaryKeyHints(type, isMovedToAnotherHierarchy));
+      removedTypes.ForEach(type => GenerateCleanupByPrimaryKeyHints(type, isMovedToAnotherHierarchy, conflictsByTable.ContainsKey(type.Hierarchy.Root)));
     }
 
-    private void GenerateCleanupByPrimaryKeyHints(StoredTypeInfo removedType, bool isMovedToAnotherHierarchy)
+    private void GenerateCleanupByPrimaryKeyHints(StoredTypeInfo removedType, bool isMovedToAnotherHierarchy, bool conflictsWithNewType)
     {
       var hierarchy = removedType.Hierarchy;
       switch (hierarchy.InheritanceSchema) {
-        case InheritanceSchema.ClassTable:
-          IEnumerable<StoredTypeInfo> typesToProcess;
-          typesToProcess = !isMovedToAnotherHierarchy
-            ? EnumerableUtils.One(removedType).Concat(removedType.AllAncestors)
-            : removedType.AllAncestors;
-          foreach (var type in typesToProcess) {
-            var identities1 = new[] { new IdentityPair(
-              GetColumnPath(type, GetTypeIdMappingName(type)),
-              removedType.TypeId.ToString(),
-              true) };
-            schemaHints.Add(
-              new DeleteDataHint(GetTablePath(type), identities1, isMovedToAnotherHierarchy));
+        case InheritanceSchema.ClassTable: {
+          if (!conflictsWithNewType) {
+            var typesToProcess = !isMovedToAnotherHierarchy
+              ? EnumerableUtils.One(removedType).Concat(removedType.AllAncestors)
+              : (IEnumerable<StoredTypeInfo>) removedType.AllAncestors;
+            foreach (var type in typesToProcess) {
+              var identities1 = new[] { new IdentityPair(
+                GetColumnPath(type, GetTypeIdMappingName(type)),
+                removedType.TypeId.ToString(),
+                true)
+              };
+              schemaHints.Add(
+                new DeleteDataHint(GetTablePath(type), identities1, isMovedToAnotherHierarchy, conflictsWithNewType));
+            }
+          }
+          else if (isMovedToAnotherHierarchy) {
+            foreach (var type in hierarchy.Types) {
+              schemaHints.Add(
+                new DeleteDataHint(GetTablePath(type), Array.Empty<IdentityPair>(), isMovedToAnotherHierarchy, conflictsWithNewType));
+            }
           }
           break;
+        }
         case InheritanceSchema.SingleTable:
-          var rootType = hierarchy.Root;
-          var identities2 = new[] { new IdentityPair(
+          if (!conflictsWithNewType) {
+            var rootType = hierarchy.Root;
+            var identities2 = new[] { new IdentityPair(
               GetColumnPath(rootType, GetTypeIdMappingName(rootType)),
               removedType.TypeId.ToString(),
               true) };
-          schemaHints.Add(
-            new DeleteDataHint(GetTablePath(rootType), identities2, isMovedToAnotherHierarchy));
+            schemaHints.Add(
+              new DeleteDataHint(GetTablePath(rootType), identities2, isMovedToAnotherHierarchy, conflictsWithNewType));
+          }
+          else if (!isMovedToAnotherHierarchy) {
+            var rootType = hierarchy.Root;
+            schemaHints.Add(
+              new DeleteDataHint(GetTablePath(rootType), Array.Empty<IdentityPair>(), isMovedToAnotherHierarchy, conflictsWithNewType));
+          }
           break;
-        case InheritanceSchema.ConcreteTable:
-          // ConcreteTable schema doesn't include TypeId
-          schemaHints.Add(
-            new DeleteDataHint(GetTablePath(removedType), Array.Empty<IdentityPair>(), isMovedToAnotherHierarchy));
+        case InheritanceSchema.ConcreteTable: {
+          //ConcreteTable schema doesn't include TypeId
+          if (!conflictsWithNewType) {
+            schemaHints.Add(
+                new DeleteDataHint(GetTablePath(removedType), Array.Empty<IdentityPair>(), isMovedToAnotherHierarchy));
+          }
+          else if (!isMovedToAnotherHierarchy) {
+            var typesToProcess = hierarchy.Types;
+            foreach (var type in typesToProcess) {
+              schemaHints.Add(
+                new DeleteDataHint(GetTablePath(type), Array.Empty<IdentityPair>(), isMovedToAnotherHierarchy, conflictsWithNewType));
+            }
+          }
           break;
+        }
         default:
           throw Exceptions.InternalError(string.Format(Strings.ExInheritanceSchemaIsInvalid, hierarchy.InheritanceSchema), UpgradeLog.Instance);
       }
@@ -598,6 +608,30 @@ private List<string> GetAffectedColumns(StoredTypeInfo type, StoredFieldInfo fie
       return affectedColumns;
     }
 
+    private Dictionary<StoredTypeInfo, StoredTypeInfo> GetRecreatedTypes(IReadOnlyList<StoredTypeInfo> removedTypes)
+    {
+      // Return types that were removed but there is a new type
+      // which uses the same table in database (and|or) schema
+      // that means further comparison will not find schema changes (table removes or renames),
+      // but data in the table has different meaning so old data should be cleaned.
+
+      var capacity = currentModel.Types.Length - typeMapping.Count;
+      var currentTables = new Dictionary<string, StoredTypeInfo>(capacity, StringComparer.Ordinal);
+      foreach (var t in currentNonConnectorTypes.Where(t => !reverseTypeMapping.ContainsKey(t))) {
+        currentTables.Add($"{t.MappingDatabase}.{t.MappingSchema}.{t.MappingName}", t);
+      }
+
+      var conflictsByTable = new Dictionary<StoredTypeInfo, StoredTypeInfo>();
+
+      foreach (var rType in removedTypes) {
+        var rTypeIdentifier = $"{rType.MappingDatabase}.{rType.MappingSchema}.{rType.MappingName}";
+        if (suspiciousTypes.Contains(rType) && currentTables.TryGetValue(rTypeIdentifier, out var nType)) {
+          conflictsByTable.Add(rType, nType);
+        }
+      }
+      return conflictsByTable;
+    }
+
     private List<StoredTypeInfo> GetRemovedTypes()
     {
       return (

Orm/Xtensive.Orm/Orm/Upgrade/Internals/SchemaComparer.cs

@@ -235,7 +235,7 @@ private static void GetCrossHierarchicalMovements(IEnumerable<NodeAction> action
     {
       (from action in actions.OfType<DataAction>()
         let deleteDataHint = action.DataHint as DeleteDataHint
-        where deleteDataHint!=null && deleteDataHint.PostCopy
+        where deleteDataHint!=null && (deleteDataHint.PostCopy || deleteDataHint.TableChangedOwner)
         select action).ForEach(output.Add);
     }