Search code examples
c#asp.net-coreentity-framework-coreone-to-manyunit-of-work

EF Core 5 One-to-many relationship problem


In this example a user has zero or many bills, one bill can be assigned to one user. Bill can also be created but never assigned.

public class User
{
  public int Id{ get; set; }   
  public List<Bill> bills{ get; set; }
}
        
public class Bill
{
  public int Id { get; set; }
        
  public int userId{ get; set; }
  public User user{ get; set; }
}

I've also added this in my DB context configuration:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
 modelBuilder.Entity<Bill>()
             .HasOne(b => b.user)
             .WithMany(u => u.bills)
             .HasForeignKey(b => b.userId);
}

I've realized it through a unit of work + repository pattern. In my BillService.cs I would like to have a method that allows me to update/add a bill and assign it to a user.

If the user doesn't exist in DB it should add it. If the user exists it should update it.

I've tried two approaches. First:

public async Task<void> AddUpdateBill(AddBillModel model){
    Bill bill= await unitOfWork.BillRepository.GetByID(model.billId);
    
    if( unitOfWork.UserRepo.GetById(model.userId) == null){
        unitOfWork.UserRepo.Insert(model.user);
    }else{
        unitOfWork.UserRepo.Update(model.user);
    }
    bill.user = model.user;
    unitOfWork.BillRepository.Update(bill);
    unitOfWork.Save();
}

Second:

public async Task<void> AddUpdateBill(AddBillModel model)
{
    Bill bill= await unitOfWork.BillRepository.GetByID(model.billId);
    bill.user = model.user;
    unitOfWork.BillRepository.Update(bill);
    unitOfWork.Save();
}

In both cases, I've got the problem of duplicated primary-key or entity already tracked.

Which is the best approach or the right way to do it?

EDIT: Sorry, BillRepo and BillRepository are the same class.

public async Task<Bill> GetByID(int id)
{
   return await context
           .bill
           .Include(b => b.user)
           .Where(b=> b.id == id)
           .FirstOrDefaultAsync();
}

public void Update(Bill bill)
{
   context.Entry(bill).CurrentValues.SetValues(bill);
}

Solution

  • The first approach seems more right (to me). First of all, comply with the naming rules: all properties must begin with upper case characters. "Bills", "UserId", "User" in your case.

    if( unitOfWork.UserRepo.GetById(model.userId) == null){
        unitOfWork.UserRepo.Insert(model.user);
    }else{
        unitOfWork.UserRepo.Update(model.user);
    }
    bill.user = model.user;
    

    You don't need it here

    bill.user = model.user;
    

    because you have just attached your entity to context and updated/inserted it.

    Also, don't forget to format your code, for example https://learn.microsoft.com/ru-ru/dotnet/csharp/programming-guide/inside-a-program/coding-conventions

    It would be useful to consider inserting/updating your entities not straight from the model, something like:

    if( unitOfWork.UserRepo.GetById(model.userId) == null){
        var user = new User 
        {
           //set properties
        };
        unitOfWork.UserRepo.Insert(user);
        unitOfWork.Save();
        bill.userId = user.Id;
    }