DataMapper, Validation order and Short-Circuit

DataMapper, Validation order and Short-Circuit

Old forum URL: forums.lhotka.net/forums/t/2257.aspx

ballistic posted on Thursday, February 01, 2007

I am having trouble getting my objects to validate in order and to short-circuit.

I have a Member Class where I have a username and an email.  In my AddBusinessRules() method I call the validation rules that check the input format first (since no db calls are made), I then have the rules that actually check if a username or email already exists at a higher priority (>50).  In addition, I set the ProcessThroughPriority to go right before the db calls (49).

Now, when I use DataMapper.Map it goes through and reads the values from my form and sets my object with those properties.  Since each of my properties calls the PropertyHasChanged() after it has been set, this envokes the validation to occur, but it does not follow the priorities I set in the AddBusinessRules(), furthermore, even if I set the e.Severity = Validation.RuleSeverity.Error in a validation method that which has been given a priority < the ProcessThroughPriority, other validation rules are still getting called.

I probablymisunderstand how the validation works so I'm hoping someone please shed light on this.

Thank you,

- Andy

xal replied on Thursday, February 01, 2007

I think you got it backwards... 0 is the highest priority and rules in that priority run first, then 1, then 2, etc.
If a rule at level 2 returns an error, level 3 and beyond are not validated.

Andrés

ballistic replied on Thursday, February 01, 2007

You are right, what I ment to say by "higher priority", was that the number was higher (>50).

Below is my AddBusinessRules method which shows the format validation first, and then the validation that actually goes out to the db.

    Protected Overrides Sub AddBusinessRules()

        ' Check username is valid format
        ValidationRules.AddRule(Of MemberAccount)(AddressOf isUsernameValidFormat, "Username", 5)

        ' Check email address is valid format
        ValidationRules.AddRule(Of MemberAccount)(AddressOf isEmailValidFormat, "Email", 10)

        ValidationRules.ProcessThroughPriority = 49

        '' Begin Database calls
        ' Check username is unique
        ValidationRules.AddRule(Of MemberAccount)(AddressOf isUsernameUnique, "Username", 50)

    End Sub


The problem I am having is that each time the variables are set, the PropertyHasChanged() is called, which triggers my validation to be called on that specific property, which does not follow the order in the AddBusinessRules.

What I'm looking for is for all the validation rules to get called in that order and if any fail with a priority < 50 to not execute the ones with a priority >= 50.

Any suggestions?

ballistic replied on Friday, February 02, 2007

Think I found a solution to my problem.  My main issue was having the validation called specifically for a property whenever I call PropertyHasChanged().  Fortunately, Rocky allows us the ability to overload that function!!

Protected Overloads Sub PropertyHasChanged(ByVal propertyName As String)

'ValidationRules.CheckRules(propertyName)  '<-- removed the call to validate
MarkDirty(True)
OnPropertyChanged(propertyName)

End Sub

Also, I call the the CheckRules() to validate all my rules, in order, with the ability to short-circuit them.

Public Overrides Function Save() As MemberAccount

ValidationRules.CheckRules()
Return MyBase.Save

End Function

Using this method, I was able to achieve the behavior I was after. 

Please let me know if anyone sees any problems with this approach.

- Andy

xal replied on Friday, February 02, 2007

Right, priority is aimed at one property, not a set of properties.
Maybe this raises the need for a new functionality. A lot of us have rules that depend on more than one property, and in an asp like scenario, the same rule may be triggered more than once because we set all properties at once.

It would be nice to have something like the pattern used in windows forms like:

myObj.SuspendValidation()
<set all props here>
myObj.ResumeValidation()

The rules manager could keep track of what it needs to validate when the validation is resumed and thus call those validation rules once.

Andrés

ajj3085 replied on Friday, February 02, 2007

The biggest problem I see is that your UI will think the object is valid and you usually enable / disable the save button based on the IsValid property.  So your UI would allow the same, the BO would suddenly check rules, find one that is broken and throw an exception.

That seems to be using exceptions to handle runtime logic which is not usually a good idea.  But with your changes, you've forced your way into that pattern.

ballistic replied on Friday, February 02, 2007

ajj3085,

I am using CSLA for a site, which means I do have the user data right after it has been entered in a form, so I cannot disable the submit button if a field is not valid.  Instead I have all the data once they click on submit, which I believe presents an opportunity to have all my formatting checks done first, and then all the db calls done vs. validating each property as it gets set.

You are right, using the approach I described, I am depending on exceptions to tell me if there was a problem since I am not doing individual property validation.

The UI would call the Save() method, which would call the CheckRules() to check if ALL the properties are valid and expect a ValidationException if they are not.  The UI would not make use of isValid and instead would always call Save() and depend on the exceptions, which doesn't seem to be an ideal approach.

The only way I can think of making use of the isValid is by having the UI call the ValidationRules.CheckRules() and then if isValid, call Save.  Does it make sense for the UI to call CheckRules?

I am also looking at Andres' suggestion about using SuspendValidation() which gives me the flexibility to do individual property validation instead of batch validation if I ever needed that approach.

- Andy

ajj3085 replied on Friday, February 02, 2007

Hi,

Even if you can't disable the Save button, you'd probably want to check IsValid before calling save.  If IsValid is false, I would think your page would redisplay with errors.  At least that's the normal pattern, and its probably best to stick to it.

If you need to do this, maybe what you should do is expose a method called Check to the UI, which would just call the CheckRules.  I think that would be better than the try catch method.

I actually didn't see Andre's suggestion until now, so I can't comment on it.

Andy

xal replied on Friday, February 02, 2007

ajj,
(I'd say andy, but all three of us in this conversation can be called that way, so trying to avoid confusion... :D )

I don't think that pattern I mentioned would ever be a problem because:

You'd rarely use it in a windows forms environment, since most of us use databinding.
Even if you did use it and your object was using databinding and you use IsSavable to enable / disable  the save button, you should be aware of the issues it may have and so, you'd refresh that binding. Maybe raising OnUnknownPropertyChanged is enough, depending how you implemented that.


I think the aim of something like that would be to solve the issue in asp, where you usually set all your props at once and you have dependant properties.
It could be used for batch processing also.

The point is that it would be a feature that you're in no way forced to use.


Andrés

JoeFallon1 replied on Friday, February 02, 2007

I use this pattern in my web app Save Button handler:

If mData.IsValid Then
  mData = CType(mData.Save, MyBO)
  ClearState()
  Util.DisplayMessage("Save succeeded", "Your changes to " & mData.code & " were saved.", "Home.aspx")
Else
  Util.DisplayMessage("Save failed", "Save failed because: " & vbCrLf & _
     AllRules.GetAllBrokenRulesString(mData), "ReturnToThisPage.aspx")
End If

By calling IsValid before Save we can display the list of broken rules if necessary.

Also, in my code for IsValid I sometimes call CheckRules() one last time. Or I may check some specific rules only. This way, all values from the web form have already been unbound into the BO and CheckRules can handle any required dependencies. (For example, on a search form I have 5 check boxes, if none of them are checked then they can't search for something. But I do not want to evaluate this rule 5 times - once for each property set - I only want to evaluate it once all 5 properites have been set.

Joe

ballistic replied on Friday, February 02, 2007

Thank you all.

After taking your input, this is the approach I am now considering.

Override the PropertyHasChanged so that it does validate each property one at a time. Along with that, use xal's method to turn this feature on when I need it. Otherwise, allow PropertyHasChanged to behave as normal.

myObj.SuspendValidation()
<set all props here>
myObj.ResumeValidation()

Protected Overloads Sub PropertyHasChanged(ByVal propertyName As String)

If Not (_IndividualPropertyValidationSuspended) Then 'Validation is not suspended on a per property basis
ValidationRules.CheckRules(propertyName)
End If
MarkDirty(True)
OnPropertyChanged(propertyName)

End Sub

Also, call CheckRules from the object's IsValid method which will validate the properties and return false if any have been broken, instead of depending o the Save() method to raise an exception.

Again, let me know what you think of this approach.

- Andy

ajj3085 replied on Friday, February 02, 2007

Andre,

Sorry for creating some confusion... I actually wasn't commenting on your suggestion, I didn't see it until after I posted.  Smile [:)]

I haven't done a .Net 2 website yet, but didn't they fix the databinding so that it works more like WinForms once the page is submitted?  Not saying that would solve the problem in this thread, just a question for my own information.

Andy

xal replied on Friday, February 02, 2007

Well, there are some improvements, but the fact remains that a user can fill all of their properties in the web form, click a button and at that point, everything travels back to the server and gets set all at once.

So, at that point all of the properties get set and here's what happens:

PropertyA and PropertyB are dependant on each other, that means, whenever PropertyA gets set, rules for both properties are validated, and vice versa.

Now, at this point, you know you're going to be setting them both at the same time, one after the other, which means that all rules for both props are validated twice.
In windows forms, you usually expect immediate feedback as a property changes, so that's "ok". But in asp all of this runs on the back, and thus the double validation is not only pointless, but pottentialy consumes more resources on a stressed server. This of course all depends on the "heaviness" of your rules. If we're talking about StringRequired, it's not going to make much difference, but if you need to access the db to validate a rule that checks for uniqueness in a very big table, you don't want to run that twice when you only really need that once...

So, I'd say it would be a valuable feature in csla and hopefully Rocky will implement it.

Andrés

RockfordLhotka replied on Saturday, February 03, 2007

I might, though I think I'd do it on ValidationRules, not on BusinessBase.

In other words, I may enable the concept, but leave the choice of externalizing it to the UI up to you.

In any case, I'll add this thread to my wish list.

ajj3085 replied on Monday, February 05, 2007

Ahh, that's true, I didn't think about it beyond on page submission.  That sounds like it would be a good optimization.

Copyright (c) Marimer LLC