diff --git a/CHANGELOG.md b/CHANGELOG.md index b546e730..e5b48f68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,6 +111,17 @@ internal-only unless explicitly marked as public API. empty-body path. - `NotValidator` — removed redundant `$schema->not` truthy check after `is_bool($schema->not)` narrowing (Psalm `RedundantCondition`). +- Composition branch errors are no longer lost, duplicated, or + inconsistent between keywords (#52). A `ValidationException` thrown by + `allOf`/`anyOf`/`oneOf` now always carries a structured error list: + a value rejected during branch normalization (typically `null` against + a non-nullable branch) synthesises a `TypeMismatchError` with the + branch `dataPath`/`schemaPath` instead of an empty list; `allOf` no + longer reports each branch error twice; and the `allOf` message counts + the branches that did not match rather than one of two error buckets. + `BranchOutcome` and `ValidationResult` carry a single canonical error + list (`ValidationResult::$abstractErrors` merged into `$errors`, new + `$failedCount`), so no caller can double-count. - Infection CI job no longer OOMs; MSI thresholds realigned with the current mutation score so the gate is neither green-by-default nor unreachable. diff --git a/src/Validator/Schema/OneOfValidatorWithContext.php b/src/Validator/Schema/OneOfValidatorWithContext.php index 78263ff8..9e57977e 100644 --- a/src/Validator/Schema/OneOfValidatorWithContext.php +++ b/src/Validator/Schema/OneOfValidatorWithContext.php @@ -12,10 +12,15 @@ use Duyler\OpenApi\Validator\Exception\AbstractValidationError; use Duyler\OpenApi\Validator\Exception\DiscriminatorDataError; use Duyler\OpenApi\Validator\Exception\InvalidDataTypeException; +use Duyler\OpenApi\Validator\Exception\NestedValidationError; use Duyler\OpenApi\Validator\Exception\OneOfError; +use Duyler\OpenApi\Validator\Exception\TypeMismatchError; use Duyler\OpenApi\Validator\Exception\ValidationException; +use Duyler\OpenApi\Validator\TypeFormatter; +use function implode; use function is_array; +use function sprintf; final readonly class OneOfValidatorWithContext { @@ -83,6 +88,18 @@ private function validateWithDiscriminator(mixed $data, Schema $schema, Validati $this->discriminatorValidator->validate($data, $schema, $this->document, $dataPath, $context); } + /** + * @param string|array|null $type + */ + private function formatSchemaType(string|array|null $type): string + { + return match (true) { + null === $type => 'object', + is_array($type) => implode('|', $type), + default => $type, + }; + } + private function hasNullableSchema(array $oneOf): bool { return array_any($oneOf, fn(Schema $subSchema): bool => $subSchema->nullable @@ -93,16 +110,16 @@ private function validateWithoutDiscriminator(mixed $data, array $oneOf, Validat { $validCount = 0; $errors = []; - $abstractErrors = []; $rootValidator = $this->dependencies->rootSchemaValidator($this->document, $this->configuration); - foreach ($oneOf as $subSchema) { + foreach ($oneOf as $index => $subSchema) { if (false === $subSchema instanceof Schema) { continue; } $childContext = $context->forkForBranch(); + $schemaPath = sprintf('/oneOf/%d', $index); try { $allowNull = $context->nullableAsType && ($subSchema->nullable @@ -112,27 +129,33 @@ private function validateWithoutDiscriminator(mixed $data, array $oneOf, Validat ++$validCount; $context->mergeChildAnnotations($childContext); } catch (AbstractValidationError $e) { - $abstractErrors[] = $e; + $errors[] = $e; } catch (InvalidDataTypeException) { - continue; - } catch (ValidationException $e) { - $errors[] = new ValidationException( - message: 'Invalid data for oneOf schema: ' . $e->getMessage(), - previous: $e, - errors: $e->getErrors(), + $errors[] = new TypeMismatchError( + expected: $this->formatSchemaType($subSchema->type), + actual: TypeFormatter::format($data), + dataPath: $context->breadcrumbs->currentPath(), + schemaPath: $schemaPath, ); + } catch (ValidationException $e) { + $branchErrors = $e->getErrors(); + + if ([] === $branchErrors) { + $branchErrors = [new NestedValidationError( + dataPath: $context->breadcrumbs->currentPath(), + schemaPath: $schemaPath, + message: $e->getMessage(), + )]; + } + + $errors = [...$errors, ...$branchErrors]; } } if (0 === $validCount) { - $allErrors = $abstractErrors; - foreach ($errors as $error) { - $allErrors = [...$allErrors, ...$error->getErrors()]; - } - throw new ValidationException( 'Exactly one of schemas must match, but none did', - errors: $allErrors, + errors: $errors, ); } diff --git a/src/Validator/SchemaValidator/AbstractCompositionalValidator.php b/src/Validator/SchemaValidator/AbstractCompositionalValidator.php index 23fce04a..6fe80d0c 100644 --- a/src/Validator/SchemaValidator/AbstractCompositionalValidator.php +++ b/src/Validator/SchemaValidator/AbstractCompositionalValidator.php @@ -7,12 +7,16 @@ use Duyler\OpenApi\Schema\Model\Schema; use Duyler\OpenApi\Validator\Exception\AbstractValidationError; use Duyler\OpenApi\Validator\Exception\InvalidDataTypeException; +use Duyler\OpenApi\Validator\Exception\NestedValidationError; use Duyler\OpenApi\Validator\Exception\TooManyErrorsError; +use Duyler\OpenApi\Validator\Exception\TypeMismatchError; use Duyler\OpenApi\Validator\Exception\ValidationException; use Duyler\OpenApi\Validator\Schema\SchemaValueNormalizer; use Duyler\OpenApi\Validator\Error\ValidationContext; use Duyler\OpenApi\Validator\SchemaValidator\Internal\BranchOutcome; +use Duyler\OpenApi\Validator\TypeFormatter; +use function array_values; use function count; use function sprintf; @@ -30,41 +34,46 @@ protected function validateSchemas( string $schemaType, ): ValidationResult { $validCount = 0; + $failedCount = 0; $errors = []; - $abstractErrors = []; $dataPath = $this->getDataPath($context); - foreach ($schemas as $subSchema) { - $outcome = $this->validateBranch($data, $subSchema, $context, $schemaType); + foreach ($schemas as $index => $subSchema) { + $outcome = $this->validateBranch($data, $subSchema, $context, $schemaType, $index); if ($outcome->matched) { ++$validCount; continue; } + ++$failedCount; + foreach ($outcome->errors as $error) { $errors[] = $error; - } - - foreach ($outcome->abstractErrors as $error) { - $abstractErrors[] = $error; - if (self::MAX_COMPOSITION_ERRORS <= count($abstractErrors)) { - $abstractErrors[] = new TooManyErrorsError( + if (self::MAX_COMPOSITION_ERRORS <= count($errors)) { + $errors[] = new TooManyErrorsError( max: self::MAX_COMPOSITION_ERRORS, dataPath: $dataPath, ); - return new ValidationResult($validCount, $errors, $abstractErrors); + return new ValidationResult($validCount, $errors, $failedCount); } } } - return new ValidationResult($validCount, $errors, $abstractErrors); + return new ValidationResult($validCount, $errors, $failedCount); } - private function validateBranch(mixed $data, Schema $subSchema, ?ValidationContext $context, string $schemaType): BranchOutcome - { + private function validateBranch( + mixed $data, + Schema $subSchema, + ?ValidationContext $context, + string $schemaType, + int $index, + ): BranchOutcome { + $schemaPath = sprintf('/%s/%d', $schemaType, $index); + try { $normalizedData = $this->normalizeForBranch($data, $subSchema, $context); $validator = $this->createSchemaValidator(); @@ -77,32 +86,31 @@ private function validateBranch(mixed $data, Schema $subSchema, ?ValidationConte $validator->validate($normalizedData, $subSchema, null); } - return new BranchOutcome(matched: true, errors: [], abstractErrors: []); - } catch (InvalidDataTypeException $e) { + return new BranchOutcome(matched: true, errors: []); + } catch (InvalidDataTypeException) { return new BranchOutcome( matched: false, - errors: [new ValidationException( - sprintf('Invalid data type for %s schema: %s', $schemaType, $e->getMessage()), - previous: $e, + errors: [new TypeMismatchError( + expected: $this->formatSchemaType($subSchema->type, 'object'), + actual: TypeFormatter::format($data), + dataPath: $this->getDataPath($context), + schemaPath: $schemaPath, )], - abstractErrors: [], ); } catch (ValidationException $e) { - /** @var list $abstractErrors */ - $abstractErrors = []; - foreach ($e->getErrors() as $err) { - if ($err instanceof AbstractValidationError) { - $abstractErrors[] = $err; - } + $errors = array_values($e->getErrors()); + + if ([] === $errors) { + $errors = [new NestedValidationError( + dataPath: $this->getDataPath($context), + schemaPath: $schemaPath, + message: $e->getMessage(), + )]; } - return new BranchOutcome( - matched: false, - errors: [$e], - abstractErrors: $abstractErrors, - ); + return new BranchOutcome(matched: false, errors: $errors); } catch (AbstractValidationError $e) { - return new BranchOutcome(matched: false, errors: [], abstractErrors: [$e]); + return new BranchOutcome(matched: false, errors: [$e]); } } diff --git a/src/Validator/SchemaValidator/AllOfValidator.php b/src/Validator/SchemaValidator/AllOfValidator.php index 11929b61..16241c28 100644 --- a/src/Validator/SchemaValidator/AllOfValidator.php +++ b/src/Validator/SchemaValidator/AllOfValidator.php @@ -9,7 +9,6 @@ use Duyler\OpenApi\Validator\Exception\ValidationException; use Override; -use function count; use function sprintf; final readonly class AllOfValidator extends AbstractCompositionalValidator implements KeywordApplicable @@ -29,18 +28,10 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex $result = $this->validateSchemas($schema->allOf, $data, $context, 'allOf'); - if ([] !== $result->errors || [] !== $result->abstractErrors) { - $allErrors = $result->abstractErrors; - - foreach ($result->errors as $exception) { - foreach ($exception->getErrors() as $error) { - $allErrors[] = $error; - } - } - + if (0 !== $result->failedCount) { throw new ValidationException( - sprintf('All of the schemas must match, but %d failed', count($result->errors)), - errors: $allErrors, + sprintf('All of the schemas must match, but %d failed', $result->failedCount), + errors: $result->errors, ); } } diff --git a/src/Validator/SchemaValidator/AnyOfValidator.php b/src/Validator/SchemaValidator/AnyOfValidator.php index e4a2cdc2..1a5348da 100644 --- a/src/Validator/SchemaValidator/AnyOfValidator.php +++ b/src/Validator/SchemaValidator/AnyOfValidator.php @@ -29,7 +29,7 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex if (0 === $result->validCount) { throw new ValidationException( 'At least one of the schemas must match, but none did', - errors: $result->abstractErrors, + errors: $result->errors, ); } } diff --git a/src/Validator/SchemaValidator/Internal/BranchOutcome.php b/src/Validator/SchemaValidator/Internal/BranchOutcome.php index 28102d76..f2d467d3 100644 --- a/src/Validator/SchemaValidator/Internal/BranchOutcome.php +++ b/src/Validator/SchemaValidator/Internal/BranchOutcome.php @@ -4,19 +4,17 @@ namespace Duyler\OpenApi\Validator\SchemaValidator\Internal; -use Duyler\OpenApi\Validator\Exception\AbstractValidationError; -use Duyler\OpenApi\Validator\Exception\ValidationException; +use Duyler\OpenApi\Validator\Exception\ValidationErrorInterface; /** @internal */ final readonly class BranchOutcome { /** - * @param list $errors - * @param list $abstractErrors + * @param list $errors Canonical error list for the branch, + * empty only when the branch matched. */ public function __construct( public bool $matched, public array $errors, - public array $abstractErrors, ) {} } diff --git a/src/Validator/SchemaValidator/OneOfValidator.php b/src/Validator/SchemaValidator/OneOfValidator.php index 6c9176d0..fa976964 100644 --- a/src/Validator/SchemaValidator/OneOfValidator.php +++ b/src/Validator/SchemaValidator/OneOfValidator.php @@ -30,7 +30,7 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex if (0 === $result->validCount) { throw new ValidationException( 'Exactly one of the schemas must match, but none did', - errors: $result->abstractErrors, + errors: $result->errors, ); } diff --git a/src/Validator/SchemaValidator/ValidationResult.php b/src/Validator/SchemaValidator/ValidationResult.php index c4257d95..6b9503cd 100644 --- a/src/Validator/SchemaValidator/ValidationResult.php +++ b/src/Validator/SchemaValidator/ValidationResult.php @@ -5,15 +5,13 @@ namespace Duyler\OpenApi\Validator\SchemaValidator; use Duyler\OpenApi\Validator\Exception\ValidationErrorInterface; -use Duyler\OpenApi\Validator\Exception\ValidationException; final readonly class ValidationResult { public function __construct( public readonly int $validCount, - /** @var array */ - public readonly array $errors, /** @var array */ - public readonly array $abstractErrors, + public readonly array $errors, + public readonly int $failedCount = 0, ) {} } diff --git a/tests/Functional/Response/CompositionBranchErrorsTest.php b/tests/Functional/Response/CompositionBranchErrorsTest.php new file mode 100644 index 00000000..dbbb5337 --- /dev/null +++ b/tests/Functional/Response/CompositionBranchErrorsTest.php @@ -0,0 +1,103 @@ +psrFactory = new Psr17Factory(); + $this->validator = OpenApiValidatorBuilder::create() + ->fromYamlString(self::SPEC) + ->build(); + } + + #[Test] + public function null_body_reports_a_structured_error_with_a_data_path(): void + { + $exception = $this->assertRejects('null'); + + $this->assertNotEmpty($exception->getErrors()); + $this->assertNotSame('', $this->validator->getFormattedErrors($exception)); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function failing_body_reports_each_error_exactly_once(): void + { + $exception = $this->assertRejects('{"type":"widgets"}'); + + $identities = array_map( + static fn($error): string => $error->keyword() . '@' . $error->dataPath(), + $exception->getErrors(), + ); + + $this->assertCount(1, $identities); + $this->assertCount(count(array_unique($identities)), $identities); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + private function assertRejects(string $body): ValidationException + { + $request = $this->psrFactory->createServerRequest('GET', '/forms'); + $operation = $this->validator->validateRequest($request); + + $response = $this->psrFactory->createResponse(200) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->psrFactory->createStream($body)); + + try { + $this->validator->validateResponse($response, $operation); + } catch (ValidationException $e) { + return $e; + } + + $this->fail('Expected the response body to be rejected'); + } +} diff --git a/tests/Functional/Schema/CompositionBranchErrorsTest.php b/tests/Functional/Schema/CompositionBranchErrorsTest.php new file mode 100644 index 00000000..fb59449e --- /dev/null +++ b/tests/Functional/Schema/CompositionBranchErrorsTest.php @@ -0,0 +1,202 @@ +validator = OpenApiValidatorBuilder::create() + ->fromYamlString(self::SPEC) + ->build(); + } + + #[Test] + public function all_of_reports_a_structured_error_when_null_is_rejected_by_every_branch(): void + { + $exception = $this->assertRejects('AllOfWrapper', null); + + $this->assertNotEmpty($exception->getErrors()); + $this->assertHasDataPath($exception); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function any_of_reports_a_structured_error_when_null_is_rejected_by_every_branch(): void + { + $exception = $this->assertRejects('AnyOfWrapper', null); + + $this->assertNotEmpty($exception->getErrors()); + $this->assertHasDataPath($exception); + $this->assertSame('At least one of the schemas must match, but none did', $exception->getMessage()); + } + + #[Test] + public function one_of_reports_a_structured_error_when_null_is_rejected_by_every_branch(): void + { + $exception = $this->assertRejects('OneOfWrapper', null); + + $this->assertNotEmpty($exception->getErrors()); + $this->assertHasDataPath($exception); + $this->assertSame('Exactly one of schemas must match, but none did', $exception->getMessage()); + } + + #[Test] + public function all_of_counts_the_failing_branch_when_the_value_is_not_an_object(): void + { + $exception = $this->assertRejects('AllOfWrapper', 'not-an-object'); + + $this->assertCount(1, $exception->getErrors()); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function all_of_reports_a_failing_branch_error_exactly_once(): void + { + $exception = $this->assertRejects('AllOfWrapper', ['type' => 'widgets']); + + $this->assertCount(1, $exception->getErrors()); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function all_of_counts_every_failing_branch(): void + { + $exception = $this->assertRejects('AllOfTwoBranches', ['type' => 'widgets']); + + $this->assertSame('All of the schemas must match, but 2 failed', $exception->getMessage()); + $this->assertCount(2, $exception->getErrors()); + $this->assertNoDuplicates($exception); + } + + /** + * `oneOf` is deliberately excluded: it is dispatched to + * {@see OneOfValidatorWithContext}, which evaluates + * keyword groups in a different order, so it surfaces a different — but equally + * valid — violation of the same branch. See the "Related observation" in #52. + */ + #[Test] + public function all_of_and_any_of_report_the_same_errors_for_the_same_failing_branch(): void + { + $allOf = $this->identities($this->assertRejects('AllOfWrapper', ['type' => 'widgets'])); + $anyOf = $this->identities($this->assertRejects('AnyOfWrapper', ['type' => 'widgets'])); + + $this->assertSame($allOf, $anyOf); + $this->assertCount(1, $this->identities($this->assertRejects('OneOfWrapper', ['type' => 'widgets']))); + } + + #[Test] + public function formatted_errors_are_never_empty_and_never_repeat(): void + { + $cases = [ + ['AllOfWrapper', null], + ['AnyOfWrapper', null], + ['OneOfWrapper', null], + ['AllOfWrapper', 'not-an-object'], + ['AllOfWrapper', ['type' => 'widgets']], + ['AnyOfWrapper', ['type' => 'widgets']], + ['OneOfWrapper', ['type' => 'widgets']], + ['AllOfTwoBranches', ['type' => 'widgets']], + ]; + + foreach ($cases as [$schema, $data]) { + $exception = $this->assertRejects($schema, $data); + + $this->assertNotSame('', $this->validator->getFormattedErrors($exception), $schema); + $this->assertNoDuplicates($exception, $schema); + } + } + + private function assertRejects(string $schema, mixed $data): ValidationException + { + try { + $this->validator->validateSchema($data, '#/components/schemas/' . $schema); + } catch (ValidationException $e) { + return $e; + } + + $this->fail('Expected ' . $schema . ' to reject the value'); + } + + private function assertHasDataPath(ValidationException $exception): void + { + foreach ($exception->getErrors() as $error) { + if ('' !== $error->dataPath()) { + $this->addToAssertionCount(1); + return; + } + } + + $this->fail('Expected at least one error carrying a dataPath'); + } + + private function assertNoDuplicates(ValidationException $exception, string $message = ''): void + { + $identities = $this->identities($exception); + + $this->assertCount(count(array_unique($identities)), $identities, $message); + } + + /** + * @return list + */ + private function identities(ValidationException $exception): array + { + return array_map( + static fn($error): string => $error->keyword() . '@' . $error->dataPath() . '@' . $error->schemaPath(), + $exception->getErrors(), + ); + } +} diff --git a/tests/Integration/Validator/Schema/OneOfValidatorWithContextTest.php b/tests/Integration/Validator/Schema/OneOfValidatorWithContextTest.php index bf631e5c..bab5ffe1 100644 --- a/tests/Integration/Validator/Schema/OneOfValidatorWithContextTest.php +++ b/tests/Integration/Validator/Schema/OneOfValidatorWithContextTest.php @@ -24,6 +24,7 @@ use PHPUnit\Framework\TestCase; use RuntimeException; +use function array_map; use function sprintf; final class OneOfValidatorWithContextTest extends TestCase @@ -415,6 +416,118 @@ public function validate_exception_contains_errors_when_none_match(): void } } + #[Test] + public function validate_reports_structured_errors_when_null_is_rejected_by_every_branch(): void + { + $schema = new Schema( + oneOf: [ + new Schema(type: 'object', required: ['type', 'id']), + ], + ); + + try { + $this->validator->validateWithContextIgnoringDiscriminator(null, $schema, $this->context); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertNotEmpty($e->getErrors()); + self::assertSame('type', $e->getErrors()[0]->keyword()); + self::assertSame('/', $e->getErrors()[0]->dataPath()); + self::assertSame('/oneOf/0', $e->getErrors()[0]->schemaPath()); + } + } + + #[Test] + public function validate_reports_each_branch_error_exactly_once(): void + { + $schema = new Schema( + oneOf: [ + new Schema(type: 'object', required: ['name']), + ], + ); + + try { + $this->validator->validateWithContextIgnoringDiscriminator(['other' => 'value'], $schema, $this->context); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertCount(1, $e->getErrors()); + self::assertSame('required', $e->getErrors()[0]->keyword()); + } + } + + /** + * @return iterable + */ + public static function rejectedNullBranchProvider(): iterable + { + yield 'untyped branch' => [new Schema(required: ['a']), 'object']; + yield 'scalar type' => [new Schema(type: 'string'), 'string']; + yield 'type array' => [new Schema(type: ['string', 'integer']), 'string|integer']; + } + + #[Test] + #[DataProvider('rejectedNullBranchProvider')] + public function validate_names_the_branch_type_when_null_is_rejected(Schema $branch, string $expectedType): void + { + $schema = new Schema(oneOf: [$branch]); + + try { + $this->validator->validateWithContextIgnoringDiscriminator(null, $schema, $this->context); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertCount(1, $e->getErrors()); + self::assertSame( + sprintf('Expected type "%s", but got "null" at /', $expectedType), + $e->getErrors()[0]->message(), + ); + } + } + + #[Test] + public function validate_accumulates_errors_from_every_failing_branch(): void + { + $schema = new Schema( + oneOf: [ + new Schema(type: 'object', required: ['a'], properties: ['a' => new Schema(type: 'integer')]), + new Schema(type: 'object', required: ['b']), + ], + ); + + try { + $this->validator->validateWithContextIgnoringDiscriminator(['a' => 'not-an-int'], $schema, $this->context); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + $keywords = array_map( + static fn($error): string => $error->keyword(), + $e->getErrors(), + ); + + self::assertSame(['type', 'required'], $keywords); + } + } + + #[Test] + public function validate_preserves_every_error_reported_by_a_single_branch(): void + { + $schema = new Schema( + oneOf: [ + new Schema( + type: 'object', + properties: ['a' => new Schema(type: 'integer')], + additionalProperties: false, + ), + ], + ); + + try { + $this->validator->validateWithContextIgnoringDiscriminator(['x' => 1, 'y' => 2], $schema, $this->context); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertCount(2, $e->getErrors()); + self::assertSame('additionalProperties', $e->getErrors()[0]->keyword()); + self::assertSame('additionalProperties', $e->getErrors()[1]->keyword()); + } + } + #[Test] public function validate_without_discriminator_with_ref(): void { diff --git a/tests/Unit/Validator/SchemaValidator/CompositionBranchErrorsTest.php b/tests/Unit/Validator/SchemaValidator/CompositionBranchErrorsTest.php new file mode 100644 index 00000000..e43d2770 --- /dev/null +++ b/tests/Unit/Validator/SchemaValidator/CompositionBranchErrorsTest.php @@ -0,0 +1,215 @@ +dependencies = new ValidatorDependencies( + pool: new ValidatorPool(), + formatRegistry: BuiltinFormats::create(), + ); + } + + /** + * @return iterable + */ + public static function compositionKeywordProvider(): iterable + { + $dependencies = new ValidatorDependencies( + pool: new ValidatorPool(), + formatRegistry: BuiltinFormats::create(), + ); + + yield 'allOf' => [new AllOfValidator($dependencies), new Schema(allOf: [self::formIdentifier()])]; + yield 'anyOf' => [new AnyOfValidator($dependencies), new Schema(anyOf: [self::formIdentifier()])]; + yield 'oneOf' => [new OneOfValidator($dependencies), new Schema(oneOf: [self::formIdentifier()])]; + } + + #[Test] + #[DataProvider('compositionKeywordProvider')] + public function null_rejected_by_every_branch_reports_a_structured_error(KeywordApplicable $validator, Schema $schema): void + { + $exception = $this->assertRejects($validator, $schema, null); + + $this->assertNotEmpty($exception->getErrors()); + + foreach ($exception->getErrors() as $error) { + $this->assertSame('/', $error->dataPath()); + } + } + + #[Test] + #[DataProvider('compositionKeywordProvider')] + public function failing_branch_is_reported_exactly_once(KeywordApplicable $validator, Schema $schema): void + { + $exception = $this->assertRejects($validator, $schema, ['type' => 'widgets']); + + $identities = array_map( + static fn($error): string => $error->keyword() . '@' . $error->dataPath() . '@' . $error->schemaPath(), + $exception->getErrors(), + ); + + $this->assertCount(1, $identities); + $this->assertCount(count(array_unique($identities)), $identities); + } + + #[Test] + public function all_of_counts_the_failing_branch_for_a_non_object_value(): void + { + $validator = new AllOfValidator($this->dependencies); + $schema = new Schema(allOf: [self::formIdentifier()]); + + $exception = $this->assertRejects($validator, $schema, 'not-an-object'); + + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + $this->assertCount(1, $exception->getErrors()); + } + + #[Test] + public function all_of_counts_every_failing_branch(): void + { + $validator = new AllOfValidator($this->dependencies); + $schema = new Schema(allOf: [ + self::formIdentifier(), + new Schema(type: 'object', required: ['widget']), + ]); + + $exception = $this->assertRejects($validator, $schema, ['type' => 'widgets']); + + $this->assertSame('All of the schemas must match, but 2 failed', $exception->getMessage()); + $this->assertCount(2, $exception->getErrors()); + } + + #[Test] + public function all_of_counts_only_the_branches_that_did_not_match(): void + { + $validator = new AllOfValidator($this->dependencies); + $schema = new Schema(allOf: [ + new Schema(type: 'string'), + new Schema(type: 'string', minLength: 20), + ]); + + $exception = $this->assertRejects($validator, $schema, 'short'); + + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function every_keyword_reports_the_same_errors_for_the_same_failing_branch(): void + { + $identities = []; + + foreach (self::compositionKeywordProvider() as $keyword => [$validator, $schema]) { + $identities[$keyword] = array_map( + static fn($error): string => $error->keyword() . '@' . $error->dataPath() . '@' . $error->schemaPath(), + $this->assertRejects($validator, $schema, ['type' => 'widgets'])->getErrors(), + ); + } + + $this->assertSame($identities['allOf'], $identities['anyOf']); + $this->assertSame($identities['allOf'], $identities['oneOf']); + } + + #[Test] + public function rejected_null_is_reported_against_the_branch_that_rejected_it(): void + { + $validator = new AllOfValidator($this->dependencies); + $schema = new Schema(allOf: [ + new Schema(type: 'string'), + self::formIdentifier(), + ]); + + $exception = $this->assertRejects($validator, $schema, null); + + $this->assertCount(2, $exception->getErrors()); + $this->assertSame('Expected type "string", but got "null" at /', $exception->getErrors()[0]->message()); + $this->assertSame('/allOf/0', $exception->getErrors()[0]->schemaPath()); + $this->assertSame('Expected type "object", but got "null" at /', $exception->getErrors()[1]->message()); + $this->assertSame('/allOf/1', $exception->getErrors()[1]->schemaPath()); + $this->assertSame('All of the schemas must match, but 2 failed', $exception->getMessage()); + } + + #[Test] + public function all_of_preserves_every_error_reported_by_a_single_branch(): void + { + $validator = new AllOfValidator($this->dependencies); + $schema = new Schema(allOf: [ + new Schema( + type: 'object', + properties: ['a' => new Schema(type: 'integer')], + additionalProperties: false, + ), + ]); + + $exception = $this->assertRejects($validator, $schema, ['x' => 1, 'y' => 2]); + + $this->assertCount(2, $exception->getErrors()); + $this->assertSame('All of the schemas must match, but 1 failed', $exception->getMessage()); + } + + #[Test] + public function composition_errors_are_capped(): void + { + $branches = []; + + for ($i = 0; $i < 30; ++$i) { + $branches[] = new Schema(type: 'object', required: ['missing' . $i]); + } + + $validator = new AllOfValidator($this->dependencies); + $exception = $this->assertRejects($validator, new Schema(allOf: $branches), []); + + $this->assertCount(21, $exception->getErrors()); + $this->assertSame('composition', $exception->getErrors()[20]->keyword()); + } + + private static function formIdentifier(): Schema + { + return new Schema( + type: 'object', + required: ['type', 'id'], + properties: [ + 'type' => new Schema(type: 'string', enum: ['forms']), + 'id' => new Schema(type: 'string'), + ], + ); + } + + private function assertRejects(KeywordApplicable $validator, Schema $schema, mixed $data): ValidationException + { + try { + $validator->validate($data, $schema); + } catch (ValidationException $e) { + return $e; + } + + $this->fail('Expected the value to be rejected'); + } +} diff --git a/tests/Unit/Validator/SchemaValidator/ValidationResultTest.php b/tests/Unit/Validator/SchemaValidator/ValidationResultTest.php index 6803f2c8..3d6280c0 100644 --- a/tests/Unit/Validator/SchemaValidator/ValidationResultTest.php +++ b/tests/Unit/Validator/SchemaValidator/ValidationResultTest.php @@ -5,7 +5,6 @@ namespace Duyler\OpenApi\Test\Unit\Validator\SchemaValidator; use Duyler\OpenApi\Validator\Exception\AbstractValidationError; -use Duyler\OpenApi\Validator\Exception\ValidationException; use Duyler\OpenApi\Validator\SchemaValidator\ValidationResult; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -17,61 +16,52 @@ final class ValidationResultTest extends TestCase #[Test] public function create_result_with_valid_data(): void { - $result = new ValidationResult(1, [], []); + $result = new ValidationResult(1, []); $this->assertSame(1, $result->validCount); $this->assertSame([], $result->errors); - $this->assertSame([], $result->abstractErrors); + $this->assertSame(0, $result->failedCount); } #[Test] public function create_result_with_errors(): void { - $error = new ValidationException('Test error'); - $result = new ValidationResult(0, [$error], []); + $error = $this->createStub(AbstractValidationError::class); + $result = new ValidationResult(0, [$error], 1); $this->assertSame(0, $result->validCount); $this->assertCount(1, $result->errors); $this->assertSame($error, $result->errors[0]); - $this->assertSame([], $result->abstractErrors); - } - - #[Test] - public function create_result_with_abstract_errors(): void - { - $abstractError = $this->createStub(AbstractValidationError::class); - $result = new ValidationResult(0, [], [$abstractError]); - - $this->assertSame(0, $result->validCount); - $this->assertSame([], $result->errors); - $this->assertCount(1, $result->abstractErrors); - $this->assertSame($abstractError, $result->abstractErrors[0]); + $this->assertSame(1, $result->failedCount); } #[Test] public function properties_are_readonly(): void { - $result = new ValidationResult(5, [], []); + $result = new ValidationResult(5, []); $this->assertSame(5, $result->validCount); } #[Test] - public function create_result_with_multiple_errors_and_abstract_errors(): void + public function create_result_with_multiple_errors(): void { - $error1 = new ValidationException('Error 1'); - $error2 = new ValidationException('Error 2'); - $abstractError1 = $this->createStub(AbstractValidationError::class); - $abstractError2 = $this->createStub(AbstractValidationError::class); + $error1 = $this->createStub(AbstractValidationError::class); + $error2 = $this->createStub(AbstractValidationError::class); - $result = new ValidationResult( - 1, - [$error1, $error2], - [$abstractError1, $abstractError2], - ); + $result = new ValidationResult(1, [$error1, $error2], 2); $this->assertSame(1, $result->validCount); $this->assertCount(2, $result->errors); - $this->assertCount(2, $result->abstractErrors); + $this->assertSame(2, $result->failedCount); + } + + #[Test] + public function failed_count_defaults_to_zero(): void + { + $error = $this->createStub(AbstractValidationError::class); + $result = new ValidationResult(0, [$error]); + + $this->assertSame(0, $result->failedCount); } }