Code cleanup and handle providers that don't support inserted ids when trying to include graph

Author: fabien.menager

src/PhenX.EntityFrameworkCore.BulkInsert.MySql/MySqlBulkInsertProvider.cs

@@ -18,6 +18,9 @@ internal class MySqlBulkInsertProvider(ILogger<MySqlBulkInsertProvider> logger)
     /// <inheritdoc />
     protected override string AddTableCopyBulkInsertId => $"ALTER TABLE {{0}} ADD {BulkInsertId} INT AUTO_INCREMENT PRIMARY KEY;";
 
+    /// <inheritdoc />
+    public override bool SupportsOutputInsertedIds => false;
+
     /// <inheritdoc />
     protected override string GetTempTableName(string tableName) => $"#_temp_bulk_insert_{tableName}_{Helpers.RandomString(6)}";
 

src/PhenX.EntityFrameworkCore.BulkInsert.Oracle/OracleBulkInsertProvider.cs

@@ -19,6 +19,9 @@ internal class OracleBulkInsertProvider(ILogger<OracleBulkInsertProvider>? logge
     /// <inheritdoc />
     protected override string AddTableCopyBulkInsertId => ""; // No need to add an ID column in Oracle
 
+    /// <inheritdoc />
+    public override bool SupportsOutputInsertedIds => false;
+
     /// <inheritdoc />
     /// <summary>
     /// The temporary table name is generated with a random 8-character suffix to ensure uniqueness, and is limited to less than 30 characters,

src/PhenX.EntityFrameworkCore.BulkInsert/Extensions/PublicExtensions.DbSet.cs

@@ -167,8 +167,9 @@ public static async Task ExecuteBulkInsertAsync<T, TOptions>(
                     "Either disable IncludeGraph or remove the onConflict parameter.");
             }
 
-            var orchestrator = new GraphBulkInsertOrchestrator();
-            await orchestrator.InsertGraphAsync(context, entities, options, provider, cancellationToken);
+            var orchestrator = new GraphBulkInsertOrchestrator(context);
+            await orchestrator.InsertGraph(false, entities, options, provider, cancellationToken);
+
             return;
         }
 
@@ -228,9 +229,10 @@ public static void ExecuteBulkInsert<T, TOptions>(
                     "Either disable IncludeGraph or remove the onConflict parameter.");
             }
 
-            var orchestrator = new GraphBulkInsertOrchestrator();
-            orchestrator.InsertGraphAsync(context, entities, options, provider, CancellationToken.None)
+            var orchestrator = new GraphBulkInsertOrchestrator(context);
+            orchestrator.InsertGraph(true, entities, options, provider, CancellationToken.None)
                 .GetAwaiter().GetResult();
+
             return;
         }
 

src/PhenX.EntityFrameworkCore.BulkInsert/Graph/GraphBulkInsertOrchestrator.cs

@@ -4,31 +4,14 @@
 
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
-using Microsoft.EntityFrameworkCore.Metadata;
+using Microsoft.Extensions.Logging;
 
 using PhenX.EntityFrameworkCore.BulkInsert.Abstractions;
 using PhenX.EntityFrameworkCore.BulkInsert.Metadata;
 using PhenX.EntityFrameworkCore.BulkInsert.Options;
 
 namespace PhenX.EntityFrameworkCore.BulkInsert.Graph;
 
-/// <summary>
-/// Result of a graph insert operation.
-/// </summary>
-/// <typeparam name="T">The root entity type.</typeparam>
-internal sealed class GraphInsertResult<T> where T : class
-{
-    /// <summary>
-    /// The root entities that were inserted.
-    /// </summary>
-    public required IReadOnlyList<T> RootEntities { get; init; }
-
-    /// <summary>
-    /// Total count of all entities inserted across all types.
-    /// </summary>
-    public required int TotalInsertedCount { get; init; }
-}
-
 /// <summary>
 /// Orchestrates bulk insertion of entity graphs with FK propagation.
 /// </summary>
@@ -37,25 +20,35 @@ internal sealed class GraphBulkInsertOrchestrator
     private static readonly ConcurrentDictionary<(Type, string), Action<object, object?>> PropertySetters = new();
     private static readonly ConcurrentDictionary<(Type, string), Func<object, object?>> PropertyGetters = new();
 
+    private readonly DbContext _context;
     private readonly MetadataProvider _metadataProvider;
+    private readonly ILogger<GraphBulkInsertOrchestrator>? _logger;
 
-    public GraphBulkInsertOrchestrator()
+    public GraphBulkInsertOrchestrator(DbContext context)
     {
-        _metadataProvider = new MetadataProvider();
+        _context = context;
+        _metadataProvider = context.GetService<MetadataProvider>();
+        _logger = context.GetService<ILogger<GraphBulkInsertOrchestrator>>();
     }
 
     /// <summary>
     /// Orchestrates the bulk insert of an entity graph.
     /// </summary>
-    public async Task<GraphInsertResult<T>> InsertGraphAsync<T>(
-        DbContext context,
+    public async Task<GraphInsertResult<T>> InsertGraph<T>(
+        bool sync,
         IEnumerable<T> entities,
         BulkInsertOptions options,
         IBulkInsertProvider provider,
         CancellationToken ctk) where T : class
     {
+        if (!provider.SupportsOutputInsertedIds)
+        {
+            throw new NotSupportedException(
+                $"The bulk insert provider '{provider.GetType().Name}' does not support returning generated IDs, which is required for IncludeGraph operations.");
+        }
+
         // 1. Collect and sort entities
-        var collector = new GraphEntityCollector(context, options);
+        var collector = new GraphEntityCollector(_context, options);
         var collectionResult = collector.Collect(entities);
 
         if (collectionResult.EntitiesByType.Count == 0)
@@ -68,7 +61,7 @@ public async Task<GraphInsertResult<T>> InsertGraphAsync<T>(
         }
 
         var totalInserted = 0;
-        var graphMetadata = new GraphMetadata(context, options);
+        var graphMetadata = new GraphMetadata(_context, options);
 
         // 2. Insert in dependency order (parents first)
         foreach (var entityType in collectionResult.InsertionOrder)
@@ -80,18 +73,18 @@ public async Task<GraphInsertResult<T>> InsertGraphAsync<T>(
             }
 
             // Propagate FK values from already-inserted parents
-            PropagateParentForeignKeys(entitiesToInsert, entityType, graphMetadata, context);
+            PropagateParentForeignKeys(entitiesToInsert, entityType, graphMetadata);
 
             // Insert entities of this type
-            await InsertEntitiesOfTypeAsync(context, entityType, entitiesToInsert, options, provider, ctk);
+            await InsertEntitiesOfType(sync, _context, entityType, entitiesToInsert, options, provider, ctk);
 
             totalInserted += entitiesToInsert.Count;
         }
 
         // 3. Insert join table records for many-to-many relationships
         if (collectionResult.JoinRecords.Count > 0)
         {
-            await InsertJoinRecordsAsync(context, collectionResult.JoinRecords, options, provider, ctk);
+            await InsertJoinRecords(sync, _context, collectionResult.JoinRecords, options, provider, ctk);
         }
 
         // Return root entities
@@ -109,8 +102,7 @@ public async Task<GraphInsertResult<T>> InsertGraphAsync<T>(
     private static void PropagateParentForeignKeys(
         List<object> entities,
         Type entityType,
-        GraphMetadata graphMetadata,
-        DbContext context)
+        GraphMetadata graphMetadata)
     {
         var efEntityType = graphMetadata.GetEntityType(entityType);
         if (efEntityType == null)
@@ -162,7 +154,8 @@ private static void PropagateParentForeignKeys(
         }
     }
 
-    private async Task InsertEntitiesOfTypeAsync(
+    private async Task InsertEntitiesOfType(
+        bool sync,
         DbContext context,
         Type entityType,
         List<object> entities,
@@ -218,7 +211,7 @@ private async Task InsertEntitiesGenericAsync<TEntity>(
         }
     }
 
-    private static void CopyGeneratedIds<TEntity>(
+    private void CopyGeneratedIds<TEntity>(
         List<TEntity> originalEntities,
         List<TEntity> insertedEntities,
         TableMetadata tableInfo) where TEntity : class
@@ -228,10 +221,10 @@ private static void CopyGeneratedIds<TEntity>(
             // Count mismatch - this can happen if the bulk insert operation
             // doesn't preserve order. Log a warning for debugging purposes.
             // The graph insert will continue but FK propagation may be incomplete.
-            System.Diagnostics.Debug.WriteLine(
-                $"Warning: IncludeGraph ID propagation failed for {typeof(TEntity).Name}. " +
-                $"Original count: {originalEntities.Count}, Inserted count: {insertedEntities.Count}. " +
-                "Foreign key values may not be correctly propagated to dependent entities.");
+            _logger?.LogWarning(
+                "IncludeGraph ID propagation failed for {EntityType}. Original count: {OriginalCount}, Inserted count: {InsertedCount}. Foreign key values may not be correctly propagated to dependent entities.",
+                typeof(TEntity).Name, originalEntities.Count, insertedEntities.Count);
+
             return;
         }
 
@@ -254,7 +247,8 @@ private static void CopyGeneratedIds<TEntity>(
         }
     }
 
-    private async Task InsertJoinRecordsAsync(
+    private async Task InsertJoinRecords(
+        bool sync,
         DbContext context,
         List<JoinRecord> joinRecords,
         BulkInsertOptions options,
@@ -293,9 +287,11 @@ private async Task InsertJoinRecordsAsync(
                 var joinEntry = Activator.CreateInstance(joinEntityType);
                 if (joinEntry == null)
                 {
-                    System.Diagnostics.Debug.WriteLine(
-                        $"Warning: IncludeGraph failed to create join entry for {joinEntityType.Name}. " +
-                        "Many-to-many relationship may be incomplete.");
+                    _logger?.LogWarning(
+                        "IncludeGraph failed to create join entry for {EntityType}. Many-to-many relationship may be incomplete.",
+                        joinEntityType.Name
+                    );
+
                     continue;
                 }
 
@@ -342,12 +338,13 @@ private async Task InsertJoinRecordsAsync(
             if (joinEntities.Count > 0)
             {
                 // Insert join entities
-                await InsertJoinEntitiesAsync(context, joinEntityType, joinEntities, options, provider, ctk);
+                await InsertJoinEntities(sync, context, joinEntityType, joinEntities, options, provider, ctk);
             }
         }
     }
 
-    private async Task InsertJoinEntitiesAsync(
+    private async Task InsertJoinEntities(
+        bool sync,
         DbContext context,
         Type joinEntityType,
         List<object> joinEntities,
@@ -369,7 +366,7 @@ private async Task InsertJoinEntitiesAsync(
             .GetMethod(nameof(IBulkInsertProvider.BulkInsert))!
             .MakeGenericMethod(joinEntityType);
 
-        var result = method.Invoke(provider, [false, context, tableInfo, joinEntities, options, null, ctk]);
+        var result = method.Invoke(provider, [sync, context, tableInfo, joinEntities, options, null, ctk]);
         if (result is not Task task)
         {
             throw new InvalidOperationException(

src/PhenX.EntityFrameworkCore.BulkInsert/Graph/GraphCollectionResult.cs

@@ -0,0 +1,22 @@
+namespace PhenX.EntityFrameworkCore.BulkInsert.Graph;
+
+/// <summary>
+/// Result of collecting entities from an object graph.
+/// </summary>
+internal sealed class GraphCollectionResult
+{
+    /// <summary>
+    /// Entities grouped by type.
+    /// </summary>
+    public required Dictionary<Type, List<object>> EntitiesByType { get; init; }
+
+    /// <summary>
+    /// Types in topological insertion order (parents before children).
+    /// </summary>
+    public required IReadOnlyList<Type> InsertionOrder { get; init; }
+
+    /// <summary>
+    /// Many-to-many join records to insert after both sides are inserted.
+    /// </summary>
+    public required List<JoinRecord> JoinRecords { get; init; }
+}

src/PhenX.EntityFrameworkCore.BulkInsert/Graph/GraphEntityCollector.cs

@@ -8,38 +8,6 @@
 
 namespace PhenX.EntityFrameworkCore.BulkInsert.Graph;
 
-/// <summary>
-/// Result of collecting entities from an object graph.
-/// </summary>
-internal sealed class GraphCollectionResult
-{
-    /// <summary>
-    /// Entities grouped by type.
-    /// </summary>
-    public required Dictionary<Type, List<object>> EntitiesByType { get; init; }
-
-    /// <summary>
-    /// Types in topological insertion order (parents before children).
-    /// </summary>
-    public required IReadOnlyList<Type> InsertionOrder { get; init; }
-
-    /// <summary>
-    /// Many-to-many join records to insert after both sides are inserted.
-    /// </summary>
-    public required List<JoinRecord> JoinRecords { get; init; }
-}
-
-/// <summary>
-/// Represents a join table record for many-to-many relationships.
-/// </summary>
-internal sealed class JoinRecord
-{
-    public required Type JoinEntityType { get; init; }
-    public required object LeftEntity { get; init; }
-    public required object RightEntity { get; init; }
-    public required NavigationMetadata Navigation { get; init; }
-}
-
 /// <summary>
 /// Collects all entities from an object graph for bulk insertion.
 /// </summary>
@@ -82,20 +50,27 @@ public GraphCollectionResult Collect<T>(IEnumerable<T> rootEntities) where T : c
         };
     }
 
-    private void CollectEntity(object entity, int depth)
+    private void CollectEntity(object? entity, int depth)
     {
-        if (entity == null || !_visited.Add(entity))
+        if (entity == null)
         {
-            // Already visited or null
+            // Null entity, nothing to collect
             return;
         }
 
-        // Check max depth
+        // Check max depth before marking as visited to avoid permanently
+        // excluding entities that might be reachable at a valid depth later.
         if (_options.MaxGraphDepth > 0 && depth > _options.MaxGraphDepth)
         {
             return;
         }
 
+        if (!_visited.Add(entity))
+        {
+            // Already visited
+            return;
+        }
+
         var entityType = entity.GetType();
         var efEntityType = _graphMetadata.GetEntityType(entityType);
 
@@ -133,33 +108,37 @@ private void CollectEntity(object entity, int depth)
 
             if (navigation.IsCollection)
             {
-                if (value is IEnumerable collection)
+                if (value is not IEnumerable collection)
                 {
-                    foreach (var item in collection)
+                    continue;
+                }
+
+                foreach (var item in collection)
+                {
+                    if (item == null)
+                    {
+                        continue;
+                    }
+
+                    if (navigation.IsManyToMany)
                     {
-                        if (item != null)
+                        // Record join table entry
+                        _joinRecords.Add(new JoinRecord
                         {
-                            if (navigation.IsManyToMany)
-                            {
-                                // Record join table entry
-                                _joinRecords.Add(new JoinRecord
-                                {
-                                    JoinEntityType = navigation.JoinEntityType!.ClrType,
-                                    LeftEntity = entity,
-                                    RightEntity = item,
-                                    Navigation = navigation,
-                                });
-                            }
-                            else
-                            {
-                                // For one-to-many, set the inverse navigation property
-                                // so that FK propagation can find the parent
-                                SetInverseNavigation(entity, item, navigation);
-                            }
-
-                            CollectEntity(item, depth + 1);
-                        }
+                            JoinEntityType = navigation.JoinEntityType!.ClrType,
+                            LeftEntity = entity,
+                            RightEntity = item,
+                            Navigation = navigation,
+                        });
                     }
+                    else
+                    {
+                        // For one-to-many, set the inverse navigation property
+                        // so that FK propagation can find the parent
+                        SetInverseNavigation(entity, item, navigation);
+                    }
+
+                    CollectEntity(item, depth + 1);
                 }
             }
             else