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

Allow coercion to boolean from '0' and '1' #345

Closed

Conversation

JanTvrdik
Copy link

@JanTvrdik JanTvrdik commented Jan 11, 2017

Both '1' and '0' are very common string representations of boolean values.

@JanTvrdik JanTvrdik force-pushed the coerse_to_boolean_from_str_int branch from 01c73b0 to 80963f8 Compare January 11, 2017 13:49
@shmax
Copy link
Collaborator

shmax commented Jan 11, 2017

I'm not totally sure about this one, mainly because if you want to use "1" and "0" you can just define a type of "integer" and use the string coercion that already exists, right? I want to check a few other libraries and see what their policy is for this kind of thing... will have more information soon.

@moderndeveloperllc
Copy link

That would seem to be contrary to RFC 7159 that defines JSON grammar. There is a caveat that JSON parsers "MAY accept non-JSON forms or extensions", but that could be tricky. The XML Schema Part 2 spec does define true, false, 0, and 1 as legal literals for a boolean, but not sure how much that matters.

It would be great if @awwright could be summoned to give his input on 1/0/true/false for boolean in JSON Schema as opposed to only true/false.

@shmax
Copy link
Collaborator

shmax commented Jan 11, 2017

@moderndeveloperllc I don't think this really has anything to do with the JSON spec, or even the JSON schema spec. We're talking about a piece of functionality that is commonly layered onto validation libraries, specifically. This library already supports some coercion (it has to be invoked explicitly with the coerce method), and the question here is whether string 1 and string 0 should be coerced to boolean.

I haven't had a chance to research this in depth yet, but one of the most robust and fully featured validation libraries I know of does not coerce string "1" and "0" to boolean (though it does coerce numeric 1 and 0 to boolean):

https://github.com/epoberezkin/ajv/blob/master/COERCION.md

@JanTvrdik
Copy link
Author

My use case is to validate query parameters ($_GET) with JSON schema, therefore coercion from integers to booleans would not help me as query parameters are always string or array.

@shmax
Copy link
Collaborator

shmax commented Jan 12, 2017

Of course, that exact use case is why we added coercion in the first place. My point is that there is no need for you to coerce your string 1/0 to boolean; you can simply change your expected type to "integer", then check them in your client PHP code for truthiness the exact same way you would check a boolean for truthiness.

The end result is functionally the same, and we don't need to push coercion beyond reasonable limits.

@awwright
Copy link

@JanTvrdik I'm not sure what "coercion" means in this case. All a validator does is accept an instance and return "valid" or "invalid".

What's the proposed change in behavior, and how does it vary from the current behavior?

@shmax
Copy link
Collaborator

shmax commented Jan 13, 2017

@shmax
Copy link
Collaborator

shmax commented Jan 13, 2017

One more comment for @JanTvrdik:

toBoolean is only a protected method--not private--which means that you are free to simply subclass TypeConstraint and override toBoolean in your own code to get the functionality you desire. I can provide a code sample if you have trouble.

@awwright
Copy link

Oooh yes.

Of course, JSON Schema only specifies validation. But I've implemented "coercion" like this, i.e. take some data (JSON or otherwise) and cast it into JSON valid against a certain schema. I did this for taking a URI and turning it into a JSON-compatible object.

This just gets a little complex if you support multiple data types, for example type: ["string", "boolean"]. That's the only consideration I can think of.

@shmax
Copy link
Collaborator

shmax commented Jan 15, 2017

I was about to argue that "1" and "0" are different from the cases already handled in that they are not simply a reversal of the stringification performed by operations like http_build_query, but I just tried it and 'http_build_query' does string-ify true and false as 1 and 0.

<?php
echo http_build_query(array(
    "numPuppies"=>7,
    "likesChocolate"=>true
));

// numPuppies=7&likesChocolate=1

Sooo, I guess I'm not as sure about this, now. I would still like to know why the folks on the other project don't coerce "1" and "0"...

@shmax
Copy link
Collaborator

shmax commented Jan 15, 2017

So we have this elegant counter-example and explanation from the author of ajv: ajv-validator/ajv#395 (comment)

Essentially, because coercion of string 1 and 0 to boolean is not reversible, we run into cases where validation on type lists (and possibly other complex nested structures) can unexpectedly fail with coercion turned on, when it would pass with it turned off.

So, @JanTvrdik I think your options are still the same:

  1. stay with your string 1 and 0 but change your schema to expect an integer. Use coerce. Your client code can continue to use the same logical checks you would do as if they were boolean
  2. Prepare your http requests such that true and false are stringified to "true" and "false" instead of "1" and "0", and coerce
  3. Subclass TypeConstraint in your own code and do whatever you like with toBoolean, but be prepared for subtle issues like the one described by @epoberezkin

@JanTvrdik
Copy link
Author

JanTvrdik commented Jan 16, 2017

we have this elegant counter-example and explanation from the author of ajv

@shmax But his points are irrelevant in context of this library because this library does not suport multiple types with anyOf nor does it support reversing the coercion. You cannot reproduce his example with this library.

@shmax
Copy link
Collaborator

shmax commented Jan 16, 2017

You cannot reproduce his example with this library.

I did try it, and what happened was exactly what he predicted. Did you get different results?

this library does not suport multiple types with anyOf

What do you mean? Do you mean that coercion is not fully supported? If you can supply a failing unit test we can investigate it together.

nor does it support reversing the coercion

Nobody is saying it should support reversing the coercion. When we say that the coercion of 1 and 0 is not reversible we mean that it is one way; once coerced its original form is lost, so that a further rule that might have otherwise validated its original form will fail (like in the schema sample we've been looking at). It's just too much of a transformation. Like @epoberezkin says, this can happen with the other coercion types as well, but it's much less likely.

I guess I don't understand why you're fighting so hard on this? There are many different ways for you to get the functionality you want, and none of them have to involve changing this repo.

I'll list out a few for you, complete with samples.

Coerce to integer instead of boolean

$value = "0"; // json_decode('{}');
$schema = json_decode('{ 
	"type": "integer",
	"enum":[0,1]
}');
$validator->coerce($value, $schema);

echo gettype($value); // "integer"
echo $value; // 0

Subclass TypeConstraint and just do whatever you want

class MyTypeConstraint extends \JsonSchema\Constraints\TypeConstraint {
    protected function toBoolean($value) {
        if($value == "1") {
            return true;
        } else if ($value == "0"){
            return false;
        } 

       return parent::toBoolean($value);
    }
}

$factory = new \JsonSchema\Constraints\Factory();
$factory->setConstraintClass( "type", "\path\to\MyTypeConstraint" );
$validator =  new \JsonSchema\Validator($factory);

$value = "0"; // json_decode('{}');
$schema = json_decode('{ 
	"type": "boolean"
}');
$validator->coerce($value, $schema);

echo gettype($value); // "boolean"

If you're really determined you could also see about modifying the core coercion code so that coerced values are not passed down the chain when evaluating lists and other complex scenarios, but like the other guy says it might not be worth the added complexity, especially when you already have much lighter alternatives.

@bighappyface
Copy link
Collaborator

@JanTvrdik @shmax is this PR still in play?

@shmax
Copy link
Collaborator

shmax commented Feb 21, 2017

@bighappyface My opinion is no, especially considering we haven't heard a peep from @JanTvrdik in quite a while.

@bighappyface
Copy link
Collaborator

Closing until further notice from @JanTvrdik

@vicary
Copy link

vicary commented Jul 3, 2018

For historical reasons we did way too much of assumption to substitute "0" and "1" where a true boolean is not possible, say $_GET and for mysql columns requries indexing.

I'd vote for it if this is where we can start "coercing" things back to standard.

For the case of ajv-validator/ajv#395 (comment), I am not sure any coerce should happen for anyOf, oneOf and allOf when the supplied value is valid in that array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants