Entity property is set prior to SaveChangesAsync then sometimes is saved to the database as null, and changed to null in the entity

I have an intermittent problem. I have a C# API project that reads an entity from the database, updates its properties, then calls DbContext.SaveChangesAsync. The entity has a nullable int DepartmentId property. Sometimes the DepartmentId column in the database table is updated correctly. However, sometimes the DepartmentId column is set to NULL (after previously being populated), while the other columns appear to be updated correctly.

I'm testing this using Swagger, passing in the same values each time, but the results are not consistent. All the other columns seem to be saved correctly but sometimes the DepartmentId is saved correctly and sometimes it's saved as NULL.

Here's the entity that's being saved:

public class LeaveRequest
{
    public int LeaveRequestId { get; set; }
    public int CompanyCode { get; set; }
    public int EmployeeId { get; set; }
    public string LeaveCode { get; set; }
    public DateTime FromDate { get; set; }
    public DateTime ToDate { get; set; }
    public int LeaveStatusId { get; set; }
    public int? DepartmentId { get; set; }

    public virtual Department Department { get; set; }
}

Here is the Department entity, and its parent Company:

public class Department
{
    public Department()
    {
        LeaveRequest = new HashSet<LeaveRequest>();
    }

    public int CompanyCode { get; set; }
    public int DepartmentId { get; set; }
    public string Description { get; set; }
    public string Parent { get; set; }
    public int? ParentDepartment { get; set; }

    public virtual Company CompanyCodeNavigation { get; set; }

    public virtual ICollection<LeaveRequest> LeaveRequest { get; set; }
}

public class Company
{
    public Company()
    {
        Department = new HashSet<Department>();
    }

    public int CompanyCode { get; set; }

    public virtual ICollection<Department> Department { get; set; }
}

Here's the relevant code from the OnModelCreating(ModelBuilder modelBuilder) method:

modelBuilder.Entity<LeaveRequest>(entity =>
{
    entity.ToTable("Leave_Request");

    entity.HasIndex(e => e.LeaveStatusId);

    entity.HasIndex(e => new { e.EmployeeId, e.FromDate });

    entity.HasIndex(e => new { e.FromDate, e.ToDate });

    entity.Property(e => e.FromDate).HasColumnType("datetime");

    entity.Property(e => e.LeaveCode)
        .HasMaxLength(3)
        .IsUnicode(false);

    entity.Property(e => e.ToDate).HasColumnType("datetime");

    entity.HasOne(d => d.Department)
        .WithMany(p => p.LeaveRequest)
        .HasForeignKey(d => new { d.CompanyCode, d.DepartmentId })
        .HasConstraintName("FK_Leave_Request_Department");
});

modelBuilder.Entity<Department>(entity =>
{
    entity.HasKey(e => new { e.CompanyCode, e.DepartmentId })
        .HasName("PK__Department__4CA06362");

    entity.Property(e => e.CompanyCode).HasColumnName("Company_Code");

    entity.Property(e => e.DepartmentId)
        .HasColumnName("Department_Id")
        .ValueGeneratedOnAdd();

    entity.Property(e => e.Description)
        .HasMaxLength(68)
        .IsUnicode(false);

    entity.Property(e => e.Parent)
        .HasMaxLength(68)
        .IsUnicode(false);

    entity.Property(e => e.ParentDepartment).HasColumnName("Parent_Department");

    entity.HasOne(d => d.CompanyCodeNavigation)
        .WithMany(p => p.Department)
        .HasForeignKey(d => d.CompanyCode)
        .OnDelete(DeleteBehavior.ClientSetNull)
        .HasConstraintName("FK__Departmen__Compa__797309D9");
});

modelBuilder.Entity<Company>(entity =>
{
    entity.HasKey(e => e.CompanyCode)
        .HasName("PK__Company__44FF419A");

    entity.Property(e => e.CompanyCode).HasColumnName("Company_Code")
});

The foreign key mentioned for the Leave_Request table, FK_Leave_Request_Department, is disabled in the database.

Here is the code that saves the updated LeaveRequest entity:

private async Task<SaveLeaveRequest> UpdateLeaveRequestAsync(Data.Models.LeaveRequest leaveRequestForDb, 
    CancellationToken cancellationToken = default)
{
    var leaveRequestFromDb = await _dbContext.LeaveRequest
            .FirstOrDefaultAsync(lr => lr.LeaveRequestId == leaveRequestForDb.LeaveRequestId, cancellationToken);

    // AutoMapper will update the target object rather than create a new one.
    _leaveMapper.Map(leaveRequestForDb, leaveRequestFromDb);

    try
    {
        await _dbContext.SaveChangesAsync(cancellationToken);

        var savedRequest = _leaveMapper.Map<SaveLeaveRequest>(leaveRequestFromDb);
        return savedRequest;
    }
    catch
    {
        throw new ApplicationException("Failed to update leave request in database.");
    }
}

Putting a breakpoint on the SaveChangesAsync line I can see the entity's DepartmentId property is set correctly immediately prior to calling SaveChangesAsync. However immediately afterwards, if the database column is set to NULL then the entity DepartmentId property has been changed to null as well.

This is using EF Core 3.1.14 with .NET Core 3.1 for the data project and .NET 7.0 for the API project. The target is a SQL Server database with compatibility level 130 (SQL Server 2016), hosted on a SQL Server 2019 instance.

The Leave_Request database table has a trigger on it. The trigger builds dynamic SQL then executes it, to save the columns that have changed, along with their values before and after the change, to an audit table. The audit records show only one update to the table each time the value saved to DepartmentId column is NULL; it's not showing two updates with the correct value being written first then being overwritten by NULL.

Can anyone help with debugging hints, or suggest why the same input might result in different values saved to the DepartmentId column in the database (while all other columns seem to be fine)?

Solution

I would start by debugging output scenarios to see if in either successful or failure cases whether EF is pre-filling the Department or not. When exposing both FKs and Navigation properties the behaviour of setting the FK can vary depending on whether the navigation property was eager loaded or pre-populated vs. completely un-set. If an earlier query in the DbContext lifetime happened to load the Department then EF will populate it when loading the Leave Request.

Make sure you also don't have code that might be setting the navigation property to #null when changing departments. Whether explicit code or mapper config. The navigation property generally takes precedence to FK properties.

For instance if you load a LeaveRequest that has a Dept set without eager loading the Department you will get one of two scenarios:

LeaveRequest 
    DepartmentId = 4 
    Department = Null // DbContext was not already tracking Department ID #4

LeaveRequest 
    DepartmentId = 4 
    Department = Department(Id 4) // DbContext was tracking Department ID #4

If your update code/mapper does:

leaveRequest.DepartmentId = 5;

In the first case it should update without an issue. In the second case it may update, but may not. I have had mixed results with different EF versions.

If your update code/mapper does:

leaveRequest.Department = null;
leaveRequest.DepartmentId = 5;

Then you will definitely have a problem with output like you are experiencing if the Department happened to be pre-populated by the DbContext. Regardless of what gets set first, the navigation property change takes precedence. EF will interpret those two lines as "Remove the department" when a Department was populated. If the Department is not pre-loaded by the DbContext then Null=Null so the change tracker will respect the DepartmentId change.

As a general rule if you have a navigation property you should update via that and protect the FK or leave it a shadow property. Using navigation properties the safest way to update would be:

var leaveRequestFromDb = _context.LeaveRequests
    .Include(x => x.Department)
    .Single(x => x.LeaveRequestId == leaveRequestForDb.LeaveRequestId);

// Mapper can copy values across...

if (leaveRequestForDb.DepartmentId != leaveRequestFromDb.DepartmentId)
{
    var newDepartment = _context.Departments
        .Single(x => x.DepartmentId == leaveRequestForDb.DepartmentId);

    leaveRequestFromDb.Department = newDepartment;
}

_context.SaveChanges();

If you want to avoid the round trip to the database for the department you can hack the update provided you don't serialize the Department or otherwise depend on it being set to a complete department:

if (leaveRequestForDb.DepartmentId != leaveRequestFromDb.DepartmentId)
{
    var newDepartment = _context.Departments.Local
        .FirstOrdefault(x => x.DepartmentId == leaveRequestForDb.DepartmentId);
    bool stubUsed = false
    if (newDepartment == null)
    {
        newDepartment = new Department { DepartmentId = leaveRequestForDb.DepartmentId };
        _context.Departments.Attach(newDepartment);
        stubUsed = true;
    }
    leaveRequestFromDb.Department = newDepartment;
}

_context.SaveChanges();

if (stubUsed)
    _context.Entry(newDepartment).State = EntityState.Detached;

The code is a bit more involved, but basically check the local tracking cache (no round trip) if found, use it, otherwise create a new stub, attach to the DbContext (to treat as an existing row) and use that, then if we use a stub we want to be sure to detach the stub after saving so any further reads that request the Department for that Id don't get served the stub from the tracking cache.

Stubbing entities isn't ideal as it trusts that the row exists rather than asserts it, resulting in an error on SaveChanges rather than the call to fetch the Department. You also want to avoid leaving stubs in the tracking cache or risk getting incomplete data with other queries; but it is an option. If performance is paramount (within reason where EF is still worth using) for API operations I would recommend a bounded context that uses entities with just FK associations and no navigation properties to potentially trip updates up.