From cb28bfd4848e8d6dcf5b682e3ace624a8d69fa96 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 15 Oct 2011 17:38:55 +0200 Subject: [PATCH 1/6] Improve Error Messages in ClassMetadata and UnitOfWork --- lib/Doctrine/ORM/Mapping/ClassMetadata.php | 19 +++++++++++++++++ lib/Doctrine/ORM/Mapping/MappingException.php | 5 +++++ lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- .../ORM/Mapping/AbstractMappingDriverTest.php | 6 +++++- .../Mapping/BasicInheritanceMappingTest.php | 1 + .../ORM/Mapping/ClassMetadataFactoryTest.php | 4 ++-- .../Tests/ORM/Mapping/ClassMetadataTest.php | 4 ++-- .../AbstractClassMetadataExporterTest.php | 21 +++++++++++++------ 8 files changed, 51 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadata.php b/lib/Doctrine/ORM/Mapping/ClassMetadata.php index 9caa79e11..2c9ec63b7 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadata.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadata.php @@ -122,6 +122,25 @@ class ClassMetadata extends ClassMetadataInfo $this->reflFields[$mapping['fieldName']] = $refProp; } + /** + * Validates & completes the basic mapping information that is common to all + * association mappings (one-to-one, many-ot-one, one-to-many, many-to-many). + * + * @param array $mapping The mapping. + * @return array The updated mapping. + * @throws MappingException If something is wrong with the mapping. + */ + protected function _validateAndCompleteAssociationMapping(array $mapping) + { + $mapping = parent::_validateAndCompleteAssociationMapping($mapping); + + if ( ! class_exists($mapping['targetEntity']) ) { + #throw MappingException::invalidTargetEntityClass($mapping['targetEntity'], $this->name, $mapping['fieldName']); + } + + return $mapping; + } + /** * Extracts the identifier values of an entity of this class. * diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 97fbf94f6..a359081c8 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -298,4 +298,9 @@ class MappingException extends \Doctrine\ORM\ORMException { return new self("Entity '" . $className . "' has no method '" . $methodName . "' to be registered as lifecycle callback."); } + + public static function invalidTargetEntityClass($targetEntity, $sourceEntity, $associationName) + { + return new self("The target-entity " . $targetEntity . " cannot be found in '" . $sourceEntity."#".$associationName."'."); + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index f53ec67a7..8af2a7337 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -594,8 +594,8 @@ class UnitOfWork implements PropertyChangedListener throw new InvalidArgumentException("A new entity was found through the relationship '" . $assoc['sourceEntity'] . "#" . $assoc['fieldName'] . "' that was not" . " configured to cascade persist operations for entity: " . self::objToStr($entry) . "." - . " Explicitly persist the new entity or configure cascading persist operations" - . " on the relationship. If you cannot find out which entity causes the problem" + . " Explicitly call EntityManager#persist() on this entity or configure to cascade persist " + . " the association. If you cannot find out which entity causes the problem" . " implement '" . $assoc['targetEntity'] . "#__toString()' to get a clue."); } $this->persistNew($targetClass, $entry); diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index 98c43a5b9..05498c1d5 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -551,4 +551,8 @@ class Dog extends Animal { } -} \ No newline at end of file +} + +class Address {} +class Phonenumber {} +class Group {} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php index 93504ec53..b4ed5b34c 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php @@ -197,6 +197,7 @@ class MappedSuperclassBase { private $mappedRelated1; private $transient; } +class MappedSuperclassRelated1 {} /** @Entity */ class EntitySubClass2 extends MappedSuperclassBase { diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index dbb82f054..868dac7cd 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -32,12 +32,12 @@ class ClassMetadataFactoryTest extends \Doctrine\Tests\OrmTestCase // Add a mapped field $cm1->mapField(array('fieldName' => 'id', 'type' => 'integer', 'id' => true)); // and a mapped association - $cm1->mapOneToOne(array('fieldName' => 'other', 'targetEntity' => 'Other', 'mappedBy' => 'this')); + $cm1->mapOneToOne(array('fieldName' => 'other', 'targetEntity' => 'TestEntity1', 'mappedBy' => 'this')); // and an association on the owning side $joinColumns = array( array('name' => 'other_id', 'referencedColumnName' => 'id') ); - $cm1->mapOneToOne(array('fieldName' => 'association', 'targetEntity' => 'Other', 'joinColumns' => $joinColumns)); + $cm1->mapOneToOne(array('fieldName' => 'association', 'targetEntity' => 'TestEntity1', 'joinColumns' => $joinColumns)); // and an id generator type $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO); diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php index fed31d9c5..7a36f808a 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php @@ -29,7 +29,7 @@ class ClassMetadataTest extends \Doctrine\Tests\OrmTestCase $cm->setParentClasses(array("UserParent")); $cm->setCustomRepositoryClass("UserRepository"); $cm->setDiscriminatorColumn(array('name' => 'disc', 'type' => 'integer')); - $cm->mapOneToOne(array('fieldName' => 'phonenumbers', 'targetEntity' => 'Bar', 'mappedBy' => 'foo')); + $cm->mapOneToOne(array('fieldName' => 'phonenumbers', 'targetEntity' => 'CmsAddress', 'mappedBy' => 'foo')); $cm->markReadOnly(); $cm->addNamedQuery(array('name' => 'dql', 'query' => 'foo')); $this->assertEquals(1, count($cm->associationMappings)); @@ -52,7 +52,7 @@ class ClassMetadataTest extends \Doctrine\Tests\OrmTestCase $oneOneMapping = $cm->getAssociationMapping('phonenumbers'); $this->assertTrue($oneOneMapping['fetch'] == ClassMetadata::FETCH_LAZY); $this->assertEquals('phonenumbers', $oneOneMapping['fieldName']); - $this->assertEquals('Doctrine\Tests\Models\CMS\Bar', $oneOneMapping['targetEntity']); + $this->assertEquals('Doctrine\Tests\Models\CMS\CmsAddress', $oneOneMapping['targetEntity']); $this->assertTrue($cm->isReadOnly); $this->assertEquals(array('dql' => 'foo'), $cm->namedQueries); } diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php index c9bfdccf0..8e263ba40 100644 --- a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php @@ -98,6 +98,8 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest public function testExportDirectoryAndFilesAreCreated() { + $this->_deleteDirectory(__DIR__ . '/export/'.$this->_getType()); + $type = $this->_getType(); $metadataDriver = $this->_createMetadataDriver($type, __DIR__ . '/' . $type); $em = $this->_createEntityManager($metadataDriver); @@ -320,12 +322,6 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest $this->assertEquals('user', $class->associationMappings['address']['inversedBy']); } - public function __destruct() - { - $type = $this->_getType(); - $this->_deleteDirectory(__DIR__ . '/export/'.$this->_getType()); - } - protected function _deleteDirectory($path) { if (is_file($path)) { @@ -338,4 +334,17 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest return rmdir($path); } } +} + +class Address +{ + +} +class Phonenumber +{ + +} +class Group +{ + } \ No newline at end of file From 3aea203b9ca77df65f55f036080a9af653194cbf Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 22 Oct 2011 11:28:07 +0200 Subject: [PATCH 2/6] Throw exception if target entity is not found. --- lib/Doctrine/ORM/Mapping/ClassMetadata.php | 4 ++-- .../Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadata.php b/lib/Doctrine/ORM/Mapping/ClassMetadata.php index 2c9ec63b7..b03a6c7b3 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadata.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadata.php @@ -134,8 +134,8 @@ class ClassMetadata extends ClassMetadataInfo { $mapping = parent::_validateAndCompleteAssociationMapping($mapping); - if ( ! class_exists($mapping['targetEntity']) ) { - #throw MappingException::invalidTargetEntityClass($mapping['targetEntity'], $this->name, $mapping['fieldName']); + if ( ! \Doctrine\Common\ClassLoader::classExists($mapping['targetEntity']) ) { + throw MappingException::invalidTargetEntityClass($mapping['targetEntity'], $this->name, $mapping['fieldName']); } return $mapping; diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php index 7a36f808a..bc3268344 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php @@ -482,4 +482,15 @@ class ClassMetadataTest extends \Doctrine\Tests\OrmTestCase $this->setExpectedException("Doctrine\ORM\Mapping\MappingException", "Entity 'Doctrine\Tests\Models\CMS\CmsUser' has no method 'notfound' to be registered as lifecycle callback."); $cm->addLifecycleCallback('notfound', 'postLoad'); } + + /** + * @group ImproveErrorMessages + */ + public function testTargetEntityNotFound() + { + $cm = new ClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'); + + $this->setExpectedException("Doctrine\ORM\Mapping\MappingException", "The target-entity Doctrine\Tests\Models\CMS\UnknownClass cannot be found in 'Doctrine\Tests\Models\CMS\CmsUser#address'."); + $cm->mapManyToOne(array('fieldName' => 'address', 'targetEntity' => 'UnknownClass')); + } } From 5392737de422f1d68a9e135eb35985ba4b76b4ee Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 22 Oct 2011 12:40:12 +0200 Subject: [PATCH 3/6] Improved and extracted UnitOfWork error messages --- .../ORM/ORMInvalidArgumentException.php | 81 +++++++++++++++++++ lib/Doctrine/ORM/UnitOfWork.php | 33 +++----- .../Functional/UnitOfWorkLifecycleTest.php | 71 ++++++++++++++++ 3 files changed, 164 insertions(+), 21 deletions(-) create mode 100644 lib/Doctrine/ORM/ORMInvalidArgumentException.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/UnitOfWorkLifecycleTest.php diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php new file mode 100644 index 000000000..b4edc5dd2 --- /dev/null +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -0,0 +1,81 @@ +. + */ + +namespace Doctrine\ORM; + +class ORMInvalidArgumentException extends \InvalidArgumentException +{ + static public function scheduleInsertForManagedEntity($entity) + { + return new self("A managed+dirty entity " . self::objToStr($entity) . " can not be scheduled for insertion."); + } + + static public function scheduleInsertForRemovedEntity($entity) + { + return new self("Removed entity " . self::objToStr($entity) . " can not be scheduled for insertion."); + } + + static public function scheduleInsertTwice($entity) + { + return new self("Entity " . self::objToStr($entity) . " can not be scheduled for insertion twice."); + } + + static public function entityWithoutIdentity($className, $entity) + { + throw new self( + "The given entity of type '" . $className . "' (".self::objToStr($entity).") has no identity/no " . + "id values set. It cannot be added to the identity map." + ); + } + + static public function readOnlyRequiresManagedEntity($entity) + { + return new self("Only managed entities can be marked or checked as read only. But " . self::objToStr($entity) . " is not"); + } + + static public function newEntityFoundThroughRelationship(array $assoc, $entry) + { + return new self("A new entity was found through the relationship '" + . $assoc['sourceEntity'] . "#" . $assoc['fieldName'] . "' that was not" + . " configured to cascade persist operations for entity: " . self::objToStr($entry) . "." + . " To solve this issue: Either explicitly call EntityManager#persist()" + . " on this unknown entity or configure cascade persist " + . " this association in the mapping for example @ManyToOne(..,cascade={\"persist\"}). " + . " If you cannot find out which entity causes the problem" + . " implement '" . $assoc['targetEntity'] . "#__toString()' to get a clue."); + } + + static public function detachedEntityFoundThroughRelationship(array $assoc, $entry) + { + throw new self("A detached entity of type " . $assoc['targetEntity'] . " (" . self::objToStr($entry) . ") " + . " was found through the relationship '" . $assoc['sourceEntity'] . "#" . $assoc['fieldName'] . "' " + . "during cascading a persist operation."); + } + + /** + * Helper method to show an object as string. + * + * @param object $obj + * @return string + */ + private static function objToStr($obj) + { + return method_exists($obj, '__toString') ? (string)$obj : get_class($obj).'@'.spl_object_hash($obj); + } +} \ No newline at end of file diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 1e6d13fb5..2b696063f 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -595,25 +595,16 @@ class UnitOfWork implements PropertyChangedListener $oid = spl_object_hash($entry); if ($state == self::STATE_NEW) { if ( ! $assoc['isCascadePersist']) { - throw new InvalidArgumentException("A new entity was found through the relationship '" - . $assoc['sourceEntity'] . "#" . $assoc['fieldName'] . "' that was not" - . " configured to cascade persist operations for entity: " . self::objToStr($entry) . "." - . " Explicitly call EntityManager#persist() on this entity or configure to cascade persist " - . " the association. If you cannot find out which entity causes the problem" - . " implement '" . $assoc['targetEntity'] . "#__toString()' to get a clue."); + throw ORMInvalidArgumentException::newEntityFoundThroughRelationship($assoc, $entry); } $this->persistNew($targetClass, $entry); $this->computeChangeSet($targetClass, $entry); - } else if ($state == self::STATE_REMOVED) { - return new InvalidArgumentException("Removed entity detected during flush: " - . self::objToStr($entry).". Remove deleted entities from associations."); } else if ($state == self::STATE_DETACHED) { // Can actually not happen right now as we assume STATE_NEW, // so the exception will be raised from the DBAL layer (constraint violation). - throw new InvalidArgumentException("A detached entity was found through a " - . "relationship during cascading a persist operation."); + throw ORMInvalidArgumentException::detachedEntityFoundThroughRelationship($assoc, $entry); } - // MANAGED associated entities are already taken into account + // MANAGED and REMOVED associated entities are already taken into account // during changeset calculation anyway, since they are in the identity map. } } @@ -662,7 +653,7 @@ class UnitOfWork implements PropertyChangedListener $oid = spl_object_hash($entity); if ( ! isset($this->entityStates[$oid]) || $this->entityStates[$oid] != self::STATE_MANAGED) { - throw new InvalidArgumentException('Entity must be managed.'); + throw new InvalidArgumentException("Entity of type '" . $class->name . "' must be managed."); } /* TODO: Just return if changetracking policy is NOTIFY? @@ -907,14 +898,14 @@ class UnitOfWork implements PropertyChangedListener { $oid = spl_object_hash($entity); - if (isset($this->entityUpdates[$oid])) { - throw new InvalidArgumentException("Dirty entity can not be scheduled for insertion."); - } if (isset($this->entityDeletions[$oid])) { - throw new InvalidArgumentException("Removed entity can not be scheduled for insertion."); + throw ORMInvalidArgumentException::scheduleInsertForRemovedEntity($entity); + } + if (isset($this->originalEntityData[$oid]) && ! isset($this->entityInsertions[$oid])) { + throw ORMInvalidArgumentException::scheduleInsertForManagedEntity($entity); } if (isset($this->entityInsertions[$oid])) { - throw new InvalidArgumentException("Entity can not be scheduled for insertion twice."); + throw ORMInvalidArgumentException::scheduleInsertTwice($entity); } $this->entityInsertions[$oid] = $entity; @@ -1071,7 +1062,7 @@ class UnitOfWork implements PropertyChangedListener $classMetadata = $this->em->getClassMetadata(get_class($entity)); $idHash = implode(' ', $this->entityIdentifiers[spl_object_hash($entity)]); if ($idHash === '') { - throw new InvalidArgumentException("The given entity has no identity."); + throw ORMInvalidArgumentException::entityWithoutIdentity($classMetadata->name, $entity); } $className = $classMetadata->rootEntityName; if (isset($this->identityMap[$className][$idHash])) { @@ -2466,7 +2457,7 @@ class UnitOfWork implements PropertyChangedListener public function markReadOnly($object) { if ( ! is_object($object) || ! $this->isInIdentityMap($object)) { - throw new InvalidArgumentException("Managed entity required"); + throw ORMInvalidArgumentException::readOnlyRequiresManagedEntity($object); } $this->readOnlyObjects[spl_object_hash($object)] = true; } @@ -2481,7 +2472,7 @@ class UnitOfWork implements PropertyChangedListener public function isReadOnly($object) { if ( ! is_object($object) ) { - throw new InvalidArgumentException("Managed entity required"); + throw ORMInvalidArgumentException::readOnlyRequiresManagedEntity($object); } return isset($this->readOnlyObjects[spl_object_hash($object)]); } diff --git a/tests/Doctrine/Tests/ORM/Functional/UnitOfWorkLifecycleTest.php b/tests/Doctrine/Tests/ORM/Functional/UnitOfWorkLifecycleTest.php new file mode 100644 index 000000000..08e3720ca --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/UnitOfWorkLifecycleTest.php @@ -0,0 +1,71 @@ +useModelSet('cms'); + parent::setUp(); + } + + public function testScheduleInsertManaged() + { + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin"; + $user->status = "active"; + $this->_em->persist($user); + $this->_em->flush(); + + $this->setExpectedException("Doctrine\ORM\ORMInvalidArgumentException", "A managed+dirty entity Doctrine\Tests\Models\CMS\CmsUser"); + $this->_em->getUnitOfWork()->scheduleForInsert($user); + } + + public function testScheduleInsertDeleted() + { + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin"; + $user->status = "active"; + $this->_em->persist($user); + $this->_em->flush(); + + $this->_em->remove($user); + + $this->setExpectedException("Doctrine\ORM\ORMInvalidArgumentException", "Removed entity Doctrine\Tests\Models\CMS\CmsUser"); + $this->_em->getUnitOfWork()->scheduleForInsert($user); + } + + public function testScheduleInsertTwice() + { + $user = new CmsUser(); + $user->username = "beberlei"; + $user->name = "Benjamin"; + $user->status = "active"; + + $this->_em->getUnitOfWork()->scheduleForInsert($user); + + $this->setExpectedException("Doctrine\ORM\ORMInvalidArgumentException", "Entity Doctrine\Tests\Models\CMS\CmsUser"); + $this->_em->getUnitOfWork()->scheduleForInsert($user); + } + + public function testAddToIdentityMapWithoutIdentity() + { + $user = new CmsUser(); + + $this->setExpectedException("Doctrine\ORM\ORMInvalidArgumentException", "The given entity of type 'Doctrine\Tests\Models\CMS\CmsUser' (Doctrine\Tests\Models\CMS\CmsUser@"); + $this->_em->getUnitOfWork()->registerManaged($user, array(), array()); + } + + public function testMarkReadOnlyNonManaged() + { + $user = new CmsUser(); + + $this->setExpectedException("Doctrine\ORM\ORMInvalidArgumentException", "Only managed entities can be marked or checked as read only. But Doctrine\Tests\Models\CMS\CmsUser@"); + $this->_em->getUnitOfWork()->markReadOnly($user); + } +} \ No newline at end of file From a8906ce572a7b2ac69aeb02d66581dfdded49f13 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 22 Oct 2011 12:49:33 +0200 Subject: [PATCH 4/6] Stringify entity in all UnitOfWork exceptions --- .../ORM/ORMInvalidArgumentException.php | 6 +++++ lib/Doctrine/ORM/UnitOfWork.php | 23 ++++++++++--------- .../Tests/ORM/Functional/Locking/LockTest.php | 2 +- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php index b4edc5dd2..a4a03c41a 100644 --- a/lib/Doctrine/ORM/ORMInvalidArgumentException.php +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -68,6 +68,12 @@ class ORMInvalidArgumentException extends \InvalidArgumentException . "during cascading a persist operation."); } + static public function entityNotManaged($entity) + { + throw new self("Entity " . self::objToStr($entity) . " is not managed. An entity is managed if its fetched " . + "from the database or registered as new through EntityManager#persist"); + } + /** * Helper method to show an object as string. * diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2b696063f..a80079041 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -653,7 +653,7 @@ class UnitOfWork implements PropertyChangedListener $oid = spl_object_hash($entity); if ( ! isset($this->entityStates[$oid]) || $this->entityStates[$oid] != self::STATE_MANAGED) { - throw new InvalidArgumentException("Entity of type '" . $class->name . "' must be managed."); + throw ORMInvalidArgumentException::entityNotManaged($entity); } /* TODO: Just return if changetracking policy is NOTIFY? @@ -935,10 +935,10 @@ class UnitOfWork implements PropertyChangedListener { $oid = spl_object_hash($entity); if ( ! isset($this->entityIdentifiers[$oid])) { - throw new InvalidArgumentException("Entity has no identity."); + throw new InvalidArgumentException("Entity has no identity." . self::objToStr($entity)); } if (isset($this->entityDeletions[$oid])) { - throw new InvalidArgumentException("Entity is removed."); + throw new InvalidArgumentException("Entity is removed." . self::objToStr($entity)); } if ( ! isset($this->entityUpdates[$oid]) && ! isset($this->entityInsertions[$oid])) { @@ -1160,7 +1160,7 @@ class UnitOfWork implements PropertyChangedListener $classMetadata = $this->em->getClassMetadata(get_class($entity)); $idHash = implode(' ', $this->entityIdentifiers[$oid]); if ($idHash === '') { - throw new InvalidArgumentException("The given entity has no identity."); + throw new InvalidArgumentException("The given entity has no identity." . self::objToStr($entity)); } $className = $classMetadata->rootEntityName; if (isset($this->identityMap[$className][$idHash])) { @@ -1291,9 +1291,9 @@ class UnitOfWork implements PropertyChangedListener break; case self::STATE_DETACHED: // Can actually not happen right now since we assume STATE_NEW. - throw new InvalidArgumentException("Detached entity passed to persist()."); + throw new InvalidArgumentException("Detached entity passed to persist()." . self::objToStr($entity)); default: - throw new UnexpectedValueException("Unexpected entity state: $entityState."); + throw new UnexpectedValueException("Unexpected entity state: $entityState." . self::objToStr($entity)); } $this->cascadePersist($entity, $visited); @@ -1350,9 +1350,9 @@ class UnitOfWork implements PropertyChangedListener $this->scheduleForDelete($entity); break; case self::STATE_DETACHED: - throw new InvalidArgumentException("A detached entity can not be removed."); + throw new InvalidArgumentException("A detached entity can not be removed." . self::objToStr($entity)); default: - throw new UnexpectedValueException("Unexpected entity state: $entityState."); + throw new UnexpectedValueException("Unexpected entity state: $entityState." . self::objToStr($entity)); } } @@ -1417,7 +1417,8 @@ class UnitOfWork implements PropertyChangedListener if ($managedCopy) { // We have the entity in-memory already, just make sure its not removed. if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { - throw new InvalidArgumentException('Removed entity detected during merge.' + throw new InvalidArgumentException( + 'Removed entity ' . self::objToStr($managedCopy) . ' detected during merge.' . ' Can not merge with a removed entity.'); } } else { @@ -1624,7 +1625,7 @@ class UnitOfWork implements PropertyChangedListener $entity ); } else { - throw new InvalidArgumentException("Entity is not MANAGED."); + throw ORMInvalidArgumentException::entityNotManaged($entity); } $this->cascadeRefresh($entity, $visited); @@ -1789,7 +1790,7 @@ class UnitOfWork implements PropertyChangedListener public function lock($entity, $lockMode, $lockVersion = null) { if ($this->getEntityState($entity, self::STATE_DETACHED) != self::STATE_MANAGED) { - throw new InvalidArgumentException("Entity is not MANAGED."); + throw ORMInvalidArgumentException::entityNotManaged($entity); } $entityName = get_class($entity); diff --git a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php index 6917e7252..62c787409 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/Locking/LockTest.php @@ -71,7 +71,7 @@ class LockTest extends \Doctrine\Tests\OrmFunctionalTestCase { public function testLockUnmanagedEntity_ThrowsException() { $article = new CmsArticle(); - $this->setExpectedException('InvalidArgumentException', 'Entity is not MANAGED.'); + $this->setExpectedException('InvalidArgumentException', 'Entity Doctrine\Tests\Models\CMS\CmsArticle'); $this->_em->lock($article, LockMode::OPTIMISTIC, $article->version + 1); } From 719e05e53e9e376c7d4138a1d20070ece2cfc57b Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 22 Oct 2011 12:57:55 +0200 Subject: [PATCH 5/6] Extract more messages into ORMInvalidArgumentException --- .../ORM/ORMInvalidArgumentException.php | 20 +++++++++++++++++++ lib/Doctrine/ORM/UnitOfWork.php | 14 ++++++------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php index a4a03c41a..878ee4b71 100644 --- a/lib/Doctrine/ORM/ORMInvalidArgumentException.php +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -19,6 +19,11 @@ namespace Doctrine\ORM; +/** + * Contains exception messages for all invalid lifecycle state exceptions inside UnitOfWork + * + * @author Benjamin Eberlei + */ class ORMInvalidArgumentException extends \InvalidArgumentException { static public function scheduleInsertForManagedEntity($entity) @@ -74,6 +79,21 @@ class ORMInvalidArgumentException extends \InvalidArgumentException "from the database or registered as new through EntityManager#persist"); } + static public function entityHasNoIdentity($entity, $operation) + { + throw new self("Entity has no identity, therefore " . $operation ." cannot be performed. " . self::objToStr($entity)); + } + + static public function entityIsRemoved($entity, $operation) + { + throw new self("Entity is removed, therefore " . $operation ." cannot be performed. " . self::objToStr($entity)); + } + + static public function detachedEntityCannot($entity, $operation) + { + throw new self("A detached entity was found during " . $operation . " " . self::objToStr($entity)); + } + /** * Helper method to show an object as string. * diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index a80079041..e4168a1a0 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -935,10 +935,10 @@ class UnitOfWork implements PropertyChangedListener { $oid = spl_object_hash($entity); if ( ! isset($this->entityIdentifiers[$oid])) { - throw new InvalidArgumentException("Entity has no identity." . self::objToStr($entity)); + throw ORMInvalidArgumentException::entityHasNoIdentity($entity, "scheduling for update"); } if (isset($this->entityDeletions[$oid])) { - throw new InvalidArgumentException("Entity is removed." . self::objToStr($entity)); + throw ORMInvalidArgumentException::entityIsRemoved($entity, "schedule for update"); } if ( ! isset($this->entityUpdates[$oid]) && ! isset($this->entityInsertions[$oid])) { @@ -1160,7 +1160,7 @@ class UnitOfWork implements PropertyChangedListener $classMetadata = $this->em->getClassMetadata(get_class($entity)); $idHash = implode(' ', $this->entityIdentifiers[$oid]); if ($idHash === '') { - throw new InvalidArgumentException("The given entity has no identity." . self::objToStr($entity)); + throw ORMInvalidArgumentException::entityHasNoIdentity($entity, "remove from identity map"); } $className = $classMetadata->rootEntityName; if (isset($this->identityMap[$className][$idHash])) { @@ -1291,7 +1291,7 @@ class UnitOfWork implements PropertyChangedListener break; case self::STATE_DETACHED: // Can actually not happen right now since we assume STATE_NEW. - throw new InvalidArgumentException("Detached entity passed to persist()." . self::objToStr($entity)); + throw ORMInvalidArgumentException::detachedEntityCannot($entity, "persisted"); default: throw new UnexpectedValueException("Unexpected entity state: $entityState." . self::objToStr($entity)); } @@ -1350,7 +1350,7 @@ class UnitOfWork implements PropertyChangedListener $this->scheduleForDelete($entity); break; case self::STATE_DETACHED: - throw new InvalidArgumentException("A detached entity can not be removed." . self::objToStr($entity)); + throw ORMInvalidArgumentException::detachedEntityCannot($entity, "removed"); default: throw new UnexpectedValueException("Unexpected entity state: $entityState." . self::objToStr($entity)); } @@ -1417,9 +1417,7 @@ class UnitOfWork implements PropertyChangedListener if ($managedCopy) { // We have the entity in-memory already, just make sure its not removed. if ($this->getEntityState($managedCopy) == self::STATE_REMOVED) { - throw new InvalidArgumentException( - 'Removed entity ' . self::objToStr($managedCopy) . ' detected during merge.' - . ' Can not merge with a removed entity.'); + throw ORMInvalidArgumentException::entityIsRemoved($managedCopy, "merge"); } } else { // We need to fetch the managed copy in order to merge. From b91689fe2fb986fc705431d5f0f05a90eb6341a4 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 21 Dec 2011 23:57:33 +0100 Subject: [PATCH 6/6] Update common with fix on interface detection --- lib/vendor/doctrine-common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vendor/doctrine-common b/lib/vendor/doctrine-common index febecd320..18d11e0a5 160000 --- a/lib/vendor/doctrine-common +++ b/lib/vendor/doctrine-common @@ -1 +1 @@ -Subproject commit febecd320261d4a49a83b0c51286182cba1fa587 +Subproject commit 18d11e0a54f8c4e726940a3753e3c2949dbf1c02