Skip to content

Commit

Permalink
fix(#2222): Fix properties with default values getting marked as requ…
Browse files Browse the repository at this point in the history
…ired (#2248)

* fix(#2222): Fix default value properties getting marked as required

* Implement PR review feedback

* Fix tests for PHP 7.4

* Fix code style

* refactor promoted properties tests

* remove readonly

* remove testEntityWithPromotedPropertiesWithDefaults

* add license header

---------

Co-authored-by: DjordyKoert <djordy.koert@yoursurprise.com>
  • Loading branch information
DominicLuidold and DjordyKoert committed Mar 30, 2024
1 parent 196ef00 commit 7f46161
Show file tree
Hide file tree
Showing 10 changed files with 318 additions and 13 deletions.
Expand Up @@ -84,6 +84,10 @@ private function processPropertyAnnotations($reflection, OA\Property $property,
return;
}

if (!Generator::isDefault($property->default)) {
return;
}

$existingRequiredFields = Generator::UNDEFINED !== $this->schema->required ? $this->schema->required : [];
$existingRequiredFields[] = $propertyName;

Expand Down
24 changes: 20 additions & 4 deletions src/ModelDescriber/ObjectModelDescriber.php
Expand Up @@ -130,6 +130,19 @@ public function describe(Model $model, OA\Schema $schema)
return null !== $value;
});

// Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222
// Promoted properties with a value initialized by the constructor are not considered to have a default value
// and are therefore not returned by ReflectionClass::getDefaultProperties(); see https://bugs.php.net/bug.php?id=81386
$reflClassConstructor = $reflClass->getConstructor();
$reflClassConstructorParameters = null !== $reflClassConstructor ? $reflClassConstructor->getParameters() : [];
foreach ($reflClassConstructorParameters as $parameter) {
if (!$parameter->isDefaultValueAvailable()) {
continue;
}

$defaultValues[$parameter->name] = $parameter->getDefaultValue();
}

foreach ($propertyInfoProperties as $propertyName) {
$serializedName = null !== $this->nameConverter ? $this->nameConverter->normalize($propertyName, $class, null, $model->getSerializationContext()) : $propertyName;

Expand All @@ -142,6 +155,13 @@ public function describe(Model $model, OA\Schema $schema)

$property = Util::getProperty($schema, $serializedName);

// Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222
// Property default value has to be set before SymfonyConstraintAnnotationReader::processPropertyAnnotations()
// is called to prevent wrongly detected required properties
if (Generator::UNDEFINED === $property->default && array_key_exists($propertyName, $defaultValues)) {
$property->default = $defaultValues[$propertyName];
}

// Interpret additional options
$groups = $model->getGroups();
if (isset($groups[$propertyName]) && is_array($groups[$propertyName])) {
Expand All @@ -156,10 +176,6 @@ public function describe(Model $model, OA\Schema $schema)
continue;
}

if (Generator::UNDEFINED === $property->default && array_key_exists($propertyName, $defaultValues)) {
$property->default = $defaultValues[$propertyName];
}

$types = $this->propertyInfo->getTypes($class, $propertyName);
if (null === $types || 0 === count($types)) {
throw new \LogicException(sprintf('The PropertyInfo component was not able to guess the type of %s::$%s. You may need to add a `@var` annotation or use `@OA\Property(type="")` to make its type explicit.', $class, $propertyName));
Expand Down
14 changes: 14 additions & 0 deletions tests/Functional/Configs/AlternativeNamesPHP80Entities.yaml
@@ -0,0 +1,14 @@
# Removes the `80` suffix from the entity names used in annotation entities.
nelmio_api_doc:
models:
names:
- alias: EntityWithPromotedPropertiesWithDefaults
type: Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithPromotedPropertiesWithDefaults80
- alias: EntityWithAlternateType
type: Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithAlternateType80

# Clean unused components from the OpenAPI schema.
services:
OpenApi\Processors\CleanUnusedComponents:
tags:
- { name: 'nelmio_api_doc.swagger.processor', priority: -100 }
14 changes: 14 additions & 0 deletions tests/Functional/Configs/AlternativeNamesPHP81Entities.yaml
@@ -0,0 +1,14 @@
# Removes the `81` suffix from the entity names used in attribute entities.
nelmio_api_doc:
models:
names:
- alias: EntityWithPromotedPropertiesWithDefaults
type: Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithPromotedPropertiesWithDefaults81
- alias: EntityWithAlternateType
type: Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithAlternateType81

# Clean unused components from the OpenAPI schema.
services:
OpenApi\Processors\CleanUnusedComponents:
tags:
- { name: 'nelmio_api_doc.swagger.processor', priority: -100 }
42 changes: 42 additions & 0 deletions tests/Functional/Controller/PromotedPropertiesController80.php
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;

use Nelmio\ApiDocBundle\Annotation\Model;
use Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithPromotedPropertiesWithDefaults80;
use OpenApi\Annotations as OA;
use Symfony\Component\Routing\Annotation\Route;

class PromotedPropertiesController80
{
/**
* @Route("/entity-with-promoted-properties-with-defaults", methods={"GET"})
*
* @OA\Get(
* operationId="getEntityWithPromotedPropertiesWithDefaults",
* )
*
* @OA\Response(
* response="204",
* description="Operation automatically detected",
* ),
*
* @OA\RequestBody(
*
* @Model(type=EntityWithPromotedPropertiesWithDefaults80::class))
* )*/
public function entityWithPromotedPropertiesWithDefaults()
{
}
}
37 changes: 37 additions & 0 deletions tests/Functional/Controller/PromotedPropertiesController81.php
@@ -0,0 +1,37 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;

use Nelmio\ApiDocBundle\Annotation\Model;
use Nelmio\ApiDocBundle\Tests\Functional\Entity\EntityWithPromotedPropertiesWithDefaults81;
use OpenApi\Attributes as OA;
use Symfony\Component\Routing\Annotation\Route;

class PromotedPropertiesController81
{
#[Route('/entity-with-promoted-properties-with-defaults', methods: ['GET'])]
#[OA\Get(
operationId: 'getEntityWithPromotedPropertiesWithDefaults',
)]
#[OA\Response(
response: 204,
description: 'Operation automatically detected',
)]
#[OA\RequestBody(
content: new Model(type: EntityWithPromotedPropertiesWithDefaults81::class),
)]
public function entityWithPromotedPropertiesWithDefaults()
{
}
}
78 changes: 69 additions & 9 deletions tests/Functional/ControllerTest.php
Expand Up @@ -44,18 +44,26 @@ protected function getOpenApiDefinition($area = 'default'): OA\OpenApi
}

/**
* @dataProvider provideControllers
* @dataProvider provideAnnotationTestCases
* @dataProvider provideAttributeTestCases
* @dataProvider provideUniversalTestCases
*
* @param array{name: string, type: string}|null $controller
* @param string[] $extraConfigs
*/
public function testControllers(?string $controllerName, ?string $fixtureName = null, array $extraConfigs = []): void
public function testControllers(?array $controller, ?string $fixtureName = null, array $extraConfigs = []): void
{
$controllerName = $controller['name'] ?? null;
$controllerType = $controller['type'] ?? null;

$fixtureName = $fixtureName ?? $controllerName ?? $this->fail('A fixture name must be provided.');

$routingConfiguration = function (RoutingConfigurator $routes) use ($controllerName) {
$routingConfiguration = function (RoutingConfigurator $routes) use ($controllerName, $controllerType) {
if (null === $controllerName) {
return;
}

$routes->withPath('/')->import(__DIR__."/Controller/$controllerName.php", 'attribute');
$routes->withPath('/')->import(__DIR__."/Controller/$controllerName.php", $controllerType);
};

$this->configurableContainerFactory->create([], $routingConfiguration, $extraConfigs);
Expand All @@ -69,22 +77,74 @@ public function testControllers(?string $controllerName, ?string $fixtureName =

self::assertSame(
self::getFixture($fixtureDir),
$this->getOpenApiDefinition()->toJson()
$this->getOpenApiDefinition()->toJson(),
);
}

public static function provideControllers(): iterable
public static function provideAttributeTestCases(): iterable
{
if (PHP_VERSION_ID < 80100) {
return;
}

$type = Kernel::MAJOR_VERSION === 5 ? 'annotation' : 'attribute';

yield 'Promoted properties defaults attributes' => [
[
'name' => 'PromotedPropertiesController81',
'type' => $type,
],
'PromotedPropertiesDefaults',
[__DIR__.'/Configs/AlternativeNamesPHP81Entities.yaml'],
];

if (version_compare(Kernel::VERSION, '6.3.0', '>=')) {
yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2209' => ['Controller2209'];
yield 'MapQueryString' => ['MapQueryStringController'];
yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2209' => [
[
'name' => 'Controller2209',
'type' => $type,
],
];
yield 'MapQueryString' => [
[
'name' => 'MapQueryStringController',
'type' => $type,
],
];
yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2191' => [
'MapQueryStringController',
[
'name' => 'MapQueryStringController',
'type' => $type,
],
'MapQueryStringCleanupComponents',
[__DIR__.'/Configs/CleanUnusedComponentsProcessor.yaml'],
];
}
}

public static function provideAnnotationTestCases(): iterable
{
if (!TestKernel::isAnnotationsAvailable()) {
return;
}

if (PHP_VERSION_ID >= 80000) {
yield 'Promoted properties defaults annotations' => [
[
'name' => 'PromotedPropertiesController80',
'type' => 'annotation',
],
'PromotedPropertiesDefaults',
[__DIR__.'/Configs/AlternativeNamesPHP80Entities.yaml'],
];
}
}

/**
* Test cases that are universal and can be run without depending on the existence of a specific feature.
*/
public static function provideUniversalTestCases(): iterable
{
yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2224' => [
null,
'VendorExtension',
Expand Down
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\ApiDocBundle\Tests\Functional\Entity;

use Symfony\Component\Validator\Constraints as Assert;

class EntityWithPromotedPropertiesWithDefaults80
{
/**
* @Assert\NotBlank()
*/
public string $nonNullableNonPromotedPropertyWithDefault;

public function __construct(
int $nonNullableNonPromotedProperty,
?string $nullableNonPromotedProperty,

string $nonNullableNonPromotedPropertyWithDefault = 'nonNullableNonPromotedPropertyWithDefault',
?int $nullableNonPromotedPropertyWithDefault = null,

public int $nonNullablePromotedPropertyWithDefault = 4711,
public ?string $nullablePromotedPropertyWithDefault = null,
) {
$this->nonNullableNonPromotedPropertyWithDefault = $nonNullableNonPromotedPropertyWithDefault;
}
}
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the NelmioApiDocBundle package.
*
* (c) Nelmio
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\ApiDocBundle\Tests\Functional\Entity;

use Symfony\Component\Validator\Constraints as Assert;

class EntityWithPromotedPropertiesWithDefaults81
{
#[Assert\NotBlank]
public readonly string $nonNullableNonPromotedPropertyWithDefault;

public function __construct(
int $nonNullableNonPromotedProperty,
?string $nullableNonPromotedProperty,

string $nonNullableNonPromotedPropertyWithDefault = 'nonNullableNonPromotedPropertyWithDefault',
?int $nullableNonPromotedPropertyWithDefault = null,

public readonly int $nonNullablePromotedPropertyWithDefault = 4711,
public readonly ?string $nullablePromotedPropertyWithDefault = null,
) {
$this->nonNullableNonPromotedPropertyWithDefault = $nonNullableNonPromotedPropertyWithDefault;
}
}
50 changes: 50 additions & 0 deletions tests/Functional/Fixtures/PromotedPropertiesDefaults.json
@@ -0,0 +1,50 @@
{
"openapi": "3.0.0",
"info": {
"title": "",
"version": "0.0.0"
},
"paths": {
"/entity-with-promoted-properties-with-defaults": {
"get": {
"operationId": "getEntityWithPromotedPropertiesWithDefaults",
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/EntityWithPromotedPropertiesWithDefaults"
}
}
}
},
"responses": {
"204": {
"description": "Operation automatically detected"
}
}
}
}
},
"components": {
"schemas": {
"EntityWithPromotedPropertiesWithDefaults": {
"properties": {
"nonNullableNonPromotedPropertyWithDefault": {
"type": "string",
"default": "nonNullableNonPromotedPropertyWithDefault"
},
"nonNullablePromotedPropertyWithDefault": {
"type": "integer",
"default": 4711
},
"nullablePromotedPropertyWithDefault": {
"type": "string",
"default": null,
"nullable": true
}
},
"type": "object"
}
}
}
}

0 comments on commit 7f46161

Please sign in to comment.