Monday, July 18, 2011

Entity Framework: AsNoTracking() and context.DbSet.Find(pk), a dangerous combo

Using context.YourDbSetHere.AsNoTracking() (line #11), all your succeeding context.YourDbSetHere.Find(pk) (line #35) will always hit the database

Actually, it's the combo of context.YourSourceDbSetHere.AsNoTracking() and context.DestinationDbSetHere.Add(context.YourSourceDbSetHere.Find(pk)) that is dangerous.


There's no clean way for a stub/proxy object (an object with only had its ID property set, the rest of the properties are left with their default value(i.e. null, empty string, zero, etc)) to represent an entity on many-to-many inserts.




[HttpPost]
public ActionResult Save(MovieInputViewModel input)
{
 using (var db = new TheMovieContext())
 {
  try
  {
   bool isNew = input.TheMovie.MovieId == 0;

   input.SelectedGenres = input.SelectedGenres ?? new List<int>();                    
   input.GenreSelections = db.Genres.AsNoTracking().OrderBy(x => x.GenreName).ToList();


   var movie = !isNew ? db.Movies.Find(input.TheMovie.MovieId) : input.TheMovie;                    

   if (isNew)
    db.Movies.Add(movie);
   else
   {
    db.Entry(movie).Property("Version").OriginalValue = input.TheMovie.Version;
    db.Entry(movie).State = System.Data.EntityState.Unchanged;


    // dirty this all the time even this is not changed by the user, 
    // so we have a simplified mechanism for concurrent update 
    // when the associated Genre(s) is modified. 
    // June 3, 2012: Above comment is stale already. For detecting concurrent update, use TimeStamp attribute instead of dirtying-technique+Concurrency attribute combo technique.
    // Read the concurrent update technique using Timestamp at this post: http://www.ienablemuch.com/2011/07/using-checkbox-list-on-aspnet-mvc-with_16.html
    db.Entry(movie).Property("MovieName").IsModified = true;
   }


   movie.Genres = movie.Genres ?? new List<Genre>();
   movie.Genres.Clear();
   foreach (int g in input.SelectedGenres)
   {
    movie.Genres.Add(db.Genres.Find(g));    
   }

   UpdateModel(movie, "TheMovie", includeProperties: null, excludeProperties: new string[] { "Version" });


   db.SaveChanges();

   db.Entry(movie).Reload();
   input.TheMovie.Version = movie.Version;

   
   ModelState.Remove("TheMovie.MovieId");

   // No need to remove TheMovie.Version, ASP.NET MVC is not preserving the ModelState of variables with byte array type.
   // Hence, with byte array, the HiddenFor will always gets its value from the model, not from the ModelState
   // ModelState.Remove("TheMovie.Version"); 


   input.MessageToUser = input.MessageToUser + " " + string.Format("Saved. {0}", isNew ? "ID is " + input.TheMovie.MovieId : "");

  }
  catch (DbUpdateConcurrencyException)
  {
   ModelState.AddModelError("", 
    "The record you attempted to edit was already modified by another user since you last loaded it. Open the latest changes on this record");
  }
 }

 return View("Input", input);
}



To solve that problem, monitor your context's ChangeTracker, line #35 to 47. Even you are using AsNoTracking, line #35 to 47 will not make a database roundtrip to fetch the Genre's row(s), inserting into many-to-many table will be streamlined.

[HttpPost]
public ActionResult Save(MovieInputViewModel input)
{
 using (var db = new TheMovieContext())
 {
  try
  {
   bool isNew = input.TheMovie.MovieId == 0;

   input.SelectedGenres = input.SelectedGenres ?? new List<int>();                    
   input.GenreSelections = db.Genres.AsNoTracking().OrderBy(x => x.GenreName).ToList();


   var movie = !isNew ? db.Movies.Find(input.TheMovie.MovieId) : input.TheMovie;                    

   if (isNew)
    db.Movies.Add(movie);
   else
   {
    db.Entry(movie).Property("Version").OriginalValue = input.TheMovie.Version;
    db.Entry(movie).State = System.Data.EntityState.Unchanged;


    // dirty this all the time even this is not changed by the user, 
    // so we have a simplified mechanism for concurrent update 
    // when the associated Genre(s) is modified
    db.Entry(movie).Property("MovieName").IsModified = true;
   }


   movie.Genres = movie.Genres ?? new List<Genre>();
   movie.Genres.Clear();
   foreach (int g in input.SelectedGenres)
   {
    var cachedGenre = db.ChangeTracker.Entries<Genre>().SingleOrDefault(x => x.Entity.GenreId == g);

    Genre gx = null;
    if (cachedGenre != null)
     gx = cachedGenre.Entity;
    else
    {
     gx = new Genre { GenreId = g };
     db.Entry(gx).State = EntityState.Unchanged;
     input.MessageToUser = input.MessageToUser + "Not yet in cache " + g.ToString() + ";";
    }

    movie.Genres.Add(gx);
   }

   UpdateModel(movie, "TheMovie", includeProperties: null, excludeProperties: new string[] { "Version" });



   db.SaveChanges();

   db.Entry(movie).Reload();
   input.TheMovie.Version = movie.Version;


   ModelState.Remove("TheMovie.MovieId");

   // No need to remove TheMovie.Version, ASP.NET MVC is not preserving the ModelState of variables with byte array type.
   // Hence, with byte array, the HiddenFor will always gets its value from the model, not from the ModelState
   // ModelState.Remove("TheMovie.Version"); 


   input.MessageToUser = input.MessageToUser + " " + string.Format("Saved. {0}", isNew ? "ID is " + input.TheMovie.MovieId : "");

  }
  catch (DbUpdateConcurrencyException)
  {
   ModelState.AddModelError("", 
    "The record you attempted to edit was already modified by another user since you last loaded it. Open the latest changes on this record");
  }
 }

 return View("Input", input);
}


This article could have been titled:
Entity Framework: Representing an entity with a stub/proxy object is a one big exercise of leaky abstraction.

Microsoft could borrow some technology from NHibernate for their Entity Framework. They should implement context.Load<EntityHere>(pkHere). C'mon Microsoft, spare us the trouble of leaky abstractions of your Entity Framework :-)


Related to Using checkbox list on ASP.NET MVC with Entity Framework 4.1


Sample outputs

On first save:



On second save:



No comments:

Post a Comment