Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] Support entities with owned types in ExecuteUpdate setter property lambda #29723

Merged
merged 1 commit into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public class RelationalQueryableMethodTranslatingExpressionVisitor : QueryableMe
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly bool _subquery;

private static readonly bool QuirkEnabled28727
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue28727", out var enabled) && enabled;

/// <summary>
/// Creates a new instance of the <see cref="QueryableMethodTranslatingExpressionVisitor" /> class.
/// </summary>
Expand Down Expand Up @@ -1178,11 +1181,25 @@ static Expression PruneOwnedIncludes(IncludeExpression includeExpression)
foreach (var (propertyExpression, _) in propertyValueLambdaExpressions)
{
var left = RemapLambdaBody(source, propertyExpression);
left = left.UnwrapTypeConversion(out _);
if (!IsValidPropertyAccess(RelationalDependencies.Model, left, out var ese))

EntityShaperExpression? ese;

if (QuirkEnabled28727)
{
AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print()));
return null;
left = left.UnwrapTypeConversion(out _);
if (!IsValidPropertyAccess(RelationalDependencies.Model, left, out ese))
{
AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print()));
return null;
}
}
else
{
if (!TryProcessPropertyAccess(RelationalDependencies.Model, ref left, out ese))
{
AddTranslationErrorDetails(RelationalStrings.InvalidPropertyInSetProperty(propertyExpression.Print()));
return null;
}
}

if (entityShaperExpression is null)
Expand Down Expand Up @@ -1382,6 +1399,66 @@ when methodCallExpression.Method.IsGenericMethod
}
}

// For property setter selectors in ExecuteUpdate, we support only simple member access, EF.Function, etc.
// We also unwrap casts to interface/base class (#29618), as well as IncludeExpressions (which occur when the target entity has
// owned entities, #28727).
static bool TryProcessPropertyAccess(
IModel model,
ref Expression expression,
[NotNullWhen(true)] out EntityShaperExpression? entityShaperExpression)
{
expression = expression.UnwrapTypeConversion(out _);

if (expression is MemberExpression { Expression : not null } memberExpression
&& Unwrap(memberExpression.Expression) is EntityShaperExpression ese)
{
expression = memberExpression.Update(ese);
entityShaperExpression = ese;
return true;
}

if (expression is MethodCallExpression mce)
{
if (mce.TryGetEFPropertyArguments(out var source, out _)
&& Unwrap(source) is EntityShaperExpression ese1)
{
if (source != ese1)
{
var rewrittenArguments = mce.Arguments.ToArray();
rewrittenArguments[0] = ese1;
expression = mce.Update(mce.Object, rewrittenArguments);
}

entityShaperExpression = ese1;
return true;
}

if (mce.TryGetIndexerArguments(model, out var source2, out _)
&& Unwrap(source2) is EntityShaperExpression ese2)
{
expression = mce.Update(ese2, mce.Arguments);
entityShaperExpression = ese2;
return true;
}
}

entityShaperExpression = null;
return false;

static Expression Unwrap(Expression expression)
{
expression = expression.UnwrapTypeConversion(out _);

while (expression is IncludeExpression includeExpression)
{
expression = includeExpression.EntityExpression;
}

return expression;
}
}

// Old quirked implementation only
static bool IsValidPropertyAccess(
IModel model,
Expression expression,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,26 @@ public virtual Task Update_where_keyless_entity_mapped_to_sql_query(bool async)
s => s.SetProperty(e => e.Name, "Eagle"),
rowsAffectedCount: 1));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Update_with_interface_in_property_expression(bool async)
=> AssertUpdate(
async,
ss => ss.Set<Coke>(),
e => e,
s => s.SetProperty(c => ((ISugary)c).SugarGrams, 0),
rowsAffectedCount: 1);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
=> AssertUpdate(
async,
ss => ss.Set<Coke>(),
e => e,
// ReSharper disable once RedundantCast
s => s.SetProperty(c => EF.Property<int>((ISugary)c, nameof(ISugary.SugarGrams)), 0),
rowsAffectedCount: 1);

protected abstract void ClearLog();
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,25 @@ public class OtherReference
}

#nullable enable

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Update_non_owned_property_on_entity_with_owned(bool async)
{
var contextFactory = await InitializeAsync<Context28671>(
onModelCreating: mb =>
{
mb.Entity<Owner>().OwnsOne(o => o.OwnedReference);
});

await AssertUpdate(
async,
contextFactory.CreateContext,
ss => ss.Set<Owner>(),
s => s.SetProperty(o => o.Title, "SomeValue"),
rowsAffectedCount: 0);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Delete_predicate_based_on_optional_navigation(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,14 @@ public override Task Update_where_hierarchy_derived(bool async)
=> AssertTranslationFailed(
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Kiwi"),
() => base.Update_where_hierarchy_derived(async));

public override Task Update_with_interface_in_property_expression(bool async)
=> AssertTranslationFailed(
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"),
() => base.Update_with_interface_in_property_expression(async));

public override Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
=> AssertTranslationFailed(
RelationalStrings.ExecuteOperationOnTPT("ExecuteUpdate", "Coke"),
() => base.Update_with_interface_in_EF_Property_in_property_expression(async));
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;

public class InheritanceBulkUpdatesSqlServerTest : InheritanceBulkUpdatesTestBase<InheritanceBulkUpdatesSqlServerFixture>
{
public InheritanceBulkUpdatesSqlServerTest(InheritanceBulkUpdatesSqlServerFixture fixture)
public InheritanceBulkUpdatesSqlServerTest(
InheritanceBulkUpdatesSqlServerFixture fixture,
ITestOutputHelper testOutputHelper)

: base(fixture)
{
ClearLog();
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

[ConditionalFact]
Expand Down Expand Up @@ -205,6 +209,32 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool
AssertExecuteUpdateSql();
}

public override async Task Update_with_interface_in_property_expression(bool async)
{
await base.Update_with_interface_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE [d]
SET [d].[SugarGrams] = 0
FROM [Drinks] AS [d]
WHERE [d].[Discriminator] = N'Coke'
""");
}

public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
{
await base.Update_with_interface_in_EF_Property_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE [d]
SET [d].[SugarGrams] = 0
FROM [Drinks] AS [d]
WHERE [d].[Discriminator] = N'Coke'
""");
}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
AssertSql();
}

public override async Task Update_non_owned_property_on_entity_with_owned(bool async)
{
await base.Update_non_owned_property_on_entity_with_owned(async);

AssertSql(
"""
UPDATE [o]
SET [o].[Title] = N'SomeValue'
FROM [Owner] AS [o]
""");
}

public override async Task Delete_predicate_based_on_optional_navigation(bool async)
{
await base.Delete_predicate_based_on_optional_navigation(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;

public class TPCInheritanceBulkUpdatesSqlServerTest : TPCInheritanceBulkUpdatesTestBase<TPCInheritanceBulkUpdatesSqlServerFixture>
{
public TPCInheritanceBulkUpdatesSqlServerTest(TPCInheritanceBulkUpdatesSqlServerFixture fixture)
public TPCInheritanceBulkUpdatesSqlServerTest(
TPCInheritanceBulkUpdatesSqlServerFixture fixture,
ITestOutputHelper testOutputHelper)
: base(fixture)
{
ClearLog();
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

[ConditionalFact]
Expand Down Expand Up @@ -176,6 +179,30 @@ SELECT COUNT(*)
""");
}

public override async Task Update_with_interface_in_property_expression(bool async)
{
await base.Update_with_interface_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE [c]
SET [c].[SugarGrams] = 0
FROM [Coke] AS [c]
""");
}

public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
{
await base.Update_with_interface_in_EF_Property_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE [c]
SET [c].[SugarGrams] = 0
FROM [Coke] AS [c]
""");
}

public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool async)
{
await base.Update_where_keyless_entity_mapped_to_sql_query(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,20 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool
AssertExecuteUpdateSql();
}

public override async Task Update_with_interface_in_property_expression(bool async)
{
await base.Update_with_interface_in_property_expression(async);

AssertExecuteUpdateSql();
}

public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
{
await base.Update_with_interface_in_EF_Property_in_property_expression(async);

AssertExecuteUpdateSql();
}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;

public class InheritanceBulkUpdatesSqliteTest : InheritanceBulkUpdatesTestBase<InheritanceBulkUpdatesSqliteFixture>
{
public InheritanceBulkUpdatesSqliteTest(InheritanceBulkUpdatesSqliteFixture fixture)
public InheritanceBulkUpdatesSqliteTest(
InheritanceBulkUpdatesSqliteFixture fixture,
ITestOutputHelper testOutputHelper)
: base(fixture)
{
ClearLog();
// Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

[ConditionalFact]
Expand Down Expand Up @@ -196,6 +199,30 @@ public override async Task Update_where_keyless_entity_mapped_to_sql_query(bool
AssertExecuteUpdateSql();
}

public override async Task Update_with_interface_in_property_expression(bool async)
{
await base.Update_with_interface_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE "Drinks" AS "d"
SET "SugarGrams" = 0
WHERE "d"."Discriminator" = 'Coke'
""");
}

public override async Task Update_with_interface_in_EF_Property_in_property_expression(bool async)
{
await base.Update_with_interface_in_EF_Property_in_property_expression(async);

AssertExecuteUpdateSql(
"""
UPDATE "Drinks" AS "d"
SET "SugarGrams" = 0
WHERE "d"."Discriminator" = 'Coke'
""");
}

protected override void ClearLog()
=> Fixture.TestSqlLoggerFactory.Clear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ public override async Task Delete_aggregate_root_when_table_sharing_with_non_own
AssertSql();
}

public override async Task Update_non_owned_property_on_entity_with_owned(bool async)
{
await base.Update_non_owned_property_on_entity_with_owned(async);

AssertSql(
"""
UPDATE "Owner" AS "o"
SET "Title" = 'SomeValue'
""");
}

public override async Task Delete_predicate_based_on_optional_navigation(bool async)
{
await base.Delete_predicate_based_on_optional_navigation(async);
Expand Down
Loading