Error when trying to track instance of an entity while updating the database entries
Solution 1:
This is a common issue when implementing things like Repositories where you are passing entity references around and the scope of the DbContext isn't clearly defined.
The problem is you are redefining entities far, far too much. Take your controller method:
string notificationText = _notificationRepository.GetNotificationById(notificationId).isSeen;
This is telling the Repository to load an entire notification, then retrieve it's Text property. Chances are that instance is being tracked by the DbContext, and that DbContext's scope is for the life of the request. When you call your Update method, that instance that was loaded by GetNotificationById() is still tracked, yet you are creating a new instance of the entity to pass to your update method, then in your update method creating yet another instance of the entity, all with the same Id.
This is completely unnecessary. I also cannot fathom why you'd go through the trouble to update the notification, then call a method to Delete the notification??
[HttpPost]
public JsonResult UpdateNotification(int notificationId, int userId)
{
try
{
var notification = _notificationRepository.GetNotificationById(notificationId);
notification.IsSeen = string.Format("{0},{1}", notificationText, userId);
_context.SaveChanges();
return Json(new { success = true});
}
catch(Exception)
{
return Json(new { success = false});
}
}
Since the repository loads an entity, we can use the change tracking on that entity, then use the scoped DbContext to save the changes. This really defeats the purpose behind adding a repository layer over-top the DbContext, as you could just fetch the entity using the DbContext itself. Normally with a unit of work pattern you'd have something more like:
using (var scope = _unitOfWork.CreateScope())
{
var notification = _notificationRepository.GetNotificationById(notificationId);
notification.IsSeen = string.Format("{0},{1}", notificationText, userId);
scope.SaveChanges();
}
Where the repositories have a means to resolve the DbContext from the UoW scope they are called under. (I.e. using a locator or having the scope is passed into each method call)
The main thing you will want to avoid though is constantly newing up entity classes for the same row. (Id) Fetch an entity, update it's value leveraging EF's built in change tracking, and call SaveChanges(). If you are ever in a situation where you have a detached entity that you want to update, first check the DbContext to see if it is tracking an entity before attaching. For instance if you modify the details in a detached Notification entity passed into the repository and want to use Update or attach and set modified state on the entity/properties:
bool INotificationRepository.UpdateNotification(Notification notification)
{
try
{
var existingNotification = context.Notifications.Local.SingleOrdefault(x => x.notificationId == notification.notificationId);
if (existingNotification != null)
{
existingNotification.isSeen = notification.isSeen;
}
else
{
context.Attach(notification);
context.Entry(selectedNotification).Property(x => x.isSeen).IsModified = true;
}
return true;
}
catch (Exception)
{
return false;
}
}
Solution 2:
Why don't you use a simpler approach? When can you reuse an entity from a repository? Example below:
Controller code
[HttpPost]
public JsonResult UpdateNotification(int notificationId, int userId)
{
var notification = _notificationRepository.GetNotificationById(notificationId);
notification.isSeen = string.Format("{0},{1}", notificationText, userId);
bool result = _notificationRepository.UpdateNotification(notification);
if (result == true)
{
DeleteNotification(notificationId);
return Json(new { success = true});
}
else
{
return Json(new { success = false });
}
}
Repository implementation
bool INotificationRepository.UpdateNotification(Notification notification)
{
try
{
context.Update(notification);
}
catch (Exception)
{
return false;
}
}