Where to run a duplicate check for an entity
Your problem with ValidateEntity appears to be that the validation occurs on SaveChanges and this is too late for you. But in Entity Framework 5.0 you can call the validation earlier if you wish using DbContext.GetValidationErrors. And of course you could also just call DbContext.ValidateEntity directly. This is how I do it:
-
Override the
ValidateEntity
method on theDbContext
:protected override DbEntityValidationResult ValidateEntity(DbEntityEntry entityEntry, IDictionary<object, object> items) { //base validation for Data Annotations, IValidatableObject var result = base.ValidateEntity(entityEntry, items); //You can choose to bail out before custom validation //if (result.IsValid) // return result; CustomValidate(result); return result; } private void CustomValidate(DbEntityValidationResult result) { ValidateOrganisation(result); ValidateUserProfile(result); } private void ValidateOrganisation(DbEntityValidationResult result) { var organisation = result.Entry.Entity as Organisation; if (organisation == null) return; if (Organisations.Any(o => o.Name == organisation.Name && o.ID != organisation.ID)) result.ValidationErrors .Add(new DbValidationError("Name", "Name already exists")); } private void ValidateUserProfile(DbEntityValidationResult result) { var userProfile = result.Entry.Entity as UserProfile; if (userProfile == null) return; if (UserProfiles.Any(a => a.UserName == userProfile.UserName && a.ID != userProfile.ID)) result.ValidationErrors.Add(new DbValidationError("UserName", "Username already exists")); }
-
Embed
Context.SaveChanges
in a try catch and create a method to accessContext.GetValidationErrors(
). This is in myUnitOfWork
class:public Dictionary<string, string> GetValidationErrors() { return _context.GetValidationErrors() .SelectMany(x => x.ValidationErrors) .ToDictionary(x => x.PropertyName, x => x.ErrorMessage); } public int Save() { try { return _context.SaveChanges(); } catch (DbEntityValidationException e) { //http://blogs.infosupport.com/improving-dbentityvalidationexception/ var errors = e.EntityValidationErrors .SelectMany(x => x.ValidationErrors) .Select(x => x.ErrorMessage); string message = String.Join("; ", errors); throw new DataException(message); } }
-
In my controller, call
GetValidationErrors()
after adding the entity to the context but beforeSaveChanges()
:[HttpPost] public ActionResult Create(Organisation organisation, string returnUrl = null) { _uow.OrganisationRepository.InsertOrUpdate(organisation); foreach (var error in _uow.GetValidationErrors()) ModelState.AddModelError(error.Key, error.Value); if (!ModelState.IsValid) return View(); _uow.Save(); if (string.IsNullOrEmpty(returnUrl)) return RedirectToAction("Index"); return Redirect(returnUrl); }
My base repository class implements InsertOrUpdate
like this:
protected virtual void InsertOrUpdate(T e, int id)
{
if (id == default(int))
{
// New entity
context.Set<T>().Add(e);
}
else
{
// Existing entity
context.Entry(e).State = EntityState.Modified;
}
}
I still recommend adding a unique constraint to the database because that will absolutely guarantee your data integrity and provide an index that can improve the efficiency, but overriding ValidateEntry gives loads of control over how and when validation occurs.
The most reliable place to perform this logic would be the database itself by declaring an unique field constraint on the name column. When someone attempts to insert or update an existing entity and try to set its name to an existing name a constraint violation exception will be thrown that you could catch and interpret in your data access layer.
First, your first option above is acceptable. The fact that another dev might not trap for a failure is not a huge drawback. This is always the case with validation: You catch it as early and elegantly as you can, but the main thing is to keep the integrity of your data. Simpler to just have a constraint in the database and trap for that though, right? But yes, you want to catch it as early as possible.
If you have the scope and time, better would be to have a smarter object that would handle saving, and perhaps other things you need handled consistently. There's many ways of doing this. It might be a wrapper for your entity. (See Decorator pattern, though I prefer to have my object consistently have a Data
property for access to the entity). It might require a relevant entity in order to be instantiated. Your controller would give the entity to this smart object to save (again, perhaps instantiating this smart object with your entity.) This smart object would know all the validation logic necessary and make sure it happens.
As an example, you could create JobRole business object. ("busJobRole", bus prefix for "business".) It could have a collection DataExceptions
. Your Controller takes the JobRole entity posted back, instantiates a busJobRole, and calls a method SaveIfValid
, which would return true if the item was saved successfully and false if there were validation problems. You then check the busJobRoles DataExceptions
property for the exact problems, and fill your modelstate, etc. Maybe like this:
// Check ModelState first for more basic errors, like email invalid format, etc., and react accordingly.
var jr = new busJobRole(modelJobRole);
if (jr.SaveIfValid == false) {
ModelState.AddModelError(jr.DataExceptions.First.GetKey(0), jr.DataExceptions.First.Get(0))
}
We follow this model so consistently, and I made an extension method for ModelState to accept a NameValue collection (returned by business objects) (vb.net version):
<Extension()> _
Public Sub AddModelErrorsFromNameValueCollection(
ByVal theModelState As ModelStateDictionary,
ByVal collectionOfIssues As NameValueCollection,
Optional ByRef prefix As String = "")
If String.IsNullOrEmpty(prefix) Then
prefix = ""
Else
prefix = prefix & "."
End If
For i = 0 To CollectionOfIssues.Count - 1
theModelState.AddModelError(prefix & CollectionOfIssues.GetKey(i),
CollectionOfIssues.Get(i))
Next
End Sub
this allows for quick, elegant adding of exceptions (determined by business objects) to the ModelState:
ModelState.AddModelErrorsFromNameValueCollection(NewApp.RuleExceptions, "TrainingRequest")
Your worry that other developers might not follow a plan you set up is very valid and good thinking. That's why your scheme needs to be consistent. For example, in my current project, I have two categories of classes that act as I've described. If they're very light, and just deal with caching and validation, they are "data manager" classes (ex: BureauDataManager). Some are true business domain object, very comprehensive, which I prefix with "bus" (ex: busTrainingRequest). The former all inherit from a common base class, to ensure consistency (and reduce code of course). Consistency allows for true encapsulation, for code discoverability, for the right code being in the right (single) place.