bartoszlenar / Validot

Validot is a performance-first, compact library for advanced model validation. Using a simple declarative fluent interface, it efficiently handles classes, structs, nested members, collections, nullables, plus any relation or combination of them. It also supports translations, custom logic extensions with tests, and DI containers.
MIT License
310 stars 19 forks source link

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

Closed kislovs closed 2 years ago

kislovs commented 2 years ago

Feature description

Example Assume we have own `Option` instead of `Nullable` (other example - [primitive obsession](https://refactoring.guru/smells/primitive-obsession) and it's solution like https://github.com/andrewlock/StronglyTypedId): ```csharp public readonly record struct Option 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(this IRuleIn> @this, Specification specification)`, but currently it's not possible to do something like that. So you forced to repeat something like ```csharp Specification> s => s .Rule(o => o.IsSome) .Rule(o => o.Some(out var value) && ...) ; ```

Feature details

bartoszlenar commented 2 years ago

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 commented 2 years ago

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.

kislovs commented 2 years ago

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!

bartoszlenar commented 2 years ago

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 commented 2 years ago

@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.

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 .

kislovs commented 2 years ago

@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 :)

bartoszlenar commented 2 years ago

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 commented 2 years ago

@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:

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

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.

kislovs commented 2 years ago

@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 :)

bartoszlenar commented 2 years ago

it's great to hear that!

anyway, thanks for your time and contributions!