From 8edf71a1ae0d41725bbd8c73e786762dc9cc1aab Mon Sep 17 00:00:00 2001
From: Woody Gilk <woody.gilk@gmail.com>
Date: Tue, 11 Aug 2026 07:26:11 -0500
Subject: [PATCH] fix: Cap composition error collection, not branch iteration
 (#54)

AbstractCompositionalValidator::validateSchemas() stopped collecting
errors at MAX_COMPOSITION_ERRORS with a `return`, which left the outer
`foreach ($schemas ...)` loop as well. Every branch after the cap went
unevaluated, so $validCount could not grow: anyOf reported "At least one
of the schemas must match, but none did" for data that a later branch
matches, and oneOf could under-count matches. Validation outcome
depended on branch declaration order, which anyOf semantics forbid.

The cap now tracks a $capped flag and breaks out of the error loop only;
the branch loop continues so every branch is still evaluated. Error
formatting is still skipped for post-cap branches, so the cap keeps
doing the work it was added for.

Error output is unchanged: 20 collected errors plus exactly one
TooManyErrorsError marker, no matter how many branches follow.

Closes #54
---
 CHANGELOG.md                                  |  16 +-
 .../AbstractCompositionalValidator.php        |   8 +-
 .../Schema/CompositionBranchOrderTest.php     | 163 ++++++++++++
 ...CompositionBranchOrderIndependenceTest.php | 249 ++++++++++++++++++
 4 files changed, 434 insertions(+), 2 deletions(-)
 create mode 100644 tests/Functional/Schema/CompositionBranchOrderTest.php
 create mode 100644 tests/Unit/Validator/SchemaValidator/CompositionBranchOrderIndependenceTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b546e730..abb558b5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+
+- `anyOf`/`oneOf` no longer depend on branch declaration order. The
+  `MAX_COMPOSITION_ERRORS` cap in `AbstractCompositionalValidator` used
+  `return` to stop collecting errors, which also abandoned the remaining
+  branches — so a branch that would match went unevaluated whenever
+  earlier branches produced 20+ errors, and `anyOf` reported "At least
+  one of the schemas must match, but none did". The cap now bounds error
+  collection only; every branch is still evaluated. Error output is
+  unchanged (20 errors plus one `TooManyErrorsError`). (#54)
+
 ## [0.7.0]
 
 Preparation for the 1.0.0 stable release. This section tracks work that
@@ -676,7 +689,8 @@ fail-closes on unresolvable callback expressions.
 ### Changed
 - Set `symfony/yaml` requirement to `^7.0`.
 
-[Unreleased]: https://github.com/duyler/openapi/compare/0.6.0...HEAD
+[Unreleased]: https://github.com/duyler/openapi/compare/0.7.0...HEAD
+[0.7.0]: https://github.com/duyler/openapi/compare/0.6.0...0.7.0
 [0.6.0]: https://github.com/duyler/openapi/compare/0.5.0...0.6.0
 [0.5.0]: https://github.com/duyler/openapi/compare/0.4.1...0.5.0
 [0.4.1]: https://github.com/duyler/openapi/compare/0.4.0...0.4.1
diff --git a/src/Validator/SchemaValidator/AbstractCompositionalValidator.php b/src/Validator/SchemaValidator/AbstractCompositionalValidator.php
index 23fce04a..3c9f2368 100644
--- a/src/Validator/SchemaValidator/AbstractCompositionalValidator.php
+++ b/src/Validator/SchemaValidator/AbstractCompositionalValidator.php
@@ -32,6 +32,7 @@ protected function validateSchemas(
         $validCount = 0;
         $errors = [];
         $abstractErrors = [];
+        $capped = false;
         $dataPath = $this->getDataPath($context);
 
         foreach ($schemas as $subSchema) {
@@ -42,6 +43,10 @@ protected function validateSchemas(
                 continue;
             }
 
+            if ($capped) {
+                continue;
+            }
+
             foreach ($outcome->errors as $error) {
                 $errors[] = $error;
             }
@@ -55,7 +60,8 @@ protected function validateSchemas(
                         dataPath: $dataPath,
                     );
 
-                    return new ValidationResult($validCount, $errors, $abstractErrors);
+                    $capped = true;
+                    break;
                 }
             }
         }
diff --git a/tests/Functional/Schema/CompositionBranchOrderTest.php b/tests/Functional/Schema/CompositionBranchOrderTest.php
new file mode 100644
index 00000000..24cb0f1c
--- /dev/null
+++ b/tests/Functional/Schema/CompositionBranchOrderTest.php
@@ -0,0 +1,163 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Duyler\OpenApi\Test\Functional\Schema;
+
+use Duyler\OpenApi\Builder\OpenApiValidatorBuilder;
+use Duyler\OpenApi\Builder\OpenApiValidatorInterface;
+use Duyler\OpenApi\Validator\Exception\TooManyErrorsError;
+use Duyler\OpenApi\Validator\Exception\ValidationException;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+use function array_filter;
+
+/**
+ * Regression for issue #54, driven through the public builder path with the
+ * shape that surfaced it in the wild: a JSON:API `included` array whose items
+ * are an anyOf over allOf-composed resource schemas.
+ *
+ * Each non-matching resource branch emits ~12 errors (a `type` enum miss plus
+ * an `additionalProperties: false` attribute object), so two branches ahead of
+ * the matching one exceed MAX_COMPOSITION_ERRORS. Before the fix the cap
+ * returned out of the branch loop, so `MatchingLast` failed while the
+ * identical `MatchingFirst` branch set passed.
+ *
+ * @internal
+ */
+#[CoversClass(OpenApiValidatorBuilder::class)]
+final class CompositionBranchOrderTest extends TestCase
+{
+    private const string JSON_API_INCLUDED_SPEC = <<<'YAML'
+openapi: 3.2.0
+info:
+  title: composition-branch-order
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    Patient:
+      allOf:
+        - type: object
+          required: [type]
+          properties:
+            type: { type: string, enum: [patients] }
+        - type: object
+          properties:
+            attributes:
+              type: object
+              additionalProperties: false
+              properties:
+                first_name: { type: string }
+                last_name: { type: string }
+    Flow:
+      allOf:
+        - type: object
+          required: [type]
+          properties:
+            type: { type: string, enum: [flows] }
+        - type: object
+          properties:
+            attributes:
+              type: object
+              additionalProperties: false
+              properties:
+                a: { type: string }
+                b: { type: string }
+                c: { type: string }
+                d: { type: string }
+                e: { type: string }
+                f: { type: string }
+                g: { type: string }
+                h: { type: string }
+                i: { type: string }
+                j: { type: string }
+                k: { type: string }
+    MatchingLast:
+      type: array
+      items:
+        anyOf:
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Flow'
+    MatchingFirst:
+      type: array
+      items:
+        anyOf:
+          - $ref: '#/components/schemas/Flow'
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+    NoMatch:
+      type: array
+      items:
+        anyOf:
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+          - $ref: '#/components/schemas/Patient'
+YAML;
+
+    #[Test]
+    public function any_of_in_array_items_ignores_branch_order_when_the_matching_branch_is_last(): void
+    {
+        $this->validator()->validateSchema($this->included(), '#/components/schemas/MatchingLast');
+
+        $this->expectNotToPerformAssertions();
+    }
+
+    #[Test]
+    public function any_of_in_array_items_ignores_branch_order_when_the_matching_branch_is_first(): void
+    {
+        $this->validator()->validateSchema($this->included(), '#/components/schemas/MatchingFirst');
+
+        $this->expectNotToPerformAssertions();
+    }
+
+    #[Test]
+    public function unmatched_item_still_reports_a_capped_and_summarised_error_set(): void
+    {
+        $caught = null;
+
+        try {
+            $this->validator()->validateSchema($this->included(), '#/components/schemas/NoMatch');
+        } catch (ValidationException $e) {
+            $caught = $e;
+        }
+
+        self::assertNotNull($caught, 'anyOf must fail when no branch matches');
+
+        $errors = $caught->getErrors();
+        $markers = array_filter($errors, static fn($error): bool => $error instanceof TooManyErrorsError);
+
+        self::assertCount(21, $errors, 'Cap is 20 collected errors plus one TooManyErrorsError marker');
+        self::assertCount(1, $markers, 'Exactly one summary error must be appended');
+    }
+
+    private function validator(): OpenApiValidatorInterface
+    {
+        return OpenApiValidatorBuilder::create()
+            ->fromYamlString(self::JSON_API_INCLUDED_SPEC)
+            ->build();
+    }
+
+    /**
+     * A single `flows` resource: matches the Flow schema and nothing else.
+     *
+     * @return array<int, array<string, mixed>>
+     */
+    private function included(): array
+    {
+        return [
+            [
+                'type' => 'flows',
+                'attributes' => [
+                    'a' => 'x', 'b' => 'x', 'c' => 'x', 'd' => 'x', 'e' => 'x', 'f' => 'x',
+                    'g' => 'x', 'h' => 'x', 'i' => 'x', 'j' => 'x', 'k' => 'x',
+                ],
+            ],
+        ];
+    }
+}
diff --git a/tests/Unit/Validator/SchemaValidator/CompositionBranchOrderIndependenceTest.php b/tests/Unit/Validator/SchemaValidator/CompositionBranchOrderIndependenceTest.php
new file mode 100644
index 00000000..ed77443f
--- /dev/null
+++ b/tests/Unit/Validator/SchemaValidator/CompositionBranchOrderIndependenceTest.php
@@ -0,0 +1,249 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Duyler\OpenApi\Test\Unit\Validator\SchemaValidator;
+
+use Duyler\OpenApi\Schema\Model\Schema;
+use Duyler\OpenApi\Validator\Error\ValidationContext;
+use Duyler\OpenApi\Validator\Exception\OneOfError;
+use Duyler\OpenApi\Validator\Exception\TooManyErrorsError;
+use Duyler\OpenApi\Validator\Exception\ValidationErrorInterface;
+use Duyler\OpenApi\Validator\Exception\ValidationException;
+use Duyler\OpenApi\Validator\Format\BuiltinFormats;
+use Duyler\OpenApi\Validator\SchemaValidator\AllOfValidator;
+use Duyler\OpenApi\Validator\SchemaValidator\AnyOfValidator;
+use Duyler\OpenApi\Validator\SchemaValidator\OneOfValidator;
+use Duyler\OpenApi\Validator\SchemaValidator\ValidatorDependencies;
+use Duyler\OpenApi\Validator\ValidatorPool;
+use Override;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+use function array_filter;
+use function array_pop;
+use function sprintf;
+
+/**
+ * Regression for issue #54: the MAX_COMPOSITION_ERRORS cap in
+ * AbstractCompositionalValidator::validateSchemas() must cap error
+ * *collection*, not branch *iteration*.
+ *
+ * Before the fix, reaching the cap returned from validateSchemas()
+ * mid-loop, so branches declared after the noisy ones were never
+ * evaluated and $validCount could not grow. anyOf then reported
+ * "none did" for data that a later branch matches, making the outcome
+ * depend on branch declaration order.
+ *
+ * Anti-test: restoring the `return` in place of the `capped` flag makes
+ * every ordering-sensitive case below fail while the two cap-shape cases
+ * keep passing — the cap itself is not what changed.
+ *
+ * @internal
+ */
+#[CoversClass(AnyOfValidator::class)]
+#[CoversClass(OneOfValidator::class)]
+#[CoversClass(AllOfValidator::class)]
+final class CompositionBranchOrderIndependenceTest extends TestCase
+{
+    /**
+     * Two of these exceed MAX_COMPOSITION_ERRORS (20) on their own.
+     */
+    private const int ERRORS_PER_NOISY_BRANCH = 12;
+
+    private ValidatorPool $pool;
+    private AnyOfValidator $anyOf;
+    private OneOfValidator $oneOf;
+    private AllOfValidator $allOf;
+
+    #[Override]
+    protected function setUp(): void
+    {
+        $this->pool = new ValidatorPool();
+        $dependencies = new ValidatorDependencies(pool: $this->pool, formatRegistry: BuiltinFormats::create());
+
+        $this->anyOf = new AnyOfValidator($dependencies);
+        $this->oneOf = new OneOfValidator($dependencies);
+        $this->allOf = new AllOfValidator($dependencies);
+    }
+
+    /**
+     * The third noisy branch matters: it is the one that reaches the
+     * post-cap `continue`. A matching branch alone would short-circuit on
+     * `$outcome->matched` before the cap is ever consulted, so turning the
+     * `continue` back into a `break` would go unnoticed.
+     */
+    #[Test]
+    public function any_of_passes_whatever_the_position_of_the_matching_branch(): void
+    {
+        $branches = [
+            $this->noisyBranch('a'),
+            $this->noisyBranch('b'),
+            $this->noisyBranch('c'),
+            $this->matchingBranch(),
+        ];
+
+        $this->anyOf->validate($this->data(), new Schema(anyOf: $branches), $this->context());
+        $this->anyOf->validate($this->data(), new Schema(anyOf: $this->matchingFirst($branches)), $this->context());
+
+        $this->expectNotToPerformAssertions();
+    }
+
+    #[Test]
+    public function one_of_counts_a_match_declared_after_the_error_cap_is_reached(): void
+    {
+        $schema = new Schema(oneOf: [
+            $this->noisyBranch('a'),
+            $this->noisyBranch('b'),
+            $this->noisyBranch('c'),
+            $this->matchingBranch(),
+        ]);
+
+        $this->oneOf->validate($this->data(), $schema, $this->context());
+
+        $this->expectNotToPerformAssertions();
+    }
+
+    #[Test]
+    public function one_of_rejects_two_matches_declared_after_the_error_cap_is_reached(): void
+    {
+        $schema = new Schema(oneOf: [
+            $this->noisyBranch('a'),
+            $this->noisyBranch('b'),
+            $this->noisyBranch('c'),
+            $this->matchingBranch(),
+            $this->otherMatchingBranch(),
+        ]);
+
+        $this->expectException(OneOfError::class);
+
+        $this->oneOf->validate($this->data(), $schema, $this->context());
+    }
+
+    #[Test]
+    public function all_of_still_fails_when_a_branch_after_the_error_cap_fails(): void
+    {
+        $schema = new Schema(allOf: [
+            $this->noisyBranch('a'),
+            $this->noisyBranch('b'),
+            $this->matchingBranch(),
+            $this->noisyBranch('c'),
+        ]);
+
+        $caught = null;
+
+        try {
+            $this->allOf->validate($this->data(), $schema, $this->context());
+        } catch (ValidationException $e) {
+            $caught = $e;
+        }
+
+        self::assertNotNull($caught, 'allOf must fail when any branch fails');
+        self::assertSame(
+            'All of the schemas must match, but 2 failed',
+            $caught->getMessage(),
+            'Branches failing after the cap are still evaluated, but their errors are no longer collected',
+        );
+    }
+
+    #[Test]
+    public function all_of_still_passes_when_every_branch_matches(): void
+    {
+        $schema = new Schema(allOf: [$this->matchingBranch(), $this->otherMatchingBranch()]);
+
+        $this->allOf->validate($this->data(), $schema, $this->context());
+
+        $this->expectNotToPerformAssertions();
+    }
+
+    #[Test]
+    public function unmatched_data_reports_at_most_the_capped_number_of_errors(): void
+    {
+        $schema = new Schema(anyOf: [$this->noisyBranch('a'), $this->noisyBranch('b'), $this->noisyBranch('c')]);
+
+        $errors = $this->failureErrors($schema);
+
+        self::assertCount(21, $errors, 'Cap is 20 collected errors plus one TooManyErrorsError marker');
+    }
+
+    #[Test]
+    public function exactly_one_too_many_errors_marker_is_appended_however_many_branches_follow(): void
+    {
+        $branches = [];
+
+        for ($i = 0; $i < 10; ++$i) {
+            $branches[] = $this->noisyBranch('branch' . $i);
+        }
+
+        $errors = $this->failureErrors(new Schema(anyOf: $branches));
+        $markers = array_filter($errors, static fn($error): bool => $error instanceof TooManyErrorsError);
+
+        self::assertCount(1, $markers, 'Exactly one summary error must be appended');
+        self::assertCount(21, $errors, 'Branches after the cap must not append further errors');
+    }
+
+    /**
+     * @return array<int, ValidationErrorInterface>
+     */
+    private function failureErrors(Schema $schema): array
+    {
+        try {
+            $this->anyOf->validate($this->data(), $schema, $this->context());
+        } catch (ValidationException $e) {
+            return $e->getErrors();
+        }
+
+        self::fail('anyOf must fail when no branch matches');
+    }
+
+    /**
+     * @param array<int, Schema> $branches
+     *
+     * @return array<int, Schema>
+     */
+    private function matchingFirst(array $branches): array
+    {
+        $reordered = $branches;
+        $last = array_pop($reordered);
+
+        return [$last, ...$reordered];
+    }
+
+    /**
+     * Emits ERRORS_PER_NOISY_BRANCH errors against {@see self::data()}.
+     */
+    private function noisyBranch(string $prefix): Schema
+    {
+        $required = [];
+
+        for ($i = 0; $i < self::ERRORS_PER_NOISY_BRANCH; ++$i) {
+            $required[] = sprintf('%s_absent_%d', $prefix, $i);
+        }
+
+        return new Schema(type: 'object', required: $required);
+    }
+
+    private function matchingBranch(): Schema
+    {
+        return new Schema(type: 'object', required: ['id']);
+    }
+
+    private function otherMatchingBranch(): Schema
+    {
+        return new Schema(type: 'object', properties: ['id' => new Schema(type: 'string')]);
+    }
+
+    /**
+     * @return array<string, string>
+     */
+    private function data(): array
+    {
+        return ['id' => 'flow-1'];
+    }
+
+    private function context(): ValidationContext
+    {
+        return ValidationContext::create($this->pool);
+    }
+}