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

Error updating null owned fields #24581

Open
alvorteg opened this issue Apr 5, 2021 · 6 comments
Open

Error updating null owned fields #24581

alvorteg opened this issue Apr 5, 2021 · 6 comments

Comments

@alvorteg
Copy link

alvorteg commented Apr 5, 2021

I think there is a bug when updating entities with UPDATE DbContext method with nulled complex types.

Steps to reproduce

If you have these classes:

public class User {

  public string Identifier { get; set; }
  public Class2 Child { get; set; }
}

public class Class2 {

  public string Name { get; set; }
  public Class3 Surnames { get; set; }

}

public class Class3 {

  public string Surname1 { get; set; }
  public string Surname2 { get; set; }
}

Modeled with EF Core as follows:

  modelBuilder.Entity<User>().HasKey(t => t.Identifier);
  modelBuilder.Entity<User>().OwnsOne(_ => _.Child).Property(_ => _.Name).IsRequired(false);
  modelBuilder.Entity<User>().OwnsOne(_ => _.Child).OwnsOne(_ => _.Surnames);

Now you insert an object with these values:

{
    "User": {
        "Identifier": "TESTID",
        "Child": {
            "Name": "NAME1",
            "Surnames": {
                "Surname1": "SUR1",
                "Surname2": "SUR2"
            }
        }
    }
}

Row in Database:

Identifier = TESTID,
Name = Name1,
Surname1 = SUR1,
Surname2= SUR2

Actual behaviour

I want to update my object to these values:

    "User": {
        "Identifier": "TESTID",
        "Child": {
            "Name": "NAMEMODIFIED",
            "Surnames": null
        }
    }

I want to update whole object in all cases, so I use:

 public void Update(User entity) {

    this.context.Users.Update(entity);
    this.context.SaveChanges();

}

I don't want to find the entity in the database as it is not necessary in my case. So, Update method marks recursively as modified all properties, and that is the behaviour that I want.

This works perfectly with all properties (nested and owned included) except in cases where an Owned entity is NULL, as it ignores it and leaves the value it had in the database, instead of updating it to NULL.

Row in DB after update

Identifier = TESTID,
Name = NAMEMODIFIED,
Surname1 = SUR1,
Surname2= SUR2

Expected Row in DB after update

Identifier = TESTID,
Name = NAMEMODIFIED,
Surname1 = null,
Surname2= null

Include provider and version information

EF Core version: 5.0.4
Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer)
Target framework: .NET Core 3.1
Operating system: Windows 10
IDE: Visual Studio 2019 16.4.0

@alvorteg alvorteg changed the title Error updating null fields Error updating null owned fields Apr 5, 2021
@smitpatel
Copy link
Member

smitpatel commented Apr 5, 2021

Related #23229

Since there is no way to determine if Class2 instance exist by looking at its data in database this model will throw error in 6.0

@ajcvickers
Copy link
Member

@smitpatel @AndriySvyryd What is the correct way to define this as a required dependent? If I do so, will this then update the nested owned type rows even if an instance of the type does not exist?

@smitpatel
Copy link
Member

 modelBuilder.Entity<User>().Navigation(_ => _.Child).IsRequired();

Also looking at the code and method again, counter question(s)
In the scenario above, the reference navigation is set to null when passing into Update method where parent object is not fetched from database.

  • If a reference navigation is set to null do we assume it is not loaded or do we remove dependent?
  • Does answer to above question change if the dependent is required dependent?
  • Since owned navigations are always loaded (implying aggregate is always complete),
    • Should we set owned navigations as always loaded?
    • If not then how do we make determination of an optional owned reference navigation coming from user is not loaded vs deleted?

@ajcvickers
Copy link
Member

@smitpatel Those were some of the questions I had around this. Below is some repro code. Even with the navigation marked as required, the update still does not happen when Surnames is null:

context.Update(new User
{
    Identifier = "TESTID",
    Child = new Class2 {Name = "NAMEMODIFIED", Surnames = null}
});
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[@p1='TESTID' (Nullable = false) (Size = 450), @p0='NAMEMODIFIED' (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      UPDATE [User] SET [Child_Name] = @p0
      WHERE [Identifier] = @p1;

It works if the dependent is required and Surnames is set to an empty object:

context.Update(new User
{
    Identifier = "TESTID",
    Child = new Class2 {Name = "NAMEMODIFIED", Surnames = new Class3()}
});
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@p3='TESTID' (Nullable = false) (Size = 450), @p0='NAMEMODIFIED' (Size = 4000), @p1=NULL (Size = 4000), @p2=NULL (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      UPDATE [User] SET [Child_Name] = @p0, [Child_Surnames_Surname1] = @p1, [Child_Surnames_Surname2] = @p2
      WHERE [Identifier] = @p3;
      SELECT @@ROWCOUNT;

Repro code:

public class User {

    public string Identifier { get; set; }
    public Class2 Child { get; set; }
}

public class Class2 {

    public string Name { get; set; }
    public Class3 Surnames { get; set; }

}

public class Class3 {

    public string Surname1 { get; set; }
    public string Surname2 { get; set; }
}

public class SomeDbContext : DbContext
{
    private static ILoggerFactory ContextLoggerFactory
        => LoggerFactory.Create(b => b.AddConsole().SetMinimumLevel(LogLevel.Information));

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(Your.ConnectionString)
            .UseLoggerFactory(ContextLoggerFactory)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<User>().OwnsOne(_ => _.Child, b =>
        {
            b.Property(_ => _.Name).IsRequired(false);
            b.OwnsOne(_ => _.Surnames);
            b.Navigation(e => e.Surnames).IsRequired();
        });
        modelBuilder.Entity<User>(b =>
        {
            b.HasKey(t => t.Identifier);
            b.Navigation(e => e.Child).IsRequired();
        });
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Add(new User
            {
                Identifier = "TESTID",
                Child = new Class2 {Name = "NAME1", Surnames = new Class3 {Surname1 = "SUR1", Surname2 = "SUR2 "}}
            });
        
            context.SaveChanges();
        }

        using (var context = new SomeDbContext())
        {
            context.Update(new User
            {
                Identifier = "TESTID",
                Child = new Class2 {Name = "NAMEMODIFIED", Surnames = null}
            });

            context.SaveChanges();
        }
    }
}

@smitpatel
Copy link
Member

Seems like this is more than the query issue I linked above. ChangeTracker and/or update pipeline are not recognizing things properly.

@ajcvickers
Copy link
Member

Note from triage: consider as part of #1985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants