Skip to content

Commit 97e239e

Browse files
author
fabien.menager
committed
Wrap full graph insert with a transaction
1 parent 29eb67b commit 97e239e

2 files changed

Lines changed: 99 additions & 27 deletions

File tree

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

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.Extensions.Logging;
66

77
using PhenX.EntityFrameworkCore.BulkInsert.Abstractions;
8+
using PhenX.EntityFrameworkCore.BulkInsert.Extensions;
89
using PhenX.EntityFrameworkCore.BulkInsert.Metadata;
910
using PhenX.EntityFrameworkCore.BulkInsert.Options;
1011

@@ -37,6 +38,10 @@ public async Task<GraphInsertResult<T>> InsertGraph<T>(
3738
IBulkInsertProvider provider,
3839
CancellationToken ctk) where T : class
3940
{
41+
42+
using var activity = Telemetry.ActivitySource.StartActivity("InsertGraph");
43+
activity?.AddTag("synchronous", sync);
44+
4045
// 1. Collect and sort entities
4146
var collector = new GraphEntityCollector(_context, options);
4247
var collectionResult = collector.Collect(entities);
@@ -68,42 +73,54 @@ public async Task<GraphInsertResult<T>> InsertGraph<T>(
6873
$"Consider using client-generated keys (e.g., GUIDs with ValueGeneratedNever()).");
6974
}
7075

71-
// 2. Insert in dependency order (parents first)
72-
foreach (var entityType in collectionResult.InsertionOrder)
76+
var connection = await _context.GetConnection(sync, ctk);
77+
78+
try
7379
{
74-
if (!collectionResult.EntitiesByType.TryGetValue(entityType, out var entitiesToInsert) ||
75-
entitiesToInsert.Count == 0)
80+
// 2. Insert in dependency order (parents first)
81+
foreach (var entityType in collectionResult.InsertionOrder)
7682
{
77-
continue;
78-
}
83+
if (!collectionResult.EntitiesByType.TryGetValue(entityType, out var entitiesToInsert) ||
84+
entitiesToInsert.Count == 0)
85+
{
86+
continue;
87+
}
7988

80-
// Propagate FK values from already-inserted parents
81-
PropagateParentForeignKeys(entitiesToInsert, entityType, graphMetadata);
89+
// Propagate FK values from already-inserted parents
90+
PropagateParentForeignKeys(entitiesToInsert, entityType, graphMetadata);
8291

83-
// Insert entities of this type
84-
await InsertEntitiesOfType(sync, _context, entityType, entitiesToInsert, options, provider, graphMetadata, ctk);
92+
// Insert entities of this type
93+
await InsertEntitiesOfType(sync, _context, entityType, entitiesToInsert, options, provider,
94+
graphMetadata, ctk);
8595

86-
totalInserted += entitiesToInsert.Count;
87-
}
96+
totalInserted += entitiesToInsert.Count;
97+
}
8898

89-
// 3. Insert join table records for many-to-many relationships
90-
var joinRecordsInserted = 0;
91-
if (collectionResult.JoinRecords.Count > 0)
92-
{
93-
joinRecordsInserted = await InsertJoinRecords(sync, _context, collectionResult.JoinRecords, options, provider, graphMetadata, ctk);
94-
totalInserted += joinRecordsInserted;
95-
}
99+
// 3. Insert join table records for many-to-many relationships
100+
if (collectionResult.JoinRecords.Count > 0)
101+
{
102+
totalInserted += await InsertJoinRecords(sync, _context, collectionResult.JoinRecords, options,
103+
provider, graphMetadata, ctk);
104+
}
105+
106+
// Return root entities
107+
var rootEntities = collectionResult.EntitiesByType.TryGetValue(typeof(T), out var roots)
108+
? roots.Cast<T>().ToList()
109+
: [];
96110

97-
// Return root entities
98-
var rootEntities = collectionResult.EntitiesByType.TryGetValue(typeof(T), out var roots)
99-
? roots.Cast<T>().ToList()
100-
: [];
111+
// Commit the transaction if we own them.
112+
await connection.Commit(sync, ctk);
101113

102-
return new GraphInsertResult<T>
114+
return new GraphInsertResult<T>
115+
{
116+
RootEntities = rootEntities,
117+
TotalInsertedCount = totalInserted,
118+
};
119+
}
120+
finally
103121
{
104-
RootEntities = rootEntities,
105-
TotalInsertedCount = totalInserted,
106-
};
122+
await connection.Close(sync, ctk);
123+
}
107124
}
108125

109126
private static void PropagateParentForeignKeys(

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,4 +714,59 @@ await _context.ExecuteBulkInsertAsync(blogs, options =>
714714
$"Inserted {totalEntities:N0} entities in {stopwatch.Elapsed.TotalSeconds:F2}s " +
715715
$"({entitiesPerSecond:F0} entities/sec) using {_context.GetType().Name}");
716716
}
717+
718+
[SkippableFact]
719+
public async Task InsertGraph_FailureMidRun_TransactionRolledBack()
720+
{
721+
// Arrange - Create a graph where the second blog has an invalid post (null Title)
722+
// This should cause the entire transaction to be rolled back
723+
var validBlog = new Blog
724+
{
725+
TestRun = _run,
726+
Name = $"{_run}_ValidBlog",
727+
Posts = new List<Post>
728+
{
729+
new Post { TestRun = _run, Title = $"{_run}_ValidPost" }
730+
},
731+
Settings = new BlogSettings
732+
{
733+
TestRun = _run,
734+
EnableComments = true
735+
}
736+
};
737+
738+
var invalidBlog = new Blog
739+
{
740+
TestRun = _run,
741+
Name = $"{_run}_InvalidBlog",
742+
Posts = new List<Post>
743+
{
744+
new Post { TestRun = _run, Title = null! } // This should violate NOT NULL constraint
745+
}
746+
};
747+
748+
var blogs = new[] { validBlog, invalidBlog };
749+
750+
// Act & Assert - Expect an exception during insert
751+
var act = async () => await _context.ExecuteBulkInsertAsync(blogs, options =>
752+
{
753+
options.IncludeGraph = true;
754+
});
755+
756+
await act.Should().ThrowAsync<Exception>("Insert should fail due to NULL constraint violation");
757+
758+
// Assert - Verify that NOTHING was inserted due to transaction rollback
759+
var insertedBlogs = _context.Blogs.Where(b => b.TestRun == _run).ToList();
760+
insertedBlogs.Should().BeEmpty("Transaction should be rolled back - no blogs inserted");
761+
762+
var insertedPosts = _context.Posts.Where(p => p.TestRun == _run).ToList();
763+
insertedPosts.Should().BeEmpty("Transaction should be rolled back - no posts inserted");
764+
765+
var insertedSettings = _context.BlogSettings.Where(s => s.TestRun == _run).ToList();
766+
insertedSettings.Should().BeEmpty("Transaction should be rolled back - no settings inserted");
767+
768+
// Verify original entities do NOT have IDs populated (rollback means no database-generated values)
769+
validBlog.Id.Should().Be(0, "Valid blog should not have ID after rollback");
770+
invalidBlog.Id.Should().Be(0, "Invalid blog should not have ID after rollback");
771+
}
717772
}

0 commit comments

Comments
 (0)