From 0912c85a0dc80bf910356daac33c9df59c40f7b7 Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Fri, 20 Nov 2020 17:06:56 +0100 Subject: [PATCH 1/3] Improve error when the items type of an array is not specified --- Exception/UndocumentedArrayItemsException.php | 42 +++++++++++++++++++ ModelDescriber/ObjectModelDescriber.php | 11 ++++- PropertyDescriber/ArrayPropertyDescriber.php | 13 +++++- Tests/Functional/ArrayItemsErrorTest.php | 37 ++++++++++++++++ Tests/Functional/BazingaFunctionalTest.php | 2 +- .../Controller/ArrayItemsErrorController.php | 35 ++++++++++++++++ .../Functional/Entity/ArrayItemsError/Bar.php | 25 +++++++++++ .../Functional/Entity/ArrayItemsError/Foo.php | 30 +++++++++++++ Tests/Functional/JMSFunctionalTest.php | 2 +- Tests/Functional/TestKernel.php | 30 +++++++------ 10 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 Exception/UndocumentedArrayItemsException.php create mode 100644 Tests/Functional/ArrayItemsErrorTest.php create mode 100644 Tests/Functional/Controller/ArrayItemsErrorController.php create mode 100644 Tests/Functional/Entity/ArrayItemsError/Bar.php create mode 100644 Tests/Functional/Entity/ArrayItemsError/Foo.php diff --git a/Exception/UndocumentedArrayItemsException.php b/Exception/UndocumentedArrayItemsException.php new file mode 100644 index 0000000..570642f --- /dev/null +++ b/Exception/UndocumentedArrayItemsException.php @@ -0,0 +1,42 @@ +class = $class; + $this->path = $path; + + $propertyName = ''; + if (null !== $class) { + $propertyName = $class.'::'; + } + $propertyName .= $path; + + parent::__construct(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@SWG\Property(type="array", @SWG\Items(type="string"))`.', $propertyName)); + } + + public function getClass() + { + return $this->class; + } + + public function getPath() + { + return $this->path; + } +} diff --git a/ModelDescriber/ObjectModelDescriber.php b/ModelDescriber/ObjectModelDescriber.php index 7a4a269..6350bfc 100644 --- a/ModelDescriber/ObjectModelDescriber.php +++ b/ModelDescriber/ObjectModelDescriber.php @@ -15,6 +15,7 @@ use Doctrine\Common\Annotations\Reader; use EXSyst\Component\Swagger\Schema; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareTrait; +use Nelmio\ApiDocBundle\Exception\UndocumentedArrayItemsException; use Nelmio\ApiDocBundle\Model\Model; use Nelmio\ApiDocBundle\ModelDescriber\Annotations\AnnotationsReader; use Nelmio\ApiDocBundle\PropertyDescriber\PropertyDescriberInterface; @@ -144,7 +145,15 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar $propertyDescriber->setModelRegistry($this->modelRegistry); } if ($propertyDescriber->supports($type)) { - $propertyDescriber->describe($type, $property, $model->getGroups()); + try { + $propertyDescriber->describe($type, $property, $model->getGroups()); + } catch (UndocumentedArrayItemsException $e) { + if (null !== $e->getClass()) { + throw $e; // This exception is already complete + } + + throw new UndocumentedArrayItemsException($model->getType()->getClassName(), sprintf("%s%s", $propertyName, $e->getPath())); + } return; } diff --git a/PropertyDescriber/ArrayPropertyDescriber.php b/PropertyDescriber/ArrayPropertyDescriber.php index 7055df4..fe8f63a 100644 --- a/PropertyDescriber/ArrayPropertyDescriber.php +++ b/PropertyDescriber/ArrayPropertyDescriber.php @@ -14,6 +14,7 @@ namespace Nelmio\ApiDocBundle\PropertyDescriber; use EXSyst\Component\Swagger\Schema; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareTrait; +use Nelmio\ApiDocBundle\Exception\UndocumentedArrayItemsException; use Symfony\Component\PropertyInfo\Type; class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistryAwareInterface @@ -32,7 +33,7 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr { $type = $type->getCollectionValueType(); if (null === $type) { - throw new \LogicException(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@SWG\Property(type="array", @SWG\Items(type="string"))`.', $property->getTitle())); + throw new UndocumentedArrayItemsException(); } $property->setType('array'); @@ -43,7 +44,15 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr $propertyDescriber->setModelRegistry($this->modelRegistry); } if ($propertyDescriber->supports($type)) { - $propertyDescriber->describe($type, $property, $groups); + try { + $propertyDescriber->describe($type, $property, $groups); + } catch (UndocumentedArrayItemsException $e) { + if (null !== $e->getClass()) { + throw $e; // This exception is already complete + } + + throw new UndocumentedArrayItemsException(null, sprintf("%s[]", $e->getPath())); + } break; } diff --git a/Tests/Functional/ArrayItemsErrorTest.php b/Tests/Functional/ArrayItemsErrorTest.php new file mode 100644 index 0000000..24acb63 --- /dev/null +++ b/Tests/Functional/ArrayItemsErrorTest.php @@ -0,0 +1,37 @@ + 'api.example.com']); + } + + public function testModelPictureDocumentation() + { + $this->expectException(UndocumentedArrayItemsException::class); + $this->expectExceptionMessage('Property "Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError\Bar::things[]" is an array, but its items type isn\'t specified.'); + + $this->getSwaggerDefinition(); + } + + protected static function createKernel(array $options = []) + { + return new TestKernel(TestKernel::ERROR_ARRAY_ITEMS); + } +} diff --git a/Tests/Functional/BazingaFunctionalTest.php b/Tests/Functional/BazingaFunctionalTest.php index beab5fd..ab82fe6 100644 --- a/Tests/Functional/BazingaFunctionalTest.php +++ b/Tests/Functional/BazingaFunctionalTest.php @@ -122,6 +122,6 @@ class BazingaFunctionalTest extends WebTestCase protected static function createKernel(array $options = []) { - return new TestKernel(true, true); + return new TestKernel(TestKernel::USE_JMS | TestKernel::USE_BAZINGA); } } diff --git a/Tests/Functional/Controller/ArrayItemsErrorController.php b/Tests/Functional/Controller/ArrayItemsErrorController.php new file mode 100644 index 0000000..6bbabbd --- /dev/null +++ b/Tests/Functional/Controller/ArrayItemsErrorController.php @@ -0,0 +1,35 @@ + + */ +class Bar +{ + /** + * @var array[] + */ + public $things; +} diff --git a/Tests/Functional/Entity/ArrayItemsError/Foo.php b/Tests/Functional/Entity/ArrayItemsError/Foo.php new file mode 100644 index 0000000..177a84f --- /dev/null +++ b/Tests/Functional/Entity/ArrayItemsError/Foo.php @@ -0,0 +1,30 @@ + + */ +class Foo +{ + /** + * @var string + */ + public $articles; + + /** + * @var Bar[] + */ + public $bars; +} diff --git a/Tests/Functional/JMSFunctionalTest.php b/Tests/Functional/JMSFunctionalTest.php index 990f181..1532ca0 100644 --- a/Tests/Functional/JMSFunctionalTest.php +++ b/Tests/Functional/JMSFunctionalTest.php @@ -304,6 +304,6 @@ class JMSFunctionalTest extends WebTestCase protected static function createKernel(array $options = []) { - return new TestKernel(true); + return new TestKernel(TestKernel::USE_JMS); } } diff --git a/Tests/Functional/TestKernel.php b/Tests/Functional/TestKernel.php index 32e9399..edb64e4 100644 --- a/Tests/Functional/TestKernel.php +++ b/Tests/Functional/TestKernel.php @@ -33,17 +33,19 @@ use Symfony\Component\Serializer\Annotation\SerializedName; class TestKernel extends Kernel { + const USE_JMS = 1; + const USE_BAZINGA = 2; + const ERROR_ARRAY_ITEMS = 4; + use MicroKernelTrait; - private $useJMS; - private $useBazinga; + private $flags; - public function __construct(bool $useJMS = false, bool $useBazinga = false) + public function __construct(int $flags = 0) { - parent::__construct('test'.(int) $useJMS.(int) $useBazinga, true); + parent::__construct('test'.$flags, true); - $this->useJMS = $useJMS; - $this->useBazinga = $useBazinga; + $this->flags = $flags; } /** @@ -61,10 +63,10 @@ class TestKernel extends Kernel new FOSRestBundle(), ]; - if ($this->useJMS) { + if ($this->flags & self::USE_JMS) { $bundles[] = new JMSSerializerBundle(); - if ($this->useBazinga) { + if ($this->flags & self::USE_BAZINGA) { $bundles[] = new BazingaHateoasBundle(); } } @@ -91,11 +93,11 @@ class TestKernel extends Kernel $routes->import(__DIR__.'/Controller/SerializedNameController.php', '/', 'annotation'); } - if ($this->useJMS) { + if ($this->flags & self::USE_JMS) { $routes->import(__DIR__.'/Controller/JMSController.php', '/', 'annotation'); } - if ($this->useBazinga) { + if ($this->flags & self::USE_BAZINGA) { $routes->import(__DIR__.'/Controller/BazingaController.php', '/', 'annotation'); try { @@ -104,6 +106,10 @@ class TestKernel extends Kernel } catch (\ReflectionException $e) { } } + + if ($this->flags & self::ERROR_ARRAY_ITEMS) { + $routes->import(__DIR__.'/Controller/ArrayItemsErrorController.php', '/', 'annotation'); + } } /** @@ -231,7 +237,7 @@ class TestKernel extends Kernel */ public function getCacheDir() { - return parent::getCacheDir().'/'.(int) $this->useJMS; + return parent::getCacheDir().'/'.$this->flags; } /** @@ -239,7 +245,7 @@ class TestKernel extends Kernel */ public function getLogDir() { - return parent::getLogDir().'/'.(int) $this->useJMS; + return parent::getLogDir().'/'.$this->flags; } public function serialize() From 68bf1670f3e4592f52e81781c93832b694f5e5f3 Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Fri, 20 Nov 2020 17:10:21 +0100 Subject: [PATCH 2/3] Fix CS --- Exception/UndocumentedArrayItemsException.php | 2 +- ModelDescriber/ObjectModelDescriber.php | 2 +- PropertyDescriber/ArrayPropertyDescriber.php | 2 +- Tests/Functional/Entity/ArrayItemsError/Bar.php | 2 -- Tests/Functional/Entity/ArrayItemsError/Foo.php | 2 -- Tests/Functional/TestKernel.php | 2 +- 6 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Exception/UndocumentedArrayItemsException.php b/Exception/UndocumentedArrayItemsException.php index 570642f..b44c05f 100644 --- a/Exception/UndocumentedArrayItemsException.php +++ b/Exception/UndocumentedArrayItemsException.php @@ -16,7 +16,7 @@ class UndocumentedArrayItemsException extends \LogicException private $class; private $path; - public function __construct(string $class = null, string $path = "") + public function __construct(string $class = null, string $path = '') { $this->class = $class; $this->path = $path; diff --git a/ModelDescriber/ObjectModelDescriber.php b/ModelDescriber/ObjectModelDescriber.php index 6350bfc..0f41324 100644 --- a/ModelDescriber/ObjectModelDescriber.php +++ b/ModelDescriber/ObjectModelDescriber.php @@ -152,7 +152,7 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar throw $e; // This exception is already complete } - throw new UndocumentedArrayItemsException($model->getType()->getClassName(), sprintf("%s%s", $propertyName, $e->getPath())); + throw new UndocumentedArrayItemsException($model->getType()->getClassName(), sprintf('%s%s', $propertyName, $e->getPath())); } return; diff --git a/PropertyDescriber/ArrayPropertyDescriber.php b/PropertyDescriber/ArrayPropertyDescriber.php index fe8f63a..defb84f 100644 --- a/PropertyDescriber/ArrayPropertyDescriber.php +++ b/PropertyDescriber/ArrayPropertyDescriber.php @@ -51,7 +51,7 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr throw $e; // This exception is already complete } - throw new UndocumentedArrayItemsException(null, sprintf("%s[]", $e->getPath())); + throw new UndocumentedArrayItemsException(null, sprintf('%s[]', $e->getPath())); } break; diff --git a/Tests/Functional/Entity/ArrayItemsError/Bar.php b/Tests/Functional/Entity/ArrayItemsError/Bar.php index fc020cc..76a8073 100644 --- a/Tests/Functional/Entity/ArrayItemsError/Bar.php +++ b/Tests/Functional/Entity/ArrayItemsError/Bar.php @@ -11,8 +11,6 @@ namespace Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError; -use Symfony\Component\Serializer\Annotation\Groups; - /** * @author Guilhem N. */ diff --git a/Tests/Functional/Entity/ArrayItemsError/Foo.php b/Tests/Functional/Entity/ArrayItemsError/Foo.php index 177a84f..c9d7704 100644 --- a/Tests/Functional/Entity/ArrayItemsError/Foo.php +++ b/Tests/Functional/Entity/ArrayItemsError/Foo.php @@ -11,8 +11,6 @@ namespace Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError; -use Symfony\Component\Serializer\Annotation\Groups; - /** * @author Guilhem N. */ diff --git a/Tests/Functional/TestKernel.php b/Tests/Functional/TestKernel.php index edb64e4..edf9e2e 100644 --- a/Tests/Functional/TestKernel.php +++ b/Tests/Functional/TestKernel.php @@ -66,7 +66,7 @@ class TestKernel extends Kernel if ($this->flags & self::USE_JMS) { $bundles[] = new JMSSerializerBundle(); - if ($this->flags & self::USE_BAZINGA) { + if ($this->flags & self::USE_BAZINGA) { $bundles[] = new BazingaHateoasBundle(); } } From a01fd1e4f0c09793f891516cbc93d06a33ca70ba Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Sat, 28 Nov 2020 16:11:05 +0100 Subject: [PATCH 3/3] Fix error --- Tests/Functional/Entity/ArrayItemsError/Bar.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Tests/Functional/Entity/ArrayItemsError/Bar.php b/Tests/Functional/Entity/ArrayItemsError/Bar.php index 76a8073..214e99f 100644 --- a/Tests/Functional/Entity/ArrayItemsError/Bar.php +++ b/Tests/Functional/Entity/ArrayItemsError/Bar.php @@ -16,8 +16,7 @@ namespace Nelmio\ApiDocBundle\Tests\Functional\Entity\ArrayItemsError; */ class Bar { - /** - * @var array[] - */ public $things; + + public function addThing(array $thing) { } }