13

Here's the scenario:

  • ASP.NET MVC2 Web Application
  • Entity Framework 4 (Pure POCO's, Custom Data Context)
  • Repository Pattern
  • Unit of Work Pattern
  • Dependency Injection
  • Service Layer mediating Controller -> Repository

So basically, all the cool stuff. :)

Flow of events for a basic UI operation ("Adding a Post"):

  1. Controller calls Add(Post) method on service layer
  2. Service layer calls Add(T) on repository
  3. Repository calls AddObject(T) on custom data context
  4. Controller calls Commit() on Unit of Work

Now, i'm trying to work out where i can put my validation.

At this stage, i need two types of validation:

  1. Simple, independant POCO validation such as "post must have a title". This seems a natural fit for Data Annotations on the POCO's.
  2. Complex business validation, such as "cannot add a comment to a locked post". This can't be done by Data Annotations.

Now, i have been reading "Programming Entity Framework, Second Edition" by Julie Lerman (which is excellent BTW), and have been looking into hooking into the SavingChanges event in order to perform "last-minute" validation. This would be a nice way to ensure validation always happens whenever i do "something" (add, modify, delete), but it's also a little late IMO (as the items are already in the state manager) - so what can i do if validation fails, remove them?

I could of course make my POCO's implement an interface (say "IValidatable"), and call a method on this interface during this event.

But this seems "too late" for business validation - is this the consensus?

I'm basically looking for guidance here, i'm trying to design a re-usable, intelligent validation scheme for complex business logic, given my above architecture.

Another curve-ball for you - as you know, POCO's with EF mean the POCO's have all the properties on the DB - so i might have a "PostID" property, with get/set accessors (as EF needs to get/set these properties).

But the problem is, "PostID" is an identity column, so how do i protect the field from being explicity set? E.g if i (for some reason) do the following:

var post = service.FindSingle(10);
post.PostId = 10;
unitOfWork.Commit();

This will throw a SqlException. How can i prevent this? I can't "hide" the property (make it private, or even internal) as the POCO's are in a seperate assembly to the Repository.

A note on validation - i'm planning to create custom exceptions (deriving from Exception). So when validation fails, i need to throw these exceptions.

That way, i can code something like this on my controller:

[HttpPost]
public ActionResult AddPost(Post post)
{
   try
   {
      IUnitOfWork uow = new UnitOfWork();
      postService.Add(post);
      uow.Commit();
   }
   catch(InvalidPostOperation ipo)
   {
      // add error to viewmodel
   }
}

Will i have to manually do validation on the service layer everytime i do an Add? Then how can i handle Save? (as this is on the Unit of Work, not the service layer).

So to prevent this from being a "all over the place" question, here are my questions:

  1. Simple POCO validation - should this be done with Data Annotations? Pros/cons/gotchas?
  2. Under what circumstances (if any) should we be hooking into the SavingChanges event of the EF Data Context in order to provide validation?
  3. Where should i be performing complex business validation? In the service explicity, or a method on the POCO's (which i can call from service). How can i create an intelligent/reusable scheme?
  4. How can we "hide" auto-generated properties of POCO's from being tampering with?

Any thoughts would be most appreciated.

Apologize if this post is "too long", but it's an important issue and one that can be solved in many ways, so i wanted to provide all the info in order for the best possible answer.

Thanks.

EDIT

The below answer is helpful, but i'm still (ideally) looking for more thoughts. Anyone else?

RPM1984
  • 72,246
  • 58
  • 225
  • 350
  • Too many questions in one, but regarding #3, keep in mind that validation is context sensitive (or task-based). It's tempting to make an `IsValid` property on your `Post`, but somewhere along the line you'll then need an `IsValidForDraft` which is used for saving drafts that lack certain information. And certainly the POCO shouldn't be aware of the concept of "draft publishing". :) – bzlm Oct 07 '10 at 06:25
  • 1
    Aw come on, why the vote to close? I realise its a `long` question. But that's why i've got the 4 questions at the end. Just because im asking 4 things, doesn't mean this Q deserves a vote to close for being 'too vague'. All 4 q's relate to the one topic. – RPM1984 Oct 07 '10 at 06:41
  • @bzlm - yep, got your point. :) – RPM1984 Oct 07 '10 at 06:42
  • 1
    WTF? Someone just downvoted about 5 of my questions for no apparent reason. Show yourself coward! – RPM1984 Oct 10 '10 at 22:55

2 Answers2

1
  1. Well like you said, DataAnnotations is not appropriate for all situations. Cons are mainly complex validation (multiple property and multiple property different object) in my experience.
  2. If i were you, i would leave business/domain validation out of the Data Layer (EF) as much as possible. If there is a Data Layer validation scenario, then fine (eg. validating complex parent/child relationships - this is purely DB stuff).
  3. Yes, the complex business validation should be in the Service Layer or in the Model Objects (attached, via partial classes or some inheritance approach: interfaces/derived classes). There's debate about this between ActiveRecord people, Repository Pattern people and DDD people, but go with what works for you, is simple and will enable rapid deployment and low cost application maintenance. This is a simple example of how you might attach more complex validation to domain objects yet is still compatible with the DataAnnotations interface and thus is 'MVC friendly'.
  4. Good question. -one i have not found a solution i'm 100% happy with yet. I have played with the idea of private setters and it's not great. Have a quick read of this summarized Evans DDD book. It's great quick read and it might provide some insight about the purpose and difference between Model Objects and Value Objects. This is where i think object design will mitigate the problems your having with the property "tampering" (as you call it) but without fixing the property visibility. Ie, another solution might lie elsewhere. Hope this helps.
Community
  • 1
  • 1
Matt Kocaj
  • 11,278
  • 6
  • 51
  • 79
  • Thanks, im downloading and having a look at that book. I'm fairly content with basic data annotations on the POCO's, more interested in complex validation. That example is very interesting (and clever) will take that into consideration. With 4) can't make it private. As i said, the POCO's are in one assembly (no EF ref), and the EF/repo is in another. So it needs to be public. I thought there was a way to say "setter only available to a specific assembly". The thing is, i only want the repository to have "set" access to the PostId property. – RPM1984 Oct 07 '10 at 02:25
  • That visibility modifier your thinking of is `Internal` and it's for classes only i think (98%?). Not properties. Sorry i couldn't help more with #4, that's why i suggested the book. I have the same concerns. One thing that might help tho is this? Have you considered not having it in a separate assembly? I used to have all my solutions with separate Data, Service, Test and MVC/Web projects in them till i decided that's too much maintenance overhead. Now i have the Data/Model/Service all in the one project/assembly. Easier for me and i can still get the same logical separation i need. Try it. – Matt Kocaj Oct 07 '10 at 02:29
  • I could be wrong, [you may be able to set a type member to `Internal`](http://msdn.microsoft.com/en-us/library/7c5ka91b.aspx) – Matt Kocaj Oct 07 '10 at 02:43
  • yes you can - but as i said in my question (and my comments), the POCO's are in a `different assembly` to the EF/repos. Marking them `internal` will mean basically nothing can touch them (internal = only classes in assembly) – RPM1984 Oct 07 '10 at 02:45
  • haha. you're right. i must have been asleep. dang! ahh well, back to the old suggestion: collapse projects? – Matt Kocaj Oct 07 '10 at 02:47
  • RE: same assembly. no way bro. :) kind of OT, but it depends on how you have your repo setup. We return IQueryables from the repo (deferred exec) - the service layer does logic on that (filtering, ordering, paging, validation), then returns the list. This way we have a nice seperation of concerns. I dont want it all in the one assembly, i know you can put the layers in 'seperate classes'. But still, too many reasons to go into why you `shouldnt` do this. To each there own tho. – RPM1984 Oct 07 '10 at 02:47
  • No worries man. I'm prob doing the exact same thing you are but hey. Just a suggestion. – Matt Kocaj Oct 07 '10 at 02:55
  • @cottsak - BTW, i asked another Q about the POCO setter access, and StackOverflow has helped once again: http://stackoverflow.com/questions/3878391/how-can-i-hide-setters-from-all-but-one-assembly – RPM1984 Oct 07 '10 at 03:16
1

Hey, probably a bit late but here goes anyway...

It all depends on your architecture, i.e. Is there a logical seperation, in your application: UI, Service Layer, Repository Layer. If you are hooking onto the Save event, how exactly will that be done? From what I observed you would be calling the repository Layer for Persistance only right? However you are hooking onto the save event, giving control back to the Service Layer/ Business Layer whatever then forcing the save through right?

I personally feel the Service layer/ Business Layer should take care of it in completion then say, hey mr repo layer -> save this object.

With regards to validation, Data Annotations should be used with the UI, so simple valiation like [Required] etc, this will be helpful with the Client Side validation but complex business logic or complex validation should be hooked into the service layer/ business layer, that way it is reusable across all entities/ objects/ POCOS etc.

With regards to preventing certain private fields not being tampered with... only allow your service layer/ business layer to actually set the object that will be persisted (yes i mean :) ...) hand coding it, I felt this was the safest option for me anyway, as I will simple do:

var updatedpost = _repo.GetPost(post.postid);
updatedpost.comment = post.comment;
updatedpost.timestamp = datetime.now;

Kind of wasteful but that way your buseinss layer takes control, however this is just my experience I may be wrong, I have read a lot into model binding, validaiton and other stuff however there seemed to be cases where things never work as expected e.g. [Required] attribute (see Brad WIlson's) post.

Haroon
  • 3,402
  • 6
  • 43
  • 74