1

I am currently having an issue with entity framework, when trying to updated my CaapListItem entity using the Exported() method only the first items status is being updated any after that get a NULL inserted. All other fields for that element are updated correctly. As shown below:

Before Handler is run: Before running handler

After Handler is run: After running handler showing null statuses

I have stepped through this code and can see that each caaplistitem gets assigned the correct status but then when entity framework runs the SaveChanges() it strips them out. I can see via sql profiler that the two elements are set to NULL. Is there a known bug in which Enumeration classes don't play well with EF?

CaapListItem Class

public class CaapListItem : Entity, IAggregateRoot, IDeletable
    {
        public Guid Guid { get; protected set; }
        public int AgreementId { get; private set; }
        public int UserId { get; private set; }
        public CaapStatus? Status { get; private set; }
        public DateTime DateAddedToReadyForExport { get; private set; }
        public DateTime? DateExported { get; private set; }
        public DateTime? DateImported { get; private set; }
        public string? ExportFilePath { get; private set; }
        public string? ImportFilename { get; private set; }

        private bool _delete;
        public bool Delete => _delete;

         public CaapListItem(int agreementId, int userId)
        {
            Guid = Guid.NewGuid();
            AgreementId = agreementId;
            UserId = userId;
        }

        private CaapListItem() {

        }

        public void ReadyForExport()
        {
            this.Status = CaapStatus.ReadyForExport;
            DateAddedToReadyForExport = DateTime.Now;
        }

        /// <summary>
        /// Sets the item to exported
        /// </summary>
        public void Exported(string exportFilePath)
        {
            this.Status = CaapStatus.Exported;
            this.DateExported = DateTime.Now;
            this.ExportFilePath = exportFilePath;
            this.AddDomainEvent(new CaapListItemExportedDomainEvent(AgreementId));
        }
}

CaapStatus Enumeration Class

 public class CaapStatus : Enumeration
    {
        public static readonly CaapStatus ReadyForExport = new(1, nameof(ReadyForExport));
        public static readonly CaapStatus Exported = new(2, nameof(Exported));
        public static readonly CaapStatus Imported = new(3, nameof(Imported));

        private CaapStatus()
        {
        }

        private CaapStatus(int id, string name)
            : base(id, name)
        {
        }
    }

Handler that calls export on each CaapListItem and saves all to database.

 public async Task<AggregateRootGuid> Handle(CaapExportCommand request,
            CancellationToken cancellationToken)
        {
            var agreements = await _context.Agreements.Include(a => a.Terminals)
                .Where(t => request.AgreementIds.Contains(t.Guid)).ToArrayAsync()
                ?? throw new NullReferenceException($"No agreements with the Ids " +
               $"{request.AgreementIds} could be found. Please check the agreements exist.");

            var calledOffAgreements = Array.ConvertAll(agreements, item => (CalledOffAgreement)item);
            //Export to csv
            var caapResults = _caapExportService.GetCaapFieldsForExport(calledOffAgreements);
            var exportFilePath = _caapExportService.ExportToCsv(caapResults);

            foreach (var agreement in calledOffAgreements)
            {
                var caapListItem = await _context.CaapList
               .FirstOrDefaultAsync(t => t.AgreementId! == agreement.Id) ??
                    throw new NullReferenceException("$No Caap List Item could be found with the" +
                    $"Id of {agreement.Id}. Please check the agreement exists and is in the caap export list.");

                caapListItem.Exported(exportFilePath);
            }

            await _context.SaveChangesAsync(cancellationToken);

            return new AggregateRootGuid(request.AgreementIds[0]);
        }

EF type configuration

internal class EFCaapListItemConfig : IEntityTypeConfiguration<CaapListItem>
    {
        public void Configure(EntityTypeBuilder<CaapListItem> builder)
        {
            builder.ToTable("CaapList");
            builder.OwnsOne(a => a.Status, status =>
            {
                status.Property(p => p.Id).HasColumnName("CaapStatusId");
                status.Ignore(p => p.Name);
            });
        }
    }
Adam Wilson
  • 281
  • 2
  • 15
  • `CaapStatus` isn't an Enum. It's your own entity class, using a rather unfortunate base type. That class behaves the same as any other related class - you need to set it if you want it to have any value. Enums are just labeled integers. – Panagiotis Kanavos Oct 12 '22 at 08:03
  • You didn't post any EF Core code either, only "repository" classes that seem to be thin wrappers over DbContext and DbSet. Such code isn't any kind of best practice - quite often it's an *anti*pattern. That `SaveChanges` on a single-entity repo is definitely a bug. A DbContext is *already* a multi-entity repository and disconnected Unit-of-Work. It already tracks all changes made to all entities during a session/transaction/UoW and commits all of them in a single internal transaction when `SaveChanges` is called. Calling `SaveChanges` on the *entities* breaks this, requiring extra transactions – Panagiotis Kanavos Oct 12 '22 at 08:06
  • Is `Enumeration` this class? https://lostechies.com/jimmybogard/2008/08/12/enumeration-classes/ – Tim Schmelter Oct 12 '22 at 08:14
  • @TimSchmelter yes it is. – Adam Wilson Oct 12 '22 at 08:15
  • @AdamWilson write proper EF Core code first before trying to apply "patterns" and "best practices". This isn't DDD and the data modification code is spread around multiple files. If Agreement and ListItem are related why are items loaded one by one instead of eagerly, as an `Agreement.ListItems` property? What does `FindByAgreement` do, does it load tracked or untracked objects? – Panagiotis Kanavos Oct 12 '22 at 08:19
  • @PanagiotisKanavos the CaapStatus is set when the handler is called it sets as exported using the method caapListItem.Exported(exportFilePath); . On your second comment the DBcontext is wrapped by the repo but the call is made at the end of the handler so it does one transaction that commits all changes. Definitely not a bug but maybe not best practice. – Adam Wilson Oct 12 '22 at 09:02
  • Your last comment again about it not being best practice while i agree and is something we need to look at, this doesn't have correlation to the issue. FindByAgreement loads tracked objects. – Adam Wilson Oct 12 '22 at 09:04
  • Of course it's related to the issue. Where's the EF Core code in all this? It's all hidden behind missing code. `Definitely not a bug` yes it is - because a DbContext is a multi-entity *domain* repository *and* Unit of Work. Your `_caapListItemRepository.SaveChangesAsync` is also saving any changes made to `Agreement`. Start by writing clean, simple EF Code that reproduces the problem. Once you have that, you can fix it and *then* try to apply any other patterns. Not `_caapListItemRepository` but `_context.CaapListItems.Where(l=>aggreementIds.Contains(l.Id))` – Panagiotis Kanavos Oct 12 '22 at 09:10
  • @PanagiotisKanavos sorry you are correct i have updated my question with only using the context and stripped out the uneeded repos. Now am i getting the same result when running the code. Both the last two statuses are set to NULL. – Adam Wilson Oct 12 '22 at 09:30
  • So, you have exactly the same issue why this question was closed - do not initialize navigation properties if they are not collections `Status = CaapStatus.ReadyForExport;`. EF Core is not DDD, it has it's own rules. – Svyatoslav Danyliv Oct 12 '22 at 09:51
  • The relevant part is `if you initiate them in the constructor, EF won't overwrite them when materializing your object or by lazy loading. They will always have their initial values until you actively replace them. Worse still, you may even end up saving empty entities in the database!.` – Panagiotis Kanavos Oct 12 '22 at 10:12
  • @SvyatoslavDanyliv thanks for the explanation, but even with removing that from the constructor I still end up with only 1 item getting the status set. Please see updated question. – Adam Wilson Oct 12 '22 at 10:35
  • Try to add `private` parameterless constructor. – Svyatoslav Danyliv Oct 12 '22 at 10:42
  • @SvyatoslavDanyliv Thats giving the same result. – Adam Wilson Oct 12 '22 at 10:48
  • Try make `Status` with public setter. I don't know how EF should initialize properties in your case. – Svyatoslav Danyliv Oct 12 '22 at 10:51
  • @SvyatoslavDanyliv Same again, I could understand if it did it for every item but it works fine for the first one but then each one after has a NULL. – Adam Wilson Oct 12 '22 at 10:58

0 Answers0