Merge pull request #1901 from Lion-Ch/feature/issue-1780

[Issue #1780] Bug: IncludeGraph fails when first entity has null navigation property

Author: borisdj

EFCore.BulkExtensions.Core/DbContextBulkTransactionGraphUtil.cs

@@ -40,7 +40,7 @@ private static async Task ExecuteWithGraphAsync(DbContext context, IEnumerable<o
         // If this is set to false, wont' be able to support some code first model types as EFCore uses shadow properties when a relationship's foreign keys arent explicitly defined
         bulkConfig.EnableShadowProperties = true;
 
-        var graphNodes = GraphUtil.GetTopologicallySortedGraph(context, entities);
+        var graphNodes = GraphUtil.GetSortedGraph(context, entities);
 
         if (graphNodes == null)
             return;

EFCore.BulkExtensions.Core/Util/GraphUtil.cs

@@ -1,6 +1,7 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Metadata;
+
 using System;
 using System.Collections.Generic;
 using System.Linq;
@@ -9,7 +10,7 @@ namespace EFCore.BulkExtensions;
 
 internal class GraphUtil
 {
-    public static IEnumerable<GraphNode>? GetTopologicallySortedGraph(DbContext dbContext, IEnumerable<object> entities)
+    public static IEnumerable<GraphNode>? GetSortedGraph(DbContext dbContext, IEnumerable<object> entities)
     {
         if (!entities.Any())
         {
@@ -26,6 +27,16 @@ internal class GraphUtil
 
         // Sort these entities so the first entity is the least dependendant
         var topologicalSorted = TopologicalSort(dependencies.Keys, y => dependencies[y].DependsOn.Select(y => y.entity));
+
+        var withAdditionalSorting = true;
+
+        if (withAdditionalSorting) return GetNodesWithSortingByDepth(dependencies, topologicalSorted);
+
+        return GetNodes(dependencies, topologicalSorted);
+    }
+
+    private static IEnumerable<GraphNode> GetNodes(Dictionary<object, GraphDependency> dependencies, IEnumerable<object> topologicalSorted)
+    {
         var result = new List<GraphNode>();
 
         foreach (var s in topologicalSorted)
@@ -40,6 +51,79 @@ internal class GraphUtil
         return result;
     }
 
+    private static IEnumerable<GraphNode> GetNodesWithSortingByDepth(Dictionary<object, GraphDependency> dependencies, IEnumerable<object> topologicalSorted)
+    {
+        var entitiesDepth = CalculateEntitiesDepth(dependencies, topologicalSorted.ToList());
+
+        var typesDepth = CalculateTypesDepth(entitiesDepth);
+
+        var result = new List<GraphNode>();
+
+        foreach (var type in typesDepth.OrderByDescending(x => x.Value))
+        {
+            foreach (var s in topologicalSorted)
+            {
+                if (s.GetType() != type.Key) continue;
+
+                result.Add(new GraphNode
+                {
+                    Entity = s,
+                    Dependencies = dependencies[s]
+                });
+            }
+        }
+
+        return result;
+    }
+
+    /// <summary>
+    /// Calculates the depth of each entity from the entity to the top of the dependency graph. Returns a dictionary mapping entities to their depth.
+    /// </summary>
+    private static Dictionary<object, int> CalculateEntitiesDepth(
+        Dictionary<object, GraphDependency> graph,
+        IReadOnlyList<object> topologicalSorted)
+    {
+        var depthDict = graph.Keys.ToDictionary(k => k, v => 0);
+
+        for (int i = graph.Count - 1; i >= 0; i--)
+        {
+            var node = topologicalSorted[i];
+
+            var nodeDeps = graph
+                .Where(x => x.Value.DependsOn.Any(d => d.entity == node))
+                .Select(x => x.Key)
+                .ToList();
+
+            int max = 0;
+            foreach (var childNode in nodeDeps)
+                max = Math.Max(max, depthDict[childNode] + 1);
+
+            depthDict[node] = max;
+        }
+
+        return depthDict;
+    }
+
+    /// <summary>
+    /// Calculates the maximum depth from entity of a given type to the top. Returns a dictionary mapping types to their maximum depth.
+    /// </summary>
+    private static Dictionary<Type, int> CalculateTypesDepth(Dictionary<object, int> entitiesDepth)
+    {
+        var typesDepth = new Dictionary<Type, int>();
+
+        foreach (var entityDepth in entitiesDepth)
+        {
+            var type = entityDepth.Key.GetType();
+
+            if (typesDepth.TryGetValue(type, out int value))
+                typesDepth[type] = Math.Max(value, entityDepth.Value);
+            else
+                typesDepth.Add(type, entityDepth.Value);
+        }
+
+        return typesDepth;
+    }
+
     private static GraphDependency? GetFlatGraph(DbContext dbContext, object graphEntity, IDictionary<object, GraphDependency> result)
     {
         var entityType = dbContext.Model.FindEntityType(graphEntity.GetType());

EFCore.BulkExtensions.Tests/IncludeGraph/GraphDbContext.cs

@@ -25,15 +25,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             cfg.HasKey(y => y.Id);
 
             cfg.HasOne(y => y.ParentAsset).WithMany(y => y.ChildAssets);
-            cfg.HasMany(y => y.WorkOrders).WithOne(y => y.Asset).IsRequired();
+            cfg.HasMany(y => y.WorkOrders).WithOne(y => y.Asset);
         });
 
         modelBuilder.Entity<WorkOrder>(cfg =>
         {
             cfg.HasKey(y => y.Id);
 
             cfg.HasMany(y => y.WorkOrderSpares).WithOne(y => y.WorkOrder);
-            cfg.HasOne(y => y.Asset).WithMany(y => y.WorkOrders).IsRequired();
+            cfg.HasOne(y => y.Asset).WithMany(y => y.WorkOrders);
         });
 
         modelBuilder.Entity<WorkOrderSpare>(cfg =>

EFCore.BulkExtensions.Tests/IncludeGraph/IncludeGraphTests.cs

@@ -1,18 +1,21 @@
 using EFCore.BulkExtensions.SqlAdapters;
 using EFCore.BulkExtensions.Tests.IncludeGraph.Model;
 using EFCore.BulkExtensions.Tests.ShadowProperties;
+
 using Microsoft.EntityFrameworkCore;
+
 using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
+
 using Xunit;
 
 namespace EFCore.BulkExtensions.Tests.IncludeGraph;
 
 public class IncludeGraphTests : IDisposable
 {
-    private readonly static WorkOrder WorkOrder1 = new ()
+    private readonly static WorkOrder WorkOrder1 = new()
     {
         Description = "Fix belt",
         Asset = new Asset
@@ -45,7 +48,7 @@ public class IncludeGraphTests : IDisposable
             }
     };
 
-    private static readonly WorkOrder WorkOrder2 = new ()
+    private static readonly WorkOrder WorkOrder2 = new()
     {
         Description = "Fix toilets",
         Asset = new Asset
@@ -78,6 +81,7 @@ public class IncludeGraphTests : IDisposable
         }
     };
 
+
     [Theory]
     [InlineData(SqlType.SqlServer)]
     //[InlineData(DbServer.Sqlite)]
@@ -100,11 +104,14 @@ public async Task BulkInsertOrUpdate_EntityWithNestedObjectGraph_SavesGraphToDat
             wos.WorkOrder = WorkOrder2;
         }
 
-        WorkOrder1.Asset.WorkOrders.Add(WorkOrder1);
-        WorkOrder2.Asset.WorkOrders.Add(WorkOrder2);
+        if (WorkOrder1.Asset != null && WorkOrder2.Asset != null)
+        {
+            WorkOrder1.Asset.WorkOrders.Add(WorkOrder1);
+            WorkOrder2.Asset.WorkOrders.Add(WorkOrder2);
 
-        WorkOrder1.Asset.ParentAsset = WorkOrder2.Asset;
-        WorkOrder2.Asset.ChildAssets.Add(WorkOrder1.Asset);
+            WorkOrder1.Asset.ParentAsset = WorkOrder2.Asset;
+            WorkOrder2.Asset.ChildAssets.Add(WorkOrder1.Asset);
+        }
 
         var testData = GetTestData().ToList();
         var bulkConfig = new BulkConfig
@@ -136,6 +143,117 @@ private static IEnumerable<WorkOrder> GetTestData()
         yield return WorkOrder2;
     }
 
+    [Theory]
+    [MemberData(nameof(GetTestData2))]
+    public async Task BulkInsertOrUpdate_EntityWithNestedNullableObjectGraph_SavesGraphToDatabase1(IEnumerable<WorkOrder> orders)
+    {
+        ContextUtil.DatabaseType = SqlType.SqlServer;
+
+        using (var db = new GraphDbContext(ContextUtil.GetOptions<GraphDbContext>(databaseName: $"{nameof(EFCoreBulkTest)}_Graph")))
+        {
+            db.Database.EnsureDeleted();
+            await db.Database.EnsureCreatedAsync();
+
+            var bulkConfig = new BulkConfig
+            {
+                IncludeGraph = true,
+                CalculateStats = true,
+            };
+            await db.BulkInsertOrUpdateAsync(orders, bulkConfig);
+
+            Assert.NotNull(bulkConfig.StatsInfo);
+            Assert.Equal(
+                orders.Count() + orders.Where(x => x.Asset != null).Count(),
+                bulkConfig.StatsInfo.StatsNumberInserted);
+        }
+
+        using (var db = new GraphDbContext(ContextUtil.GetOptions<GraphDbContext>(databaseName: $"{nameof(EFCoreBulkTest)}_Graph")))
+        {
+            var ordersFromDb = db.WorkOrders
+                .Include(y => y.Asset)
+                .OrderBy(x => x.Id)
+                .ToList();
+
+            foreach (var orderFromDb in ordersFromDb)
+            {
+                var order = orders.First(x => x.Description == orderFromDb.Description);
+
+                if (order.Asset == null)
+                    Assert.Null(orderFromDb.Asset);
+                else
+                    Assert.NotNull(orderFromDb.Asset);
+            }
+        }
+    }
+
+    public static IEnumerable<object[]> GetTestData2()
+        =>
+        [
+            [GetTestDataWithNullInEnd()],
+            [GetTestDataWithNullInMiddle()],
+            [GetTestDataWithNullInFirst()],
+        ];
+
+    private static IEnumerable<WorkOrder> GetTestDataWithNullInEnd()
+    {
+        var baseData = GetBaseTestData();
+
+        baseData.Last().Asset = null;
+
+        return baseData;
+    }
+
+    private static IEnumerable<WorkOrder> GetTestDataWithNullInMiddle()
+    {
+        var baseData = GetBaseTestData();
+
+        baseData[1].Asset = null;
+
+        return baseData;
+    }
+
+    private static IEnumerable<WorkOrder> GetTestDataWithNullInFirst()
+    {
+        var baseData = GetBaseTestData();
+
+        baseData.First().Asset = null;
+
+        return baseData;
+    }
+
+    private static List<WorkOrder> GetBaseTestData()
+    {
+        return
+        [
+            new WorkOrder()
+            {
+                Description = "Fix belt",
+                Asset = new Asset
+                {
+                    Description = "MANU-1",
+                    Location = "WAREHOUSE-1"
+                },
+            },
+            new WorkOrder()
+            {
+                Description = "Fix toilets",
+                Asset = new Asset
+                {
+                    Description = "FLUSHMASTER-1",
+                    Location = "GYM-BLOCK-3"
+                },
+            },
+            new WorkOrder()
+            {
+                Description = "Fix door",
+                Asset = new Asset
+                {
+                    Location = "OFFICE-12"
+                },
+            }
+        ];
+    }
+
     public void Dispose()
     {
         using var db = new GraphDbContext(ContextUtil.GetOptions<GraphDbContext>(databaseName: $"{nameof(EFCoreBulkTest)}_Graph"));

EFCore.BulkExtensions.Tests/IncludeGraph/Model/WorkOrder.cs

@@ -1,12 +1,13 @@
-using System.Collections.Generic;
-
-namespace EFCore.BulkExtensions.Tests.IncludeGraph.Model;
+using System.Collections.Generic;
 
-public class WorkOrder
-{
-    public int Id { get; set; }
-    public string Description { get; set; } = null!;
-
-    public Asset Asset { get; set; } = null!;
-    public ICollection<WorkOrderSpare> WorkOrderSpares { get; set; } = new HashSet<WorkOrderSpare>();
-}
+namespace EFCore.BulkExtensions.Tests.IncludeGraph.Model;
+
+public class WorkOrder
+{
+    public int Id { get; set; }
+    public string Description { get; set; } = null!;
+
+    public int? AssetId { get; set; }
+    public Asset? Asset { get; set; }
+    public ICollection<WorkOrderSpare> WorkOrderSpares { get; set; } = new HashSet<WorkOrderSpare>();
+}