openapi-psr7-validator icon indicating copy to clipboard operation
openapi-psr7-validator copied to clipboard

Missing inner exception message/detail when using AnyOf/OneOf

Open vsouz4 opened this issue 5 years ago • 4 comments

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 🤷‍♂ 🙂

vsouz4 avatar Aug 16 '19 07:08 vsouz4

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

scaytrase avatar Aug 16 '19 07:08 scaytrase

"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

scaytrase avatar Aug 16 '19 07:08 scaytrase

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

vsouz4 avatar Aug 16 '19 08:08 vsouz4

Moved this into https://github.com/thephpleague/openapi-psr7-validator/issues/13

scaytrase avatar Sep 27 '19 07:09 scaytrase