Add option to disable validation of "format" constraint #383
Conversation
|
@bighappyface / @shmax Please let me know if you want me to slow down - I'm aware I have submitted a large number of PRs recently, and that as a result there is now a backlog of stuff awaiting merging or review. If this is too much, too fast, please say so and I'll hold back for a bit. |
|
I don't know about @bighappyface, but I don't mind the pace. We're targeting 6.0 for most of this stuff, anyway, so there's no harm if it takes him a while to catch up. |
| @@ -30,6 +30,7 @@ | |||
| const CHECK_MODE_COERCE_TYPES = 0x00000004; | |||
| const CHECK_MODE_APPLY_DEFAULTS = 0x00000008; | |||
| const CHECK_MODE_EXCEPTIONS = 0x00000010; | |||
| const CHECK_MODE_DISABLE_FORMAT = 0x00000020; | |||
There was a problem hiding this comment.
Maybe this is just a personal style thing, and it's no big deal, but for some reason I always get a little uncomfortable when I see a variable or constant named in the negative. It makes it the odd duck out, and then you wind up doing odd double-negative logic to check them in the affirmative, eg if (!disabledThing).
Is there any way to flip the polarity and enable it by default, then require the user to explicitly disable it if he doesn't want it?
There was a problem hiding this comment.
Normally, I'd agree with you. I would vastly prefer an on-by-default option.
The reason I did it this way here is because if you pass $checkMode directly, rather than going via Factory::addConfig, you'll clobber whatever the default is. In this case, that would mean a very high chance of users turning off "format" validation by accident (i.e. anybody who uses a custom $checkMode and didn't realise the implications of overwriting the default).
Thoughts?
There was a problem hiding this comment.
Good point. I guess this is the best option. 👍
|
LGTM |
* 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
Add
CHECK_MODE_DISABLE_FORMAT. If set, validation of the"format"keyword is ignored completely, and assumed successful.Why
From draft-04 onwards, the spec says we should provide an option to disable format validation: