diff --git a/CHANGELOG.md b/CHANGELOG.md index ec31d45..7117179 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). +## 0.16.1 — 2026-10-17 + +- **Fixed.** A closed object (`additionalProperties: false`) whose declared + member fails its own subschema no longer receives an extra + `additionalProperties` violation of the parent over the member's own + failure (#175). The backend marks a member as evaluated only when every + declared member passed, so the failing one still counted as unchecked when + `additionalProperties` ran; a member the sibling `properties` map declares, + or one of its `patternProperties` patterns matches, is now what the leaf + filter treats as not additional. An undeclared member, and a member that + fails the object form of `additionalProperties`, keep their leaves; the + verdict never moved. + ## 0.16.0 — 2026-10-11 - **Added.** OpenAPI 3.1 schemas now support `prefixItems`, `contains` with diff --git a/src/Internal/Schema/SchemaValidator.php b/src/Internal/Schema/SchemaValidator.php index 22a1523..352cfa5 100644 --- a/src/Internal/Schema/SchemaValidator.php +++ b/src/Internal/Schema/SchemaValidator.php @@ -5,6 +5,7 @@ namespace Rasuvaeff\OpenApiContract\Internal\Schema; use Opis\JsonSchema\Errors\ValidationError; +use Opis\JsonSchema\Helper; use Opis\JsonSchema\Schema; use Opis\JsonSchema\SchemaLoader; use Opis\JsonSchema\Schemas\ExceptionSchema; @@ -233,6 +234,9 @@ private function leaf(ValidationError $error): array { $path = $this->dataPath($error); $expected = $this->assertion($error); + if ($this->isSpuriousAdditionalProperties($error)) { + return []; + } if ($error->keyword() === 'required') { /** @var mixed $missing */ $missing = $error->args()['missing'] ?? null; @@ -251,6 +255,69 @@ private function leaf(ValidationError $error): array return [new SchemaFailure(path: $path, keyword: $error->keyword(), actual: $error->data()->value(), expected: $expected)]; } + /** + * A closed object (`additionalProperties: false`) whose declared member + * fails its own subschema still receives an `additionalProperties` error + * naming it: the backend marks a member as evaluated only when every + * declared member passed, so the failing one counts as unchecked by the + * time `additionalProperties` runs. A member the sibling `properties` + * map declares, or one of its `patternProperties` patterns matches, is + * not additional — its own subschema is what the `properties` keyword + * judged, and its failure is already reported there. If no flagged name + * escapes that, the leaf exists only because of the missing mark and is + * dropped. + * + * The object form of `additionalProperties` is left alone: there the + * backend itself judges the flagged members against the added schema, + * so a name it reports there failed both the declaration and it. + */ + private function isSpuriousAdditionalProperties(ValidationError $error): bool + { + if ($error->keyword() !== 'additionalProperties') { + return false; + } + /** @var mixed $flagged */ + $flagged = $error->args()['properties'] ?? null; + if (!is_array($flagged)) { + return false; + } + $schema = $error->schema()->info()->data(); + if (!$schema instanceof \stdClass || !property_exists($schema, 'additionalProperties') || $schema->additionalProperties !== false) { + return false; + } + + /** @var mixed $declared */ + $declared = $schema->properties ?? null; + /** @var mixed $patterned */ + $patterned = $schema->patternProperties ?? null; + /** @var array $patterns */ + $patterns = is_array($patterned) ? $patterned : (is_object($patterned) ? (array) $patterned : []); + + foreach ($flagged as $name) { + if (!is_string($name)) { + return false; + } + if (is_object($declared) && property_exists($declared, $name)) { + continue; + } + foreach (array_keys($patterns) as $pattern) { + if (!is_string($pattern) || $pattern === '') { + continue; + } + // The delimiter prefix and the flags make the pattern non-empty in every case psalm already narrowed `$pattern` to non-empty-string. + /** @var non-empty-string $regex */ + $regex = Helper::patternToRegex($pattern); + if (preg_match($regex, $name) === 1) { + continue 2; + } + } + + return false; + } + + return true; + } + /** * The assertion an error is about: the keyword and its value in the * subschema that carries it, as the compiler spelled it. diff --git a/tests/ContractTest.php b/tests/ContractTest.php index a077b2b..8a55c14 100644 --- a/tests/ContractTest.php +++ b/tests/ContractTest.php @@ -570,16 +570,19 @@ public function validatesARecursiveSchemaAtEveryDepth(): void $valid = '{"name":"a","children":[{"name":"b","children":[{"name":"c","children":[{"name":"d"}]}]}]}'; Assert::true($contract->validateRequest($request($valid))->isValid()); + // The four older leaf rungs — the two genuine failures here, and the + // backend's spurious `additionalProperties` at each closed ancestor + // whose declared member carried the failure (#175) — are now two. Assert::same( array_map(static fn(Violation $v): string => $v->code, $contract->validateRequest($request('{"name":"a","children":[{"name":"b","children":[{"nam":"c"}]}]}'))->violations), - ['request.body.schema', 'request.body.schema', 'request.body.schema', 'request.body.schema'], + ['request.body.schema', 'request.body.schema'], ); // `id` is readOnly: never demanded on a request, at any depth, and // demanded at every depth of a response. Assert::true($contract->validateRequest($request('{"name":"a","children":[{"name":"b","id":1}]}'))->isValid()); Assert::same( array_map(static fn(Violation $v): string => $v->code, $contract->validateResponse('tree.create', $response($valid))->violations), - ['response.body.schema', 'response.body.schema', 'response.body.schema', 'response.body.schema', 'response.body.schema', 'response.body.schema', 'response.body.schema'], + ['response.body.schema', 'response.body.schema', 'response.body.schema', 'response.body.schema'], ); Assert::true($contract->validateResponse('tree.create', $response('{"id":1,"name":"a","children":[{"id":2,"name":"b","children":[{"id":3,"name":"c"}]}]}'))->isValid()); @@ -685,7 +688,7 @@ public function validatesASharedComponentDeferredToDefs(): void Assert::true($contract->validateRequest($request(sprintf('{"sendTo":[%s],"billTo":[%s]}', $noCode, $address([]))))->isValid()); Assert::same( array_map(static fn(Violation $v): string => $v->code, $contract->validateRequest($request(sprintf('{"sendTo":[%s],"billTo":[%s]}', $address([]), $address(['city' => 5]))))->violations), - ['request.body.schema', 'request.body.schema'], + ['request.body.schema'], ); Assert::true($contract->validateResponse('parcels.create', $response(sprintf('{"sendTo":[%s],"billTo":[%s],"code":1}', $address([]), $address([]))))->isValid()); Assert::same( diff --git a/tests/SchemaValidatorTest.php b/tests/SchemaValidatorTest.php index 6bae685..bd79cfa 100644 --- a/tests/SchemaValidatorTest.php +++ b/tests/SchemaValidatorTest.php @@ -1063,6 +1063,80 @@ public function reportsLeafFailuresWithTheirPathAndKeyword(): void Assert::same($deep[19]->path, ['p9', 'r']); } + /** + * A closed object's declared member failing its own subschema is not + * additionally reported as an `additionalProperties` failure of the + * parent (#175): the backend marks a member as evaluated only when every + * declared member passed, so the failing one is still unchecked when + * `additionalProperties: false` runs. An undeclared member is a real + * failure of it and keeps its leaf; so does a member that failed the + * object form of `additionalProperties`, which the backend carries on + * the member's own path. + */ + public function closedObjectDoesNotReportItsDeclaredMemberAsAdditional(): void + { + $validator = new SchemaValidator(); + $closed = [ + 'type' => 'object', + 'additionalProperties' => false, + 'required' => ['a'], + 'properties' => ['a' => ['type' => 'array', 'minItems' => 1]], + ]; + + // The declared member fails its own subschema: its leaf, and no word + // about `additionalProperties` at the root. + Assert::false($validator->isValid(json_decode('{"a":[]}'), $closed, SchemaDialect::OpenApi31)); + Assert::same( + $this->render($validator->failures(json_decode('{"a":[]}'), $closed, SchemaDialect::OpenApi31)), + [['a', 'minItems', []]], + ); + + // An undeclared member beside it is `additionalProperties`'s to report. + Assert::same( + $this->render($validator->failures(json_decode('{"a":[],"b":1}'), $closed, SchemaDialect::OpenApi31)), + [['a', 'minItems', []], ['', 'additionalProperties', ['a' => [], 'b' => 1]]], + ); + + // A name `patternProperties` matches is no more additional dropped. + $patternClosed = [ + 'type' => 'object', + 'additionalProperties' => false, + 'patternProperties' => ['^x-' => ['type' => 'integer']], + 'properties' => ['x-a' => ['type' => 'integer']], + ]; + Assert::same( + $this->render($validator->failures(json_decode('{"x-a":"3"}'), $patternClosed, SchemaDialect::OpenApi31)), + [['x-a', 'type', '3']], + ); + Assert::same( + $this->render($validator->failures(json_decode('{"x-a":"3","count":9}'), $patternClosed, SchemaDialect::OpenApi31)), + [['x-a', 'type', '3'], ['', 'additionalProperties', ['x-a' => '3', 'count' => 9]]], + ); + + // Nested closed objects: the same at the member's own path. + $nested = ['type' => 'object', 'additionalProperties' => false, 'properties' => ['inner' => [ + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => ['i' => ['type' => 'string', 'minLength' => 2]], + ]]]; + Assert::same( + $this->render($validator->failures(json_decode('{"inner":{"i":"x"}}'), $nested, SchemaDialect::OpenApi31)), + [['inner.i', 'minLength', 'x']], + ); + Assert::same( + $this->render($validator->failures(json_decode('{"inner":{"i":"x","j":2}}'), $nested, SchemaDialect::OpenApi31)), + [['inner.i', 'minLength', 'x'], ['inner', 'additionalProperties', ['i' => 'x', 'j' => 2]]], + ); + + // The object form is untouched: a member failing it as well is named + // on its own path, under each keyword. + $open = ['type' => 'object', 'properties' => ['s' => ['type' => 'integer']], 'additionalProperties' => ['minLength' => 5]]; + Assert::same( + $this->render($validator->failures(json_decode('{"s":"x"}'), $open, SchemaDialect::OpenApi31)), + [['s', 'type', 'x'], ['s', 'minLength', 'x']], + ); + } + /** * A discriminated union's diagnostics follow the branch the value names * — through `mapping` as a reference or a component name, or implicitly