0

Greeting, for learning purposes I'm writing an ASP.NET Core Web API app for a movie database, diagram of which is shown below:
Database diagram img link (Sorry, not enough reputation to post images)
Here's what Context class looks like

public class MovieDbContext : DbContext
    {
        public MovieDbContext(DbContextOptions<MovieDbContext> options)
            :base(options)
        {}

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<MovieActor>()
                .HasKey(ma => new { ma.MovieId, ma.ActorId });
            modelBuilder.Entity<MovieGenre>()
                .HasKey(mg => new { mg.MovieId, mg.GenreId});
        }

        public DbSet<Movie> Movies { get; set; }
        public DbSet<Actor> Actors { get; set; }
        public DbSet<Genre> Genres { get; set; }
        public DbSet<MovieActor> MovieActors { get; set; }
        public DbSet<MovieGenre> MovieGenres { get; set; }
    }

I've read in several resources and stackoverflow posts that it's a good practice to use DTO classes for requests so I've implemented one for the Movie entity called MovieCreate.

    public class Movie
    {
        public int MovieId { get; set; }
        [Required]
        [StringLength(200)]
        public string Name { get; set; }
        [Required]
        public int Year { get; set; }
        [Required]
        [StringLength(4000)]
        public string Description { get; set; }
        [Required]
        [Column(TypeName = "decimal(3, 1)")]
        public decimal Score { get; set; }
        [Required]
        [StringLength(2000)]
        public string ImgUrl { get; set; }

        public IList<MovieActor> MovieActors { get; set; }
        public IList<MovieGenre> MovieGenres { get; set; }
    }
    public class MovieCreate
    {
        public string Name { get; set; }
        public int Year { get; set; }
        public string Description { get; set; }
        public decimal Score { get; set; }
        public string ImgUrl { get; set; }
        public IList<int> ActorIds { get; set; }
        public IList<int> GenreIds { get; set; }
    }

The difference between MovieCreate class and Movie entity is the absence of MovieId property because well we don't know the current value of the PK and even more importantly IDENTITY_INSERT parameter is OFF, so no point in having that inside DTO class. Another difference is type change of navigation properties from, for example, IList<MovieGenre> to IList<int> again caused by the lack of MovieId information.

So I'm trying to figure out the correct way to store data from POST request to the database. I have a service inside of DataAccess project that is responsible for operations with database. Here's what Create method in that service looks like:

    public async Task<MovieResponse> Create(MovieCreate newMovie)
        {
            Movie movie = newMovie.MapMovie();

            //We save changes to database first so that MovieId is assigned to movie variable
            _context.Movies.Add(movie);
            await _context.SaveChangesAsync();

            foreach (var actor in newMovie.ActorIds)
            {
                movie.MovieActors.Add(new MovieActor { ActorId = actor, MovieId = movie.MovieId });
            }

            foreach (var genre in newMovie.GenreIds)
            {
                movie.MovieGenres.Add(new MovieGenre { GenreId = genre, MovieId = movie.MovieId });
            }

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

            return movie.MapMovieResponse();
        }

I wonder if this is the correct or should I say acceptable way to do things? Also by trial and error and some googling after that I figured that by either explicitly assigning MovieId to 0 or just never assigning it, which would result in it to be default integer value(which is 0 as well), EF would assume that we didn't explicitly specified PK value so it would be generated by database when saved, so code like this also works:

        public async Task<MovieResponse> Create(MovieCreate newMovie)
        {
            Movie movie = newMovie.MapMovie();
            //technically movie.MovieId == 0 at this point, since no value was assigned to it, so it's default
            foreach (var actor in newMovie.ActorIds)
            {
                movie.MovieActors.Add(new MovieActor { ActorId = actor, MovieId = 0});
            }

            foreach (var genre in newMovie.GenreIds)
            {
                movie.MovieGenres.Add(new MovieGenre { GenreId = genre, MovieId = 0});
            }
            //And now we save changes to the database
            _context.Movies.Add(movie);
            await _context.SaveChangesAsync();

            return movie.MapMovieResponse();
        }

Also in the first version of Create method instead of adding, for example, MovieGenre data to the navigation property I could add data to MovieGenres DbSet. So which way is the "correct" one?
P.S. If it's not too much to ask I would really appreciate any tips on Update (PUT) and PATCH requests, I think especially on PATCH since from what I've seen on stackoverflow it's really complicated.

2 Answers 2

1

Both options obviously work, so I don't know if "correct" is the right term. But the second option is definitely more efficient, both from a code and performance standpoint. The code is easier to read and more compact and you are only calling SaveChanges once. You can clean up the second option slightly by removing the "MovieId = 0" for your join tables. It's assumed by EF, so you don't need it.

Personally, for updates I just pull the existing record and then manually check those properties that can be updated, do a compare to see if they changed and update if needed (a bunch of IF statements). By only touching the fields that were actually updated, EF can create a more efficient UPDATE query. Don't just automatically copy every field from the incoming update DTO to the existing entity if you don't have to.

Sorry to say I don't make use of PATCH much, so I can't talk to that.

Sign up to request clarification or add additional context in comments.

1 Comment

Thanks for the answer. That's what I did at first as well, i.e. not assigning MovieId to 0 explicitly, only did this in the code that's in the post for clarification. For the update part that's exactly what I had in mind and now I see that it's the way to go. Thanks again.
0

Change ActorIds and GenreIds to an Array of int

    public class MovieCreationDto
    {
        public string Name { get; set; }
        public int Year { get; set; }
        public string Description { get; set; }
        public decimal Score { get; set; }
        public string ImgUrl { get; set; }
        public int[] ActorIds { get; set; }
        public int[] GenreIds { get; set; }
    }

and Update your project to ASP.NET Core 5 and EF Core 5 Because Making Many to Many relationships is really easy in EF Core 5 and Querying is much more easier for Many to Many relationships.

change your way of adding Actors and Genres to Movies do it with Select(Projection) in LINQ.

What's New in EF Core 5.0

1 Comment

Thanks for the answer. I will look into .NET 5 soon-ish, just wanna figure out the basics first and have a solid ground on understanding how everything works.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.