Skip to content

[DoctrineBridge][UX] Better Doctrine exceptions #3965

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 16, 2021
Merged

[DoctrineBridge][UX] Better Doctrine exceptions #3965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96e9766
Better doctrine exception
Korbeil Jan 20, 2021
c1f8d55
chore: CS
alanpoulain Jan 20, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## 2.7.0

* Doctrine: Better exception to find which resource is linked to an exception (#3965)
* MongoDB: `date_immutable` support (#3940)

## 2.6.3
Expand Down
18 changes: 16 additions & 2 deletions src/Bridge/Doctrine/Common/Util/IdentifierManagerTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use ApiPlatform\Core\Exception\InvalidIdentifierException;
use ApiPlatform\Core\Exception\PropertyNotFoundException;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\Type as DBALType;
use Doctrine\ODM\MongoDB\DocumentManager;
Expand All @@ -29,6 +30,7 @@ trait IdentifierManagerTrait
{
private $propertyNameCollectionFactory;
private $propertyMetadataFactory;
private $resourceMetadataFactory;

/**
* Transform and check the identifier, composite or not.
Expand Down Expand Up @@ -74,7 +76,13 @@ private function normalizeIdentifiers($id, ObjectManager $manager, string $resou

$identifier = null === $identifiersMap ? $identifierValues[$i] ?? null : $identifiersMap[$propertyName] ?? null;
if (null === $identifier) {
throw new PropertyNotFoundException(sprintf('Invalid identifier "%s", "%s" was not found.', $id, $propertyName));
$exceptionMessage = sprintf('Invalid identifier "%s", "%s" was not found', $id, $propertyName);
if ($this->resourceMetadataFactory instanceof ResourceMetadataFactoryInterface) {
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
$exceptionMessage .= sprintf(' for resource "%s"', $resourceMetadata->getShortName());
}

throw new PropertyNotFoundException($exceptionMessage.'.');
}

$doctrineTypeName = $doctrineClassMetadata->getTypeOfField($propertyName);
Expand All @@ -87,7 +95,13 @@ private function normalizeIdentifiers($id, ObjectManager $manager, string $resou
$identifier = MongoDbType::getType($doctrineTypeName)->convertToPHPValue($identifier);
}
} catch (ConversionException $e) {
throw new InvalidIdentifierException(sprintf('Invalid value "%s" provided for an identifier.', $propertyName), $e->getCode(), $e);
$exceptionMessage = sprintf('Invalid value "%s" provided for an identifier', $propertyName);
if ($this->resourceMetadataFactory instanceof ResourceMetadataFactoryInterface) {
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
$exceptionMessage .= sprintf(' for resource "%s"', $resourceMetadata->getShortName());
}

throw new InvalidIdentifierException($exceptionMessage.'.', $e->getCode(), $e);
}

$identifiers[$propertyName] = $identifier;
Expand Down
4 changes: 3 additions & 1 deletion src/Bridge/Doctrine/Orm/ItemDataProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\QueryBuilder;
use Doctrine\Persistence\ManagerRegistry;
Expand All @@ -45,11 +46,12 @@ class ItemDataProvider implements DenormalizedIdentifiersAwareItemDataProviderIn
/**
* @param QueryItemExtensionInterface[] $itemExtensions
*/
public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, iterable $itemExtensions = [])
public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, iterable $itemExtensions = [], ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
{
$this->managerRegistry = $managerRegistry;
$this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
$this->propertyMetadataFactory = $propertyMetadataFactory;
$this->resourceMetadataFactory = $resourceMetadataFactory;
$this->itemExtensions = $itemExtensions;
}

Expand Down
4 changes: 3 additions & 1 deletion src/Bridge/Doctrine/Orm/SubresourceDataProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\QueryBuilder;
Expand All @@ -48,11 +49,12 @@ final class SubresourceDataProvider implements SubresourceDataProviderInterface
* @param QueryCollectionExtensionInterface[] $collectionExtensions
* @param QueryItemExtensionInterface[] $itemExtensions
*/
public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, iterable $collectionExtensions = [], iterable $itemExtensions = [])
public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, iterable $collectionExtensions = [], iterable $itemExtensions = [], ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
{
$this->managerRegistry = $managerRegistry;
$this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
$this->propertyMetadataFactory = $propertyMetadataFactory;
$this->resourceMetadataFactory = $resourceMetadataFactory;
$this->collectionExtensions = $collectionExtensions;
$this->itemExtensions = $itemExtensions;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<argument type="service" id="api_platform.metadata.property.name_collection_factory" />
<argument type="service" id="api_platform.metadata.property.metadata_factory" />
<argument type="tagged" tag="api_platform.doctrine.orm.query_extension.item" />
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
</service>

<service id="api_platform.doctrine.orm.subresource_data_provider" public="false" abstract="true">
Expand All @@ -33,6 +34,7 @@
<argument type="service" id="api_platform.metadata.property.metadata_factory" />
<argument type="tagged" tag="api_platform.doctrine.orm.query_extension.collection" />
<argument type="tagged" tag="api_platform.doctrine.orm.query_extension.item" />
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
</service>

<service id="api_platform.doctrine.orm.default.collection_data_provider" parent="api_platform.doctrine.orm.collection_data_provider" class="ApiPlatform\Core\Bridge\Doctrine\Orm\CollectionDataProvider">
Expand Down
37 changes: 21 additions & 16 deletions tests/Bridge/Doctrine/Common/Util/IdentifierManagerTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
use ApiPlatform\Core\Metadata\Property\PropertyNameCollection;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Dummy as DummyDocument;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
use ApiPlatform\Core\Tests\ProphecyTrait;
Expand All @@ -39,17 +41,18 @@ class IdentifierManagerTraitTest extends TestCase
{
use ProphecyTrait;

private function getIdentifierManagerTraitImpl(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory)
private function getIdentifierManagerTraitImpl(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory)
{
return new class($propertyNameCollectionFactory, $propertyMetadataFactory) {
return new class($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory) {
use IdentifierManagerTrait {
IdentifierManagerTrait::normalizeIdentifiers as public;
}

public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory)
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory)
{
$this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
$this->propertyMetadataFactory = $propertyMetadataFactory;
$this->resourceMetadataFactory = $resourceMetadataFactory;
}
};
}
Expand All @@ -59,7 +62,7 @@ public function __construct(PropertyNameCollectionFactoryInterface $propertyName
*/
public function testSingleIdentifier()
{
[$propertyNameCollectionFactory, $propertyMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
[$propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
'id',
]);
$objectManager = $this->getEntityManager(Dummy::class, [
Expand All @@ -68,7 +71,7 @@ public function testSingleIdentifier()
],
]);

$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory);
$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory);

$this->assertEquals($identifierManager->normalizeIdentifiers(1, $objectManager, Dummy::class), ['id' => 1]);
}
Expand All @@ -79,7 +82,7 @@ public function testSingleIdentifier()
*/
public function testSingleDocumentIdentifier()
{
[$propertyNameCollectionFactory, $propertyMetadataFactory] = $this->getMetadataFactories(DummyDocument::class, [
[$propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory] = $this->getMetadataFactories(DummyDocument::class, [
'id',
]);
$objectManager = $this->getDocumentManager(DummyDocument::class, [
Expand All @@ -88,7 +91,7 @@ public function testSingleDocumentIdentifier()
],
]);

$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory);
$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory);

$this->assertEquals($identifierManager->normalizeIdentifiers(1, $objectManager, DummyDocument::class), ['id' => 1]);
}
Expand All @@ -98,7 +101,7 @@ public function testSingleDocumentIdentifier()
*/
public function testCompositeIdentifier()
{
[$propertyNameCollectionFactory, $propertyMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
[$propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
'ida',
'idb',
]);
Expand All @@ -111,7 +114,7 @@ public function testCompositeIdentifier()
],
]);

$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory);
$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory);

$this->assertEquals($identifierManager->normalizeIdentifiers('ida=1;idb=2', $objectManager, Dummy::class), ['ida' => 1, 'idb' => 2]);
}
Expand All @@ -122,9 +125,9 @@ public function testCompositeIdentifier()
public function testInvalidIdentifier()
{
$this->expectException(PropertyNotFoundException::class);
$this->expectExceptionMessage('Invalid identifier "idbad=1;idb=2", "ida" was not found.');
$this->expectExceptionMessage('Invalid identifier "idbad=1;idb=2", "ida" was not found for resource "dummy".');

[$propertyNameCollectionFactory, $propertyMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
[$propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
'ida',
'idb',
]);
Expand All @@ -137,7 +140,7 @@ public function testInvalidIdentifier()
],
]);

$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory);
$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory);

$identifierManager->normalizeIdentifiers('idbad=1;idb=2', $objectManager, Dummy::class);
}
Expand All @@ -149,6 +152,7 @@ private function getMetadataFactories(string $resourceClass, array $identifiers)
{
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);

$nameCollection = ['foobar'];

Expand All @@ -164,8 +168,9 @@ private function getMetadataFactories(string $resourceClass, array $identifiers)
$propertyMetadataFactoryProphecy->create($resourceClass, 'foobar')->willReturn(new PropertyMetadata());

$propertyNameCollectionFactoryProphecy->create($resourceClass)->willReturn(new PropertyNameCollection($nameCollection));
$resourceMetadataFactoryProphecy->create($resourceClass)->willReturn(new ResourceMetadata('dummy'));

return [$propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal()];
return [$propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal()];
}

/**
Expand Down Expand Up @@ -218,9 +223,9 @@ public function testInvalidIdentifierConversion()
DBALType::addType('uuid', UuidType::class);

$this->expectException(InvalidIdentifierException::class);
$this->expectExceptionMessage('Invalid value "ida" provided for an identifier.');
$this->expectExceptionMessage('Invalid value "ida" provided for an identifier for resource "dummy".');

[$propertyNameCollectionFactory, $propertyMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
[$propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory] = $this->getMetadataFactories(Dummy::class, [
'ida',
]);
$objectManager = $this->getEntityManager(Dummy::class, [
Expand All @@ -229,7 +234,7 @@ public function testInvalidIdentifierConversion()
],
]);

$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory);
$identifierManager = $this->getIdentifierManagerTraitImpl($propertyNameCollectionFactory, $propertyMetadataFactory, $resourceMetadataFactory);

$identifierManager->normalizeIdentifiers('notanuuid', $objectManager, Dummy::class);
}
Expand Down
Loading