diff --git a/CHANGELOG.md b/CHANGELOG.md index b546e730..69d98525 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ 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 + +- `nullable: true` is now honoured when it sits beside a composition keyword + instead of only on the branch schemas, so the standard OAS 3.0 workaround + for a nullable `$ref` — `{allOf: [$ref], nullable: true}` — accepts `null`. + `AllOfValidator`, `AnyOfValidator`, `OneOfValidator`, + `OneOfValidatorWithContext` and `IfThenElseValidator` short-circuit when the + schema carrying the keyword is nullable, rather than dispatching `null` into + branches that reject it. `anyOf`/`oneOf` treat this as the keyword being + satisfied, not as a matching branch, so `oneOf` still enforces exactly-one + for non-null data. A `null` member of an OAS 3.1 `type` union is ordinary + JSON Schema and keeps composing — only `nullable: true` waives branches, and + only while `nullableAsType` is enabled (#50). + ## [0.7.0] Preparation for the 1.0.0 stable release. This section tracks work that diff --git a/src/Validator/Schema/OneOfValidatorWithContext.php b/src/Validator/Schema/OneOfValidatorWithContext.php index 78263ff8..e765e44a 100644 --- a/src/Validator/Schema/OneOfValidatorWithContext.php +++ b/src/Validator/Schema/OneOfValidatorWithContext.php @@ -47,6 +47,10 @@ private function validate(mixed $data, Schema $schema, ValidationContext $contex return; } + if (null === $data && SchemaValueNormalizer::isNullableSchema($schema, $context->nullableAsType)) { + return; + } + if ($useDiscriminator && null !== $schema->discriminator) { $this->validateWithDiscriminator($data, $schema, $context); return; diff --git a/src/Validator/Schema/SchemaValueNormalizer.php b/src/Validator/Schema/SchemaValueNormalizer.php index 29149f2c..dcf5cb0c 100644 --- a/src/Validator/Schema/SchemaValueNormalizer.php +++ b/src/Validator/Schema/SchemaValueNormalizer.php @@ -4,6 +4,7 @@ namespace Duyler\OpenApi\Validator\Schema; +use Duyler\OpenApi\Schema\Model\Schema; use Duyler\OpenApi\Validator\Exception\InvalidDataTypeException; use stdClass; @@ -50,6 +51,11 @@ public static function normalize(mixed $value, bool $allowNull = false): array|i )); } + public static function isNullableSchema(Schema $schema, bool $nullableAsType): bool + { + return $nullableAsType && $schema->nullable; + } + /** * @param string|array|null $type */ diff --git a/src/Validator/SchemaValidator/AbstractSchemaValidator.php b/src/Validator/SchemaValidator/AbstractSchemaValidator.php index 52890165..1ecd9dbd 100644 --- a/src/Validator/SchemaValidator/AbstractSchemaValidator.php +++ b/src/Validator/SchemaValidator/AbstractSchemaValidator.php @@ -4,9 +4,11 @@ namespace Duyler\OpenApi\Validator\SchemaValidator; +use Duyler\OpenApi\Schema\Model\Schema; use Duyler\OpenApi\Validator\Error\ValidationContext; use Duyler\OpenApi\Validator\PregExecutor; use Duyler\OpenApi\Validator\Schema\RegexValidator; +use Duyler\OpenApi\Validator\Schema\SchemaValueNormalizer; use Duyler\OpenApi\Validator\ValidatorPool; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; @@ -20,6 +22,12 @@ public function __construct( protected readonly ValidatorDependencies $dependencies, ) {} + protected function acceptsNullAsNullable(mixed $data, Schema $schema, ?ValidationContext $context): bool + { + return null === $data + && SchemaValueNormalizer::isNullableSchema($schema, $context?->nullableAsType ?? true); + } + protected function getDataPath(?ValidationContext $context): string { if (null === $context) { diff --git a/src/Validator/SchemaValidator/AllOfValidator.php b/src/Validator/SchemaValidator/AllOfValidator.php index 11929b61..2940d505 100644 --- a/src/Validator/SchemaValidator/AllOfValidator.php +++ b/src/Validator/SchemaValidator/AllOfValidator.php @@ -27,6 +27,10 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex return; } + if ($this->acceptsNullAsNullable($data, $schema, $context)) { + return; + } + $result = $this->validateSchemas($schema->allOf, $data, $context, 'allOf'); if ([] !== $result->errors || [] !== $result->abstractErrors) { diff --git a/src/Validator/SchemaValidator/AnyOfValidator.php b/src/Validator/SchemaValidator/AnyOfValidator.php index e4a2cdc2..c140435a 100644 --- a/src/Validator/SchemaValidator/AnyOfValidator.php +++ b/src/Validator/SchemaValidator/AnyOfValidator.php @@ -24,6 +24,10 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex return; } + if ($this->acceptsNullAsNullable($data, $schema, $context)) { + return; + } + $result = $this->validateSchemas($schema->anyOf, $data, $context, 'anyOf'); if (0 === $result->validCount) { diff --git a/src/Validator/SchemaValidator/IfThenElseValidator.php b/src/Validator/SchemaValidator/IfThenElseValidator.php index 61b447c3..e7bbb4eb 100644 --- a/src/Validator/SchemaValidator/IfThenElseValidator.php +++ b/src/Validator/SchemaValidator/IfThenElseValidator.php @@ -32,6 +32,10 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex return; } + if ($this->acceptsNullAsNullable($data, $schema, $context)) { + return; + } + if (is_bool($schema->if)) { $this->routeThenOrElse(schema: $schema, data: $data, context: $context, ifValid: $schema->if); diff --git a/src/Validator/SchemaValidator/OneOfValidator.php b/src/Validator/SchemaValidator/OneOfValidator.php index 6c9176d0..eb2f1939 100644 --- a/src/Validator/SchemaValidator/OneOfValidator.php +++ b/src/Validator/SchemaValidator/OneOfValidator.php @@ -25,6 +25,10 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex return; } + if ($this->acceptsNullAsNullable($data, $schema, $context)) { + return; + } + $result = $this->validateSchemas($schema->oneOf, $data, $context, 'oneOf'); if (0 === $result->validCount) { diff --git a/src/Validator/SchemaValidator/TypeValidator.php b/src/Validator/SchemaValidator/TypeValidator.php index 4d985cac..b27f7e0e 100644 --- a/src/Validator/SchemaValidator/TypeValidator.php +++ b/src/Validator/SchemaValidator/TypeValidator.php @@ -8,6 +8,7 @@ use Duyler\OpenApi\Validator\EmptyArrayStrategy; use Duyler\OpenApi\Validator\Error\ValidationContext; use Duyler\OpenApi\Validator\Exception\TypeMismatchError; +use Duyler\OpenApi\Validator\Schema\SchemaValueNormalizer; use Duyler\OpenApi\Validator\TypeFormatter; use Override; @@ -41,7 +42,7 @@ public function validate(mixed $data, Schema $schema, ?ValidationContext $contex $nullableAsType = $context?->nullableAsType ?? true; - if (null === $data && $schema->nullable && $nullableAsType) { + if (null === $data && SchemaValueNormalizer::isNullableSchema($schema, $nullableAsType)) { return; } diff --git a/tests/Functional/Response/NullableCompositionTest.php b/tests/Functional/Response/NullableCompositionTest.php new file mode 100644 index 00000000..ab3d0046 --- /dev/null +++ b/tests/Functional/Response/NullableCompositionTest.php @@ -0,0 +1,131 @@ +psrFactory = new Psr17Factory(); + } + + #[Test] + public function null_relationship_is_accepted_through_nested_properties(): void + { + $this->validateResponseBody( + '{"data":{"type":"actions","id":"1","relationships":{"form":{"data":null}}}}', + ); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function populated_relationship_is_accepted_through_nested_properties(): void + { + $this->validateResponseBody( + '{"data":{"type":"actions","id":"1","relationships":{"form":{"data":{"type":"forms","id":"7"}}}}}', + ); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function invalid_relationship_is_still_rejected_through_nested_properties(): void + { + $this->expectException(ValidationException::class); + + $this->validateResponseBody( + '{"data":{"type":"actions","id":"1","relationships":{"form":{"data":{"type":"widgets"}}}}}', + ); + } + + private function validateResponseBody(string $body): void + { + $validator = OpenApiValidatorBuilder::create() + ->fromYamlString(self::SPEC) + ->build(); + + $operation = $validator->validateRequest( + $this->psrFactory->createServerRequest('GET', '/actions/1'), + ); + + $response = $this->psrFactory->createResponse(200) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->psrFactory->createStream($body)); + + $validator->validateResponse($response, $operation); + } +} diff --git a/tests/Unit/Validator/SchemaValidator/AllOfValidatorTest.php b/tests/Unit/Validator/SchemaValidator/AllOfValidatorTest.php index eda9245f..22f3c0ce 100644 --- a/tests/Unit/Validator/SchemaValidator/AllOfValidatorTest.php +++ b/tests/Unit/Validator/SchemaValidator/AllOfValidatorTest.php @@ -8,6 +8,7 @@ use Duyler\OpenApi\Validator\SchemaValidator\ValidatorDependencies; use Duyler\OpenApi\Schema\Model\Schema; +use Duyler\OpenApi\Validator\Error\ValidationContext; use Duyler\OpenApi\Validator\Exception\ValidationException; use Duyler\OpenApi\Validator\ValidatorPool; use Duyler\OpenApi\Validator\Format\BuiltinFormats; @@ -237,4 +238,32 @@ public function allOf_propagates_invalid_data_type_errors(): void self::assertGreaterThan(0, count($errors)); } } + + #[Test] + public function allOf_passes_when_parent_schema_is_nullable_and_data_is_null(): void + { + $schema = new Schema( + nullable: true, + allOf: [new Schema(type: 'object', required: ['id'])], + ); + + $this->validator->validate(null, $schema); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function allOf_rejects_null_when_parent_nullable_is_not_honored(): void + { + $schema = new Schema( + nullable: true, + allOf: [new Schema(type: 'object', required: ['id'])], + ); + + $context = ValidationContext::create($this->pool, nullableAsType: false); + + $this->expectException(ValidationException::class); + + $this->validator->validate(null, $schema, $context); + } } diff --git a/tests/Unit/Validator/SchemaValidator/AnyOfValidatorTest.php b/tests/Unit/Validator/SchemaValidator/AnyOfValidatorTest.php index f65ec47f..cfc602c3 100644 --- a/tests/Unit/Validator/SchemaValidator/AnyOfValidatorTest.php +++ b/tests/Unit/Validator/SchemaValidator/AnyOfValidatorTest.php @@ -239,4 +239,32 @@ public function anyOf_passes_when_multiple_nullable_schemas_match_null(): void $this->expectNotToPerformAssertions(); } + + #[Test] + public function anyOf_passes_when_parent_schema_is_nullable_and_data_is_null(): void + { + $schema = new Schema( + nullable: true, + anyOf: [new Schema(type: 'object', required: ['id'])], + ); + + $this->validator->validate(null, $schema); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function anyOf_rejects_null_when_parent_nullable_is_not_honored(): void + { + $schema = new Schema( + nullable: true, + anyOf: [new Schema(type: 'object', required: ['id'])], + ); + + $context = ValidationContext::create($this->pool, nullableAsType: false); + + $this->expectException(ValidationException::class); + + $this->validator->validate(null, $schema, $context); + } } diff --git a/tests/Unit/Validator/SchemaValidator/IfThenElseValidatorTest.php b/tests/Unit/Validator/SchemaValidator/IfThenElseValidatorTest.php index 5b2f6c01..52b89440 100644 --- a/tests/Unit/Validator/SchemaValidator/IfThenElseValidatorTest.php +++ b/tests/Unit/Validator/SchemaValidator/IfThenElseValidatorTest.php @@ -8,6 +8,8 @@ use Duyler\OpenApi\Validator\SchemaValidator\ValidatorDependencies; use Duyler\OpenApi\Schema\Model\Schema; +use Duyler\OpenApi\Validator\Error\ValidationContext; +use Duyler\OpenApi\Validator\Exception\InvalidDataTypeException; use Duyler\OpenApi\Validator\Exception\MaximumError; use Duyler\OpenApi\Validator\ValidatorPool; use Duyler\OpenApi\Validator\Format\BuiltinFormats; @@ -156,4 +158,34 @@ public function not_apply_else_when_if_valid(): void $this->expectNotToPerformAssertions(); } + + #[Test] + public function skip_branches_when_parent_schema_is_nullable_and_data_is_null(): void + { + $schema = new Schema( + nullable: true, + if: new Schema(type: 'string'), + else: new Schema(type: 'object', required: ['id']), + ); + + $this->validator->validate(null, $schema); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function apply_branches_to_null_when_parent_nullable_is_not_honored(): void + { + $schema = new Schema( + nullable: true, + if: new Schema(type: 'string'), + else: new Schema(type: 'object', required: ['id']), + ); + + $context = ValidationContext::create($this->pool, nullableAsType: false); + + $this->expectException(InvalidDataTypeException::class); + + $this->validator->validate(null, $schema, $context); + } } diff --git a/tests/Unit/Validator/SchemaValidator/NullableCompositionTest.php b/tests/Unit/Validator/SchemaValidator/NullableCompositionTest.php new file mode 100644 index 00000000..3c4866d6 --- /dev/null +++ b/tests/Unit/Validator/SchemaValidator/NullableCompositionTest.php @@ -0,0 +1,375 @@ + + */ + public static function nullableParentSchemas(): iterable + { + yield 'allOf + nullable' => ['AllOfNullable']; + yield 'type + allOf + nullable' => ['TypedAllOfNullable']; + yield 'anyOf + nullable' => ['AnyOfNullable']; + yield 'oneOf + nullable' => ['OneOfNullable']; + yield 'nullable branch inside allOf' => ['AllOfBranchNullable']; + yield 'nullable without composition' => ['PlainNullable']; + } + + /** + * @return iterable + */ + public static function allSchemas(): iterable + { + yield from self::nullableParentSchemas(); + yield 'allOf without nullable' => ['AllOfNotNullable']; + } + + #[Test] + #[DataProvider('nullableParentSchemas')] + public function null_is_accepted_by_nullable_schema(string $schemaName): void + { + $this->validator()->validateSchema(null, '#/components/schemas/' . $schemaName); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + #[DataProvider('allSchemas')] + public function valid_object_is_accepted(string $schemaName): void + { + $this->validator()->validateSchema(self::validObject(), '#/components/schemas/' . $schemaName); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function null_is_rejected_when_composition_is_not_nullable(): void + { + $this->expectException(ValidationException::class); + + $this->validator()->validateSchema(null, '#/components/schemas/AllOfNotNullable'); + } + + #[Test] + #[DataProvider('allSchemas')] + public function invalid_object_is_still_rejected(string $schemaName): void + { + $this->expectException(ValidationException::class); + + $this->validator()->validateSchema(['type' => 'wrong'], '#/components/schemas/' . $schemaName); + } + + #[Test] + #[DataProvider('nullableParentSchemas')] + public function null_is_rejected_when_nullable_as_type_is_disabled(string $schemaName): void + { + $this->expectException(ValidationException::class); + + $this->validator(nullableAsType: false)->validateSchema(null, '#/components/schemas/' . $schemaName); + } + + #[Test] + public function oneOf_still_rejects_data_matching_more_than_one_branch(): void + { + $yaml = <<<'YAML' + openapi: 3.0.3 + info: + title: Nullable oneOf overlap + version: 1.0.0 + components: + schemas: + Overlapping: + nullable: true + oneOf: + - type: object + required: [id] + properties: + id: + type: string + - type: object + required: [id] + properties: + id: + type: string + YAML; + + $validator = OpenApiValidatorBuilder::create()->fromYamlString($yaml)->build(); + + $validator->validateSchema(null, '#/components/schemas/Overlapping'); + + $this->expectException(ValidationException::class); + + $validator->validateSchema(['id' => '1'], '#/components/schemas/Overlapping'); + } + + #[Test] + public function oas_31_null_type_union_is_honored_inside_and_outside_compositions(): void + { + $validator = OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC_31)->build(); + + foreach (['UnionOutside', 'UnionOnBranch', 'NullableOnParent'] as $schemaName) { + $validator->validateSchema(null, '#/components/schemas/' . $schemaName); + $validator->validateSchema(['id' => '1'], '#/components/schemas/' . $schemaName); + } + + $this->expectNotToPerformAssertions(); + } + + /** + * A `null` member of a `type` union is ordinary JSON Schema and keeps composing: `allOf` + * branches still have to match. Only OAS `nullable: true` — which the spec defines as + * "allows sending a null value for the defined schema" — waives them. + */ + #[Test] + public function oas_31_null_type_union_on_parent_does_not_waive_composition_branches(): void + { + $validator = OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC_31)->build(); + + $validator->validateSchema(['id' => '1'], '#/components/schemas/UnionOnParent'); + + $this->expectException(ValidationException::class); + + $validator->validateSchema(null, '#/components/schemas/UnionOnParent'); + } + + #[Test] + public function discriminated_oneOf_with_nullable_parent_accepts_null_and_still_discriminates(): void + { + $yaml = <<<'YAML' + openapi: 3.0.3 + info: + title: Nullable discriminated oneOf + version: 1.0.0 + components: + schemas: + Cat: + type: object + required: [petType, meows] + properties: + petType: + type: string + meows: + type: boolean + Dog: + type: object + required: [petType, barks] + properties: + petType: + type: string + barks: + type: boolean + Pet: + nullable: true + discriminator: + propertyName: petType + mapping: + cat: '#/components/schemas/Cat' + dog: '#/components/schemas/Dog' + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + YAML; + + $validator = OpenApiValidatorBuilder::create()->fromYamlString($yaml)->build(); + + $validator->validateSchema(null, '#/components/schemas/Pet'); + $validator->validateSchema(['petType' => 'cat', 'meows' => true], '#/components/schemas/Pet'); + + $this->expectException(ValidationException::class); + + $validator->validateSchema(['petType' => 'cat', 'barks' => true], '#/components/schemas/Pet'); + } + + /** + * @return iterable + */ + public static function inPlaceApplicators(): iterable + { + yield 'if/else' => ['IfElseNullable']; + yield 'not' => ['NotNullable']; + yield 'dependentSchemas' => ['DependentSchemasNullable']; + } + + #[Test] + #[DataProvider('inPlaceApplicators')] + public function nullable_parent_accepts_null_for_other_in_place_applicators(string $schemaName): void + { + $validator = OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC_IN_PLACE)->build(); + + $validator->validateSchema(null, '#/components/schemas/' . $schemaName); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function nullable_parent_does_not_stop_if_else_from_routing_non_null_data(): void + { + $validator = OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC_IN_PLACE)->build(); + + $validator->validateSchema('a string', '#/components/schemas/IfElseNullable'); + $validator->validateSchema(['id' => '1'], '#/components/schemas/IfElseNullable'); + + $this->expectException(ValidationException::class); + + $validator->validateSchema(['id' => 1], '#/components/schemas/IfElseNullable'); + } + + /** + * @return array{type: string, id: string} + */ + private static function validObject(): array + { + return ['type' => 'forms', 'id' => '1']; + } + + private function validator(bool $nullableAsType = true): OpenApiValidatorInterface + { + $builder = OpenApiValidatorBuilder::create()->fromYamlString(self::SPEC); + + $builder = $nullableAsType + ? $builder->enableNullableAsType() + : $builder->disableNullableAsType(); + + return $builder->build(); + } +} diff --git a/tests/Unit/Validator/SchemaValidator/OneOfValidatorTest.php b/tests/Unit/Validator/SchemaValidator/OneOfValidatorTest.php index d1b3627f..7e5b833a 100644 --- a/tests/Unit/Validator/SchemaValidator/OneOfValidatorTest.php +++ b/tests/Unit/Validator/SchemaValidator/OneOfValidatorTest.php @@ -270,4 +270,35 @@ public function oneOf_passes_when_single_nullable_schema_matches_null(): void $this->expectNotToPerformAssertions(); } + + #[Test] + public function oneOf_passes_when_parent_schema_is_nullable_and_data_is_null(): void + { + $schema = new Schema( + nullable: true, + oneOf: [ + new Schema(type: 'object', required: ['id']), + new Schema(type: 'object', required: ['name']), + ], + ); + + $this->validator->validate(null, $schema); + + $this->expectNotToPerformAssertions(); + } + + #[Test] + public function oneOf_rejects_null_when_parent_nullable_is_not_honored(): void + { + $schema = new Schema( + nullable: true, + oneOf: [new Schema(type: 'object', required: ['id'])], + ); + + $context = ValidationContext::create($this->pool, nullableAsType: false); + + $this->expectException(ValidationException::class); + + $this->validator->validate(null, $schema, $context); + } }