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

Document behaviors around setting owned navigations to null #4669

Open
LunicLynx opened this issue Feb 12, 2024 · 4 comments
Open

Document behaviors around setting owned navigations to null #4669

LunicLynx opened this issue Feb 12, 2024 · 4 comments

Comments

@LunicLynx
Copy link

Structured data in nested Json can't be set to null

Currently it is not possible to update nested structured data in a json column to null, just by attaching an entity to the context.

I can understand this behavior for non json columns as it would be more involved to update the database. But for json columns the serialization should not be a merge of navigation and non navigation properties.

Given the example below. After attaching owner to the database and saving it, the MoreData property should be null.

To be clear the data we want to attach is coming via an asp controller.

To make this work right now, we would need to:

  1. Check the incoming data for null in the field.
  2. Save the state to a bool. (for example isNull)
  3. If it isNull set it to a temporary value.
  4. Attach the entity to the context.
  5. Based on isNull remove the temporary value again.

Maybe there is another solution? So far we were not able to find one which did not involve dealing with the previous data or tricking the change tracker into recognizing the non null state first.

Code

using System.Diagnostics;
using Microsoft.EntityFrameworkCore;

var myDbContext = new MyDbContext();
myDbContext.Database.EnsureDeleted();
myDbContext.Database.EnsureCreated();

// Create data and detach
var owner = myDbContext.JsonOwners.SingleOrDefault(o => o.Data.Name == "Data");
if (owner == null)
{
    owner = new JsonOwner()
    {
        Data = new Data()
        {
            Name = "Data",
            MoreData = new NestedData()
            {
                NestedName = "NestedData"
            }
        }
    };
    myDbContext.JsonOwners.Add(owner);
}
myDbContext.SaveChanges();
myDbContext.Entry(owner).State = EntityState.Detached;

// Set to nested data to null and re-attach
// In reality this data comes via an http call and is attached to the context.
owner.Data.MoreData = null;
myDbContext.Update(owner);
myDbContext.SaveChanges();

// Getting by name and asserting
var d = myDbContext.JsonOwners.Single(o => o.Data.Name == "Data");
Debug.Assert(d.Data.MoreData == null, "d.Data.MoreData == null");

return;

public class JsonOwner
{
    public Guid Id { get; set; }

    public Data Data { get; set; }
}

public class Data
{
    public string Name { get; set; }
    public NestedData? MoreData { get; set; }
}

public class NestedData
{
    public string NestedName { get; set; }
}

public class MyDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EfLab;Trusted_Connection=True;");
    }

    public DbSet<JsonOwner> JsonOwners { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<JsonOwner>(e =>
        {
            e.OwnsOne(p => p.Data, x =>
            {
                x.ToJson();
                x.OwnsOne(r => r.MoreData);
            });
        });
    }
}

Version information

EF Core version: 8.0.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer (8.0.1)
Target framework: .NET 8
Operating system: Windows 11

@maumar maumar self-assigned this Feb 12, 2024
@ajcvickers
Copy link
Member

Note for team: still repros on latest daily; not a regression.

@maumar
Copy link
Contributor

maumar commented Feb 17, 2024

Note: also repros for non-json owned entities

@ajcvickers ajcvickers assigned ajcvickers and unassigned maumar Feb 28, 2024
@ajcvickers
Copy link
Member

ajcvickers commented Feb 29, 2024

Note for triage: I investigated this more. If this was not an owned type, this would be the correct behavior. This is because, in effect, the MoreData navigation isn't loaded. What this should do for owned types depends on whether or not we allow optional owned types to be not loaded. From an aggregate perspective, not allowed them to be unloaded is fine. That is, we could assume that if the navigation is null, then the owned type does not exist in the database, as opposed to it just isn't loaded. From the perspective of how people actually use owned types, it may not be fine.

Given that we are unsure where owned types are going, I propose we do nothing here immediately.

It's also worth noting that this works:

myDbContext.Update(owner);
owner.Data.MoreData = null;
myDbContext.SaveChanges();

This is because in this case EF is tracking the entity when the nav is set to null, so EF correctly detects this as a delete.

@LunicLynx
Copy link
Author

LunicLynx commented Feb 29, 2024

The example only works if MoreData was something (not null) at the time it was attached.

@ajcvickers ajcvickers changed the title Structured data in nested Json can't be set to null Document behaviors around setting owned navigations to null Mar 7, 2024
@ajcvickers ajcvickers transferred this issue from dotnet/efcore Mar 7, 2024
@ajcvickers ajcvickers added this to the Backlog milestone Mar 7, 2024
@ajcvickers ajcvickers removed their assignment Mar 7, 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