I have implemented the Repository pattern in my Web APi 2 + Entity Framework 6.1 + Angular 1.4 SPA site. The application allows management of a martial arts club. One of its functionalities is to allow to handle belt promotions. This involves some validations i.e. You can't add a promotion to a higher belt with a date lower than an existing promotion to a lower belt. Currently i have this validation as part of the repository. This is obviously wrong. I have a couple of questions:
- Where should I put this validation? Should it be in the controller?
- Should I create a separate class for validation and use it in controller?
- How should i return potential validation errors to the front end? Should it be an exception with a message?
- Is it a good practice to return results of more complex validations from the API and display those messages in the UI?
- Should I have one repository as controler field or instantiate it with each request? Does't instantiating with each request create a risk of issues with database connection?
Here is my current implementation of the repository and controller classes:
public class PromotionsRepository : RepositoryBase, IPromotionsRepository
{
public PromotionsRepository()
{
}
public async Task<List<Promotion>> GetPromotionsForMember(int memberId)
{
using (var context = new BjjClubEntities())
{
try
{
var promotions = await context.Members.Include("Promotions").Where(member => member.MemberId == memberId).SingleOrDefaultAsync();
return promotions.Promotions.OrderBy(promotion => promotion.Date).ToList();
}
catch (Exception ex)
{
LogError(ex);
return null;
}
}
}
public async Task AddPromotions(int memberId, List<Promotion> newPromotions)
{
using (var context = new BjjClubEntities())
{
try
{
newPromotions.ForEach(newPromotion =>
{
var editedMember = context.Members.Include("Promotions").Where(member => member.MemberId == memberId).SingleOrDefault();
// can't promote to a lower belt or lower number of stripes
var conflictingPromotionsBefore = editedMember.Promotions
.Where(promotion => newPromotion.Date > promotion.Date)
.Where(promotion => newPromotion.Belt < promotion.Belt || (newPromotion.Belt == promotion.Belt && newPromotion.Stripes < promotion.Stripes)).ToList();
// when inserting promotion that occured at a previous date
// no promotions after this date should be lower
var conflictingPromotionsAfter = editedMember.Promotions
.Where(promotion => newPromotion.Date < promotion.Date)
.Where(promotion => newPromotion.Belt > promotion.Belt || (newPromotion.Belt == promotion.Belt && newPromotion.Stripes > promotion.Stripes)).ToList();
// can't have two promotions on the same day
var conflictingPromotions = editedMember.Promotions
.Where(promotion => newPromotion.Date.Date == promotion.Date.Date).ToList();
conflictingPromotions.AddRange(conflictingPromotionsBefore);
conflictingPromotions.AddRange(conflictingPromotionsAfter);
if (conflictingPromotions.Count > 0)
{
var errorMessage = string.Join(", ", conflictingPromotions.Select(promotion => promotion.Date.ToShortDateString()).ToList());
throw new ArgumentException($"Promotions from the following dates conflict with the one You are trying to add: {errorMessage}");
}
editedMember.Promotions.Add(newPromotion);
});
await context.SaveChangesAsync();
}
catch (Exception ex)
{
LogError(ex);
throw ex;
}
}
}
public async Task DeletePromotion(int promotionId)
{
using (var context = new BjjClubEntities())
{
try
{
var promotionToRemove = await context.Promotions.Where(promotionEvent => promotionEvent.PromotionId == promotionId).SingleOrDefaultAsync();
context.Promotions.Remove(promotionToRemove);
context.Entry(promotionToRemove).State = EntityState.Deleted;
await context.SaveChangesAsync();
}
catch (Exception ex)
{
LogError(ex);
throw ex;
}
}
}
}
Here is the controller:
public class PromotionsController : ApiController
{
private PromotionsRepository _promotionsRepository;
public PromotionsController()
{
_promotionsRepository = new PromotionsRepository();
}
[Authorize]
[HttpGet]
[Route("Promotions/{memberId:int}", Name = "PromotionsByMemberId")]
public async Task<IHttpActionResult> GetPromotionsForMember(int memberId)
{
return Ok(await _promotionsRepository.GetPromotionsForMember(memberId));
}
[Authorize]
[HttpPost]
[Route("Promotions", Name = "AddPromotionForMember")]
public async Task<IHttpActionResult> AddPromotionsForMember([FromBody] List<Promotion> newPromotions)
{
int memberId = newPromotions[0].MemberId;
PromotionValidator validator = new PromotionValidator(memberId, new PromotionsRepository());
if (newPromotions.Count == 0)
{
throw new ArgumentException("No promotions to add");
}
Common.ValidationResult result = await validator.ValidateNewPromotions(newPromotions);
if (!result.Result)
{
var message = new HttpResponseMessage(HttpStatusCode.Conflict)
{
Content = new StringContent(result.ValidationMessage)
};
throw new HttpResponseException(message);
}
await _promotionsRepository.AddPromotions(memberId, newPromotions);
return Ok();
}
[Authorize]
[HttpDelete]
[Route("Promotions/{id:int}", Name = "DeletePromotion")]
public async Task<IHttpActionResult> DeletePromotion(int id)
{
try
{
await _promotionsRepository.DeletePromotion(id);
return Ok();
}
catch (Exception ex)
{
var message = new HttpResponseMessage(HttpStatusCode.Conflict)
{
Content = new StringContent(ex.Message)
};
throw new HttpResponseException(message);
}
}
}
Try/Catch. Just create a single global exception filter that handles/logs all exceptions. Also, don't usethrow ex, just dothrow. Also, you should learn how to use dependency injection in order to instantiate instances of your classes, such as the Repository class/DbContext. DI can scope instances to the lifetime of the request, and will automatically dispose when finished. Lastly, your repository class isn't so much a repository because it contains a lot of business logic - just call it a service class. \$\endgroup\$