Input requested: Business rule subsystem

I'm in the middle of reimplementing the business/validation rules subsystem for CSLA 4, and would value the community's thoughts on the following:

The CSLA business/validation rules subsystem has operated at a method level through its existence. You use AddRule() to link a delegate pointer to a method to a property of your business object.

That has advantages - it is low overhead and it is easy to implement a rule - it is just a method after all.

public static bool MyRule(object target, RuleArgs e)
{
  // return true if success or
  // set e.Description and return false if failure
}

The primary alternative model is to elevate a rule to an object level. So a rule is defined as a method in a class, where that class typically implements an IBusinessRule interface or something like that.

public class MyRule : IBusinessRule
{
  public void Rule(RuleContext e)
  {
    // use e.Results.AddResult() to indicate failure
  }
}

This requires an instance of MyRule be created, so there's a little overhead, and a little more code since the rule isn't just a method - it is also a class.

But the advantage is that there can be a BusinessRule base class that you subclass to create rules. That base class can expose protected members much like ObjectFactory does.

public class MyRule : BusinessRule
{
  public override void Rule(RuleContext e)
  {
    var x = ReadProperty(e.Target, Customer.IdProperty);
    LoadProperty(e.Target, Customer.DataProperty, x + 1);
    // use e.Results.AddResult() to indicate failure
  }
}

This would impact AddRule() of course - because your code would be more like this:

BusinessRules.AddRule(new MyRule(), IdProperty);

I'm strongly leaning toward doing this however, because it radically simplifies the process of passing parameters. Since the rule is your code you can have a meaningful ctor.

public class MaxLength : BusinessRule
{
  public int Max { get; private set; }

  public MaxLength(int max)
  {
    Max = max;
  }

  public override void Rule(RuleContext e)
  {
    if (e.PropertyValues[0].ToString().Length > Max)
      e.Results.AddResult("Value exceeds max length", RuleSeverity.Error);
  }
}

Then the add is like this:

BusinessRules.AddRule(new MaxLength(50), NameProperty);

which is much, much simpler than the current model where you have to create a custom RuleArgs subclass and the business object developer has to magically know to use your custom args type, etc.

One final point - no matter what, the CSLA 4 model will be a breaking change - no existing rule methods will carry forward without at least a little change. So arguing against this idea because it is a breaking change isn't really valid - I'm more looking for thoughts on whether the idea has broad merit over sticking with the current delegate/method-based model.

Thoughts?

I like it.

The RuleArgs classes were simple enough but if we can just get rid of them then even better. The idea of a Base Class for rules is interesting too.

Here is one vote in favor.

Joe

 

A few thoughts... just throwing this out there.

Would each instance have its own instance of the rule objects?  Would it be somehow possible to "custom build" rule instances? 

Do you think this would lead to a rule object which does too many things?  Or would you consider it valid for there to essentially be multiple rules.  Say a ContactNameRule, which 1) requires a value, 2) says the value can't be more than 15 characters and 3) forces the value to all upper case.

Will we have lots of problems because people make rule objects which have their own state, and thus one instance of a BO interfers with another?  Along those lines, what about thread saftey?  Especially if the rule instances are shared (one per type) and two BOs on two threads execute the same rule at the same time.

Good thoughts Andy.

Each type will have its own instance of the rule object. So you can have a rule type and apply it to different business object types - a different rule instance per business object type.

I'm not sure what you mean by "custom build"?

You could create a rule object that does many things. It is a specific goal to be able to create a "rule" that is actually invoking numerous rules - such as (for example) an external rules engine or workflow. Or all the ValidationRule attributes for a property. That's a goal regardless of whether a rule is an object or a method.

It will be absolutely possible for people to create bad rule objects :)   Rule objects must be designed to be threadsafe - so the Rule() method must be atomic and stateless. I can't help people do that right, but I can't provide ObjectFactory-like support without putting the rule methods in an object instance.

Hello,

I'd personally prefer a class that implements IBusinessRule. It's easier to read and enforces code reuse.

Thanks
-Blake Niemyjski

Hi,

I like the idea of a Rule base class, so one vote in favor from me!

Cheers,
Herman

Rocky, 
My vote for instance rule.  The real question to me is what available on RuleContext.  Looks like you can access property values and add validation result. 

It'll be great if we RuleContext also keep previous value, broken rule result for other property, and also maybe ability to mark a property to also be validated.

Ricky

This is good, but I think you need to up the ante a little bit here.

Unless I am missing something obvious, I see no reason why no to be able to include the PropertyInfo parameter in the rule class too.

So can you please consider something like the following?


---------------------------------------------------------------------------------
// Base class for all CSLA rules.
abstract class BusinessRule<T>
{
// To be used by the CSLA to determined
// what property this rule applies to
internal PropertyInfo<T> ThePropInfo
{
get;
private set;
}

protected BusinessRule(PropertyInfo<T> prop)
{
ThePropInfo = prop;
}

abstract public void Rule(RuleContext e);
}


---------------------------------------------------------------------------------
// Implementation of BusinessRule<T>.
// Note: This is a rule for **string** property
// so generic T is specified as string.
class MaxLength : BusinessRule<string>
{
public int Max
{
get;
private set;
}

// This class is meant to be used by a string
// property and we are able to enforce that
// right on the constructor.
public MaxLength(int max, PropertyInfo<string> theProp)
: base(theProp)
{
Max = max;
}

public override void Rule(RuleContext e)
{
// My rules was design to work with string
// and since I only allow PropertyInfo<string>
// types I can rest assured that I wont get any
// surprised dealing with properties that don’t
// return a string.
}
}


---------------------------------------------------------------------------------
// Usage.
BusinessRules.AddRule(new MaxLength(50, NameProperty));

Binding the entire concept to IPropertyInfo is maybe not a bad idea - though it would pose challenges for property-independent rules. That may be solvable another way though.

The rule itself wouldn't work as you suggest. A rule is independent of any specific object type - or at least can be. Think about these things as being not unlike ValidationAttribute subclasses in the Microsoft DataAnnotations model - but more flexible in their usage perhaps - in that these can be for business processing and not just validation.

In my current (very fluid) thinking there are just a small number of types:

1. BusinessRule (abstract implementation of IBusinessRule)
1. DefaultDescription
2. DefaultSeverity
3. DefaultStopProcessing
4. UserState - object
5. Rule(RuleContext context)
2. RuleContext (passed to rule on invocation)
1. InputPropertyValues - Dictionary<IPropertyInfo, object>
2. PrimaryProperty - IPropertyInfo
3. Result - List<RuleResult>
4. RuleDefinition - RuleMethod
5. Target - object
6. IsAsync
7. AddResult(RuleResult result)
8. Complete()
3. RuleMethod (links rule to object property)
1. InputProperties - List<IPropertyInfo>
2. IsBusy
3. PrimaryProperty - IPropertyInfo
4. Priority
5. Rule - IBusinessRule
6. Invoke()
7. BeginInvoke()
4. RuleResult (contains results that feed into BrokenRules)
1. Description
2. OutputPropertyValues - Dictionary<IPropertyInfo, object>
3. Properties - List<IPropertyInfo>
4. Severity
5. StopProcessing
6. Success - bool
5. BusinessRuleManager (essentially the existing manager)
1. AddRule()
2. AddDependentProperty()
3. CheckRules()
4. etc...

BusinessRuleManager will maintain lists of RuleMethod objects - the associations between rule objects and business object properties.

Each association will have its own instance of an IBusinessRule/BusinessRule.

Each invocation of a Rule() method will get a new instance of RuleContext, which will contain copies of the required business object property values. This ensures that the rule can always safely use those values, even in an async context.

The RuleContext does include the Target property - a real reference to the business object. Using that in an async context will be certain doom, but I'm not sure I should coddle people. If they do stupid things like using Target while async, that's their problem. Or should I ensure that is null if the rule is invoked async? Maybe I should do a little coddling?

The RuleResult is pretty rough right now - I'm still working out how to redo BrokenRulesCollection to be somewhat more efficient in terms of its key values The challenge here, is that you now get to make up your own results - plural. So I might just always blank the broken rules for a property and then add in any results values - dropping the unique rulename key requirement entirely.

I also don't yet know how or if the rule:// URI concept will carry through. I'm not so sure it even matters now that DataAnnotations exist and do basically the same job but better.

For example, here's a MaxLength rule using the proposed model:

    public class MaxLength : BusinessRule
    {
      public int Max { get; private set; }

      public MaxLength(int max)
      {
        Max = max;
      }

      protected override void Rule(RuleContext context)
      {
        var value = context.InputPropertyValues[context.PrimaryProperty].ToString();
        if (value.Length > Max)
          context.AddResult(
            string.Format("{0} value too long", context.PrimaryProperty.FriendlyName));
      }
    }

No reflection needed because the property value(s) are provided to the rule. No magic needed to get the friendly name, because it is in the IPropertyInfo object. No custom RuleArgs subclass needed, because the Max value is provided to the rule when the rule is instantiated.

 

The only thing I would say is that it seems we are loosing a bit of compile time checking going from existing AddRule and "staiuc bool SomeRule(T tartet, RuleArgs e)" to the new model.

Thanks
Sergey

True - at the moment the types are not generic, so the Target property is of type object.

I tried making the types generic, but that seemed to be complicating things substantially. The reason is that in the end the only place you care about Target being a specific type is in the rule itself. That implies that IBusinessRule and BusinessRule need to be generic, and that they'd have a constraint on the type of T. Which means the rule types would always be hard-coded to a specific business object type.

That is a high price to pay, I think, for avoiding what comes down to one cast.

I like the new Rule class - so add my vote for that.

Have you considered changes  to (removing) FriendlyProperyName in PropertyInfo? Seems like it is only going to work as expected in Client scenarios and not in serverside applications. If you have a multi-culture application and have FriendlyName read from a resource file the serverside part would only use the "culture" from the first caller that initialized the static PropertyInfo objects.

I hadn't thought about removing FriendlyName.

DataAnnotations has an attribute for a display/friendly name. Ideally that attribute would be used as the source value for a friendly name - and it supports localization.

So perhaps the answer is to provide a function (maybe in the BusinessRule base class or in PropertyInfo<T>) that gets the display name value from DataAnnotations.

how useful is the Target property given that it's usage in an async context will cause problems.... in fact, what would the solution for an async 'mutex' rule look like?