fix: Cap composition error collection, not branch iteration (#54)

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

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;
                 }
             }
         }

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',
+                ],
+            ],
+        ];
+    }
+}