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

Validation of zero values #19

Open
mex opened this issue Dec 20, 2016 · 10 comments
Open

Validation of zero values #19

mex opened this issue Dec 20, 2016 · 10 comments

Comments

@mex
Copy link

mex commented Dec 20, 2016

This library currently marks a zero value for a required property as an error (

if reflect.DeepEqual(reflect.Zero(val.Type()).Interface(), val.Interface()) {
).

But this doesn't really make sense in an API, because you should be allowed to send in a 0 for an integer property or a false for a boolean property even if it is required.

In regards to integers, the minimum option should define whether 0 is allowed, not the required option.

In regards to booleans, the required check doesn't make any sense at all in its current form as you aren't able to send anything other than true.

@casualjim
Copy link
Member

required values are rendered as pointers normally, the zero value for that is nil
Do you have a specification or an example where it doesn't render it as a pointer?

@mex
Copy link
Author

mex commented Dec 20, 2016

I have a case, where I have set a boolean as required and x-nullable: false, because I don't want the pointer. I want a value that's either true or false - and nothing else.

"isPrimary": {
    "description": "Bla bla",
    "type": "boolean",
    "required": true,
    "x-nullable": false
}

In this case I get a validation error, if I send in "isPrimary": false, because of that validation check.

@casualjim
Copy link
Member

You're explicitly disabling the ability to detect if a value is required or not with x-nullable: false.
I assume you want to fail when the key isPrimary isn't in the json? Isn't the absence of the key the same as false?

@mex
Copy link
Author

mex commented Dec 20, 2016

I want it to provide a validation error, if isPrimary is absent. And no, I wouldn't say so. I don't want truthy/falsy scenarios.

I'm starting to grasp the complexity of my issue as it would require some intermediate unmarshalling into a struct with pure pointers, perform the validation and then make a conversion to my desired result of a struct with values.

@casualjim
Copy link
Member

there are a number of helpers in the swag package to make working with pointers easier.
https://github.com/go-openapi/swag/blob/master/convert_types.go#L66-L78

@mex
Copy link
Author

mex commented Dec 20, 2016

Yeah, I know about that, but I just don't want those conversions everywhere in my code (we are talking about around 100 endpoints with 10+ properties a piece).

It makes for ugly code to use pointers for required and values for optional in the API layer, while using values for required and pointers for optional in the database layer.

@casualjim
Copy link
Member

I have a PR that deals with this, but I can't merge because it's a breaking change, and a big one.
That PR allows for detection of absence of key as well as null value and uses only value properties.
I think the opportunity to fix this behavior will need to wait until openapi spec v3 is released so that people expect actual breakage

@mex
Copy link
Author

mex commented Dec 20, 2016

Cool, looking forward to that then! I'll manage with absence ~= false until then.

@dtravin
Copy link

dtravin commented Oct 14, 2017

@casualjim can you publish the PR you mentioned?
I guess there is a place for RequiredBool method that does not have the reflect.DeepEqual check

@fredbi
Copy link
Member

fredbi commented Dec 23, 2017

See alse go-swagger/go-swagger#959 (related but not duplicate)

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

5 participants