Upsert with .Net Core Web API

Question

I am using .NET Core 2.2, EF Core and C#. I have created a web API project.

I was requested to do an Upsert (insert or update). The entities structure is:

• Movie

• User

• Rating: this is used to give a rating to a movie, so a new rating will have the MovieId (from Movie) the UserId (from User) and the Rating value.

In the Web API controller I have this request for the result of the upsert:

• 404 (if movie or user is not found)

• 400 (if the rating is an invalid value, the values can be an integer from 1 to 5)

• 200 (OK)

So, according to the request, I have implemented this code, but it does not look correct according to the SOLID principles. Is there any way to refactor this code to improve it? Obviously: making one method for the update and other to create is not an option as it does not respect the request.

This is the code:

[Route("apid/movies")]
[ApiController]
public class MoviesDController : ControllerBase
{
 private readonly IRatingRepository _repository;
 private readonly IUserRepository _userRepository;
 private readonly IMovieRepository _movieRepository;
 public MoviesDController(IRatingRepository repository, IUserRepository userRepository, IMovieRepository movieRepository)
 {
 _repository = repository;
 _userRepository = userRepository;
 _movieRepository = movieRepository;
 }
 // POST: apid/movies
 [HttpPost]
 public IActionResult Upsert([FromBody] Rating rating)
 {
 if (!ModelState.IsValid)
 {
 return BadRequest(ModelState);
 }
 //1) Get the User
 var user = _userRepository.GetById(rating.UserId);
 if (user == null)
 {
 return NotFound("User not found...");
 }
 //2) Get the Movie
 var movie = _movieRepository.GetById(rating.MovieId);
 if (movie == null)
 {
 return NotFound("Movie not found...");
 }
 //3) Get Rating
 var existingRating = _repository.GetMovieRating(rating.UserId, rating.MovieId);
 if (existingRating == null)
 {
 //Insert
 _repository.InsertRating(rating);
 return Ok("Rating added");
 }
 existingRating.RatingValue = rating.RatingValue;
 //Update
 _repository.UpdateRating(existingRating);
 return Ok("Rating updated");
 }
}

• t3chb0t: it does not look correct according to the SOLID principles - how do you figure? If you know what is wrong what don't try improve it first? What makes think it's not SOLID exactly?
• MarcosF8: That is my best code at moment actually. I have no ideas, as I normally write one method to insert and another to update.
• Robert Perry: If you're worried about SOLID; the big screaming problem for me is that theres too much logic in your controller.. Move that out to your Business Logic layer..

Answer 1

Foreign Keys on DB Schema
You might want to look at adding some foreign keys and navigation properties so that you dont need to make 3 seperate calls to the database to do an update. At the very least your Rating should be able to link back to the Movie. That would save 1 call.

Intelligent Controllers
You want to avoid adding business logic to controller classes. All of those repositories and the logic for what happens during an upsert should come out into a seperate class to fit with the Single Responsibility Principle. Controllers are 'directors' of calls from the outside into internal code and should be as empty, dumb or anaemic as possible.

p.s. I revisited this because it was still on the list of 'unanswered' questions in my SE filter