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

Missing inner exception message/detail when using AnyOf/OneOf #61

Open
vsouz4 opened this issue Aug 16, 2019 · 4 comments
Open

Missing inner exception message/detail when using AnyOf/OneOf #61

vsouz4 opened this issue Aug 16, 2019 · 4 comments

Comments

@vsouz4
Copy link

vsouz4 commented Aug 16, 2019

Not sure that could be considered as a bug or maybe an improvement...

When checking the solution applied for #57 I realised that this library discards the exceptions from the inner validations done with OneOf and AnyOf:

Here, for example src/Schema/Keywords/OneOf.php:

        foreach ($oneOf as $schema) {
            try {
                $schemaValidator->validate($data, $schema, $this->dataBreadCrumb);
                $matchedCount++;
            } catch (SchemaMismatch $e) {
                // that did not match... its ok         <<<<<<<<< HERE >>>>>>>>>>
            }
        }

        if ($matchedCount !== 1) {
            throw KeywordMismatch::fromKeyword('oneOf', $data, sprintf('Data must match exactly one schema, but matched %d', $matchedCount));
        }

so if we have in definition something like:

      properties:
        some_property_here:
          oneOf:
            - type: object
            - type: string
              maxLength: 10
              minLength: 1

And send a request with:

{
    "some_property_here": "String with more than 10 characters"
}

One would expected to see the message:
Keyword validation failed: Data must match exactly one schema, but matched 0 from the OneOf (which is ok and displaying correctly) but also the reason why it didn't match:
Length of '35' must be shorter or equal to 10 but this last message gets discarded when we catch SchemaMismatch $e and don't do anything with it...

So maybe there's a way to display those failed constraints and messages to get more specific errors?

The idea I had was to use the previous feature from exceptions as you're doing already in some places. So here (with some line breaks to make it more readable):

        $prev = null;
        foreach ($oneOf as $schema) {
            try {
                $schemaValidator->validate($data, $schema, $this->dataBreadCrumb);
                $matchedCount++;
            } catch (SchemaMismatch $e) {
                $prev = $e; // that did not match... its ok
            }
        }

...

            throw KeywordMismatch::fromKeyword(
                'oneOf',
                $data,
                sprintf('Data must match exactly one schema, but matched %d', $matchedCount),
                $prev // <<< HERE, there's already this argument in `fromKeyword` method
            );

Of course you need to consider that this catch block might get executed more than once, and you would need to somehow get/group all those inner exceptions (with my example I'm overriding $prev everytime)... idk, maybe we could start with at least 1 specific exception (which is better than none) and implement this multiple exception handling later 🤷‍♂ 🙂

@scaytrase
Copy link
Contributor

I think one can create extended exception like InvalidSchemaCombination extends KeywordMismatch with accumulated exceptions per schema. I would consider this a new feature

@scaytrase
Copy link
Contributor

"Previous" idea won't work easily, do demonstrate it just swap your oneOf schemas order. you would get an error that string is not an object - not really useful error

@vsouz4
Copy link
Author

vsouz4 commented Aug 16, 2019

you're right, the InvalidSchemaCombination would be better solution 🙂

@scaytrase
Copy link
Contributor

Moved this into thephpleague/openapi-psr7-validator#13

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

2 participants