Skip to content

Commit 9f337a9

Browse files
CopilotPhenX
andcommitted
Address PR review feedback: add tests, validation, and docs improvements
Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com>
1 parent 2514aac commit 9f337a9

5 files changed

Lines changed: 119 additions & 10 deletions

File tree

docs/graph-insert.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,7 @@ await dbContext.ExecuteBulkInsertAsync(new[] { blog }, o =>
131131

132132
- **Shadow foreign keys**: Currently not supported. Add a CLR property for foreign keys.
133133
- **Circular references**: Handled gracefully by tracking visited entities, but may result in incomplete graphs.
134+
- **Owned entities**: Owned entity types are not included in graph traversal and are not inserted when using `IncludeGraph = true`.
135+
- **Self-referencing hierarchies**: Multi-level self-referencing hierarchies (e.g., Category → Children) require multiple insert operations. Root entities can be inserted, but nested children with FK references to other entities of the same type within the same batch are not supported.
136+
- **Many-to-many join tables**: Entities on both sides of many-to-many relationships are traversed and inserted. However, automatic join table population only works with explicit join entity types (not `Dictionary<string, object>` shared-type entities).
134137
- **OnConflict/Upsert**: Not currently supported with `IncludeGraph = true`.

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,13 @@ public static async Task ExecuteBulkInsertAsync<T, TOptions>(
160160

161161
if (options.IncludeGraph)
162162
{
163+
if (onConflict != null)
164+
{
165+
throw new InvalidOperationException(
166+
"OnConflict options cannot be used together with IncludeGraph. " +
167+
"Either disable IncludeGraph or remove the onConflict parameter.");
168+
}
169+
163170
var orchestrator = new GraphBulkInsertOrchestrator();
164171
await orchestrator.InsertGraphAsync(context, entities, options, provider, cancellationToken);
165172
return;
@@ -214,6 +221,13 @@ public static void ExecuteBulkInsert<T, TOptions>(
214221

215222
if (options.IncludeGraph)
216223
{
224+
if (onConflict != null)
225+
{
226+
throw new InvalidOperationException(
227+
"OnConflict options cannot be used together with IncludeGraph. " +
228+
"Either disable IncludeGraph or remove the onConflict parameter.");
229+
}
230+
217231
var orchestrator = new GraphBulkInsertOrchestrator();
218232
orchestrator.InsertGraphAsync(context, entities, options, provider, CancellationToken.None)
219233
.GetAwaiter().GetResult();

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ private static void PropagateParentForeignKeys(
121121
// For each FK relationship, propagate PK values from parent entities
122122
foreach (var fk in efEntityType.GetForeignKeys())
123123
{
124-
var principalEntityType = fk.PrincipalEntityType;
125124
var dependentNavigation = fk.DependentToPrincipal;
126125

127126
if (dependentNavigation == null)
@@ -300,14 +299,24 @@ private async Task InsertJoinRecordsAsync(
300299
continue;
301300
}
302301

302+
// Check if the join entity is a dictionary (shared-type entity)
303+
var isDictionary = joinEntry is IDictionary<string, object>;
304+
303305
// Set FK values for left entity
304306
for (var i = 0; i < fk.Properties.Count; i++)
305307
{
306308
var fkProp = fk.Properties[i];
307309
var pkProp = fk.PrincipalKey.Properties[i];
308310

309311
var pkValue = GetPropertyValue(record.LeftEntity, pkProp.Name);
310-
SetPropertyValue(joinEntry, fkProp.Name, pkValue);
312+
if (isDictionary)
313+
{
314+
((IDictionary<string, object>)joinEntry)[fkProp.Name] = pkValue!;
315+
}
316+
else
317+
{
318+
SetPropertyValue(joinEntry, fkProp.Name, pkValue);
319+
}
311320
}
312321

313322
// Set FK values for right entity
@@ -317,7 +326,14 @@ private async Task InsertJoinRecordsAsync(
317326
var pkProp = inverseFk.PrincipalKey.Properties[i];
318327

319328
var pkValue = GetPropertyValue(record.RightEntity, pkProp.Name);
320-
SetPropertyValue(joinEntry, fkProp.Name, pkValue);
329+
if (isDictionary)
330+
{
331+
((IDictionary<string, object>)joinEntry)[fkProp.Name] = pkValue!;
332+
}
333+
else
334+
{
335+
SetPropertyValue(joinEntry, fkProp.Name, pkValue);
336+
}
321337
}
322338

323339
joinEntities.Add(joinEntry);
@@ -353,7 +369,12 @@ private async Task InsertJoinEntitiesAsync(
353369
.GetMethod(nameof(IBulkInsertProvider.BulkInsert))!
354370
.MakeGenericMethod(joinEntityType);
355371

356-
var task = (Task)method.Invoke(provider, [false, context, tableInfo, joinEntities, options, null, ctk])!;
372+
var result = method.Invoke(provider, [false, context, tableInfo, joinEntities, options, null, ctk]);
373+
if (result is not Task task)
374+
{
375+
throw new InvalidOperationException(
376+
$"The BulkInsert method for join entity type '{joinEntityType.Name}' did not return a Task as expected.");
377+
}
357378
await task;
358379
}
359380

src/PhenX.EntityFrameworkCore.BulkInsert/Metadata/GraphMetadata.cs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,9 @@ private void TopologicalSort(
100100

101101
// Get dependencies (types that this type references via FKs)
102102
var navigations = GetNavigations(type);
103-
foreach (var nav in navigations)
103+
foreach (var nav in navigations.Where(n => n.IsDependentToPrincipal && validTypes.Contains(n.TargetType) && n.TargetType != type))
104104
{
105-
// Only consider dependent-to-principal navigations (this entity has the FK)
106-
if (nav.IsDependentToPrincipal && validTypes.Contains(nav.TargetType) && nav.TargetType != type)
107-
{
108-
TopologicalSort(nav.TargetType, validTypes, visited, visiting, result);
109-
}
105+
TopologicalSort(nav.TargetType, validTypes, visited, visiting, result);
110106
}
111107

112108
visiting.Remove(type);

tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Graph/GraphTestsBase.cs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,4 +359,79 @@ public async Task InsertGraph_SyncVariant_Works()
359359
var insertedPosts = _context.Posts.Where(p => p.TestRun == _run).ToList();
360360
insertedPosts.Should().HaveCount(1);
361361
}
362+
363+
[SkippableFact]
364+
public async Task InsertGraph_SelfReferencing_InsertsRootOnly()
365+
{
366+
// Arrange - Self-referencing hierarchies require multiple inserts in order
367+
// This test verifies that root entities without parents can be inserted
368+
var rootCategory = new Category
369+
{
370+
TestRun = _run,
371+
Name = $"{_run}_RootCategory",
372+
Parent = null,
373+
ParentId = null
374+
};
375+
376+
// Act
377+
await _context.ExecuteBulkInsertAsync(new[] { rootCategory }, options =>
378+
{
379+
options.IncludeGraph = true;
380+
});
381+
382+
// Assert
383+
var insertedCategories = _context.Categories.Where(c => c.TestRun == _run).ToList();
384+
insertedCategories.Should().HaveCount(1);
385+
386+
var insertedRoot = insertedCategories.First();
387+
insertedRoot.Id.Should().BeGreaterThan(0);
388+
insertedRoot.ParentId.Should().BeNull();
389+
}
390+
391+
[SkippableFact]
392+
public async Task InsertGraph_ManyToMany_TraversesRelatedEntities()
393+
{
394+
// Note: Many-to-many join table insertion requires explicit join entity types.
395+
// Dictionary<string, object> join entities are not supported by the bulk insert infrastructure.
396+
// This test verifies that many-to-many navigations are traversed and related entities are collected.
397+
398+
// Arrange - Create a post with tags
399+
var tag1 = new Tag { TestRun = _run, Name = $"{_run}_Tag1" };
400+
var tag2 = new Tag { TestRun = _run, Name = $"{_run}_Tag2" };
401+
402+
var blog = new Blog
403+
{
404+
TestRun = _run,
405+
Name = $"{_run}_BlogWithTaggedPost",
406+
Posts = new List<Post>
407+
{
408+
new Post
409+
{
410+
TestRun = _run,
411+
Title = $"{_run}_TaggedPost",
412+
Tags = new List<Tag> { tag1, tag2 }
413+
}
414+
}
415+
};
416+
417+
// Act
418+
await _context.ExecuteBulkInsertAsync(new[] { blog }, options =>
419+
{
420+
options.IncludeGraph = true;
421+
});
422+
423+
// Assert - Verify the entities were inserted (even if join table wasn't populated)
424+
var insertedBlog = _context.Blogs.FirstOrDefault(b => b.TestRun == _run);
425+
insertedBlog.Should().NotBeNull();
426+
insertedBlog!.Id.Should().BeGreaterThan(0);
427+
428+
var insertedPost = _context.Posts.FirstOrDefault(p => p.TestRun == _run);
429+
insertedPost.Should().NotBeNull();
430+
insertedPost!.Id.Should().BeGreaterThan(0);
431+
432+
// Tags should be inserted as they were traversed via many-to-many navigation
433+
var insertedTags = _context.Tags.Where(t => t.TestRun == _run).ToList();
434+
insertedTags.Should().HaveCount(2);
435+
insertedTags.Should().AllSatisfy(t => t.Id.Should().BeGreaterThan(0));
436+
}
362437
}

0 commit comments

Comments
 (0)