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

Joi.any().when is not evaluated if empty #332

Closed
asafdav opened this issue May 19, 2014 · 5 comments
Closed

Joi.any().when is not evaluated if empty #332

asafdav opened this issue May 19, 2014 · 5 comments
Assignees
Labels
bug Bug or defect
Milestone

Comments

@asafdav
Copy link
Contributor

asafdav commented May 19, 2014

Joi.any().when is not evaluated it its value is empty even if the when condition is fulfilled.

For example:

 Joi.object({
   a: Joi.string().when('b', {is: true, then: Joi.required()}),
   b: Joi.boolean().default(false)
})

If you use this object to validate {b: true} the returned value will remain {b: true} but it actually should return an error.

asafdav added a commit to asafdav/joi that referenced this issue May 19, 2014
@hueniverse
Copy link
Contributor

You should follow the rules of the bug hunt and open a failing test. The fix is not right but the issue looks valid.

@hueniverse hueniverse added the bug label May 20, 2014
@asafdav
Copy link
Contributor Author

asafdav commented May 20, 2014

Sure, I will create the test, sorry for that.

I just wonder why you say the fix is not right, I'd love to learn from that.

I went through the code, and as long as "undefined" is in the "valid" list, when an empty value will be received it will automatically pass.

I saw how any().required is implemented and figured I can use the same method. Logically it makes sense to say that fields with "when" clause must be evaluated on every validation cycle.

I tried and saw that my fix didn't break anything, and it actually solved my issue.

Thanks in advance.

@hueniverse
Copy link
Contributor

Take a look at the commit above. This was a bit more complex. First, normal alternative doesn't require a match by default if the key is missing so the change belonged in the any.when(), not in the alternatives.when(). Second, once you add that change, the otherwise case breaks due to bad logic in concat().

@hueniverse hueniverse added this to the 4.3.0 milestone May 21, 2014
@hueniverse hueniverse self-assigned this May 21, 2014
@hueniverse hueniverse reopened this May 21, 2014
@hueniverse
Copy link
Contributor

Looking at your actual failing test, I am not sure what's the right solution for that one. Basically, you can now (with the fix) make this work by removing alternatives() (just Joi.when()). Calling when() directly on an alternatives type doesn't do what you expect it to. I need to think about this and if this is a bug for an edge case or not. Either way, the core of the issue was a confirmed bug.

@asafdav
Copy link
Contributor Author

asafdav commented May 21, 2014

I see, I'm glad I could make you think :)

Thanks for the elaborate explanation and for the workaround!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

No branches or pull requests

2 participants