-
Notifications
You must be signed in to change notification settings - Fork 356
[BUGFIX] Don't throw exceptions until after checking anyOf / oneOf #394
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[BUGFIX] Don't throw exceptions until after checking anyOf / oneOf #394
Conversation
Nice one LGTM |
Thanks :-). Having thought about this a little more, I might refactor this to take advantage of bubbled exceptions, rather than just turning them off during anyOf / oneOf - that would be better from a performance perspective. |
I should also add tests that cover this case. |
6b35f61
to
a45875d
Compare
@shmax How's that? |
It's a thing of beauty. LGTM |
Thanks :-) |
Grr. Apparently I committed the pre-style-fix version. Travelling now, will commit the correct version once I'm home. |
a45875d
to
e67aa7a
Compare
There we go - much better :-). |
$this->assertTrue($v->isValid()); | ||
} | ||
|
||
public function testNoPrematureOneOfException() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I seeing things or is this method identical to testNoPrematureAnyOfException
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not seeing things - thanks for spotting that one! Will sort once I'm back at my desk later today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bighappyface Fixed.
e67aa7a
to
0d46d20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
* Add URI translation for retrieval & add local copies of spec schema * Add use line for InvalidArgumentException & adjust scope (#372) Fixes issue #371 * add quiet option (#382) * add quiet option * use verbose instead of quiet * add quiet option * always output dump-schema * always output dump-schema-url * fix typo and ws * [BUGFIX] Add provided schema under a dummy / internal URI (fixes #376) (#378) * Add provided schema under a dummy / internal URI (fixes #376) In order to resolve internal $ref references within a user-provided schema, SchemaStorage needs to know about the schema. As user-supplied schemas do not have an associated URI, use a dummy / internal one instead. * Remove dangling use * Change URI to class constant on SchemaStorage * Add option to disable validation of "format" constraint (#383) * Add more unit tests (#366) * Add test coverage for coercion API * Complete test coverage for SchemaStorage * Add test coverage for ObjectIterator * Add exception test for JsonPointer * MabeEnum\Enum appears to use singletons - add testing const * Don't check this line for coverage mbstring is on all test platforms, so this line will never be reached. * Add test for TypeConstraint::validateTypeNameWording() * Add test for exception on TypeConstraint::validateType() * PHPunit doesn't like an explanation with its @codeCoverageIgnore... * Add various tests for UriRetriever * Add tests for FileGetContents * Add tests for JsonSchema\Uri\Retrievers\Curl * Add missing bad-syntax test file * Restrict ignore to the exception line only * Fix exception scope * Allow the schema to be an associative array (#389) * Allow the schema to be an associative array Implements #388. * Use json_decode(json_encode()) for array -> object cast * Skip exception check on PHP versions < 5.5.0 * Skip test on HHVM, as it's happy to encode resources * Enable FILTER_FLAG_EMAIL_UNICODE for email format if present (#398) * Don't throw exceptions until after checking anyOf / oneOf (#394) Fixes #393 * Fix infinite recursion on some schemas when setting defaults (#359) (#365) * Don't try to fetch files that don't exist Throws an exception when the ref can't be resolved to a useful file URI, rather than waiting for something further down the line to fail after the fact. * Refactor defaults code to use LooseTypeCheck where appropriate * Test for not treating non-containers like arrays * Update comments * Rename variable for clarity * Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults if they are marked as required. * Workaround for $this scope issue on PHP-5.3 * Fix infinite recursion via $ref when applying defaults * Add missing second test for array case * Add test for setting a default value for null * Also fix infinite recursion via $ref for array defaults * Move nested closure into separate method * $parentSchema will always be set when $name is, so don't check it * Handle nulls properly - fixes issue #377 * Add option to also validate the schema (#357) * Remove stale files from #357 (obviated by #362) (#400) * Stop #386 sneaking in alongside another PR backport
What
Don't throw exceptions until evaluation of
anyOf
/oneOf
has completed.Why
This is a bug. Fixes #393.