Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some deprecations about invalid values #1049

Merged
merged 1 commit into from
Jun 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
UPGRADE 3.x
===========

UPGRADE FROM 3.x to 3.x
=======================
phansys marked this conversation as resolved.
Show resolved Hide resolved

### Sonata\DoctrineORMAdminBundle\Admin\FieldDescription

Deprecated `getTargetEntity()`, use `getTargetModel()` instead.

### Sonata\DoctrineORMAdminBundle\Model\ModelManager

Deprecated passing `null` as argument 2 for `find()`.
Deprecated passing `null` or an object which is in state new or removed as argument 1 for `getNormalizedIdentifier()`.
Deprecated passing `null` as argument 1 for `getUrlSafeIdentifier()`.

UPGRADE FROM 3.0 to 3.1
=======================

### Tests

All files under the ``Tests`` directory are now correctly handled as internal test classes.
You can't extend them anymore, because they are only loaded when running internal tests.
All files under the ``Tests`` directory are now correctly handled as internal test classes.
You can't extend them anymore, because they are only loaded when running internal tests.
More information can be found in the [composer docs](https://getcomposer.org/doc/04-schema.md#autoload-dev).
22 changes: 22 additions & 0 deletions src/Admin/FieldDescription.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,33 @@ 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()
phansys marked this conversation as resolved.
Show resolved Hide resolved
{
@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 since sonata-project/doctrine-orm-admin-bundle 3.x.
*/
public function getTargetModel(): ?string
phansys marked this conversation as resolved.
Show resolved Hide resolved
{
if ($this->associationMapping) {
return $this->associationMapping['targetEntity'];
}

return null;
}

public function setFieldMapping($fieldMapping)
Expand Down
2 changes: 1 addition & 1 deletion src/Builder/DatagridBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions src/Builder/FormContractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,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])) {
Expand All @@ -122,7 +122,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()
));
}
}
Expand All @@ -132,7 +132,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()
));
}

Expand Down Expand Up @@ -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()
));
}

Expand Down
44 changes: 39 additions & 5 deletions src/Model/ModelManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -260,7 +265,7 @@ public function findOneBy($class, array $criteria = [])
}

/**
* @param string $class
* @param string|object $class
*
* @return EntityManager
*/
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Admin/FieldDescriptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions tests/Builder/FormContractorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Sonata\AdminBundle\Form\Type\ModelListType;
use Sonata\AdminBundle\Form\Type\ModelType;
use Sonata\AdminBundle\Model\ModelManagerInterface;
use Sonata\DoctrineORMAdminBundle\Admin\FieldDescription;
use Sonata\DoctrineORMAdminBundle\Builder\FormContractor;
use Sonata\DoctrineORMAdminBundle\Model\ModelManager;
use Sonata\Form\Type\CollectionType;
Expand Down Expand Up @@ -73,9 +74,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 and replace `getTargetEntity()` with `getTargetModel().
$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);

Expand Down
14 changes: 14 additions & 0 deletions tests/Model/ModelManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down