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.

Sign up for GitHub

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

Add option to disable validation of "format" constraint #383

Merged
merged 1 commit into from
Mar 17, 2017
Merged

Add option to disable validation of "format" constraint #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ third argument to `Validator::validate()`, or can be provided as the third argum
| `Constraint::CHECK_MODE_COERCE_TYPES` | Convert data types to match the schema where possible |
| `Constraint::CHECK_MODE_APPLY_DEFAULTS` | Apply default values from the schema if not set |
| `Constraint::CHECK_MODE_EXCEPTIONS` | Throw an exception immediately if validation fails |
| `Constraint::CHECK_MODE_DISABLE_FORMAT` | Do not validate "format" constraints |

Please note that using `Constraint::CHECK_MODE_COERCE_TYPES` or `Constraint::CHECK_MODE_APPLY_DEFAULTS`
will modify your original data.
Expand Down
1 change: 1 addition & 0 deletions src/JsonSchema/Constraints/Constraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ abstract class Constraint extends BaseConstraint implements ConstraintInterface
const CHECK_MODE_COERCE_TYPES = 0x00000004;
const CHECK_MODE_APPLY_DEFAULTS = 0x00000008;
const CHECK_MODE_EXCEPTIONS = 0x00000010;
const CHECK_MODE_DISABLE_FORMAT = 0x00000020;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I guess this is the best option. 👍


/**
* Bubble down the path
Expand Down
2 changes: 1 addition & 1 deletion src/JsonSchema/Constraints/FormatConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class FormatConstraint extends Constraint
*/
public function check(&$element, $schema = null, JsonPointer $path = null, $i = null)
{
if (!isset($schema->format)) {
if (!isset($schema->format) || $this->factory->getConfig(self::CHECK_MODE_DISABLE_FORMAT)) {
return;
}

Expand Down
17 changes: 17 additions & 0 deletions tests/Constraints/FormatTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

namespace JsonSchema\Tests\Constraints;

use JsonSchema\Constraints\Constraint;
use JsonSchema\Constraints\Factory;
use JsonSchema\Constraints\FormatConstraint;

class FormatTest extends BaseTestCase
Expand Down Expand Up @@ -76,6 +78,21 @@ public function testInvalidFormat($string, $format)
$this->assertEquals(1, count($validator->getErrors()), 'Expected 1 error');
}

/**
* @dataProvider getInvalidFormats
*/
public function testDisabledFormat($string, $format)
{
$factory = new Factory();
$validator = new FormatConstraint($factory);
$schema = new \stdClass();
$schema->format = $format;
$factory->addConfig(Constraint::CHECK_MODE_DISABLE_FORMAT);

$validator->check($string, $schema);
$this->assertEmpty($validator->getErrors());
}

public function getValidFormats()
{
return array(
Expand Down