Search code examples
c#entity-framework-coreaspnetboilerplate

Child entity not saved when added to aggregate root


When I save the change everything looks good. CaseWorkNote entity is properly created and added to workNotes collection (property of Case entity). When CurrentUnitOfWork calls DbContext->SaveChanges() I see that my entity is there with status Added.

In the end nothing is saved to DB.

What I miss in my code or what I'm doing wrong?

Below is my code and screenshot with tracked entity.

Model:

    public class Case : FullAuditedAggregateRoot<Guid>
    {
        [Required]
        public CaseType Type { get; set; }
        [Required]
        public string Subject { get; set; }
        public string Descripion { get; set; }
        //Aggregated entity
        private HashSet<CaseWorkNote> _workNotes;
        public IEnumerable<CaseWorkNote> WorkNotes => _workNotes?.ToList();
        //
        public CaseWorkNote AddNote(string text)
        {
            if (_workNotes is null)
            {
                _workNotes = new HashSet<CaseWorkNote>();
            }
            CaseWorkNote workNote = CaseWorkNote.Create(this, text);
            _workNotes.Add(workNote);
            return workNote;
        }
    }
    public class CaseWorkNote : FullAuditedEntity
    {
        [ForeignKey("CaseId")]
        [Required]
        public Case Case { get; private set; }    
        [Required]
        public string Text { get; set; }            
        private CaseWorkNote() : base() { }
        public static CaseWorkNote Create(Case kase, string text)
        {
            return new CaseWorkNote()
            {
                Case = kase,
                Text = text
            };
        }
    }

DBcontext:

    public class testDbContext : AbpZeroDbContext<Tenant, Role, User, testDbContext>
    {
        public DbSet<Case> Cases { get; set; }
        public DbSet<CaseWorkNote> CaseWorkNotes { get; set; }
        public testDbContext(DbContextOptions<testDbContext> options)
            : base(options) { }
        public override int SaveChanges()
        {
            //Here I see CaseWorkNote entity with state = "Added"
            var entries = this.ChangeTracker.Entries();
            foreach (var item in entries)
            {
                Debug.WriteLine("State: {0}, Type: {1}", item.State.ToString(), item.Entity.GetType().FullName);
            }
            return base.SaveChanges();
        }
    }

Application Service Class:

    public class CaseAppService : AsyncCrudAppService<Case, CaseDto, Guid, PagedCaseResultRequestDto, CreateCaseDto, UpdateCaseDto>, ICaseAppService
    {
        //Removed for brevity
        ...
        //
        public async Task AddWorkNote(CreateUpdateCaseWorkNoteDto input)
        {
            var kase = await this.GetEntityByIdAsync(input.CaseId);
            kase.AddNote(input.Text);

            CurrentUnitOfWork.SaveChanges();
        }

        protected override async Task<Case> GetEntityByIdAsync(Guid id)
        {
            var kase = await Repository
                .GetAllIncluding(c => c.WorkNotes)
                .FirstOrDefaultAsync(c => c.Id == id);

            if (kase == null)
            {
                throw new EntityNotFoundException(typeof(Case), id);
            }
            return kase;
        }

        public async Task<ListResultDto<CaseWorkNoteDto>> GetWorkNotes(EntityDto<Guid> entity)
        {
            var kase = await this.GetEntityByIdAsync(entity.Id);
            return new ListResultDto<CaseWorkNoteDto>(MapToEntityDto(kase).WorkNotes.ToList());
        }
    }

thanks

debugger - you can see the entity is there.


Solution

  • The problem is caused by the default EF Core property access mode and ToList() call here

    public IEnumerable<CaseWorkNote> WorkNotes => _workNotes?.ToList();
    

    Not sure what type of methodology are you following, but you are violating the simple good design rule that property (and especially collection type) should not allocate on each get. Not only because it is inefficient, but also allows the "smart" client like EF Core to detect the actual type as List and try using it to add items when loading related data.

    In reality with this type of implementation they are adding to a list which is discarded, in other words - nowhere. So EF Core loading related data / navigation property fixup doesn't work, which also may affect the change tracker and lead to weird behaviors.

    To fix the EF Core issue, you should configure EF Core to use directly the backing field. The easiest way is to set it globally inside the OnModelCreating override:

    modelBuilder.UsePropertyAccessMode(PropertyAccessMode.Field);
    

    It also can be set per entity or per entity property, but I would suggest the above, moreover one of the expected changes in EF Core 3.0 is that Backing fields will be used by default.

    Anyway, now the problem in question will be solved.

    Still, it will be better to follow the good practices. The _workNotes member should be initialized with initializer or in class constructor, and property getter should return it directly. If the idea was to prevent the caller to get access to the private member by casting the result, then there are other ways to prevent that which does not clone the collection content. For instance:

    //Aggregated entity
    private readonly HashSet<CaseWorkNote> _workNotes = new HashSet<CaseWorkNote>();
    public IEnumerable<CaseWorkNote> WorkNotes => _workNotes.Select(e => e);
    //
    

    Regardless of whether you keep your current implementation of the navigation property or not, you must let EF Core use the backing field directly.