As an exercise I was asked to moved logic from controller to service layer. With methods such as Get or GetById it wasn't a problem. Difficult part is to move Update and Patch methods. Below, you will find what I do have in Controler class and how I've changed them to the new requirement.
ToursController.cs before change
public class ToursController : ControllerBase
{
private readonly ToursRepository _repo;
private readonly IMapper _mapper;
public ToursController(ToursRepository repository, IMapper mapper)
{
_repo = repository ?? throw new ArgumentNullException(nameof(repository));
_mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
}
[HttpPut("{id}")]
public async Task<ActionResult> Update(int id, TourForUpdateDto updateDto)
{
var tour = await _repo.GetByIdAsync(id);
if (tour is null)
{
return NotFound();
}
_mapper.Map(updateDto, tour);
await _repo.SaveChangesAsync();
return NoContent();
}
[HttpPatch("{id}")]
public async Task<ActionResult> Patch(int id, JsonPatchDocument patchDocument)
{
var tour = await _repo.GetByIdAsync(id);
if (tour is null)
{
return NotFound();
}
var updateDto = _mapper.Map<TourForUpdateDto>(tour);
patchDocument.ApplyTo(updateDto);
if (!ModelState.IsValid)
{
return BadRequest(ModelState);
}
if (!TryValidateModel(patchDocument))
{
return BadRequest(ModelState);
}
_mapper.Map(updateDto, tour);
await _repo.SaveChangesAsync();
return NoContent();
}
}
ToursController.cs after change
public class ToursController : ControllerBase
{
private readonly ToursRepository _repo;
private readonly IMapper _mapper;
private readonly ToursService _toursService;
public ToursController(ToursRepository repository, IMapper mapper)
{
_repo = repository ?? throw new ArgumentNullException(nameof(repository));
_mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
_toursService = toursService ?? throw new ArgumentNullException(nameof(toursService));
}
[HttpPut("{id}")]
public async Task<ActionResult> Update(int id, TourForUpdateDto updateDto)
{
if (!_toursService.Exists(id))
{
return NotFound();
}
await _toursService.Update(id, updateDto);
return NoContent();
}
[HttpPatch("{id}")]
public async Task<ActionResult> Patch(int id, JsonPatchDocument patchDocument)
{
// No idea how to update this method.
var tour = await _repo.GetByIdAsync(id);
if (tour is null)
{
return NotFound();
}
var updateDto = _mapper.Map<TourForUpdateDto>(tour);
patchDocument.ApplyTo(updateDto);
if (!ModelState.IsValid)
{
return BadRequest(ModelState);
}
if (!TryValidateModel(patchDocument))
{
return BadRequest(ModelState);
}
_mapper.Map(updateDto, tour);
await _repo.SaveChangesAsync();
return NoContent();
}
}
ToursService.cs after change
using AutoMapper;
using BandManager.Api.Models;
using BandManager.Api.Services.Repositories;
public class ToursService
{
private readonly ToursRepository _repo;
private readonly IMapper _mapper;
public ToursService(ToursRepository repo, IMapper mapper)
{
_repo = repo ?? throw new ArgumentNullException(nameof(repo));
_mapper = mapper ?? throw new ArgumentNullException(nameof(mapper));
}
public async Task Update(int id, TourForUpdateDto updateDto)
{
var tour = await GetByIdAsync(id);
if (tour is null)
return;
_mapper.Map(updateDto, tour);
await _repo.SaveChangesAsync();
}
public async Task<TourDto?> GetByIdAsync(int id)
{
return _mapper.Map<TourDto>(await _repo.GetByIdAsync(id));
}
public bool Exists(int id)
{
return _repo.Exists(id);
}
}
Questions and thoughts
- Should I ever return objects which are implementing
IActionResultor any MVC related types from my services? My gut feeling is - no. - Should service be able to use
AutoMapperto convert entities to DTOs? I think, yes. - Should my service, in the same method call, be able to call
SaveChangesAsyncon repository?