.NET Entity Framework - duplicate entities

Question

Usefull Context

I currently have two entities that look as below.

MovieSerie

    public class MovieSerie
    {
        [Key]
        public Guid MovieSerieId { get; set; }

        [Required]
        [MaxLength(128)]
        public string Title { get; set; }

        [Required]
        [MaxLength(256)]
        public string Description { get; set; }

        public virtual ICollection<Movie> Movies { get; set; }
    }

Movie

    public class Movie
    {
        [Key]
        public Guid MovieId { get; set; }

        [Required]
        [MaxLength(128)]
        public string Title { get; set; }

        public virtual MovieSerie MovieSerie { get; set; }
    }

I have removed some properties that were unused so far so the example is a bit more readable.

These entities have a one-to-many relationship because a MovieSerie contains multiple movies but a movie can only belong to one MovieSerie.

The problem

When I am trying to make a new movie from Postman by providing an EXISTING MovieSerie, I am getting an exception. The exception looks as below.

Duplicate entry '\xA9\xCE\x0E\x1E\x9A\xAE\xA2G\x91<\xE6\xE3-\x88C\xE9' for key 'movieseries.PRIMARY'

So I figured out that it is trying to make a new MovieSerie when I am providing a MovieSerie object. The raw JSON from the request that I am trying to send from Postman looks like below.

{
    "MovieId" : "6aa8c134-689c-45e2-bf60-cd0eb5473cc2",
    "Title" : "TestMovie",
    "MovieSerie" : {
        "movieSerieId": "1e0ecea9-ae9a-47a2-913c-e6e32d8843e9",
        "title": "Harry Potter",
        "description": "This contains the Harry Potter serie"
    }
}

The POST method to save the movie is shown below.

        [HttpPost]
        public async Task<ActionResult<Movie>> PostMovie(Movie movie)
        {
            if (movie == null)
            {
                return BadRequest("No movie object provided");
            }

            else if (movie.MovieSerie != null)
            {
                if (!_validator.MovieSerieExists(movie.MovieSerie.MovieSerieId))
                {
                    return BadRequest("The movie serie does not exists in the database");
                }
            }

            _context.Movies.Add(movie);
            await _context.SaveChangesAsync();

            return CreatedAtAction("GetMovie", new { id = movie.MovieId }, movie);
        }

Could someone give me any insight into what I am doing wrong? Why is it trying to make a new entity while it already exists? What should I change to get the wished behavior?

I tried to provide all information required, however, let me know if I missed something.

EDIT ADDED DBCONTEXT

            modelBuilder.Entity<MovieSerie>(entity =>
            {
                entity.HasKey(movieSerie => movieSerie.MovieSerieId);
                entity.Property(movieSerie => movieSerie.Title).IsRequired();
                entity.Property(movieSerie => movieSerie.Description).IsRequired();
                entity.HasMany(ms => ms.Movies)
                .WithOne(m => m.MovieSerie);
            });

            modelBuilder.Entity<Movie>(entity =>
            {
                entity.HasKey(movie => movie.MovieId);
                entity.Property(movie => movie.Title).IsRequired();
                entity.HasOne(m => m.MovieSerie)
                .WithMany(s => s.Movies);
            });

Answer 1

This is what happens when passing entities between server and client in ASP.Net. When your DbContext is lifetime scoped to a request, the entities are loaded by a DbContext and passed to the view, but then what you pass back on the Post call is a JSON object that is deserialized into an entity class definition. On this request, neither the Movie or it's associated related entities are tracked by the DbContext.

When you tell the Post's DbContext to Add the movie, any child entities on that movie will be treated as new entities as well, resulting in duplicate records.

How to avoid this:

Option 1: Use ViewModels to avoid confusing data coming from views with entities. (Data state) This is always my recommended option. This avoids confusion about what objects you are dealing with, and also means you can reduce the amount of data being sent over the wire. As entities get larger, sending entities back and forth means larger payloads for fields your view doesn't need. ViewModels can be populated to serve just the fields that the view will interact with. Automapper can help largely with turning entity graphs into ViewModels with it's ProjectTo method.

So if we had a view for creating a Movie (Movie/Create) and that view listed a the movie series to choose from, it might search/fetch series:

[Serializable]
public class MovieSeriesSummaryViewModel
{
    public Guid MovieSeriesId { get; set; }
    public string Name { get; set; }
}

Then when the controller goes to search/retrieve those series to choose from:

var series = _context.MovieSeries
      // .Where(x => [search criteria...])
      .ProjectTo<MovieSeriesSummaryViewModel>(config)
      .ToList();

or

var series = _context.MovieSeries
      // .Where(x => [search criteria...])
      .Select( x = > new MovieSeriesSummaryViewModel
      {
          MovieSeriesId = x.MovieSeriesId,
          Name = x.Name
      }).ToList();

a PostMovie action accepts a PostMovieViewModel:

[Serializable]
public class PostMovieViewModel
{
    public string MovieName { get; set; }
    public Guid? MovieSeriesId { get; set; }
    // ... 
}

The create movie view model only needs to pass the series ID (if applicable) and the required fields to create a new movie. From there we associate the series from the DbContext when creating our new Movie:

[HttpPost]
public async Task<ActionResult<PostMovieViewModel>> PostMovie(PostMovieViewModel movieVM)
{
    var movieSeries = movieVM.MovieSeriesId.HasValue 
        ? _context.MovieSeries.Single(x => x.MovieSeriesId == movieVM.MovieSeriesId.Value)
        : null;
    var movie = new Movie
    {
        Name = movieVM.Name,
        MovieSeries = movieSeries
    };
    _context.Movies.Add(movie);
    await _context.SaveChangesAsync();
}

The key point here is that we fetch the existing series from the Context to associate to the new movie. Fetching entities by ID is quite fast and serves as a meaningful validation that the data we passed in is complete.

Option 2: Re-associate all references. The underlying problem with passing deserialize objects and treating them as entities is that the DbContext isn't tracking them. There are 2 ways you can fix this, either tell the DbContext to track them, or replace the references with tracked objects.

2a - Replacing references

[HttpPost]
public async Task<ActionResult<Movie>> PostMovie(Movie movie)
{
     if (movie.MovieSeries != null)
     {
         var existingMovieSeries = _context.MovieSeries
             .Single(x => MovieSeriesId == movie.MovieSeries.MovieSeriesId);
         movie.MovieSeries = existingMovieSeries; // Replace the reference.
     }
     _context.Movies.Add(movie);
     await _context.SaveChanges();
}

This still potentially means going to the DB for all references, and forgetting to will result in silent duplication issues.

2b - Track related entities. This one I saved for last as it can seem simple, but can trip you up...

[HttpPost]
public async Task<ActionResult<Movie>> PostMovie(Movie movie)
{
     if (movie.MovieSeries != null)
         _context.Attach(movie.MovieSeries);

     _context.Movies.Add(movie);
     await _context.SaveChanges();
}

That looks simple, and would work most of the time, but if the DbContext is already tracking that movie series for any reason, the Attach method will fail. This is an error that could appear intermittently at runtime depending on the particular actions/data combinations. (I.e. updating 2 movies /w same series or conditionally calling a method that loads that series) The proper check would be:

[HttpPost]
public async Task<ActionResult<Movie>> PostMovie(Movie movie)
{
     if (movie.MovieSeries != null)
     {   
         var existingMovieSeries = _context.MovieSeries.Local
             .SingleOrDefault(x => x.MovieSeriesId == movie.MovieSeries.MovieSeriesId);
         if (existingMovieSeries == null)
             _context.Attach(movie.MovieSeries);
         else
             movie.MovieSeries = existingMovieSeries;
     }
     _context.Movies.Add(movie);
     await _context.SaveChanges();
}

Checking MovieSeries.Local checks to see if the DbContext is tracking the series. (without hitting the DB) If not, we can attach it. If it is, we need to replace the reference. This can be a lot of boiler plate code to put in for every reference on a new object. When attaching entities coming from a view, it is also important not to ever set the entity state for that entity to Modified without first verifying the data is valid. (Which would require loading the entity first anyway) Doing so could allow users to alter data in ways you don't intend as setting an entity to Modified will update all fields on that entity. (Where loading an entity and then copying across values means only those values you change will be updated)

Answer 2

Your problem is that you are passing the whole movie serie object. This is not something you should do. The idea of relational databases is to, as the name suggest, relate tables. This relationships are done using keys (foreign keys). In your particular case, you need to define a foreign key column in your Movie table, to relate it to MovieSeries, as follows:

public class Movie
{
    [Key]
    public Guid MovieId { get; set; }
    public int MovieSerieId {get; set; }
    [Required]
    [MaxLength(128)]
    public string Title { get; set; }

    [ForeignKey("MovieSerieID")]
    public virtual MovieSerie MovieSerie { get; set; }
}

As you can see, im specifying that the MovieSerieID attribute is a foreign key. The virtual MovieSerie attribute is used by EF to get all the details of your foreign key.

Now, you can create your movie passing only the MovieSerieid, as follows:

{
    "MovieId" : "6aa8c134-689c-45e2-bf60-cd0eb5473cc2",
    "Title" : "TestMovie",
    "MovieSerieId": "1e0ecea9-ae9a-47a2-913c-e6e32d8843e9"
}