aurelia / validation

A validation plugin for Aurelia.
MIT License
132 stars 128 forks source link

Allow validation rule re-use or maybe decouple rule creation from validation group creation #100

Closed grofit closed 8 years ago

grofit commented 9 years ago

Summary

After chatting in gitter it was mentioned that hopefully there will soon be support for #10, which kinda ties into this. However let me bore you for a few minutes with a scenario and how the problem causes duplication of rules.

Scenario (Lots of waffle)

So if I have a model, lets say Item, which looks like this (using TS):

export class Item
{
   public Name: string;
   public Description: string;
   public Weight: number;
   public Value: number;
}

Now to validate this object I need to do 3 things:

That's all fine and I have an Create Item Page with the following validation setup:

export class CreateItemVm
{
    public Item = new Item();

    private itemValidationGroup = validation.on(this.Item) // lets assume `this` will work here
        .ensure("Name")
            .isNotEmpty()
            .hasMinLength(5)
        .ensure('Description')
            .hasMinLength(5)
        .ensure('Weight')
            .containsOnlyDigits()
        .ensure("Value")
             .isNotEmpty()
            .containsOnlyDigits();
}

Then I can add as many methods within my VM as I want and can then do:

this.itemValidationGroup.validate()
   .then(() => { /* do something, its valid */ })
   .catch(() => { /* do something, its failed */});

Now at this point its fine and this is an intended use case I assume, however then lets assume I need to re-use this validation on another page, such as an Edit Item Page, I then have to duplicate the above rules for this VM unless I make a class to separate the rule generation out to make it more reuseable, like so:

export class ItemValidationRules
{
    public createValidatorFor(item: Item) {
        return validation.on(item)
        .ensure("Name")
            .isNotEmpty()
            .hasMinLength(5)
        .ensure('Description')
            .hasMinLength(5)
        .ensure('Weight')
            .containsOnlyDigits()
        .ensure("Value")
             .isNotEmpty()
            .containsOnlyDigits();
    }
}

Now at this point I can quite happily replace all bespoke validation logic with a call to this class (Which can be DI'ed in) and then I can re-use the validation rules in both pages.

HOWEVER! then we get a requirement to create an Inventory page where you can view all your existing items and edit them in the list or create new ones in a modal and add them to the list.

So our inventory object would look something like:

export class Inventory
{
   public Items: Array<Item>;
}

Then our inventory page would look something like:

export class InventoryVm
{
    public Inventory: Inventory; // Powers the list that can be quick edited
    public NewItem: Item; // The new item which can be created in a modal then added to the list if valid

    private itemValidationRules: ItemValidationRules;
    private newItemValidationGroup = this.itemValidationRules.createValidatorFor(this.NewItem);

    private inventoryValidationRules = validation.on(this.Inventory)
        .ensureForEach("Items") // Assuming #10 is done
            .ensure("Name")
                .IsNotEmpty()
                .etc(); // repeat all the same stuff as in the Item validation
}

The problem

Now at this point the problem rears its head. As the way that validation is setup (being explicitly tied to the creation of a validation group) you cannot really get around being able to re-use the same validation rules in both scenarios. So without writing lots of custom stuff it is not possible to re-use the validation rules in a composite way as it stands.

Now ideally I just want to have the notion of "this is how to validate an item", so I can basically re-use those same rules to generate as many groups as I want, be it within a larger entity where I want cascading rules (such as inventory, or a character who has an inventory which contains many items) or an individual object such as an Item.

So we were discussing the possibility of something like adding an extension like usingRulesFrom(existingValidationGroup) which could be used like so in the previous example:

private inventoryValidationRules = validation.on(this.Inventory)
    .ensureForEach("Items") // Assuming #10 is done
        .useRulesFrom(newItemValidationGroup);

However I am not really sure how realistic this is to do as I dont know if the validation group contains enough metadata to extract and rebuild the rules on a new object.

So I am open to ideas at this point as I guess my ideal scenario would be something like:

var itemValidationRules = validation.createRuleset()
    .ensure("Name")
        .isNotEmpty()
        .hasMinLength(5)
    .ensure('Description')
        .hasMinLength(5)
    .ensure('Weight')
        .containsOnlyDigits()
    .ensure("Value")
         .isNotEmpty()
        .containsOnlyDigits();

var itemValidationGroup = validation.on(someItem)
    .useRuleset(itemValidaitonRules);

var inventoryValidationGroup = validation.on(someInventory)
    .ensureForEach("Items")
        .useRuleset(itemValidationRules);

Then that way you can split the rule definitions and the validation groups up and use them in a composite manner, this also makes it easier to make cascading validation but still use each validation rule at the individual level.

Also its worth mentioning without the starting the notion of cascading validation as mentioned in #10 a lot of other things on top of this issue become a lot more cumbersome, like trying to manage a list of entities which can dynamically have elements added or removed, you would need to manually keep re-creating and removing validation (which is one reason why I originally favoured external classes handling the creation of validation groups), and if you were to remove one how would you know which validation group to remove from the list... I guess if they maintained order it may be ok, but hopefully this also shows why #10 is super important as a stepping stone for this sort of thing.

The End

I hope I have not bored you too much and the points raised here are helpful to see how in larger more complex models the current validation paradigm is going to cause lots of headaches.

janvanderhaegen commented 9 years ago

Hey Grofit,

The @ensure decorator works wonders, but using the fluent API causes a lot of repetition. I think it would be pretty easy to do this using a callback function, actually:

var rulesSet1 = validation.createRuleSet( (v) => v.ensure('firstName').isNotEmpty() ); //and more rules
var group = validation.on(this.VM).useRuleSet(rulesSet1);

Doing the decoupling this way (using a callback in the createRuleSet method) will allow me to reuse the @ensure decorator code internally when setting up the group.

Secondly, the useRuleSet opens up a nice entry point to add rule sets created not just with the createRuleSet method, but we can generate rule sets from JSON schemas, have them generated from Entity Framework models, etc...

Interesting... Let's agree that #10 is highest prio, but we'll want this before hitting beta too :)

dotnetrules commented 9 years ago

Hmmmm I'm liking this also......

grofit commented 9 years ago

I am aware of the @ensure syntax and part of me likes the validation rules being on the models, part of me dislikes it. Coming from Knockout the validation is metadata on the actual model so it is easy to cascade and ripple down, but then your models are tied to the validator.

Anyway good to know you are interested in the idea, the style you propose is fine as that way you could easily have a method handle that step so the validation rules setup could be offloaded to another class (I only mention this because I know some of my validation can be verbose, so putting it in an isolated class makes it a bit neater and easier to maintain).

grofit commented 9 years ago

Any news on this one? (No worries if not)

plwalters commented 8 years ago

Thanks for submitting this / commenting on this. At this time we are closing this because we have completely re-written and are deprecating the previous feature set. If you feel this should be re-opened please feel free to review this blog post and submit either again on this repository, or on the new validatejs bridge repository

Thanks again!