I'm developing a .NET Core 3.1 Razor Page Application. I'm using Entity Framework Core and a Unit of Work pattern with Generic Repository. I'm also using AddScoped to register all my services, e.g. UnitOfWork and Repositories etc., i.e., one DbContext shared amongst repositories per HttpRequest.

services.AddDbContextPool<MyContext>(opt => opt.UseSqlServer(Configuration.GetConnectionString("MyConnection"))
         .EnableSensitiveDataLogging());

services.AddScoped<IUnitOfWork, UnitOfWork>();
services.AddScoped<IGenericRepository<Domain.List>, ListRepository>();
services.AddScoped<IGenericRepository<Domain.ListItem>, ListItemRepository>();
//etc...

I've written some code for a user to update their name and email. Once the user enters the data, I call a function UserAlreadyExists to validate if the email address already exists in the database. This validation code is used for new users registering on the application, but also for users who are already registered and are updating their details. The below code is used when an existing user is attempting to update their details.

public IUnitOfWork UoW { get; set; }

[BindProperty]
public Domain.User UserObj { get; set; }

public IActionResult OnPost()
{
    if(ModelState.IsValid)
    {
        if(UserObj.Id > 0)
        {
           //Update user

           //Validation to ensure if email updated, it's not already in use
           if(UserAlreadyExists(UserObj))
           {
              TempData["Message"] = "Email address already exists within database.";
              return Page();
           }

           UoW.UserRepository.Update(UserObj);
           UoW.SaveChanges();
           TempData["Message"] = "User updated.";

        }
        else
        {
          //Add user code here               
        }

        return RedirectToPage("List");
    }

   return Page();
}
    

This is the code that checks for the user's existence

private bool UserAlreadyExists(Domain.User user)
{
   bool alreadyExists = true;

   var existingUser = UoW.UserRepository
            .Find(u => u.Email.Trim().ToUpper() == user.Email.Trim().ToUpper())
            .FirstOrDefault();
        
    // Existing user
    if (existingUser == null)
    {
        alreadyExists = false;
    }
    else if (user.Email == existingUser.Email)
    {
        if (user.Id == existingUser.Id)
        {
            //User updating their details, but not their email
            alreadyExists = false;
        }               
    }

   return alreadyExists;
}

When an existing user tries to update their name, I get this error:

The instance of entity type 'User' cannot be tracked because another instance with the key value '{Id: 1}' is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached

I understand the error, and it's happening inside the UserAlreadyExists function. When an existing user attempts to update their name, the User entity is passed into the function and that entity contains the primary key. Then I create another User entity within the validation function called existingUser, and this is where the problem is - I now have 2 user entities with the same primary key value.

I'm wondering should I just detach the existingUser entity from the context prior to save changes, or maybe there is a better approach?

Any feedback appreciated.

Thanks.

CodePudding user response：

It's unclear what the UoW and UserRepository classes do. The error is caused because UserAlreadyExists loads the entity but doesn't update it. From the error it looks like UserRepository.Update is trying to attach the DTO again, even though the entity with the same ID is already loaded.

There are two options:

• Only load the ID instead of the entire entity
• Update the loaded object

Load only the ID

If EF Core was used directly, UserAlreadyExists could be rewritten to check for existence using LINQ:

private bool UserAlreadyExists(Domain.User user)
{
    var id=await _context.Users.Where(u=>u.Email==user.Email.Trim())
                         .Select(u=>u.Id)
                         .FirstOrDefault();
    return (user.Id==id);
}

That's all it takes, because user.Id is already known to be >0, and the emails already match.

u.Email should not be modified because that prevents the database server from using any indexes to speed up searching for the email. In SQL Server it's common to use case-insensitive collations so there's no reason to use ToLower().

After that, whatever Update does should work, because there's no entity being tracked. If EF Core was used, we could write:

if(!UserAlreadyExists(UserObj))
{
    _context.Users.Update(UserObj);
    _context.SaveChanges();
}

Update the loaded entity

The Razor CRUD tutorial shows how to use the ControllerBase.TryUpdateModelAsync method to update an already loaded entity. Instead of checking if a user exists, we can load the entity directly and update it:

var existingUser= _context.Users.Where(u=>u.Email==UserObj.Email.Trim())
                               .FirstOrDefault();
if(existingUser==null)
{
    _context.Users.Add(UserObj);
    _context.SaveChanges();
}
else if(existingUser.Id==UserObj.Id)
{
    if (await TryUpdateModelAsync<User>(existingUser))
    {
        _context.SaveChanges();
    }
}

TryUpdateModelAsync will use the Model properties so it doesn't need access to the UserObj object.

CodePudding user response：

Remember that in the end of the day, entity's db context is both a unit of work ( mind your injections ) and a generic repository for all models defined in the context, so.. you can actually go by without re-wrapping everything on something you will have to maintain later.

For this scenario, since you probably dont have the "AsNoTracking" available ( due to not working directly with ef as your repo ), you can change the code in 2 ways:

1. perform a count on the db, with the filters you want, so that you return the logical "is there a user already" instead of the actual user row
2. perform a select over a DTO object, that is an object that is not actually been tracked. Make a projection, select the properties you want to select, and map them to a plain class, specific for the task