diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php index adb0cd529..02c0f99aa 100644 --- a/lib/Doctrine/ORM/ORMInvalidArgumentException.php +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -18,6 +18,7 @@ */ namespace Doctrine\ORM; +use Doctrine\ORM\Mapping\ClassMetadata; /** * Contains exception messages for all invalid lifecycle state exceptions inside UnitOfWork @@ -187,6 +188,30 @@ class ORMInvalidArgumentException extends \InvalidArgumentException return new self("Binding entities to query parameters only allowed for entities that have an identifier."); } + /** + * @param ClassMetadata $targetClass + * @param array $assoc + * @param mixed $actualValue + * + * @return self + */ + public static function invalidAssociation(ClassMetadata $targetClass, $assoc, $actualValue) + { + $expectedType = 'Doctrine\Common\Collections\Collection|array'; + + if (($assoc['type'] & ClassMetadata::TO_ONE) > 0) { + $expectedType = $targetClass->getName(); + } + + return new self(sprintf( + 'Expected value of type "%s" for association field "%s#$%s", got "%s" instead.', + $expectedType, + $assoc['sourceEntity'], + $assoc['fieldName'], + is_object($actualValue) ? get_class($actualValue) : gettype($actualValue) + )); + } + /** * Helper method to show an object as string. * diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 5c2eb7bc0..32e4b8603 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -830,6 +830,10 @@ class UnitOfWork implements PropertyChangedListener $targetClass = $this->em->getClassMetadata($assoc['targetEntity']); foreach ($unwrappedValue as $key => $entry) { + if (! ($entry instanceof $targetClass->name)) { + throw ORMInvalidArgumentException::invalidAssociation($targetClass, $assoc, $entry); + } + $state = $this->getEntityState($entry, self::STATE_NEW); if ( ! ($entry instanceof $assoc['targetEntity'])) { @@ -2180,12 +2184,29 @@ class UnitOfWork implements PropertyChangedListener case ($relatedEntities instanceof Collection): case (is_array($relatedEntities)): + if (($assoc['type'] & ClassMetadata::TO_MANY) <= 0) { + throw ORMInvalidArgumentException::invalidAssociation( + $this->em->getClassMetadata($assoc['targetEntity']), + $assoc, + $relatedEntities + ); + } + foreach ($relatedEntities as $relatedEntity) { $this->doPersist($relatedEntity, $visited); } + break; case ($relatedEntities !== null): + if (! $relatedEntities instanceof $assoc['targetEntity']) { + throw ORMInvalidArgumentException::invalidAssociation( + $this->em->getClassMetadata($assoc['targetEntity']), + $assoc, + $relatedEntities + ); + } + $this->doPersist($relatedEntities, $visited); break; diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index be290acb3..29f8fab85 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php @@ -1288,9 +1288,14 @@ class BasicFunctionalTest extends \Doctrine\Tests\OrmFunctionalTestCase $user->status = 'developer'; $user->address = $user; + $this->setExpectedException( + 'Doctrine\ORM\ORMInvalidArgumentException', + 'Expected value of type "Doctrine\Tests\Models\CMS\CmsAddress" for association field ' + . '"Doctrine\Tests\Models\CMS\CmsUser#$address", got "Doctrine\Tests\Models\CMS\CmsUser" instead.' + ); + $this->_em->persist($user); - $this->setExpectedException("Doctrine\ORM\ORMException", "Found entity of type Doctrine\Tests\Models\CMS\CmsUser on association Doctrine\Tests\Models\CMS\CmsUser#address, but expecting Doctrine\Tests\Models\CMS\CmsAddress"); $this->_em->flush(); } } diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index a64f2b428..df7256e44 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -2,29 +2,49 @@ namespace Doctrine\Tests\ORM; +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\NotifyPropertyChanged; +use Doctrine\Common\PropertyChangedListener; +use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\UnitOfWork; use Doctrine\Tests\Mocks\ConnectionMock; +use Doctrine\Tests\Mocks\DriverMock; use Doctrine\Tests\Mocks\EntityManagerMock; -use Doctrine\Tests\Mocks\UnitOfWorkMock; use Doctrine\Tests\Mocks\EntityPersisterMock; -use Doctrine\Tests\Models\Forum\ForumUser; +use Doctrine\Tests\Mocks\UnitOfWorkMock; use Doctrine\Tests\Models\Forum\ForumAvatar; +use Doctrine\Tests\Models\Forum\ForumUser; +use stdClass; /** * UnitOfWork tests. */ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase { - // SUT + /** + * SUT + * + * @var UnitOfWorkMock + */ private $_unitOfWork; - // Provides a sequence mock to the UnitOfWork + + /** + * Provides a sequence mock to the UnitOfWork + * + * @var ConnectionMock + */ private $_connectionMock; - // The EntityManager mock that provides the mock persisters + + /** + * The EntityManager mock that provides the mock persisters + * + * @var EntityManagerMock + */ private $_emMock; protected function setUp() { parent::setUp(); - $this->_connectionMock = new ConnectionMock(array(), new \Doctrine\Tests\Mocks\DriverMock()); + $this->_connectionMock = new ConnectionMock(array(), new DriverMock()); $this->_emMock = EntityManagerMock::create($this->_connectionMock); // SUT $this->_unitOfWork = new UnitOfWorkMock($this->_emMock); @@ -49,9 +69,9 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase public function testSavingSingleEntityWithIdentityColumnForcesInsert() { // Setup fake persister and id generator for identity generation - $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\Models\Forum\ForumUser")); + $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumUser')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\Models\Forum\ForumUser', $userPersister); - $userPersister->setMockIdGeneratorType(\Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_IDENTITY); + $userPersister->setMockIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); // Test $user = new ForumUser(); @@ -89,13 +109,13 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase { // Setup fake persister and id generator for identity generation //ForumUser - $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\Models\Forum\ForumUser")); + $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumUser')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\Models\Forum\ForumUser', $userPersister); - $userPersister->setMockIdGeneratorType(\Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_IDENTITY); + $userPersister->setMockIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); // ForumAvatar - $avatarPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\Models\Forum\ForumAvatar")); + $avatarPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumAvatar')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\Models\Forum\ForumAvatar', $avatarPersister); - $avatarPersister->setMockIdGeneratorType(\Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_IDENTITY); + $avatarPersister->setMockIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); // Test $user = new ForumUser(); @@ -120,9 +140,9 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase public function testChangeTrackingNotify() { - $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\ORM\NotifyChangedEntity")); + $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\ORM\NotifyChangedEntity')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\ORM\NotifyChangedEntity', $persister); - $itemPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\ORM\NotifyChangedRelatedItem")); + $itemPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\ORM\NotifyChangedRelatedItem')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\ORM\NotifyChangedRelatedItem', $itemPersister); $entity = new NotifyChangedEntity; @@ -164,7 +184,7 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase public function testGetEntityStateOnVersionedEntityWithAssignedIdentifier() { - $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\ORM\VersionedAssignedIdentifierEntity")); + $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\ORM\VersionedAssignedIdentifierEntity')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\ORM\VersionedAssignedIdentifierEntity', $persister); $e = new VersionedAssignedIdentifierEntity(); @@ -175,7 +195,7 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase public function testGetEntityStateWithAssignedIdentity() { - $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\Models\CMS\CmsPhonenumber")); + $persister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\Models\CMS\CmsPhonenumber')); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\Models\CMS\CmsPhonenumber', $persister); $ph = new \Doctrine\Tests\Models\CMS\CmsPhonenumber(); @@ -197,13 +217,13 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase } /** - * DDC-2086 [GH-484] Prevented "Undefined index" notice when updating. + * DDC-2086 [GH-484] Prevented 'Undefined index' notice when updating. */ public function testNoUndefinedIndexNoticeOnScheduleForUpdateWithoutChanges() { // Setup fake persister and id generator - $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata("Doctrine\Tests\Models\Forum\ForumUser")); - $userPersister->setMockIdGeneratorType(\Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_IDENTITY); + $userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumUser')); + $userPersister->setMockIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); $this->_unitOfWork->setEntityPersister('Doctrine\Tests\Models\Forum\ForumUser', $userPersister); // Create a test user @@ -227,12 +247,83 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase $this->setExpectedException('InvalidArgumentException'); $this->_unitOfWork->lock(null, null, null); } + + /** + * @group DDC-3490 + * + * @dataProvider invalidAssociationValuesDataProvider + * + * @param mixed $invalidValue + */ + public function testRejectsPersistenceOfObjectsWithInvalidAssociationValue($invalidValue) + { + $this->_unitOfWork->setEntityPersister( + 'Doctrine\Tests\Models\Forum\ForumUser', + new EntityPersisterMock( + $this->_emMock, + $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumUser') + ) + ); + + $user = new ForumUser(); + $user->username = 'John'; + $user->avatar = $invalidValue; + + $this->setExpectedException('Doctrine\ORM\ORMInvalidArgumentException'); + + $this->_unitOfWork->persist($user); + } + + /** + * @group DDC-3490 + * + * @dataProvider invalidAssociationValuesDataProvider + * + * @param mixed $invalidValue + */ + public function testRejectsChangeSetComputationForObjectsWithInvalidAssociationValue($invalidValue) + { + $metadata = $this->_emMock->getClassMetadata('Doctrine\Tests\Models\Forum\ForumUser'); + + $this->_unitOfWork->setEntityPersister( + 'Doctrine\Tests\Models\Forum\ForumUser', + new EntityPersisterMock($this->_emMock, $metadata) + ); + + $user = new ForumUser(); + + $this->_unitOfWork->persist($user); + + $user->username = 'John'; + $user->avatar = $invalidValue; + + $this->setExpectedException('Doctrine\ORM\ORMInvalidArgumentException'); + + $this->_unitOfWork->computeChangeSet($metadata, $user); + } + + /** + * Data Provider + * + * @return mixed[][] + */ + public function invalidAssociationValuesDataProvider() + { + return [ + ['foo'], + [['foo']], + [''], + [[]], + [new stdClass()], + [new ArrayCollection()], + ]; + } } /** * @Entity */ -class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged +class NotifyChangedEntity implements NotifyPropertyChanged { private $_listeners = array(); /** @@ -252,7 +343,7 @@ class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged private $items; public function __construct() { - $this->items = new \Doctrine\Common\Collections\ArrayCollection; + $this->items = new ArrayCollection; } public function getId() { @@ -281,7 +372,7 @@ class NotifyChangedEntity implements \Doctrine\Common\NotifyPropertyChanged } } - public function addPropertyChangedListener(\Doctrine\Common\PropertyChangedListener $listener) + public function addPropertyChangedListener(PropertyChangedListener $listener) { $this->_listeners[] = $listener; }