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

Miss assertJson like in phpunit #217

Open
c33s opened this issue Aug 29, 2020 · 11 comments
Open

Miss assertJson like in phpunit #217

c33s opened this issue Aug 29, 2020 · 11 comments

Comments

@c33s
Copy link

c33s commented Aug 29, 2020

No description provided.

@zerkms
Copy link
Contributor

zerkms commented Aug 29, 2020

What would be the use case?

@c33s
Copy link
Author

c33s commented Aug 29, 2020

i don't really understand the question, well to test if a string is json is the use case and to not repeat myself using constructs based upon https://github.com/sebastianbergmann/phpunit/blob/e15dfe6ebddacd7ed093b008332f775a4cc077f3/src/Framework/Constraint/String/IsJson.php

in my case i store json in the database, so its not enough to check for string, it must be json so the database is able to query that field

@zerkms
Copy link
Contributor

zerkms commented Aug 29, 2020

But why? What you do with that string next? Do you deserialise it?

@c33s
Copy link
Author

c33s commented Aug 29, 2020

i make a doctrine database type for value objects which are stored as json in the database. the conversion should of course check if the json is valid.

why is the exact usecase so important? it is simply handy to assert for json it's a common type nowadays.

it would also be handy to assert for valid path, windows path, linux path, protocoll and much more.

@zerkms
Copy link
Contributor

zerkms commented Aug 29, 2020

the conversion should of course check if the json is valid.

why assert then?

why is the exact usecase so important? it is simply handy to assert for json it's a common type nowadays.

because I personally never use JSON without deserialising it 🤷

@c33s
Copy link
Author

c33s commented Aug 29, 2020

it's a generic doctrine type to store value objects in database, so the type must not care about the data inside the json but it must care that it's valid json.

    public function convertToPHPValue($value, AbstractPlatform $platform): ?ArrayInterface
    {
        Assert::nullOrString($value);
        $this->validateConfiguredValueObjectClass();
        if (null === $value) {
            return null;
        }
        Assert::true($this->isJson($value));

        /** @var ArrayInterface $class */
        $class = $this->valueObjectClass;

        return $class::fromArray(json_decode($value, true));
    }

    private function isJson($value)
    {
        if (!is_string($value)) {
            return false;
        }

        \json_decode($value);

        if (\json_last_error()) {
            return false;
        }

        return true;
    }

An empty string is not valid JSON

you took the code from the error description not from the matches method. https://github.com/sebastianbergmann/phpunit/blob/e15dfe6ebddacd7ed093b008332f775a4cc077f3/src/Framework/Constraint/String/IsJson.php#L35

don't know how phpunit handles the error desciption output and if it maybe automatically adds a Not to the build up string but the matches implemenation looks good to me.

why assert then?

why not assert? what else to do there? ignore invalid json?

edit:
simply missing a Assert::nullOrJson($value) method above

@zerkms
Copy link
Contributor

zerkms commented Aug 29, 2020

        Assert::true($this->isJson($value));

        /** @var ArrayInterface $class */
        $class = $this->valueObjectClass;

        return $class::fromArray(json_decode($value, true));

In this code you must check the json_decode return value. If you do - you'll see the json assertion would become redundant.

@c33s
Copy link
Author

c33s commented Aug 29, 2020

what if i prefer to check it before and don't mind the double decode?

edit:
for me this way i more readable

@zerkms
Copy link
Contributor

zerkms commented Aug 29, 2020

I don't see how it makes any sense, but point taken.

@jeetonweb
Copy link

Hey Guys, If I take a JSON string as an input and want to validate it how would I do it through Assert ?

Also, can you guys give some example on extending Assert features ?

@andrew-demb
Copy link
Contributor

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

No branches or pull requests

4 participants