Fix oneof

[znewsham]

(replaces #431 - it was from the main branch which was causing trouble.)

This ended up being quite a lot larger than I initially planned. Initially it looked like it was going to be fairly simple, but alas not. All tests are passing and I added a handful of others.

I believe this fixes #428 and #410

There are a few small changes that look large, and a couple of actual large ones:

1. In checkObj - move everything after the validate call into a new function called recurse - this is called by checkObj in the case that we don't want to validate a key (or initially when we dont have a key) but also by validate which may also change the "schema" used from the root schema to the current branch schema.
2. Rather than accumulating errors, everything now returns an array of errors - these are recursively concatenated until a def.type.some completes with multiple options. At which point it either discards the entire array (in the case of a pass) or continues to concat.
3. subSchema subSchemaAffectedKey and subSchemaAffectedKeyGeneric(sometimes) are now additionally passed around - these represent the current sub schema and the affected key relative to the sub schema affectedKey and affectedKeyGeneric refer to the current key relative to the root schema.
4. If none of the provided schemas match, the entire set of accumulated errors are thrown. This behaviour is debatable - it isn't useful to be told that "a.b" and "a.c" are missing when you provided "a.d" and "a.e" - but I'm not sure how else to handle this. Perhaps a new error type that preempts all the others for a given schema '"a" did not match any of the provided schemas'?
5. add oneOfKeys() and recursiveKeys() functions to accumulate all keys in a schema (including child schemas) and those that are inside of a oneOf.
6. change checkModifier to check if a key is inside a oneOf and if so run the check against each subSchema

Thanks for providing such a vast library of test cases - it made implementing this very safe. I'm going to include this in my project where I've been hacking around this limitation for ages, and will ensure that all my schemas work, as I make heavy use of oneOf(schema1, schema2, ...)

[titivermeesch]

Would this also be a fix for #112 ?

[znewsham]

Would this also be a fix for #112 ?

yes, I believe it should be

[titivermeesch]

Is there any progress on this? Or something which I could contribute to?

Having oneOf working as expected would be awesome. Our current workaround is just using black box, which is less than ideal.

[znewsham]

As far as I'm concerned it's done. I'm using this version in production.

[titivermeesch]

@aldeed Any chance this could be in the next release?

[jankapunkt]

What's the status of this one?

[vpalos]

Is there any chance to see this merged anytime soon?
Basically the entire oneOf feature relies on this fix.
Thanks for the awesome package btw!

[titivermeesch]

@aldeed Is there a reason why this isn't getting any attention? Is there a timeline we should be aware of that is delaying this? Seems like a bug/feature that has been requested a lot, and for a long time

[titivermeesch]

What's the status of this one?

No status update at all...

[aldeed]

Thank you for your initial work on this @znewsham. I took your tests and some of your ideas and implemented in a slightly different way in e8f91dc. The main difference is recursing within the already-existing multi-type list, and adding a loop through all possible definitions, which avoids needing to prebuild the oneOfKeys list and avoids needing to pass around subschema arguments.

3.3.0 release

[znewsham]

Thanks @aldeed - I believe one of the reasons I went with the oneOfKeys approach was performance (deeply nested schemas can be wildly inefficient to check/clean) - do you have any benchmarks that get ran? I'd love to compare the performance

[aldeed]

@znewsham No, but benchmarks would be interesting in general. This package is already quite slow because of needing to handle mongo modifiers. I've often thought of trying to split that feature out so that normal object validation can be made faster (i.e. no mongoObject needed). But everything is quite tangled so I have tried a few times and failed.