Skip to content

Commit eef4d4a

Browse files
committed
Remove "Include" and "ThenInclude" in BulkDelete because it is not always clear what table to delete from.
1 parent f9997e2 commit eef4d4a

File tree

4 files changed

+181
-4
lines changed

4 files changed

+181
-4
lines changed

src/Thinktecture.EntityFrameworkCore.BulkOperations/Extensions/BulkOperationsQueryableExtensions.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ public static int BulkDelete<T>(this IQueryable<T> source)
2626
ArgumentNullException.ThrowIfNull(source);
2727

2828
var methodInfo = _bulkDelete.MakeGenericMethod(typeof(T));
29-
var expression = Expression.Call(null, methodInfo, source.Expression);
29+
var sourceExpression = IncludeRemovingVisitor.Instance.Visit(source.Expression);
30+
var expression = Expression.Call(null, methodInfo, sourceExpression);
3031
return source.Provider.Execute<int>(expression);
3132
}
3233

@@ -49,7 +50,23 @@ public static Task<int> BulkDeleteAsync<T>(
4950
throw new InvalidOperationException(CoreStrings.IQueryableProviderNotAsync);
5051

5152
var methodInfo = _bulkDelete.MakeGenericMethod(typeof(T));
52-
var expression = Expression.Call(null, methodInfo, source.Expression);
53+
var sourceExpression = IncludeRemovingVisitor.Instance.Visit(source.Expression);
54+
var expression = Expression.Call(null, methodInfo, sourceExpression);
5355
return provider.ExecuteAsync<Task<int>>(expression, cancellationToken);
5456
}
57+
58+
private class IncludeRemovingVisitor : ExpressionVisitor
59+
{
60+
public static readonly IncludeRemovingVisitor Instance = new();
61+
62+
protected override Expression VisitMethodCall(MethodCallExpression node)
63+
{
64+
// Include and ThenInclude may expand the query so it is unclear what table to DELETE from
65+
if (node.Method.DeclaringType == typeof(EntityFrameworkQueryableExtensions)
66+
&& node.Method.Name is nameof(EntityFrameworkQueryableExtensions.Include) or nameof(EntityFrameworkQueryableExtensions.ThenInclude))
67+
return node.Arguments[0];
68+
69+
return base.VisitMethodCall(node);
70+
}
71+
}
5572
}

src/Thinktecture.EntityFrameworkCore.BulkOperations/Extensions/BulkOperationsRelationalQueryableMethodTranslatingExpressionVisitorExtensions.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,14 @@ private static TableExpressionBase GetTableForDeleteOperation(SelectExpression s
100100
{
101101
TableExpressionBase? table = null;
102102

103-
if (selectExpression.Projection.Count == 0 && selectExpression.Tables.Count == 1)
104-
return selectExpression.Tables[0];
103+
if (selectExpression.Projection.Count == 0)
104+
{
105+
if (selectExpression.Tables.Count == 1)
106+
return selectExpression.Tables[0];
107+
108+
if (selectExpression.Tables.Count > 1)
109+
throw new NotSupportedException($"The provided query is referencing more than 1 table. If the entity has owned types, then please provide just one column of the table to DELETE from [example: Select(x => x.Id).BulkDeleteAsync()]. Found tables: [{String.Join(", ", selectExpression.Tables.Select(GetDisplayName))}].");
110+
}
105111

106112
foreach (ProjectionExpression projection in selectExpression.Projection)
107113
{

tests/Thinktecture.EntityFrameworkCore.SqlServer.Tests/Extensions/QueryableExtensionsTests/BulkDelete.cs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,4 +220,64 @@ public void Should_throw_if_skip_is_present()
220220
var loadedEntities = AssertDbContext.TestEntities.ToList();
221221
loadedEntities.Should().HaveCount(2);
222222
}
223+
224+
[Fact]
225+
public void Should_handle_Include_properly()
226+
{
227+
var parent = new TestEntity { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), RequiredName = "RequiredName" };
228+
var child = new TestEntity { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), RequiredName = "RequiredName", Parent = parent };
229+
ArrangeDbContext.Add(parent);
230+
ArrangeDbContext.Add(child);
231+
ArrangeDbContext.SaveChanges();
232+
233+
var affectedRows = ActDbContext.TestEntities
234+
.Include(e => e.Children)
235+
.Where(e => e.Parent!.Count == 0)
236+
.BulkDelete();
237+
238+
affectedRows.Should().Be(1);
239+
240+
var loadedEntities = AssertDbContext.TestEntities.ToList();
241+
loadedEntities.Should().BeEquivalentTo(new[] { new TestEntity { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), RequiredName = "RequiredName" } });
242+
}
243+
244+
[Fact]
245+
public async Task Should_delete_entity_with_separate_owned_types_if_column_of_main_entity_is_provided()
246+
{
247+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = new OwnedEntity { IntColumn = 1 } });
248+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } });
249+
await ArrangeDbContext.SaveChangesAsync();
250+
251+
var affectedRows = await ActDbContext.TestEntities_Own_SeparateOne
252+
.Where(e => e.SeparateEntity.IntColumn == 1)
253+
.Select(e => e.Id)
254+
.BulkDeleteAsync();
255+
256+
affectedRows.Should().Be(1);
257+
258+
var loadedEntities = await AssertDbContext.TestEntities_Own_SeparateOne.ToListAsync();
259+
loadedEntities.Should().BeEquivalentTo(new[] { new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } } });
260+
}
261+
262+
[Fact]
263+
public async Task Should_delete_separate_owned_entity_only_if_column_of_owned_entity_is_provided()
264+
{
265+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = new OwnedEntity { IntColumn = 1 } });
266+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } });
267+
await ArrangeDbContext.SaveChangesAsync();
268+
269+
var affectedRows = await ActDbContext.TestEntities_Own_SeparateOne
270+
.Where(e => e.SeparateEntity.IntColumn == 1)
271+
.Select(e => e.SeparateEntity)
272+
.BulkDeleteAsync();
273+
274+
affectedRows.Should().Be(1);
275+
276+
var loadedEntities = await AssertDbContext.TestEntities_Own_SeparateOne.ToListAsync();
277+
loadedEntities.Should().BeEquivalentTo(new[]
278+
{
279+
new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = null! },
280+
new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } }
281+
});
282+
}
223283
}

tests/Thinktecture.EntityFrameworkCore.SqlServer.Tests/Extensions/QueryableExtensionsTests/BulkDeleteAsync.cs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,98 @@ await ActDbContext.TestEntities.Skip(1).Awaiting(q => q.BulkDeleteAsync())
9999
var loadedEntities = await AssertDbContext.TestEntities.ToListAsync();
100100
loadedEntities.Should().HaveCount(2);
101101
}
102+
103+
[Fact]
104+
public async Task Should_handle_Include_properly()
105+
{
106+
var parent = new TestEntity { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), RequiredName = "RequiredName" };
107+
var child = new TestEntity { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), RequiredName = "RequiredName", Parent = parent };
108+
ArrangeDbContext.Add(parent);
109+
ArrangeDbContext.Add(child);
110+
await ArrangeDbContext.SaveChangesAsync();
111+
112+
var affectedRows = await ActDbContext.TestEntities
113+
.Include(e => e.Children)
114+
.Where(e => e.Parent!.Count == 0)
115+
.BulkDeleteAsync();
116+
117+
affectedRows.Should().Be(1);
118+
119+
var loadedEntities = await AssertDbContext.TestEntities.ToListAsync();
120+
loadedEntities.Should().BeEquivalentTo(new[] { new TestEntity { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), RequiredName = "RequiredName" } });
121+
}
122+
123+
[Fact]
124+
public async Task Should_handle_entities_with_inlined_owned_types()
125+
{
126+
var entity1 = new TestEntity_Owns_Inline { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), InlineEntity = new OwnedEntity { IntColumn = 1 } };
127+
var entity2 = new TestEntity_Owns_Inline { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), InlineEntity = new OwnedEntity { IntColumn = 2 } };
128+
ArrangeDbContext.Add(entity1);
129+
ArrangeDbContext.Add(entity2);
130+
await ArrangeDbContext.SaveChangesAsync();
131+
132+
var affectedRows = await ActDbContext.TestEntities_Own_Inline
133+
.Where(e => e.InlineEntity.IntColumn == 1)
134+
.BulkDeleteAsync();
135+
136+
affectedRows.Should().Be(1);
137+
138+
var loadedEntities = await AssertDbContext.TestEntities_Own_Inline.ToListAsync();
139+
loadedEntities.Should().BeEquivalentTo(new[] { new TestEntity_Owns_Inline { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), InlineEntity = new OwnedEntity { IntColumn = 2 } } });
140+
}
141+
142+
[Fact]
143+
public async Task Should_throw_if_entity_has_separate_owned_types()
144+
{
145+
ArrangeDbContext.Add(new TestEntity { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), RequiredName = "RequiredName" });
146+
ArrangeDbContext.Add(new TestEntity { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), RequiredName = "RequiredName" });
147+
await ArrangeDbContext.SaveChangesAsync();
148+
149+
await ActDbContext.TestEntities_Own_SeparateOne
150+
.Awaiting(q => q.BulkDeleteAsync())
151+
.Should().ThrowAsync<NotSupportedException>().WithMessage("The provided query is referencing more than 1 table. If the entity has owned types, then please provide just one column of the table to DELETE from [example: Select(x => x.Id).BulkDeleteAsync()]. Found tables: [TestEntities_Own_SeparateOne AS t, SeparateEntitiesOne AS s].");
152+
153+
var loadedEntities = await AssertDbContext.TestEntities.ToListAsync();
154+
loadedEntities.Should().HaveCount(2);
155+
}
156+
157+
[Fact]
158+
public async Task Should_delete_entity_with_separate_owned_types_if_column_of_main_entity_is_provided()
159+
{
160+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = new OwnedEntity { IntColumn = 1 } });
161+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } });
162+
await ArrangeDbContext.SaveChangesAsync();
163+
164+
var affectedRows = await ActDbContext.TestEntities_Own_SeparateOne
165+
.Where(e => e.SeparateEntity.IntColumn == 1)
166+
.Select(e => e.Id)
167+
.BulkDeleteAsync();
168+
169+
affectedRows.Should().Be(1);
170+
171+
var loadedEntities = await AssertDbContext.TestEntities_Own_SeparateOne.ToListAsync();
172+
loadedEntities.Should().BeEquivalentTo(new[] { new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } } });
173+
}
174+
175+
[Fact]
176+
public async Task Should_delete_separate_owned_entity_only_if_column_of_owned_entity_is_provided()
177+
{
178+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = new OwnedEntity { IntColumn = 1 } });
179+
ArrangeDbContext.Add(new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } });
180+
await ArrangeDbContext.SaveChangesAsync();
181+
182+
var affectedRows = await ActDbContext.TestEntities_Own_SeparateOne
183+
.Where(e => e.SeparateEntity.IntColumn == 1)
184+
.Select(e => e.SeparateEntity)
185+
.BulkDeleteAsync();
186+
187+
affectedRows.Should().Be(1);
188+
189+
var loadedEntities = await AssertDbContext.TestEntities_Own_SeparateOne.ToListAsync();
190+
loadedEntities.Should().BeEquivalentTo(new[]
191+
{
192+
new TestEntity_Owns_SeparateOne { Id = new Guid("6C410EFE-2A40-4348-8BD6-8E9B9B72F0D0"), SeparateEntity = null! },
193+
new TestEntity_Owns_SeparateOne { Id = new Guid("C004AB82-803E-4A90-B254-6032B9BBB70E"), SeparateEntity = new OwnedEntity { IntColumn = 2 } }
194+
});
195+
}
102196
}

0 commit comments

Comments
 (0)