C#•4w ago

✅ EF Core doesn't stop tracking list items.

Trying to edit an office entity and this works great for everything, except when the editedOffice passes less salesPerson ID's then there were before. If there are less id's it should remove them from the list inside the db aswell, currently it's not doing that. It is however adding new ones correctly when you pass more ids that werent there before.

public async Task<OfficeDTO?> EditOfficeAsync(OfficeEditDTO editedOffice)
{
    var office = await dbContext.Offices.Include(o => o.Address)
                                         .Include(o => o.SalesPeople)
                                         .FirstOrDefaultAsync(o => o.Id == editedOffice.Id);

    if (office == null)
        return null;

    var selectedSalesPeopleIds = office.SalesPeople.Select(o => o.Id).ToList();


    if (editedOffice.Name != null)
        office.Name = editedOffice.Name;

    if(editedOffice.Address != null)
    {

public async Task<OfficeDTO?> EditOfficeAsync(OfficeEditDTO editedOffice)
{
    var office = await dbContext.Offices.Include(o => o.Address)
                                         .Include(o => o.SalesPeople)
                                         .FirstOrDefaultAsync(o => o.Id == editedOffice.Id);

    if (office == null)
        return null;

    var selectedSalesPeopleIds = office.SalesPeople.Select(o => o.Id).ToList();


    if (editedOffice.Name != null)
        office.Name = editedOffice.Name;

    if(editedOffice.Address != null)
    {

31 Replies

Jiry_XDOP•4w ago

        if (editedOffice.Address.AddressLine != null)
            office.Address.AddressLine = editedOffice.Address.AddressLine;

        if (editedOffice.Address.PostalCode != null)
            office.Address.PostalCode = editedOffice.Address.PostalCode;

        if (editedOffice.Address.City != null)
            office.Address.City = editedOffice.Address.City;

        if (editedOffice.Address.Country != null)
            office.Address.Country = editedOffice.Address.Country;
    }

    

    if (!new HashSet<int>(editedOffice.SalesPeopleIds).SetEquals(selectedSalesPeopleIds))
        office.ReplaceSalesPeople(await dbContext.SalesPeople.Where(s => editedOffice.SalesPeopleIds.Contains(s.Id)).ToListAsync());


    await dbContext.SaveChangesAsync();


    return new OfficeDTO(
        office.Id,
        office.Name,
        new AddressDTO(
            office.Address.Id,
            office.Address.AddressLine,
            office.Address.PostalCode,
            office.Address.City,
            office.Address.Country
        ),
        office.SalesPeople.Select(s => new SalesPersonDTO(s.Id, s.Name)).ToList()
    );
}

        if (editedOffice.Address.AddressLine != null)
            office.Address.AddressLine = editedOffice.Address.AddressLine;

        if (editedOffice.Address.PostalCode != null)
            office.Address.PostalCode = editedOffice.Address.PostalCode;

        if (editedOffice.Address.City != null)
            office.Address.City = editedOffice.Address.City;

        if (editedOffice.Address.Country != null)
            office.Address.Country = editedOffice.Address.Country;
    }

    

    if (!new HashSet<int>(editedOffice.SalesPeopleIds).SetEquals(selectedSalesPeopleIds))
        office.ReplaceSalesPeople(await dbContext.SalesPeople.Where(s => editedOffice.SalesPeopleIds.Contains(s.Id)).ToListAsync());


    await dbContext.SaveChangesAsync();


    return new OfficeDTO(
        office.Id,
        office.Name,
        new AddressDTO(
            office.Address.Id,
            office.Address.AddressLine,
            office.Address.PostalCode,
            office.Address.City,
            office.Address.Country
        ),
        office.SalesPeople.Select(s => new SalesPersonDTO(s.Id, s.Name)).ToList()
    );
}

In office:

private List<SalesPerson> _salesPeople = [];

public IReadOnlyCollection<SalesPerson> SalesPeople
{
    get => _salesPeople.AsReadOnly();
    init => _salesPeople = Guard.Against.NoDuplicatesAndNotNull(value).ToList();
}

public void ReplaceSalesPeople(IEnumerable<SalesPerson> salesPeople)
{
    Guard.Against.NoDuplicatesAndNotNull(salesPeople);

    _salesPeople.Clear();
    _salesPeople.AddRange(salesPeople);

}

private List<SalesPerson> _salesPeople = [];

public IReadOnlyCollection<SalesPerson> SalesPeople
{
    get => _salesPeople.AsReadOnly();
    init => _salesPeople = Guard.Against.NoDuplicatesAndNotNull(value).ToList();
}

public void ReplaceSalesPeople(IEnumerable<SalesPerson> salesPeople)
{
    Guard.Against.NoDuplicatesAndNotNull(salesPeople);

    _salesPeople.Clear();
    _salesPeople.AddRange(salesPeople);

}

When debugging I can see it gets replaced correctly in office but savechangesasync doesnt remove the item from the list? I even tried to set them to null and that doesn't work?

public void ReplaceSalesPeople(IEnumerable<SalesPerson> salesPeople)
{
    Guard.Against.NoDuplicatesAndNotNull(salesPeople);


    var salesPeopleToRemove = _salesPeople.Where(s => !salesPeople.Any(ns => ns.Id == s.Id)).ToList();

    foreach (var salesPerson in salesPeopleToRemove)
    {
        salesPerson.Office = null;
    }
    _salesPeople.Clear();
    _salesPeople.AddRange(salesPeople);
    
}

public void ReplaceSalesPeople(IEnumerable<SalesPerson> salesPeople)
{
    Guard.Against.NoDuplicatesAndNotNull(salesPeople);


    var salesPeopleToRemove = _salesPeople.Where(s => !salesPeople.Any(ns => ns.Id == s.Id)).ToList();

    foreach (var salesPerson in salesPeopleToRemove)
    {
        salesPerson.Office = null;
    }
    _salesPeople.Clear();
    _salesPeople.AddRange(salesPeople);
    
}

Keswiik•4w ago

Are you re-using the same DB context for all of these operations?

Jiry_XDOP•4w ago

Yes I am dbContext

Keswiik•4w ago

And what type of application is this? API? some kind of native app? Are these modifications all done in one request or is this something that could be spaced out over some time? Because in general you're supposed to treat a DbContext as a single unit of work. Basically reuse the same context for the lifetime of a request, then throw it away. They're not meant to be singletons that are reused over the lifetime of an application.

Jiry_XDOP•4w ago

ASP .NET CORE Controller Rest API Yes this is one request the specified code above is the service layer Except the office mentioned that is inside the domain layer (entity class)

Keswiik•4w ago

ok, wanted to make sure it wasn't anything getting wonky with singleton dbcontext or something

Jiry_XDOP•4w ago

Oh no don't worry the dbContext is done properly via DI I think I've found something

 new() { Name = "Timmie Janssens", Office=null},

 new() { Name = "Timmie Janssens", Office=null},

I just tried to seed a salesperson without an office it might just be my migrations not allowing null... But I don't get why its not throwing an error there then

Keswiik•4w ago

what does your SalesPerson entity look like?

Jiry_XDOP•4w ago

public class SalesPerson : Entity
{
    public required string Name { get; set; }
    public int OfficeId { get; set; }
    public  Office? Office { get; set; }
}

public class SalesPerson : Entity
{
    public required string Name { get; set; }
    public int OfficeId { get; set; }
    public  Office? Office { get; set; }
}

Keswiik•4w ago

ya you probably want OfficeId to be an int? so EF realizes that it's nullable

Jiry_XDOP•4w ago

Gotcha its not enough to set Office nullable? I thought ef would infer that 😅

Keswiik•4w ago

well, you don't have to load nav properties so a nav property being nullable doesn't necessarily mean the relationship is nullable and since you've specified OfficeId I'm going to assume EF sees that, sees it's a non-nullable FK, and uses that to build the relationship but you not getting an exception makes me think EF isn't detecting the removal or isn't properly tracking values i never write my entities with IReadOnlyCollection<> and backing fields so I'm not sure if that could be causing issues

Jiry_XDOP•4w ago

Okay that seems to have worked

Keswiik•4w ago

:Blob:

Jiry_XDOP•4w ago

Its now removing it correctly Yeah that's one odd thing Its probably because of the IReadOnlyCollection no?

Keswiik•4w ago

it could be, but I've never used backing fields for nav properties

Jiry_XDOP•4w ago

I used it so I couldn't insert doubles How do you normally set this constraint?

Keswiik•4w ago

well you can prevent double inserts by configuring unique indexes too

Jiry_XDOP•4w ago

Like OfficeId in this case you mean or?

Keswiik•4w ago

i assume sales person can be assigned to a single office? (or no office)

Jiry_XDOP•4w ago

Yes one to many relationship, not required relationship

Keswiik•4w ago

what kind of potential double inserts do you think you could get then? I'm assuming this part of the API does not handle creating salespersons but only sends existing ones worst case scenario, you try adding a user to the same office twice and you likely get an exception but I usually keep all logic out of my entities and do any validation / guarding against stuff like this in the service layer

Jiry_XDOP•4w ago

I see yes in this case its true. The ReadOnlyCollection is not necessary here. It was actually a lecturer of mine who suggested this readonly list (for another entity class but I also implemented it here)m but it has no use case other than avoiding thrown Exceptions.

Keswiik•4w ago

makes sense in the case where a double insert could be possible (say a many-many relationship and you've explicitly defined a join table), you can always add a unique index that uses both FKs you'd do something like

modelBuilder.Entity<YourJoinEntity>.HasIndex(e => new() { e.ForeignKey1, e.ForeignKey2 }).IsUnique()

// or with the fluent api
// never do it this way but I think it's valid?
[Index(nameof(ForeignKey1), nameof(ForeignKey2), IsUnique = true)]
public class YourJoinEntity {
  // props here
}

modelBuilder.Entity<YourJoinEntity>.HasIndex(e => new() { e.ForeignKey1, e.ForeignKey2 }).IsUnique()

// or with the fluent api
// never do it this way but I think it's valid?
[Index(nameof(ForeignKey1), nameof(ForeignKey2), IsUnique = true)]
public class YourJoinEntity {
  // props here
}

Jiry_XDOP•4w ago

Gotcha thanks, I appreciate the help really much. Just one more question, it may be a nooby question, what do you mean here when referring to a backing field for nav properties is that the Id or? here

Keswiik•4w ago

i was referring to this

private List<SalesPerson> _salesPeople = [];

public IReadOnlyCollection<SalesPerson> SalesPeople
{
    get => _salesPeople.AsReadOnly();
    init => _salesPeople = Guard.Against.NoDuplicatesAndNotNull(value).ToList();
}

private List<SalesPerson> _salesPeople = [];

public IReadOnlyCollection<SalesPerson> SalesPeople
{
    get => _salesPeople.AsReadOnly();
    init => _salesPeople = Guard.Against.NoDuplicatesAndNotNull(value).ToList();
}

Jiry_XDOP•4w ago

Oh you meant I also could do it in the modelbuilder without defining it as a nav property?

Keswiik•4w ago

nah i was just saying that I've never used backing fields for a nav prop i usually do public List<SalesPerson> SalesPeople { get; set; } but that is mostly personal preference, I don't like putting any business logic in my entities

Jiry_XDOP•4w ago

Ooh gotcha I totally understand now 🙂 Thanks for explaining and taking time to help me I really appreciate it!

Keswiik•4w ago

np :PepoSalute:

Jiry_XDOP•4w ago

❤️

Gaming

Programming

✅ EF Core doesn't stop tracking list items.