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

Please repeal commit 26c3f42095b8a1378617eb4756fbf8282cb1ae89 #1211

Closed
deankarn opened this issue Dec 23, 2017 · 14 comments
Closed

Please repeal commit 26c3f42095b8a1378617eb4756fbf8282cb1ae89 #1211

deankarn opened this issue Dec 23, 2017 · 14 comments

Comments

@deankarn
Copy link

This commit caused breaking changes for anyone who had updated to using validator v9 from v8.

Moreover it prevents anyone from changing the the binding.Validator to use any other validation library besides go-playground/validator.v8, although I love that gin has adopted my library as their default, there are others and why the binding.StructValidator interface exists in the first place.

Lastly the addition was unnecessary, even if it made it more convenient, anyone could have just created a new instance of validator registered what they wanted and created a struct comply with binding.StructValidator, much like in my instructions on updating to use v9 of validator; which is also broken due to this change.

As an alternative, I believe that an example of how to override the default validator would be more beneficial and could easily be copy-paste.

I sincerely hope this can be corrected, although I am not a member of the gin team, I feel very close to this project and IMHO, this put in many blockers for gin's codebase and adoption going forward.

@egiathamaershada
Copy link

i also wanted to know about this topic and agree with joeybloggs. To tell the truth, i have been checking a discussion between you and sudo-suhas at #430 everyday and hoping something can be done. I am not against the commit, but i like the v8 to v9 upgrade more because of the flexibility. I'd like the most if we use v9, but i understand some people may use v8 in production, which is already pointed at #430

@benpaxton-hf
Copy link

Agreed, we've had to import validator.v8 from our own custom validation library to keep it working with Gin - it would be nice if we didn't need this.

@deankarn
Copy link
Author

deankarn commented Mar 5, 2018

@appleboy @javierprovecho ☝️

Just checking to see if any of the contributors to this project is looking into this or this project has been abandoned.

This commit was a breaking change and hurts the adoption of Gin going forward

@malozanoff
Copy link

+1 i can't run custom validators in gin now, the documentation clearly states this is possible also, so i assume this was an unintended change.
I'd rather not have to pin my project to an old gin version forever, does anyone have any other way to run custom validators now?

@appleboy
Copy link
Member

appleboy commented Mar 8, 2018

@sudo-suhas Could you have a chance to take look at this?

@sudo-suhas
Copy link
Contributor

sudo-suhas commented Mar 8, 2018

@appleboy As @egiathamaershada mentioned, I had discussed possible ways to fix this issue which I inadvertently caused but there was no response from the maintainers. I'd be more that happy to make one for this. Shall I try what @joeybloggs suggested in #430 (comment)?

@appleboy
Copy link
Member

appleboy commented Mar 8, 2018

@sudo-suhas maybe try the first way to fix this.

  1. Expose the binding.Validator to allow it to be overridden as before

PR Welcome. 👍

@sudo-suhas
Copy link
Contributor

Okay @appleboy, will do. Please give me time till tomorrow(things are a little hectic at work).

@benpaxton-hf
Copy link

@malozanoff

does anyone have any other way to run custom validators now?

If you can easily change the source of your existing validator, it's enough to add the following to it:

import (
    "gopkg.in/go-playground/validator.v8"
)

func (v yourValidator) RegisterValidation(a string, b validator.Func) error {
    return nil
}

If you can't, you could wrap it in something like the following:

import (
    "gopkg.in/go-playground/validator.v8"
)

type Validator interface {
    ValidateStruct(obj interface{}) error
}
type ValidatorWrapper struct {
    Validator
}
func (v ValidatorWrapper) RegisterValidation(a string, b validator.Func) error {
    return nil
}

@malozanoff
Copy link

How would that work? The binding interface in gin does not have a RegisterValidation anymore... So even if i add one (i did) i can't call it from my application as its not exposed in the interface.

type StructValidator interface {
	ValidateStruct(interface{}) error
}

That is the whole point of why things are broken, or what am i missing?

@deankarn
Copy link
Author

deankarn commented Mar 8, 2018

If you expose the binding.Validator that would allow people still use v8 and all they’d have to do is type assert and then have all the methods, including RegisterValidation, at their disposal.

It will also allow the validator to be overridden with others that comply with the original validation interface.

There’s no need to have any extra methods on the validator interface.

@benpaxton-hf
Copy link

@malozanoff, the current version of Gin has:

type StructValidator interface {
	ValidateStruct(interface{}) error
	RegisterValidation(string, validator.Func) error
}

If you use a version of Gin prior to 26c3f42, as it sounds like you're doing, you don't need the workaround I posted.

@sudo-suhas
Copy link
Contributor

@joeybloggs, @appleboy This issue can be closed now since #1277 was merged.

@appleboy
Copy link
Member

@sudo-suhas OK Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants