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

Allow specify property value without lambda in ExecuteUpdate #29139

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

roji
Copy link
Member

@roji roji commented Sep 19, 2022

Fixes #28968

Description

This is a non-breaking tweak to a new API introduced in EF7. It allows a simpler form to be used for ExecuteUpdate. Specifcally,

.SetProperty(p => p.Name, "Shay")

can be used instead of

.SetProperty(p => p.Name, _ => "Shay")

There may be many calls to SetProperty and setting to a value that does not depend on any other values in the entity is a very common case.

Customer impact

Shorter, more readable code for a frequently used common case.

How found

Customer suggestion.

Regression

No. New API, and this is non-breaking for existing use of that API in RC1.

Testing

New tests added.

Risk

Low; non-breaking tweak to new API.


@smitpatel as discussed offline. Added various closure tests, funcletizer seems to be humming along with 100% efficiency.

AFAICT, switching from Expression to regular non-Expression doesn't seem to create particular difficulties for dynamic expression construction. Here's a sample:

await ctx.Blogs.ExecuteUpdateAsync(sp => sp
    .SetProperty(b => b.Name, "Hello"));

// Same thing dynamically:
var setPropertyMethodInfo = typeof(SetPropertyCalls<>).MakeGenericType(typeof(Blog))
    .GetMethods()
    .Single(m => m.Name == nameof(SetPropertyCalls<Blog>.SetProperty)
                 && m.GetParameters()[0].ParameterType.IsGenericType
                 && m.GetParameters()[0].ParameterType.GetGenericTypeDefinition() == typeof(Func<,>)
                 && !m.GetParameters()[1].ParameterType.IsGenericType)
    .MakeGenericMethod(typeof(string));

var setPropertyCallsParam = Expression.Parameter(typeof(SetPropertyCalls<Blog>), "sp");
var blogParam1 = Expression.Parameter(typeof(Blog), "b");

Expression<Func<SetPropertyCalls<Blog>, SetPropertyCalls<Blog>>> setProperties =
    Expression.Lambda<Func<SetPropertyCalls<Blog>, SetPropertyCalls<Blog>>>(
        Expression.Call(
            setPropertyCallsParam,
            setPropertyMethodInfo,
            Expression.Lambda<Func<Blog, string>>(Expression.Property(blogParam1, nameof(Blog.Name)), blogParam1),
            Expression.Constant("Hello")),
        setPropertyCallsParam);

await ctx.Blogs.ExecuteUpdateAsync(setProperties);

What do you think?

Closes #28968

@roji roji requested a review from smitpatel September 19, 2022 10:39
rowsAffectedCount: 8,
(b, a) => Assert.All(a, c => Assert.Equal("Updated", c.ContactName)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Update_Where_parameter_set_constant(bool async)
{
using var context = Fixture.CreateContext();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slipped in, removing.

@ajcvickers
Copy link
Member

@roji Approved by Tactics. Please merge ASAP.

And make SetProperty accept a non-expression lambda directly

Closes dotnet#28968
@roji roji merged commit 4262584 into dotnet:release/7.0 Sep 20, 2022
@roji roji deleted the ExecuteUpdateThing branch September 20, 2022 06:50
ajcvickers pushed a commit that referenced this pull request Sep 20, 2022
And make SetProperty accept a non-expression lambda directly

Closes #28968
@ajcvickers ajcvickers removed this from the 7.0.0-rc2 milestone Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants