Validot icon indicating copy to clipboard operation
Validot copied to clipboard

Consider extensibility point for extensions which accept `Specification<T>`

Open kislovs opened this issue 3 years ago • 7 comments

Feature description

  • As for now there's only one extensibility point: custom rules - which must use RuleTemplate, but it doesn't support to accept Specification<T> directly. So it's not possible to write extension like built-in AsNullable, AsCollection, etc.
  • Problem: if you have some kind of wrapper which not covered by built-in extensions (e.g. AsNullable), you forced to repeat boilerplate code
Example

Assume we have own Option<T> instead of Nullable<T> (other example - primitive obsession and it's solution like https://github.com/andrewlock/StronglyTypedId):

public readonly record struct Option<TValue>
    where TValue : notnull
{
    private readonly TValue? val;

    internal Option(bool isSome, TValue? value = default)
    {
        IsSome = isSome;

        val = value;
    }

    public bool IsSome { get; }
    public bool IsNone => !IsSome;

    public bool Some([MaybeNullWhen(false)] out TValue value)
    {
        value = val;
        return IsSome;
    }
}

When validating we should have same logic as AsNullable:

  • Ok if Option.IsSome == false;
  • Validate Option's inner value otherwise.

We'd like to add extension AsOption<TValue>(this IRuleIn<Option<TValue>> @this, Specification<TValue> specification), but currently it's not possible to do something like that. So you forced to repeat something like

Specification<Option<int>> s => s
    .Rule(o => o.IsSome)
    .Rule(o => o.Some(out var value) && ...)
;

Feature details

  • Same as built-in rules, built-in wrappers like AsNullable must be migrated to this extensibility point.
  • One of the possible solution: make all of the internal ICommand interfaces public alongside with AddCommand method.
  • There's bunch of ((SpecificationApi<T>)@this).AddCommand(...) downcast, fluent builder should be updated to allow new functionality without such crutches.
  • Caveat: it's possible to make it harder to add new functional in the future (e.g. #2)

kislovs avatar Jul 20 '22 13:07 kislovs

Thank you, @kislovs, for your idea.

It's an interesting request and for sure I'll investigate the codebase and see how it could be done without either rewriting the big portions of it or affecting the performance.

At the moment I'm on vacations and will be back in a week time.

bartoszlenar avatar Jul 22 '22 13:07 bartoszlenar

At the moment I'm pursuing for having #3 done and published in the version 2.3. It's about adding new command and while I'm coding it, I'm also investigating how much of work would be required for functionality you described.

Too soon for conclusion, but my first impressions are, unfortunately, that it would be either a massive thing or an ugly one.

Validot is super-fast, but you pay the price of less flexible process. Technically I could make a lot of classes public and let the people code their own commands, commands extensions, scopes and context actions... but then I'd need to document the process and even then the risk of shooting yourself in the foot is just too big. It would also affect the pace of the development and codebase stability, because with all the internals opened public, major version will be bumped up regularly.

I'd need more time to think about if it's worth it, and if yes - how to do it the right way.

bartoszlenar avatar Jul 27 '22 20:07 bartoszlenar

even then the risk of shooting yourself in the foot is just too big

I think it could be acceptable, because majority of validation already could be done with current library, but if it's not possible - fork is required. But, you know, fork is very hard to use across your projects especially when you just want to make some proofs that "new stuff" is working on production on real data, not in synthetic unit tests. With opening internal "dangerous" API it's possible to code and test new functional, and then, if it's not something very special, it's straight way to open new issue here.

As little trade-off it's possible to use efcore way: mark all of the internal API with "dangerous" description. It says like "you may use it, but we don't consider you use it!".


Last thought, while doing #3 issue you could count how many internal interfaces/classes you really need - maybe it's not a lot. Less public API -> less possible problems you've described.

Will wait for the news!

kislovs avatar Jul 27 '22 23:07 kislovs

At the moment cutting Validot's guts open sounds like something I don't like to do. The core parts are polished performance-wise and marked as internal for a reason. Validot isn't based on the similar context passing from rule to rule like in FluentValidation, where for sure you can do literally everything during the process and achieve great flexibility.

Validot's selling point is the speed and memory performance and until desperately needed due to some bug, I will choose keeping the library compact and simple.

Let me complete AsType and I'll let you know how much do I like opening the API public.

bartoszlenar avatar Jul 28 '22 19:07 bartoszlenar

@kislovs By the way... isn't that your case could be handled with just an extension to the specification command chain?

What I mean is the extension that takes specification and wraps several already existing commands.

  • You can validate option.Value with regular Member and the delivered specification,
  • You can make it conditional with WithCondition that checks option.IsSome
  • You can set the path to the level up (so it's not assigned under Value path) with WithPath.

Just an idea, haven't battle-tested it, but it looks for me that it's perfectly doable to prepare a custom extension for the case you mentioned. And to be honest - for most such things.

One exception that I could think of is the dictionary, which is an interesting case. Thanks for rising an issue ticket #26 .

bartoszlenar avatar Jul 28 '22 19:07 bartoszlenar

@bartoszlenar, yeah, for example I mentioned we wrote such extension :). But initially we stuck with the #24 (we have hierarchy and each child has specific validation rules). We investigated into Validot's sources and it looked like it's possible to do such validation (.Member, .AsCollection and .WithCondition used for understanding). Then we realized that's all of the stuff is internal for now... So, I've created two issues: one for initial problem with hierarchy, and second as idea to make user defined "experiments" possible :)

kislovs avatar Jul 29 '22 00:07 kislovs

#24 will be done, maybe even in the upcoming release 2.3. for now, you could just create two validators and move the if-ology to the upper layer. will work for sure.

bartoszlenar avatar Jul 30 '22 11:07 bartoszlenar

@kislovs

The final decision is NOT to open Validot's internals for such custom extensions. Unless absolutely necessary for some really crucial cases, which I don't see at the very moment.

I also believe that starting from version 2.3.0 that includes AsConverted (#3) among with other built-in commands, users can achieve the same results utilizing the regular Validot API.

Let's break down the example and the requirements from your initial post.

The extension method could be constructed in a way it:

  • extends IRuleIn<Option<T>> returning IRuleOut<Option<T>>, to integrate with the existing fuent API
  • accepts Specification<T> as an argument, it will be used for inner value validation
  • contains everything in AsModel, because it nicely wraps all commands into a single scope, returns IRuleOut making it possible to add more parameter commands in the upper level (e.g., AsOption(...).WithMessage())
  • uses AsConverted to extract the inner value and execute specification based on it
  • uses WithCondition to execute AsConverted only if there is some value

Code example of such an extension:

public static IRuleOut<Option<T>> AsOption<T>(this IRuleIn<Option<T>> @this, Specification<T> specification)
{
    return @this.AsModel(s => s
        .AsConverted(
            option =>
            {
                option.Some(out var value);

                return value;
            }, specification)
        .WithCondition(option => option.IsSome)
    );
}

Usage:

class User
 {
     public Option<string> Name { get; set; }
 }
Specification<string> userNameSpecification = s => s.NotEmpty();

Specification<User> specification = s => s
    .Member(m => m.Name, m => m
        .AsOption(userNameSpecification)
    );

var validator = Validator.Factory.Create(specification);

validator.Validate(new User() { Name = new Option<string>(true, "Bart") }).IsValid; // true

validator.Validate(new User() { Name = new Option<string>(true, "") }).ToString() 
// Name: Must not be empty

validator.Validate(new User() { Name = new Option<string>(false) }).IsValid; // true

So the proposed solution for this and other similar cases would be to

  • create a custom extension method
  • utilize the existing Validot commands to construct the validation logic
  • use this extension method in specifications

I'm happy to reconsider this if some more extreme example shows up that can't be handled with the solution described above. Let me know @kislovs if you have anything specific in mind. Otherwise I'll be closing this issue.

bartoszlenar avatar Sep 17 '22 12:09 bartoszlenar

@bartoszlenar, I think AsConverted (#3) opens a lot of opportunities and closes a lot of current problems. I have no counterexample against closing this issue for now :)

kislovs avatar Sep 20 '22 09:09 kislovs

it's great to hear that!

anyway, thanks for your time and contributions!

bartoszlenar avatar Sep 21 '22 09:09 bartoszlenar