Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support wildcards in partial struct validation #255

Closed
jawher opened this issue Sep 19, 2016 · 7 comments
Closed

Support wildcards in partial struct validation #255

jawher opened this issue Sep 19, 2016 · 7 comments
Assignees
Milestone

Comments

@jawher
Copy link
Contributor

jawher commented Sep 19, 2016

Package version eg. v8, v9:

v9

Issue, Question or Enhancement:

validate.StructPartial is a very useful feature to restrict the validation to a struct field for example.
It is however limited by the fact that one have to list the names of all the child fields, e.g. the code example below:

Code sample, to showcase or reproduce:

In the following example, a call to validate.Struct would complain about S.Child.Name and S.Child.Age.

However, Calling validate.StructPartial with Child will return nil:

type S struct {
    Child    struct {
        Name       string `validate:"required"`
        Age        int    `validate:"gt=1"`
    }
        // other fields
}

func TestStructPartial(t *testing.T) {
    subject := S{}

    validate := New()

    errs = validate.StructPartial(subject, "Child")
    // errs is nil here
        if errs==nil {
        t.Fail()
    }

    // StructPartial must be called like this for it to work:
    errs = validate.StructPartial(subject, "Child.Name", "Child.Age")
}

Proposal

Would you be interested in a PR to add wildcard support to partial validation ? e.g.:

errs = validate.StructPartial(subject, "Child.*")

What I think is really needed for such a feature to be useful:

  • *: accepts a single level: Child.* will match Child.Name but not Child.Adress.Zip
  • **: for multi-level matching: Child.**
  • works also with slice indices: Hobbies[*]

To go even further:

  • Prefix/Suffix matching: Child.Ho*, Child.*Field

If we're really crazy:

  • Index ranges: Child.Hobbies[1:3] to only match Child.Hobbies[1], Child.Hobbies[2] and Child.Hobbies[3]
  • open-ended index ranges: Child.Hobbies[4:], Child.Hobbies[:3]
  • Or operator: Child.(Hobbies|Friends)[*].ID
  • etc.

Risks

Such a feature would impact the partial validation performance: testing a field will no longer be a simple hash lookup.

Possible workaround: Add new partial validation function and let the library user decide which variants to call

@deankarn
Copy link
Contributor

Hey @jawher I'll have to think about this I just find being Implicit rather than explicit very dangerous.

eg. Developer 1 creates struct and add partial validation

type Child struct {
    Name       string `validate:"required"`
    Age        int    `validate:"gt=1"`
}

// soma validation code, maybe not even in the same place as struct
errs = validate.StructPartial(subject, "Child.*")

Then some other developer comes along and adds another field, without knowing there is partial validation going on,

type Child struct {
    Name       string `validate:"required"`
    Age        int    `validate:"gt=1"`
    Sex        string `validate:"required"`
}

and now the Sex field is being validated, but do you want it to be? Is it going to cause problems?

I was very hesitant to add the StructExcept and StructPartial in the first place.

Risks

You are correct it will affect performance of StructPartial times the number of partials passed in. I am not against this functionality if it only affects the StructPartial and StructExcept`(yes would be nice to have in except too ) functions performance.

I would probably accept a pull request.

New Functionality

I am currently experimenting with some new functionality that will solve several requests that have been raised here including #255, #254 and #245 which may make this unnecessary.

instead of some new partial validation as you suggested, I was going to add the ability to report error directly from within the FieldLevel validation functions, much like the StructLevel's ReportError(...) function.

This would essentially allow users to use custom validations just like they are today or if they choose, to implement more complex validations on their own and just return true.

There are a few hurdles I have to work out, like validation tags currently aren't supported on struct field except for omitempty and required.

but my v9 changes to interfaces will allow me to add this functionality without making breaking changes; it will affect performance but it should be negligible. It will take me a while to complete, pretty busy right now, but I intend to get this done if you can wait.

@deankarn deankarn self-assigned this Sep 19, 2016
@deankarn deankarn added this to the v9 milestone Sep 19, 2016
@jawher
Copy link
Contributor Author

jawher commented Sep 21, 2016

@joeybloggs Thanks for the detailed and prompt reply !

I agree that wildcards will come with their fair share of unpredictability and possible surprises.

Regarding the future on-steroids fieldLevel validators, I must be missing something but I'm not sure I see how they can help with the issue at hand, i.e. partial validation filtering.

I thought about this issue some more and I think I have a possible alternative: Expose a new top-level validation function, say validate.StructFiltered for example accepting a struct as its first argument and a function which returns true/false for whether to validate a given field or not, e.g.:

validate.ValidateFiltered(user, func(fieldPath string) bool {
    return !strings.Contains(fieldPath, ".Sex")
})

The accepted parameters are to be defined: the field path is the strict minimum, but could be extended to also accept other params (currentValue, ...)

Such a solution would require minimum modification to the library: reuse the same partial/except validation code, just check for the presence of a filtering function and use if that's the case, else fallback to the includeExclude map.

Somebody else (me for example) can then provide a implementation of such a function in a separate package and use it to handle their use case:

import "github.com/jawher/validator/filter"


validate.ValidateFiltered(user, filter.Include("Child.*"))

validate.ValidateFiltered(user, filter.Exclude("Child.Hobbies[*].**"))

@deankarn
Copy link
Contributor

Hey @jawher

thanks for the reply, I like it! it is a great idea! 👍

I would definitely like to add that functionality, the only change I'd make is passing the path via a []byte otherwise there would be an allocation for every field to create the string to be passed as the field namespace is a []byte and is would pass the Struct namespace and not the other that could be different by tag.

validate.ValidateFiltered(user, func(ns []byte) bool {
    return !bytes.Contains(fieldPath, []byte(".Sex"))

    /// NOTE: []byte(".Sex") could be precomputed
})

and just use the bytes functions instead of strings

@deankarn deankarn added the new label Sep 22, 2016
@deankarn
Copy link
Contributor

Hey @jawher

I added this functionality in Release 9.2.0 please take a look and let me know if this solves it for you.

@jawher
Copy link
Contributor Author

jawher commented Sep 26, 2016

@joeybloggs Nice ! You beat me to it, I was planning to create a PR for this 👍

I'll ping back once I get a working implementation for the wildcard filtering.

@deankarn
Copy link
Contributor

Hey @jawher just checking in to see if you'd had a chance to check this out yet :)

@jawher
Copy link
Contributor Author

jawher commented Oct 21, 2016

Wow, it's been 25 days already !

I started working on it a while ago, but I'm afraid I overdone it a bit and ended up with something a touch too complex :D

I'll try to get back to it the next weeks.

@deankarn deankarn closed this as completed Jun 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants