Reuse of rules for repeated/nested models

[grofit]

I raised this against the original aurelia validation lib: (no point repeating the waffle)

https://github.com/aurelia/validation/issues/100

Still think it is applicable when you have complex models or want to re-use validation concerns. It is one of the reasons I made Treacherous to solve this issue with complex objects (which also supports aurelia)

[plwalters]

Hey re: treacherous if you want any pointers for setting up a plugin to tie in to the core validation system let me know.

As far as customizing the validation is it safe to summarize that you want to be able to have standard validation for an instance of a class but also be able to configure that on a per-instance basis? If so I think this is supported now and I can show an example.

[plwalters]

Maybe we can add a doc with samples or update the current samples app to include these kinds of explanations as well.

[grofit]

Treacherous is its own validation system, the aurelia plugin just basically sits on top of it in the view layer and just subscribes to calls. That is not too important here but the thing that is important is why I made it in the first place.

As mentioned in that original issue I raised for aurelia-validation, the first incarnation required you to explicitly re-write rules for each object in most cases (as shown in the docs). I was after the notion of a ruleset or something which encapsulated the rules required to validate a JSON object/schema. So for example in treacherous I can do:

var itemRuleset= Treacherous.createRuleset()
    .forProperty("name")
        .addRule("maxLength", 50) 
        .addRule("alphanumeric")
    .forProperty("amount")
        .addRule("number")
     // lots of other rules
    .build();

Then lets say I use an item in various places, like an inventory, trade, equipment etc rather than having to re-write those above rules within each model which uses composition I could just re-use that ruleset, like so:

var inventoryRuleset = Treacherous.createRuleset()
   .forProperty("items")
      .addRulesetForEach(itemRuleset)
   .build();

Anyway so the notion was you could wrap up the rules required for validating a specific model for re-use, be it via composition or arrays of child elements etc. It currently looks like in the new system (which seems better than the old one) my notion of a ruleset is actually the validationGroup, so I have a layer before the validation group which is the ruleset.

It is down to preference I guess as you may not always need this flexibility and re-use, but that is one of the driving reasons I wanted to support that and nested complex objects (which knockout.validation struggles with , which is another reason I wanted to improve upon that). So anyway if you think this is useful as an idea take it as an enhancement, if not no worries.

[EisenbergEffect]

The good thing is, if we don't get it into our validation library, you should be able to plug it into the new general validation mechanisms of Aurelia. However, maybe you can work with @PWKad to see how this might work in our validation library. We have the ability to create a set of validation rules that are independent of an object. We may just need to add the ability to compose them into another set. I bet it wouldn't be too hard.

[mbroadst]

I think I'm getting to a point where I agree with changing the design towards creating rulesets. Here's my current situation:

I have a dialog fragment which I'm composeing into another custom dialog element, which accepts its record (model) as part of its activation. AFAICT the dialog fragment should be in charge of validation, however in order to create all the parts (validation binding behaviors most importantly) I need to create the validator in the ctor of the VM for the dialog fragment. This is a no-go, since I will be forced to create the validator on an object that is guaranteed to change during activated. My current (undesirable) solution is:

  activated(model) {
     this.record = model;
     this.validator.object = model;
  }

From reading through the source, it seems to me that the current validation module(s) are a little too concerned with the object being validated. You need the object to create a validator, to look up the ValidationReporter, etc. Rather, I think the ValidationReporter should be associated with a Validator (or a "ruleset"), and the validator itself should take an object as input to the validate method. This would get rid of a lot of chicken-egg problems as I mentioned above, but would also facilitate the intent (I think) of this issue, allowing users to reuse similar rulesets around their codebase.

Thoughts?

[grofit]

I think the main problem here is that if you want reactive validation you need to know about the model ahead of time, as otherwise you cannot really report on validation changes, just do snapshot validation (i.e what is the state of the model at a given time).

As mentioned before it shouldnt be too hard to refactor it to be able to support, that but in most cases if you were to change the model that often then maybe you should inject in some sort of validation factory and just clear/rebuild the validation group when it is changed.

Also just to be clear the original purpose of this feature was to support composition based validation rules, i.e you define set of rules and can use them within another set of rules to allow for re-use. It sounds like you are more after having the ability to do a one off validation pass on a model for a given ruleset, which sounds fine as a feature but I think it would be separate to this feature request.

[mbroadst]

@grofit how are they not the same feature? the core issue is a separation of concerns between the validation and the model being validated. Being able to create a validator that is essentially a builder pattern object for creating a collection of rules all of which can be stored as a "validator" allows the thing to be reused elsewhere.

[mbroadst]

@grofit to clarify (now that I'm not typing on a phone 😄), if I end up having a model that I need to validate within that document fragment at the time of activation then how is that different than ? Sure, it's technically on the same data path, but its just as valid a use case for needing a reusable validation ruleset that's not dependent on the data.

You are correct that figuring out how to signal validation errors will have to be thought out, but I definitely think a first start is towards creating a reusable ruleset, and I've made a PR to being working in that direction (and discuss these issues) over here: https://github.com/aurelia/validatejs/pull/36

[apawsey]

sorry guys to just jump in, but surely with composition the point is to contain everything required within each component. Therefore the idea here is not to create some separate definition of rulesets, but to contain the validation rules within the model that is part of the component. Then the only requirement becomes one of making validation component aware... connecting my output to the root/parent view's overall validation? or am I oversimplifying somewhere?

[mbroadst]

@apawsey I can't speak for the original intent of the issue (and yeah unfortunately I think I've sort of hijacked this conversation, perhaps I should open another issue), but in the case of e.g. a dialog fragment, you can't make the validation rules for the model since the model doesn't technically exist upon creation but rather when the VM is activated and the model passed in to it.

If I think about the validator as it stands now (more abstractly), it's a builder of rules to validate . There is an initial step (during ensure) that directly couples that set of rules to a particular object, and if we could work out a way to eliminate that coupling we would be able to satisfy the need to create reusable "rulesets", as well as the use case I presented above. Now, considering the current design observes properties on a model I'm not sure what the best way to go about doing this is (if there is in fact a way) but that's the gist of what I'd like to discuss

[apawsey]

@mbroadst but in your case, in my mind at least, I would create the validation inside the model you're passing into your dialog. You have a model (ie: a class). The current validatorjs pattern is to create the validation rules inside your constructor, so create the rules in your 'model-to-be-validated-inside-your-dialog' class, and then couple the output of that validation to the display in your dialog. That way the model is responsible for validating itself, and the dialog (which is purely a presentation construct) is responsible for the presentation.

The point here to me is simple... what purpose to validation rules have on their own? Without a model they are pointless, therefore there's no need for them to exist outside that context. There are arguments to be made regarding, "I have to validate an address in four places, I don't want four copies of the validation rules, therefore I want an address ruleset", but my argument is simply that you need an address component, which contains the address validation... reuse it four times, all done :D

[mbroadst]

@apawsey Ah, that's a really good point. In fact in the tests the validator is inside the model, and in the decorators example as well, but not in the fluent example. So I was basing my whole view of this on the assumptions put forth in the fluent example (in the sample app) where the validator is created in the VM itself.

I can see merits to all of these approaches, so I suppose we'll wait until the core devs chime in with their intentions!

[EisenbergEffect]

Personally, I favor flexibility. It seems to me that we should be able to support independent rulesets. There may be some slightly different API patterns needed when those patterns are used. We may need to work through that more.

The most important point, from the perspective of Aurelia's core, is that something talks to the ValidationReporter (or many somethings, groups of somethings, etc...it shouldn't matter). The Validate binding behavior listens to the reporter and when it receives errors, it correlates those to HTML elements which are bound to the associated error properties. The Validate behavior then calls out to a renderer, passing that correlated information along so the renderer (or renderers) can display the feedback to the user.

In this way, you wouldn't even need a validation system at all. Your own app code could publish info directly to the reporter, or our validatejs could do it, or some other library could do it....or the server could send back data that did it...etc.

[plwalters]

After thinking about it more this is basically also supported though perhaps the API could be improved.

class Model {
  name: '';
}
let a = new Model();
let b = new Model();
this.validator = new Validator();
this.validator.object = { name: 'fake' };
this.validator.ensure('name').required();
this.validator.object = a;
this.validator.validate(); // reporter outputs results for a
this.validator.object = b;
this.validator.validate(); // same reporter outputs results for b now

The trick is that the config exists typically for an instance but can be shared - but can only be accessed currently by the instance through metadata.

I think we could potentially make this easier by having validate() still receive the prop name but have an overload basically where if the param is not string and is not null/undefined (basically the instance to validate) and a second optional parameter which would be the property name if you want to validate only a single property.

this.validator.validate();
this.validator.validate('name');
this.validator.validate(modelObj, 'name');

Thoughts?

[plwalters]

To follow up the same scenario above in that case would want to look something like this -

class Model {
  name: '';
}
let a = new Model();
let b = new Model();
this.validator = new Validator().rulesOnly(); // method to set up the sharable config
this.validator.ensure('name').required();
this.validator.validate(); // returns error for no object to validate
this.validator.validate(a); // reporter outputs results for a
this.validator.validate(b); // same reporter outputs results for b now

[mbroadst]

@PWKad considering this is my exact proposal I say hurrah, yes lets do it! 😄

[apawsey]

I know I'm going to be going against the grain here, and obviously there's nothing personal in it, but I just can't get on board with this discussion.

I honestly think there's a difference between flexibility and inconsistency. And this is just starting to feel like we can cover everything at the cost of losing track of what the purpose is.

Overloaded methods that sometimes take property names, and other times take objects, is going to be super confusing. Methods that change the behaviour of the validator after i've constructed it, is again going to be very confusing. Is this validator a shared one or not, the only way to tell is find if I've called that method. What if with reusable rules, people start setting up validators all over the place, and then injecting them. Knowing what those validators are doing is never going to be obvious.

I think it would be better to go back to the original point which was separate rulesets, and separate those from the validation. So a validator is tied to an object, but you can add a set of preconfigured rules to that object. Otherwise to get a clear picture of the validity of your object, you going to have to pass it through multiple validators... and that's just nasty. This is also more in line with the example @grofit posted regarding his own system.

let personValidationRules = new ValidationRuleset()
                            .ensure('firstName')
                              .required()
                            .ensure('surname')
                              .required();

class Client {
  firstName: '';
  surname: '';
  IDNumber: '';

  constructor() {
    this.validator = new Validator(this)
                       .ensure('IDNumber')
                         .required()
                         .format(someRegEx)
                       .addRuleSet(personValidationRules);
  }
}

I wrote a five page rant on why I still feel like separate rulesets is just crazy and still confusing (and I really honestly can't stress how much I think that :smile:), but if the demand is there then let's do it. It just needs to be clearly defined. I also think that some of this demand will be reduced once it's clear how to add a custom validator (like password strength).

[EisenbergEffect]

From an architectural standpoint, it's nice to have the rules separated out. Something like this:

let rules = new ValidationRules()
    .ensure('firstName').required();

However, you could have a facade that would create the rules and immediately associate them with an object.

let object = {};

ValidationRules.for(object)
    .ensure('firstName').required();

Under the hood that just gets/creates an instance of ValidationRules and then automatically links it with the object.

I think the second use case is what most people will use, esp. inside of a class:

export class Person {
  firstName: '';

  constructor() {
    ValidationRules.for(this)
        .ensure('firstName').required();
  }
}

However, having things broken apart gives the flexibility to handle use cases which have that requirement. It also means it's not hard to share/combine rule sets. You could register ValidationRules as a transient with the container so they could be injected if desired. Given a ValidationRules, you could also apply it to an instance:

let object = {};
validationRules.applyTo(object);

In fact, this is how the helper function would work:

class ValidationRules {
  static for(obj) {
    let rules = new ValidationRules();
    return rules.applyTo(obj);
  }
}

[apawsey]

Taking your example:

export class Person {
  firstName: '';

  constructor() {
    ValidationRules.for(this)
        .ensure('firstName').required();
  }
}

The problem is that once you associate validation rules with an object, they are no longer an independent ruleset, but a configured validator. So therefore the helper method would be more like:

class ValidationRules {
  static for(obj) {
    return new Validator(this);
  }
}

using the same methods that currently exist on the validator to add new rules. Essentially this is just semantic sugar.

Whereas the applyTo method would be more like (slightly psudeo code for the sake of it's 12:30 am here :smile:):

class ValidationRules {
  applyTo(validator) {
    validationRulesCollection.forEach(rule => {
      validator.addRule(rule.clone);
    }
  }
}

(I know clone doesn't really exist, as I said, it's pseudo code ish).

I'll see if I can put together a PR for this in the morning because I think the simplest way to understand this will be with some thing to look at.

[EisenbergEffect]

It's not clear in my quick sample above, the the idea with applyTo is that it creates a clone of the ValidationRules and "binds" it to a specific instance. So, it's very similar to what you are showing @apawsey

[plwalters]

@apawsey There is no structural change needed to support this as I mentioned above it's simply a helper method that should actually make it easier. As far as having two signatures for the validate method would it be less of a problem if it were two separate methods? validate and validateAgainst()?

[EisenbergEffect]

@PWKad I definitely think having different method names for each scenario is a good idea in this case.

[grofit]

I don't mind what the syntax looks like be it cloning another rule or just adding an existing rule in.

I think the difference in what I was originally proposing and what the system can currently do stems from that I think (could be wrong, not really looked through source or used it yet) you do not have the distinction between the rules for a model and the validation of a model. So in the examples above when you are setting up the rules for a model you are implicitly creating the validation group for that model at the same time right? as you start with the model as the first part in the chain of creating rules.

However when I originally put this forward into aurelia-validation I was hoping to try and separate out the notion of making rules for a model, and creating something to validate a model. This is mainly why when doing Treacherous it was key to have a separation between rules and validation so the rulesets/rulelinks are basically just POCOs so they can be passed around easily and contain no logic so its easy enough to just pass them around.

Anyway to get back on topic the cloning seems a possible solution which would allow you to achieve composable rulesets (which is what I was ultimately after).

[apawsey]

Ok, well I got carried away playing with this. There is a stupidly large and complicated PR that was the result of a few nights learning new javascript stuffs. #56 Cue the vicious attacks on my js coding...

[mbroadst]

To reiterate a few points and maybe make some new ones: I think the biggest identified problem with the current solution is that what we call a Validator is really better termed something like a SchemaValidator, where the object is at once the schema definition (constraints) as well as the thing that applies the rules and notifies.

I think the most simple, flexible solution is to fully commit to that separation:

• ValidationRules/ValidationSchema/Constraints whatever you want to call them, are simply a collection of rules which can also optionally be composed (let nameRules = firstNameRules.compose(lastNameRules);)
• Validator becomes a simple "configuration" class that provides a method: validate(data, constraints) that notifies reporters, or even takes a callback as the final parameter: validate(data, constraints, err => {})

At very least I think its starting to make sense to standardize on some names here, because I think it's causing a lot of confusion.

[apawsey]

Take 2 on a test of this... #58 ... busy creating a gist of the sample app.

[plwalters]

@mbroadst I'm not against the approach you've mentioned but to re-iterate -

Today we have

class Validator // the view-model way of adding validation rules, validating, etc...
class ValidationEngine // the universal way to get the config and the proper reporter for an object
class ValidationConfig // the underlying metadata for the object that contains rules

if we create a ValidationRules or w/e does that simply replace the current ValidationConfig?

[jdanyow]

the ValidationRules fluent API now supports rule reuse: http://blog.durandal.io/2016/06/14/new-validation-alpha-is-here/