Skip to content

Commit d4be66b

Browse files
rojiroji
authored
Support interfaces and entities with owned types in ExecuteUpdate setter property lambda (dotnet#29672)
Fixes dotnet#29618 Fixes dotnet#28727
1 parent 315be6c commit d4be66b

14 files changed

+260
-25
lines changed

‎src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,7 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio
103103
return null;
104104
}
105105

106-
if (!(expression is NewExpression
107-
|| expression is MemberInitExpression
108-
|| expression is EntityShaperExpression
109-
|| expression is IncludeExpression))
106+
if (expression is not NewExpression and not MemberInitExpression and not EntityShaperExpression and not IncludeExpression)
110107
{
111108
if (_indexBasedBinding)
112109
{

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,8 +1195,8 @@ static Expression PruneOwnedIncludes(IncludeExpression includeExpression)
11951195
foreach (var (propertyExpression, _) in propertyValueLambdaExpressions)
11961196
{
11971197
var left = RemapLambdaBody(source, propertyExpression);
1198-
left = left.UnwrapTypeConversion(out _);
1199-
if (!IsValidPropertyAccess(RelationalDependencies.Model, left, out var ese))
1198+
1199+
if (!TryProcessPropertyAccess(RelationalDependencies.Model, ref left, out var ese))
12001200
{
12011201
AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print()));
12021202
return null;
@@ -1399,36 +1399,63 @@ when methodCallExpression.Method.IsGenericMethod
13991399
}
14001400
}
14011401

1402-
static bool IsValidPropertyAccess(
1402+
// For property setter selectors in ExecuteUpdate, we support only simple member access, EF.Function, etc.
1403+
// We also unwrap casts to interface/base class (#29618), as well as IncludeExpressions (which occur when the target entity has
1404+
// owned entities, #28727).
1405+
static bool TryProcessPropertyAccess(
14031406
IModel model,
1404-
Expression expression,
1407+
ref Expression expression,
14051408
[NotNullWhen(true)] out EntityShaperExpression? entityShaperExpression)
14061409
{
1407-
if (expression is MemberExpression { Expression: EntityShaperExpression ese })
1410+
expression = expression.UnwrapTypeConversion(out _);
1411+
1412+
if (expression is MemberExpression { Expression : not null } memberExpression
1413+
&& Unwrap(memberExpression.Expression) is EntityShaperExpression ese)
14081414
{
1415+
expression = memberExpression.Update(ese);
14091416
entityShaperExpression = ese;
14101417
return true;
14111418
}
14121419

14131420
if (expression is MethodCallExpression mce)
14141421
{
14151422
if (mce.TryGetEFPropertyArguments(out var source, out _)
1416-
&& source is EntityShaperExpression ese1)
1423+
&& Unwrap(source) is EntityShaperExpression ese1)
14171424
{
1425+
if (source != ese1)
1426+
{
1427+
var rewrittenArguments = mce.Arguments.ToArray();
1428+
rewrittenArguments[0] = ese1;
1429+
expression = mce.Update(mce.Object, rewrittenArguments);
1430+
}
1431+
14181432
entityShaperExpression = ese1;
14191433
return true;
14201434
}
14211435

14221436
if (mce.TryGetIndexerArguments(model, out var source2, out _)
1423-
&& source2 is EntityShaperExpression ese2)
1437+
&& Unwrap(source2) is EntityShaperExpression ese2)
14241438
{
1439+
expression = mce.Update(ese2, mce.Arguments);
14251440
entityShaperExpression = ese2;
14261441
return true;
14271442
}
14281443
}
14291444

14301445
entityShaperExpression = null;
14311446
return false;
1447+
1448+
static Expression Unwrap(Expression expression)
1449+
{
1450+
expression = expression.UnwrapTypeConversion(out _);
1451+
1452+
while (expression is IncludeExpression includeExpression)
1453+
{
1454+
expression = includeExpression.EntityExpression;
1455+
}
1456+
1457+
return expression;
1458+
}
14321459
}
14331460

14341461
static Expression GetEntitySource(IModel model, Expression propertyAccessExpression)
@@ -1645,11 +1672,9 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
16451672
}
16461673

16471674
protected override Expression VisitExtension(Expression extensionExpression)
1648-
=> extensionExpression is EntityShaperExpression
1649-
|| extensionExpression is ShapedQueryExpression
1650-
|| extensionExpression is GroupByShaperExpression
1651-
? extensionExpression
1652-
: base.VisitExtension(extensionExpression);
1675+
=> extensionExpression is EntityShaperExpression or ShapedQueryExpression or GroupByShaperExpression
1676+
? extensionExpression
1677+
: base.VisitExtension(extensionExpression);
16531678

16541679
private Expression? TryExpand(Expression? source, MemberIdentity member)
16551680
{

‎src/Shared/ExpressionExtensions.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ public static LambdaExpression UnwrapLambdaFromQuote(this Expression expression)
2424
public static Expression? UnwrapTypeConversion(this Expression? expression, out Type? convertedType)
2525
{
2626
convertedType = null;
27-
while (expression is UnaryExpression unaryExpression
28-
&& (unaryExpression.NodeType == ExpressionType.Convert
29-
|| unaryExpression.NodeType == ExpressionType.ConvertChecked
30-
|| unaryExpression.NodeType == ExpressionType.TypeAs))
27+
while (expression is UnaryExpression
28+
{
29+
NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked or ExpressionType.TypeAs
30+
} unaryExpression)
3131
{
3232
expression = unaryExpression.Operand;
3333
if (unaryExpression.Type != typeof(object) // Ignore object conversion

‎test/EFCore.Relational.Specification.Tests/BulkUpdates/InheritanceBulkUpdatesTestBase.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,5 +158,26 @@ public virtual Task Update_where_keyless_entity_mapped_to_sql_query(bool async)
158158
s => s.SetProperty(e => e.Name, "Eagle"),
159159
rowsAffectedCount: 1));
160160

161+
[ConditionalTheory]
162+
[MemberData(nameof(IsAsyncData))]
163+
public virtual Task Update_with_interface_in_property_expression(bool async)
164+
=> AssertUpdate(
165+
async,
166+
ss => ss.Set<Coke>(),
167+
e => e,
168+
s => s.SetProperty(c => ((ISugary)c).SugarGrams, 0),
169+
rowsAffectedCount: 1);
170+
171+
[ConditionalTheory]
172+
[MemberData(nameof(IsAsyncData))]
173+
public virtual Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
174+
=> AssertUpdate(
175+
async,
176+
ss => ss.Set<Coke>(),
177+
e => e,
178+
// ReSharper disable once RedundantCast
179+
s => s.SetProperty(c => EF.Property<int>((ISugary)c, nameof(ISugary.SugarGrams)), 0),
180+
rowsAffectedCount: 1);
181+
161182
protected abstract void ClearLog();
162183
}

‎test/EFCore.Relational.Specification.Tests/BulkUpdates/NonSharedModelBulkUpdatesTestBase.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,25 @@ public class OtherReference
9090
}
9191

9292
#nullable enable
93+
94+
[ConditionalTheory]
95+
[MemberData(nameof(IsAsyncData))]
96+
public virtual async Task Update_non_owned_property_on_entity_with_owned(bool async)
97+
{
98+
var contextFactory = await InitializeAsync<Context28671>(
99+
onModelCreating: mb =>
100+
{
101+
mb.Entity<Owner>().OwnsOne(o => o.OwnedReference);
102+
});
103+
104+
await AssertUpdate(
105+
async,
106+
contextFactory.CreateContext,
107+
ss => ss.Set<Owner>(),
108+
s => s.SetProperty(o => o.Title, "SomeValue"),
109+
rowsAffectedCount: 0);
110+
}
111+
93112
[ConditionalTheory]
94113
[MemberData(nameof(IsAsyncData))]
95114
public virtual async Task Delete_predicate_based_on_optional_navigation(bool async)

‎test/EFCore.Relational.Specification.Tests/BulkUpdates/TPTInheritanceBulkUpdatesTestBase.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,14 @@ public override Task Update_where_hierarchy_derived(bool async)
5656
=> AssertTranslationFailed(
5757
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Kiwi"),
5858
() => base.Update_where_hierarchy_derived(async));
59+
60+
public override Task Update_with_interface_in_property_expression(bool async)
61+
=> AssertTranslationFailed(
62+
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"),
63+
() => base.Update_with_interface_in_property_expression(async));
64+
65+
public override Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
66+
=> AssertTranslationFailed(
67+
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"),
68+
() => base.Update_with_interface_in_EF_Property_in_property_expression(async));
5969
}

‎test/EFCore.SqlServer.FunctionalTests/BulkUpdates/InheritanceBulkUpdatesSqlServerTest.cs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;
55

66
public class InheritanceBulkUpdatesSqlServerTest : InheritanceBulkUpdatesTestBase<InheritanceBulkUpdatesSqlServerFixture>
77
{
8-
public InheritanceBulkUpdatesSqlServerTest(InheritanceBulkUpdatesSqlServerFixture fixture)
8+
public InheritanceBulkUpdatesSqlServerTest(
9+
InheritanceBulkUpdatesSqlServerFixture fixture,
10+
ITestOutputHelper testOutputHelper)
11+
912
: base(fixture)
1013
{
1114
ClearLog();
15+
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
1216
}
1317

1418
[ConditionalFact]
@@ -205,6 +209,32 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool
205209
AssertExecuteUpdateSql();
206210
}
207211

212+
public override async Task Update_with_interface_in_property_expression(bool async)
213+
{
214+
await base.Update_with_interface_in_property_expression(async);
215+
216+
AssertExecuteUpdateSql(
217+
"""
218+
UPDATE [d]
219+
SET [d].[SugarGrams] = 0
220+
FROM [Drinks] AS [d]
221+
WHERE [d].[Discriminator] = N'Coke'
222+
""");
223+
}
224+
225+
public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
226+
{
227+
await base.Update_with_interface_in_EF_Property_in_property_expression(async);
228+
229+
AssertExecuteUpdateSql(
230+
"""
231+
UPDATE [d]
232+
SET [d].[SugarGrams] = 0
233+
FROM [Drinks] AS [d]
234+
WHERE [d].[Discriminator] = N'Coke'
235+
""");
236+
}
237+
208238
protected override void ClearLog()
209239
=> Fixture.TestSqlLoggerFactory.Clear();
210240

‎test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
4141
AssertSql();
4242
}
4343

44+
public override async Task Update_non_owned_property_on_entity_with_owned(bool async)
45+
{
46+
await base.Update_non_owned_property_on_entity_with_owned(async);
47+
48+
AssertSql(
49+
"""
50+
UPDATE [o]
51+
SET [o].[Title] = N'SomeValue'
52+
FROM [Owner] AS [o]
53+
""");
54+
}
55+
4456
public override async Task Delete_predicate_based_on_optional_navigation(bool async)
4557
{
4658
await base.Delete_predicate_based_on_optional_navigation(async);

‎test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPCInheritanceBulkUpdatesSqlServerTest.cs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;
55

66
public class TPCInheritanceBulkUpdatesSqlServerTest : TPCInheritanceBulkUpdatesTestBase<TPCInheritanceBulkUpdatesSqlServerFixture>
77
{
8-
public TPCInheritanceBulkUpdatesSqlServerTest(TPCInheritanceBulkUpdatesSqlServerFixture fixture)
8+
public TPCInheritanceBulkUpdatesSqlServerTest(
9+
TPCInheritanceBulkUpdatesSqlServerFixture fixture,
10+
ITestOutputHelper testOutputHelper)
911
: base(fixture)
1012
{
1113
ClearLog();
14+
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
1215
}
1316

1417
[ConditionalFact]
@@ -176,6 +179,30 @@ FROM [Kiwi] AS [k]
176179
""");
177180
}
178181

182+
public override async Task Update_with_interface_in_property_expression(bool async)
183+
{
184+
await base.Update_with_interface_in_property_expression(async);
185+
186+
AssertExecuteUpdateSql(
187+
"""
188+
UPDATE [c]
189+
SET [c].[SugarGrams] = 0
190+
FROM [Coke] AS [c]
191+
""");
192+
}
193+
194+
public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
195+
{
196+
await base.Update_with_interface_in_EF_Property_in_property_expression(async);
197+
198+
AssertExecuteUpdateSql(
199+
"""
200+
UPDATE [c]
201+
SET [c].[SugarGrams] = 0
202+
FROM [Coke] AS [c]
203+
""");
204+
}
205+
179206
public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool async)
180207
{
181208
await base.Update_where_keyless_entity_mapped_to_sql_query(async);

‎test/EFCore.SqlServer.FunctionalTests/BulkUpdates/TPTInheritanceBulkUpdatesSqlServerTest.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,20 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool
144144
AssertExecuteUpdateSql();
145145
}
146146

147+
public override async Task Update_with_interface_in_property_expression(bool async)
148+
{
149+
await base.Update_with_interface_in_property_expression(async);
150+
151+
AssertExecuteUpdateSql();
152+
}
153+
154+
public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
155+
{
156+
await base.Update_with_interface_in_EF_Property_in_property_expression(async);
157+
158+
AssertExecuteUpdateSql();
159+
}
160+
147161
protected override void ClearLog()
148162
=> Fixture.TestSqlLoggerFactory.Clear();
149163

0 commit comments

Comments
 (0)