From 80cae68cc43c42d74b30c16dedcf004692861a90 Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Sun, 24 May 2020 19:21:26 -0300 Subject: [PATCH] Add some deprecations about invalid values --- src/Admin/FieldDescription.php | 19 ++++++++++++ src/Builder/DatagridBuilder.php | 2 +- src/Builder/FormContractor.php | 8 ++--- src/Model/ModelManager.php | 44 ++++++++++++++++++++++++---- tests/Admin/FieldDescriptionTest.php | 4 +-- tests/Builder/FormContractorTest.php | 6 ++-- tests/Model/ModelManagerTest.php | 14 +++++++++ 7 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/Admin/FieldDescription.php b/src/Admin/FieldDescription.php index 3ffb7ba33..f6fddff1c 100644 --- a/src/Admin/FieldDescription.php +++ b/src/Admin/FieldDescription.php @@ -35,11 +35,30 @@ public function setAssociationMapping($associationMapping) $this->fieldName = $associationMapping['fieldName']; } + /** + * NEXT_MAJOR: Remove this method. + * + * @deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be removed in version 4.0. Use FieldDescription::getTargetModel() instead. + */ public function getTargetEntity() + { + @trigger_error(sprintf( + 'Method %s() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be removed in version 4.0.' + .' Use %s::getTargetModel() instead.', + __METHOD__, + __CLASS__ + ), E_USER_DEPRECATED); + + return $this->getTargetModel(); + } + + final public function getTargetModel(): ?string { if ($this->associationMapping) { return $this->associationMapping['targetEntity']; } + + return null; } public function setFieldMapping($fieldMapping) diff --git a/src/Builder/DatagridBuilder.php b/src/Builder/DatagridBuilder.php index 7d6217636..9cce9028a 100644 --- a/src/Builder/DatagridBuilder.php +++ b/src/Builder/DatagridBuilder.php @@ -158,7 +158,7 @@ public function addFilter(DatagridInterface $datagrid, $type, FieldDescriptionIn if (ModelAutocompleteFilter::class === $type) { $fieldDescription->mergeOption('field_options', [ - 'class' => $fieldDescription->getTargetEntity(), + 'class' => $fieldDescription->getTargetModel(), 'model_manager' => $fieldDescription->getAdmin()->getModelManager(), 'admin_code' => $admin->getCode(), 'context' => 'filter', diff --git a/src/Builder/FormContractor.php b/src/Builder/FormContractor.php index 3658f2ef9..7fe9c6243 100644 --- a/src/Builder/FormContractor.php +++ b/src/Builder/FormContractor.php @@ -114,7 +114,7 @@ public function getDefaultOptions($type, FieldDescriptionInterface $fieldDescrip )); } - $options['class'] = $fieldDescription->getTargetEntity(); + $options['class'] = $fieldDescription->getTargetModel(); $options['model_manager'] = $fieldDescription->getAdmin()->getModelManager(); if ($this->checkFormClass($type, [ModelAutocompleteType::class])) { @@ -123,7 +123,7 @@ public function getDefaultOptions($type, FieldDescriptionInterface $fieldDescrip 'The current field `%s` is not linked to an admin.' .' Please create one for the target entity: `%s`', $fieldDescription->getName(), - $fieldDescription->getTargetEntity() + $fieldDescription->getTargetModel() )); } } @@ -133,7 +133,7 @@ public function getDefaultOptions($type, FieldDescriptionInterface $fieldDescrip 'The current field `%s` is not linked to an admin.' .' Please create one for the target entity : `%s`', $fieldDescription->getName(), - $fieldDescription->getTargetEntity() + $fieldDescription->getTargetModel() )); } @@ -165,7 +165,7 @@ public function getDefaultOptions($type, FieldDescriptionInterface $fieldDescrip 'The current field `%s` is not linked to an admin.' .' Please create one for the target entity : `%s`', $fieldDescription->getName(), - $fieldDescription->getTargetEntity() + $fieldDescription->getTargetModel() )); } diff --git a/src/Model/ModelManager.php b/src/Model/ModelManager.php index ad74b0f47..cbfc7eec5 100644 --- a/src/Model/ModelManager.php +++ b/src/Model/ModelManager.php @@ -240,7 +240,12 @@ public function lock($object, $expectedVersion) public function find($class, $id) { - if (!isset($id)) { + if (null === $id) { + @trigger_error(sprintf( + 'Passing null as argument 1 for %s() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be not allowed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + return null; } @@ -260,7 +265,7 @@ public function findOneBy($class, array $criteria = []) } /** - * @param string $class + * @param string|object $class * * @return EntityManager */ @@ -370,7 +375,13 @@ public function getIdentifierFieldNames($class) public function getNormalizedIdentifier($entity) { + // NEXT_MAJOR: Remove the following 2 checks and declare "object" as type for argument 1. if (null === $entity) { + @trigger_error(sprintf( + 'Passing null as argument 1 for %s() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be not allowed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + return null; } @@ -382,6 +393,21 @@ public function getNormalizedIdentifier($entity) UnitOfWork::STATE_NEW, UnitOfWork::STATE_REMOVED, ], true)) { + // NEXT_MAJOR: Uncomment the following exception, remove the deprecation and the return statement inside this conditional block. + // throw new \InvalidArgumentException(sprintf( + // 'Can not get the normalized identifier for %s since it is in state %u.', + // ClassUtils::getClass($entity), + // $this->getEntityManager($entity)->getUnitOfWork()->getEntityState($entity) + // )); + + @trigger_error(sprintf( + 'Passing an object which is in state %u (new) or %u (removed) as argument 1 for %s() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x' + .'and will be not allowed in version 4.0.', + UnitOfWork::STATE_NEW, + UnitOfWork::STATE_REMOVED, + __METHOD__ + ), E_USER_DEPRECATED); + return null; } @@ -402,6 +428,16 @@ public function getNormalizedIdentifier($entity) */ public function getUrlSafeIdentifier($entity) { + // NEXT_MAJOR: Remove the following check and declare "object" as type for argument 1. + if (!\is_object($entity)) { + @trigger_error(sprintf( + 'Passing other type than object for argument 1 for %s() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be not allowed in version 4.0.', + __METHOD__ + ), E_USER_DEPRECATED); + + return null; + } + return $this->getNormalizedIdentifier($entity); } @@ -447,9 +483,7 @@ public function batchDelete($class, ProxyQueryInterface $queryProxy) $entityManager->flush(); $entityManager->clear(); - } catch (\PDOException $e) { - throw new ModelManagerException('', 0, $e); - } catch (DBALException $e) { + } catch (\PDOException | DBALException $e) { throw new ModelManagerException('', 0, $e); } } diff --git a/tests/Admin/FieldDescriptionTest.php b/tests/Admin/FieldDescriptionTest.php index 6ff47eea5..079a0e87a 100644 --- a/tests/Admin/FieldDescriptionTest.php +++ b/tests/Admin/FieldDescriptionTest.php @@ -303,11 +303,11 @@ public function testGetTargetEntity(): void $field = new FieldDescription(); - $this->assertNull($field->getTargetEntity()); + $this->assertNull($field->getTargetModel()); $field->setAssociationMapping($assocationMapping); - $this->assertSame('someValue', $field->getTargetEntity()); + $this->assertSame('someValue', $field->getTargetModel()); } public function testIsIdentifierFromFieldMapping(): void diff --git a/tests/Builder/FormContractorTest.php b/tests/Builder/FormContractorTest.php index e7f3d566a..fdd6b76b8 100644 --- a/tests/Builder/FormContractorTest.php +++ b/tests/Builder/FormContractorTest.php @@ -25,6 +25,7 @@ use Sonata\AdminBundle\Form\Type\ModelType; use Sonata\AdminBundle\Model\ModelManagerInterface; use Sonata\CoreBundle\Form\Type\CollectionType as DeprecatedCollectionType; +use Sonata\DoctrineORMAdminBundle\Admin\FieldDescription; use Sonata\DoctrineORMAdminBundle\Builder\FormContractor; use Sonata\DoctrineORMAdminBundle\Model\ModelManager; use Sonata\Form\Type\CollectionType; @@ -74,9 +75,10 @@ public function testDefaultOptionsForSonataFormTypes(): void $admin->method('getModelManager')->willReturn($modelManager); $admin->method('getClass')->willReturn($modelClass); - $fieldDescription = $this->createMock(FieldDescriptionInterface::class); + // NEXT_MAJOR: Mock `FieldDescriptionInterface` instead of `FieldDescription`, when `getTargetModel()` is available. + $fieldDescription = $this->createMock(FieldDescription::class); $fieldDescription->method('getAdmin')->willReturn($admin); - $fieldDescription->method('getTargetEntity')->willReturn($modelClass); + $fieldDescription->method('getTargetModel')->willReturn($modelClass); $fieldDescription->method('getAssociationAdmin')->willReturn($admin); $admin->method('getNewInstance')->willReturn($model); diff --git a/tests/Model/ModelManagerTest.php b/tests/Model/ModelManagerTest.php index dea47c2ff..6615b409a 100644 --- a/tests/Model/ModelManagerTest.php +++ b/tests/Model/ModelManagerTest.php @@ -937,6 +937,13 @@ public function testRemove($exception): void $model->delete(new VersionedEntity()); } + /** + * NEXT_MAJOR: Remove this method. + * + * @group legacy + * + * @expectedDeprecation Passing null as argument 1 for Sonata\DoctrineORMAdminBundle\Model\ModelManager::find() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be not allowed in version 4.0. + */ public function testFindBadId(): void { $registry = $this->createMock(ManagerRegistry::class); @@ -973,6 +980,13 @@ public function getWrongEntities(): iterable yield ['sonata-project']; } + /** + * NEXT_MAJOR: Remove this method. + * + * @group legacy + * + * @expectedDeprecation Passing null as argument 1 for Sonata\DoctrineORMAdminBundle\Model\ModelManager::getNormalizedIdentifier() is deprecated since sonata-project/doctrine-orm-admin-bundle 3.x and will be not allowed in version 4.0. + */ public function testGetUrlsafeIdentifierNull(): void { $registry = $this->createMock(ManagerRegistry::class);