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

Required dependent of optional dependent is not materialized/updated properly #34707

Open
bachratyg opened this issue Sep 18, 2024 · 2 comments

Comments

@bachratyg
Copy link

An aggregate root has a 1:0..1 optional dependent with some required properties, using table splitting. This dependent entity has a 1:1 required dependent where all properties are nullable (that's why this nav is required). After the optional dependent has saved values, changes on the required dependent entity are not saved.

Steps to reproduce

Build & run.
Observe the last issued SQL command and "State before save:" line on the console output.
If the required dependent is replaced: "State before save: Added" and an update command.
If the required dependent is reused: "State before save: Detached" and no update command.
(note that both City and Street columns are updated in the former case despite the Street not changing)

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

// Prep the db & seed
using( var db = new AppDb() )
{
	db.Database.EnsureDeleted();
	db.Database.EnsureCreated();
	db.Parents.Add(new() { Id = "x", Child = new() { Name = "asd" } });
	db.SaveChanges();
}

// reproduce the issue
using( var db = new AppDb() )
{
	var parent = db.Parents.Single();
	// This will update
	//parent.Child!.Address = new() { City = "asd" };
	// This will not
	parent.Child!.Address.City = "asd";
	db.ChangeTracker.DetectChanges();
	Console.WriteLine($"State before save: {db.Entry(parent.Child.Address).State}");
	db.SaveChanges();
}

// Model
class AppDb : DbContext
{
	protected override void OnConfiguring(DbContextOptionsBuilder builder)
	{
		builder
			.UseSqlServer("Data Source=localhost;Initial Catalog=Test;Integrated Security=true")
			.LogTo(Console.WriteLine, LogLevel.Information)
			.EnableSensitiveDataLogging()
			;
	}
	protected override void OnModelCreating(ModelBuilder builder)
	{
		builder.Entity<Parent>().OwnsOne(e => e.Child).OwnsOne(e => e.Address);
	}
	public DbSet<Parent> Parents => this.Set<Parent>();
}

class Parent
{
	public required string Id { get; set; }
	public Child? Child { get; set; }
}
class Child
{
	public required string Name { get; set; }
	public Address Address { get; set; } = new();
}
class Address
{
	// All optional, hence the navigation to this entity is required
	public string? City { get; set; }
	public string? Street { get; set; }
}

First I thought this has to do with owned entities and thus part of #24581 and #1985 however it repros with plain old navigations. Replace OnModelCreating with the following (basically maps to the same table structure), steps and outcome is the same

	protected override void OnModelCreating(ModelBuilder builder)
	{
		builder.Entity<Parent>().ToTable("Parents");
		builder.Entity<Parent>().HasOne(e => e.Child).WithOne().HasForeignKey<Child>();
		builder.Entity<Parent>().Navigation(e => e.Child).AutoInclude();
		builder.Entity<Child>().Property<string>("Id");
		builder.Entity<Child>().HasKey("Id");
		builder.Entity<Child>().ToTable("Parents");
		builder.Entity<Child>().HasOne(e => e.Address).WithOne().HasForeignKey<Address>();
		builder.Entity<Child>().Navigation(e => e.Address).IsRequired().AutoInclude();
		builder.Entity<Address>().Property<string>("Id");
		builder.Entity<Address>().HasKey("Id");
		builder.Entity<Address>().ToTable("Parents");
	}

Some additional observations:

  • the navigation is required but warns OptionalDependentWithAllNullPropertiesWarning[20704] and neither the Address nor the City and Street setters are hit on materialization
  • the Address is Detached on materialization, it should be Unchanged
  • even though Detached the change tracker still maintains some knowledge of the Address or else the replace/reuse behavior could not be possibly different. It's just an unknown object.

Include provider and version information

EF Core version: 8.0.8
Database provider: Microsoft.EntityFrameworkCore.SqlServer (probably others)
Target framework: .NET 8.0

@ajcvickers
Copy link
Contributor

@bachratyg Sorry for taking a long time to respond. I think this is a bug, but if you explicitly configure both owning navigations as required, then Address is populated when the query is executed. For example:

builder.Entity<Parent>(b =>
{
    b.OwnsOne(e => e.Child, b =>
    {
        b.OwnsOne(e => e.Address);
        b.Navigation(e => e.Address).IsRequired();
    });
    b.Navigation(e => e.Child).IsRequired();
});

Note for team: I think this is a bug, because is Child is optional, then even if Address is marked as required (which it isn't in the original code, but I tried this) and the Child exists in the database, it's Address will not be loaded.

@bachratyg
Copy link
Author

bachratyg commented Dec 2, 2024

Marking the Child nav as required is not viable. The following would no longer work:

...
db.Database.EnsureCreated();
db.Parents.Add(new() { Id = "x", Child = new() { Name = "asd" } });
db.Parents.Add(new() { Id = "y" });
db.SaveChanges();
...

Alternatively if I make the child nav non-nullable and all properties nullable (just like Address) that would wreak havoc on the business logic side of things.

@ajcvickers ajcvickers added this to the Backlog milestone Dec 3, 2024
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

3 participants