Composition validators lose, duplicate, and disagree about branch errors; getErrors() can be empty

[shadowhand]

PHP version

8.5

duyler/openapi version

0.7.0 (also reproduces on main at dd09fb3)

OpenAPI spec version

3.0

Description

When validation fails inside allOf / anyOf / oneOf, the structured error list on the thrown ValidationException is unreliable. Three distinct symptoms, all in the branch-error plumbing of AbstractCompositionalValidator:

1. getErrors() returns an empty list when a branch rejects the value during normalization — in practice, whenever the data is null and no branch is nullable. getFormattedErrors() then returns an empty string, so a middleware has literally nothing to report to the client: no keyword, no dataPath, and the exception message carries no path either.
2. allOf reports every branch error twice. anyOf and oneOf report each once, so the count depends on which keyword wraps the schema.
3. The allOf message miscounts failures — "All of the schemas must match, but 0 failed" is thrown alongside a non-empty error list.

Taken together, the same value validated against the same effective schema yields a different error set depending only on the composition keyword used to wrap it, and one of those sets is empty. This is what makes a JSON:API-style error response impossible to build: you cannot tell the client which member was rejected.

This is the secondary observation split out of #50 as requested. It is independent of that fix — verified to reproduce identically with #51 applied.

Steps to reproduce

<?php

declare(strict_types=1);

use Duyler\OpenApi\Builder\OpenApiValidatorBuilder;
use Duyler\OpenApi\Validator\Exception\ValidationException;

require __DIR__ . '/vendor/autoload.php';

$validator = OpenApiValidatorBuilder::create()
    ->fromYamlFile('composition-errors.yaml')
    ->build();

$cases = [
    'allOf + null'                => ['AllOfWrapper', null],
    'anyOf + null'                => ['AnyOfWrapper', null],
    'oneOf + null'                => ['OneOfWrapper', null],
    'allOf + non-object'          => ['AllOfWrapper', 'not-an-object'],
    'branch alone + bad object'   => ['FormIdentifier', ['type' => 'widgets']],
    'allOf + bad object'          => ['AllOfWrapper', ['type' => 'widgets']],
    'anyOf + bad object'          => ['AnyOfWrapper', ['type' => 'widgets']],
    'oneOf + bad object'          => ['OneOfWrapper', ['type' => 'widgets']],
];

foreach ($cases as $label => [$schema, $data]) {
    try {
        $validator->validateSchema($data, '#/components/schemas/' . $schema);
        printf("%-26s PASSED\n", $label);
    } catch (ValidationException $e) {
        $errors = $e->getErrors();
        printf(
            "%-26s errors=%d  %-55s %s\n",
            $label,
            count($errors),
            '"' . $e->getMessage() . '"',
            implode(', ', array_map(
                static fn($err): string => $err->keyword() . '@' . ($err->dataPath() ?: '/'),
                $errors,
            )),
        );
    }
}

Schema, e.g. composition-errors.yaml:

openapi: 3.0.3
info:
  title: composition error reporting
  version: 1.0.0
components:
  schemas:
    FormIdentifier:
      type: object
      required: [type, id]
      properties:
        type:
          type: string
          enum: [forms]
        id:
          type: string
    AllOfWrapper:
      allOf:
        - $ref: '#/components/schemas/FormIdentifier'
    AnyOfWrapper:
      anyOf:
        - $ref: '#/components/schemas/FormIdentifier'
    OneOfWrapper:
      oneOf:
        - $ref: '#/components/schemas/FormIdentifier'

Actual result

allOf + null               errors=0  "All of the schemas must match, but 1 failed"
anyOf + null               errors=0  "At least one of the schemas must match, but none did"
oneOf + null               errors=0  "Exactly one of schemas must match, but none did"
allOf + non-object         errors=1  "All of the schemas must match, but 0 failed"           type@/
branch alone + bad object  errors=1  "Missing required properties: ... at /"                 required@/
allOf + bad object         errors=2  "All of the schemas must match, but 1 failed"           enum@/type, enum@/type
anyOf + bad object         errors=1  "At least one of the schemas must match, but none did"  enum@/type
oneOf + bad object         errors=1  "Exactly one of schemas must match, but none did"       required@/

Expected: a non-empty, de-duplicated error list for every failing case, and a failure count in the message that matches the number of branches that actually failed.

The same three symptoms show up through the PSR-7 path. With content.application/json.schema set to {allOf: [$ref FormIdentifier]}:

body "null"                → "All of the schemas must match, but 1 failed", getErrors() = 0
                             getFormattedErrors() = ""
body {"type":"widgets"}    → "All of the schemas must match, but 1 failed", getErrors() = 2
                             getFormattedErrors() = the same enum error printed twice

Root cause

1. Empty list. AbstractCompositionalValidator::validateBranch() (src/Validator/SchemaValidator/AbstractCompositionalValidator.php:81-89) catches the InvalidDataTypeException that SchemaValueNormalizer::normalize() throws for a value it cannot hand to a branch, and wraps it in a bare ValidationException with no errors: argument and abstractErrors: []:

} catch (InvalidDataTypeException $e) {
    return new BranchOutcome(
        matched: false,
        errors: [new ValidationException(
            sprintf('Invalid data type for %s schema: %s', $schemaType, $e->getMessage()),
            previous: $e,
        )],
        abstractErrors: [],
    );
}

Nothing structured is ever synthesised for the rejected value — unlike TypeValidator, which throws a TypeMismatchError carrying dataPath and schemaPath. AllOfValidator merges abstractErrors (empty) with each wrapper's getErrors() (also empty); AnyOfValidator and OneOfValidator forward only $result->abstractErrors. Either way the final list is [].

2. Duplication in allOf. validateBranch()'s catch (ValidationException $e) (:90-103) returns the branch failure twice — once as errors: [$e] and again as abstractErrors: [...] extracted from $e->getErrors(). AllOfValidator::validate() (src/Validator/SchemaValidator/AllOfValidator.php:32-45) then seeds $allErrors from $result->abstractErrors and appends every $exception->getErrors(), so each AbstractValidationError lands in the list twice. AnyOfValidator and OneOfValidator only read abstractErrors, which is why the three keywords disagree.

3. Miscounted failures. AllOfValidator formats the message with count($result->errors), but a branch that fails by throwing a bare AbstractValidationError is recorded in abstractErrors only (:104-106), so it never reaches that counter — hence "but 0 failed" next to a reported error.

Suggested direction: give BranchOutcome a single, canonical per-branch error list rather than two overlapping ones, synthesise a TypeMismatchError (with the branch's dataPath/schemaPath) where InvalidDataTypeException is currently swallowed, and derive the failure count from the number of branches that did not match.

Related observation

branch alone + bad object reports required@/ while allOf + bad object reports enum@/type for the same data — and neither reports both violations. Branches are validated through AbstractCompositionalValidator::createSchemaValidator() (the non-context SchemaValidator) rather than the SchemaValidatorWithContext path used at the top level, and the two evaluate keyword groups in a different order, so which violation surfaces first depends on the wrapper. Possibly worth its own issue; noting it here because it is visible in the same repro output.

[milinsky]

@shadowhand Thanks for your issue. I will soon release a large number of fixes and improvements from the dev branch and return to your pull requests.