From 8272ffd23fd66bac13a0dc074c868a00b82c7707 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 20 Oct 2012 01:27:53 +0200 Subject: [PATCH 1/3] Proxy generation as of doctrine/common#168 - DCOM-96 --- .gitignore | 3 +- lib/Doctrine/ORM/Id/AssignedGenerator.php | 2 +- .../ORM/Mapping/ClassMetadataInfo.php | 11 +- lib/Doctrine/ORM/PersistentCollection.php | 7 +- lib/Doctrine/ORM/Proxy/Autoloader.php | 77 +-- lib/Doctrine/ORM/Proxy/Proxy.php | 2 +- lib/Doctrine/ORM/Proxy/ProxyException.php | 69 --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 465 ++++++------------ .../ORM/Tools/DebugUnitOfWorkListener.php | 2 +- lib/Doctrine/ORM/UnitOfWork.php | 36 +- lib/vendor/doctrine-common | 2 +- .../Functional/ProxiesLikeEntitiesTest.php | 34 +- .../ORM/Performance/ProxyPerformanceTest.php | 165 +++++++ .../Tests/ORM/Proxy/AutoloaderTest.php | 62 --- .../Tests/ORM/Proxy/ProxtFactoryTest.php | 89 ++++ .../ORM/Proxy/ProxyClassGeneratorTest.php | 204 -------- .../Proxy/fixtures/NonNamespacedProxies.php | 13 - 17 files changed, 474 insertions(+), 769 deletions(-) delete mode 100644 lib/Doctrine/ORM/Proxy/ProxyException.php create mode 100644 tests/Doctrine/Tests/ORM/Performance/ProxyPerformanceTest.php delete mode 100644 tests/Doctrine/Tests/ORM/Proxy/AutoloaderTest.php create mode 100644 tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php delete mode 100644 tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php delete mode 100644 tests/Doctrine/Tests/ORM/Proxy/fixtures/NonNamespacedProxies.php diff --git a/.gitignore b/.gitignore index b49fac4bf..8c613c8e2 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ lib/Doctrine/Common lib/Doctrine/DBAL /.settings/ .buildpath -.project \ No newline at end of file +.project +.idea diff --git a/lib/Doctrine/ORM/Id/AssignedGenerator.php b/lib/Doctrine/ORM/Id/AssignedGenerator.php index 853ff3bbd..121cc8fd0 100644 --- a/lib/Doctrine/ORM/Id/AssignedGenerator.php +++ b/lib/Doctrine/ORM/Id/AssignedGenerator.php @@ -52,7 +52,7 @@ class AssignedGenerator extends AbstractIdGenerator $identifier = array(); foreach ($idFields as $idField) { - $value = $class->reflFields[$idField]->getValue($entity); + $value = $class->getFieldValue($entity, $idField); if ( ! isset($value)) { throw ORMException::entityMissingAssignedIdForField($entity, $idField); diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 4cb748bbe..fc808d3e5 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -605,7 +605,7 @@ class ClassMetadataInfo implements ClassMetadata /** * The ReflectionProperty instances of the mapped class. * - * @var array + * @var \ReflectionProperty[] */ public $reflFields = array(); @@ -693,13 +693,14 @@ class ClassMetadataInfo implements ClassMetadata return $id; } - $value = $this->reflFields[$this->identifier[0]]->getValue($entity); + $id = $this->identifier[0]; + $value = $this->reflFields[$id]->getValue($entity); - if ($value !== null) { - return array($this->identifier[0] => $value); + if (null === $value) { + return array(); } - return array(); + return array($id => $value); } /** diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index a7d4abb3f..dbfac3716 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -853,9 +853,10 @@ final class PersistentCollection implements Collection, Selectable $newObjects = $this->coll->matching($criteria)->toArray(); } - $targetClass = $this->em->getClassMetadata(get_class($this->owner)); - - $id = $targetClass->getSingleIdReflectionProperty()->getValue($this->owner); + $id = $this->em + ->getClassMetadata(get_class($this->owner)) + ->getSingleIdReflectionProperty() + ->getValue($this->owner); $builder = Criteria::expr(); $ownerExpression = $builder->eq($this->backRefFieldName, $id); $expression = $criteria->getWhereExpression(); diff --git a/lib/Doctrine/ORM/Proxy/Autoloader.php b/lib/Doctrine/ORM/Proxy/Autoloader.php index d495ef89a..9b2e2cdfa 100644 --- a/lib/Doctrine/ORM/Proxy/Autoloader.php +++ b/lib/Doctrine/ORM/Proxy/Autoloader.php @@ -19,82 +19,11 @@ namespace Doctrine\ORM\Proxy; -use Doctrine\ORM\Configuration; -use Closure; +use Doctrine\Common\Proxy\Autoloader as BaseAutoloader; /** - * Special Autoloader for Proxy classes because them not being PSR-0 compatible. - * - * @author Benjamin Eberlei + * @deprecated use \Doctrine\Common\Proxy\Autoloader instead */ -class Autoloader +class Autoloader extends BaseAutoloader { - /** - * Resolves proxy class name to a filename based on the following pattern. - * - * 1. Remove Proxy namespace from class name - * 2. Remove namespace seperators from remaining class name. - * 3. Return PHP filename from proxy-dir with the result from 2. - * - * @param string $proxyDir - * @param string $proxyNamespace - * @param string $className - * - * @return string - * - * @throws ProxyException - */ - static public function resolveFile($proxyDir, $proxyNamespace, $className) - { - if (0 !== strpos($className, $proxyNamespace)) { - throw ProxyException::notProxyClass($className, $proxyNamespace); - } - - $className = str_replace('\\', '', substr($className, strlen($proxyNamespace) +1)); - return $proxyDir . DIRECTORY_SEPARATOR . $className.'.php'; - } - - /** - * Registers and returns autoloader callback for the given proxy dir and - * namespace. - * - * @param string $proxyDir - * @param string $proxyNamespace - * @param \Closure $notFoundCallback Invoked when the proxy file is not found. - * - * @return \Closure - */ - static public function register($proxyDir, $proxyNamespace, Closure $notFoundCallback = null) - { - $proxyNamespace = ltrim($proxyNamespace, "\\"); - $autoloader = function($className) use ($proxyDir, $proxyNamespace, $notFoundCallback) { - if (0 === strpos($className, $proxyNamespace)) { - $file = Autoloader::resolveFile($proxyDir, $proxyNamespace, $className); - - if ($notFoundCallback && ! file_exists($file)) { - $notFoundCallback($proxyDir, $proxyNamespace, $className); - } - - require $file; - } - }; - - spl_autoload_register($autoloader, true, true); - - return $autoloader; - } - - /** - * Registers and returns autoloader callback from a Configuration instance - * - * @param Configuration $config - * @param \Closure $notFoundCallback - * - * @return \Closure - */ - static public function registerFromConfiguration(Configuration $configuration, Closure $notFoundCallback) - { - return self::register($configuration->getProxyDir(), $configuration->getProxyNamespace(), $notFoundCallback); - } } - diff --git a/lib/Doctrine/ORM/Proxy/Proxy.php b/lib/Doctrine/ORM/Proxy/Proxy.php index ed588af6d..f478d4905 100644 --- a/lib/Doctrine/ORM/Proxy/Proxy.php +++ b/lib/Doctrine/ORM/Proxy/Proxy.php @@ -19,7 +19,7 @@ namespace Doctrine\ORM\Proxy; -use Doctrine\Common\Persistence\Proxy as BaseProxy; +use Doctrine\Common\Proxy\Proxy as BaseProxy; /** * Interface for proxy classes. diff --git a/lib/Doctrine/ORM/Proxy/ProxyException.php b/lib/Doctrine/ORM/Proxy/ProxyException.php deleted file mode 100644 index 7f28a20e0..000000000 --- a/lib/Doctrine/ORM/Proxy/ProxyException.php +++ /dev/null @@ -1,69 +0,0 @@ -. - */ - -namespace Doctrine\ORM\Proxy; - -/** - * ORM Proxy Exception. - * - * @license http://www.opensource.org/licenses/lgpl-license.php LGPL - * @link www.doctrine-project.com - * @since 1.0 - * @author Benjamin Eberlei - */ -class ProxyException extends \Doctrine\ORM\ORMException -{ - /** - * @return ProxyException - */ - public static function proxyDirectoryRequired() - { - return new self("You must configure a proxy directory. See docs for details"); - } - - /** - * @return ProxyException - */ - public static function proxyDirectoryNotWritable() - { - return new self("Your proxy directory must be writable."); - } - - /** - * @return ProxyException - */ - public static function proxyNamespaceRequired() - { - return new self("You must configure a proxy namespace. See docs for details"); - } - - /** - * @param $className - * @param $proxyNamespace - * - * @return ProxyException - */ - public static function notProxyClass($className, $proxyNamespace) - { - return new self(sprintf( - "The class %s is not part of the proxy namespace %s", - $className, $proxyNamespace - )); - } -} diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 47dd2c28c..de67e2f86 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -20,53 +20,62 @@ namespace Doctrine\ORM\Proxy; use Doctrine\ORM\EntityManager; -use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\EntityNotFoundException; + use Doctrine\Common\Util\ClassUtils; +use Doctrine\Common\Proxy\Proxy; +use Doctrine\Common\Proxy\ProxyGenerator; /** * This factory is used to create proxy objects for entities at runtime. * * @author Roman Borschel * @author Giorgio Sironi + * @author Marco Pivetta * @since 2.0 */ class ProxyFactory { /** - * The EntityManager this factory is bound to. - * - * @var \Doctrine\ORM\EntityManager + * @var EntityManager The EntityManager this factory is bound to. */ - private $_em; + private $em; /** - * Whether to automatically (re)generate proxy classes. - * - * @var bool + * @var \Doctrine\ORM\UnitOfWork The UnitOfWork this factory uses to retrieve persisters */ - private $_autoGenerate; + private $uow; + + /** + * @var ProxyGenerator the proxy generator responsible for creating the proxy classes/files. + */ + private $proxyGenerator; + + /** + * @var bool Whether to automatically (re)generate proxy classes. + */ + private $autoGenerate; /** - * The namespace that contains all proxy classes. - * * @var string */ - private $_proxyNamespace; + private $proxyNs; /** - * The directory that contains all proxy classes. - * * @var string */ - private $_proxyDir; + private $proxyDir; /** - * Used to match very simple id methods that don't need - * to be proxied since the identifier is known. - * - * @var string + * @var array definitions (indexed by requested class name) for the proxy classes. + * Each element is an array containing following items: + * "fqcn" - FQCN of the proxy class + * "initializer" - Closure to be used as proxy __initializer__ + * "cloner" - Closure to be used as proxy __cloner__ + * "identifierFields" - list of field names for the identifiers + * "reflectionFields" - ReflectionProperties for the fields */ - const PATTERN_MATCH_ID_METHOD = '((public\s)?(function\s{1,}%s\s?\(\)\s{1,})\s{0,}{\s{0,}return\s{0,}\$this->%s;\s{0,}})i'; + private $definitions = array(); /** * Initializes a new instance of the ProxyFactory class that is @@ -76,365 +85,195 @@ class ProxyFactory * @param string $proxyDir The directory to use for the proxy classes. It must exist. * @param string $proxyNs The namespace to use for the proxy classes. * @param boolean $autoGenerate Whether to automatically generate proxy classes. - * - * @throws ProxyException */ public function __construct(EntityManager $em, $proxyDir, $proxyNs, $autoGenerate = false) { - if ( ! $proxyDir) { - throw ProxyException::proxyDirectoryRequired(); - } - if ( ! $proxyNs) { - throw ProxyException::proxyNamespaceRequired(); - } - $this->_em = $em; - $this->_proxyDir = $proxyDir; - $this->_autoGenerate = $autoGenerate; - $this->_proxyNamespace = $proxyNs; + $this->em = $em; + $this->uow = $em->getUnitOfWork(); + $this->proxyDir = $proxyDir; + $this->proxyNs = $proxyNs; + $this->autoGenerate = $autoGenerate; } /** * Gets a reference proxy instance for the entity of the given type and identified by * the given identifier. * - * @param string $className - * @param mixed $identifier - * + * @param string $className + * @param mixed $identifier * @return object */ public function getProxy($className, $identifier) { - $fqn = ClassUtils::generateProxyClassName($className, $this->_proxyNamespace); - - if (! class_exists($fqn, false)) { - $fileName = $this->getProxyFileName($className); - if ($this->_autoGenerate) { - $this->_generateProxyClass($this->_em->getClassMetadata($className), $fileName, self::$_proxyClassTemplate); - } - require $fileName; + if ( ! isset($this->definitions[$className])) { + $this->initProxyDefinitions($className); } - $entityPersister = $this->_em->getUnitOfWork()->getEntityPersister($className); + $definition = $this->definitions[$className]; + $fqcn = $definition['fqcn']; + $identifierFields = $definition['identifierFields']; + /* @var $reflectionFields \ReflectionProperty[] */ + $reflectionFields = $definition['reflectionFields']; + $proxy = new $fqcn($definition['initializer'], $definition['cloner']); - return new $fqn($entityPersister, $identifier); - } + foreach ($identifierFields as $idField) { + $reflectionFields[$idField]->setValue($proxy, $identifier[$idField]); + } - /** - * Generates the Proxy file name. - * - * @param string $className - * @param string|null $baseDir Optional base directory for proxy file name generation. - * If not specified, the directory configured on the Configuration of the - * EntityManager will be used by this factory. - * @return string - */ - private function getProxyFileName($className, $baseDir = null) - { - $proxyDir = $baseDir ?: $this->_proxyDir; - - return $proxyDir . DIRECTORY_SEPARATOR . '__CG__' . str_replace('\\', '', $className) . '.php'; + return $proxy; } /** * Generates proxy classes for all given classes. * - * @param array $classes The classes (ClassMetadata instances) for which to generate proxies. - * @param string|null $toDir The target directory of the proxy classes. If not specified, the - * directory configured on the Configuration of the EntityManager used - * by this factory is used. - * + * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata[] $classes The classes (ClassMetadata instances) + * for which to generate proxies. + * @param string $proxyDir The target directory of the proxy classes. If not specified, the + * directory configured on the Configuration of the EntityManager used + * by this factory is used. * @return int Number of generated proxies. */ - public function generateProxyClasses(array $classes, $toDir = null) + public function generateProxyClasses(array $classes, $proxyDir = null) { - $proxyDir = $toDir ?: $this->_proxyDir; - $proxyDir = rtrim($proxyDir, DIRECTORY_SEPARATOR); - $num = 0; + $generated = 0; foreach ($classes as $class) { - /* @var $class ClassMetadata */ - if ($class->isMappedSuperclass || $class->reflClass->isAbstract()) { + /* @var $class \Doctrine\ORM\Mapping\ClassMetadataInfo */ + if ($class->isMappedSuperclass || $class->getReflectionClass()->isAbstract()) { continue; } - $proxyFileName = $this->getProxyFileName($class->name, $proxyDir); + $generator = $this->getProxyGenerator(); - $this->_generateProxyClass($class, $proxyFileName, self::$_proxyClassTemplate); - $num++; + $proxyFileName = $generator->getProxyFileName($class->getName(), $proxyDir); + $generator->generateProxyClass($class, $proxyFileName); + $generated += 1; } - return $num; + return $generated; } /** - * Generates a proxy class file. - * - * @param ClassMetadata $class Metadata for the original class. - * @param string $fileName Filename (full path) for the generated class. - * @param string $file The proxy class template data. - * - * @return void - * - * @throws ProxyException + * @param ProxyGenerator $proxyGenerator */ - private function _generateProxyClass(ClassMetadata $class, $fileName, $file) + public function setProxyGenerator(ProxyGenerator $proxyGenerator) { - $methods = $this->_generateMethods($class); - $sleepImpl = $this->_generateSleep($class); - $cloneImpl = $class->reflClass->hasMethod('__clone') ? 'parent::__clone();' : ''; // hasMethod() checks case-insensitive - - $placeholders = array( - '', - '', '', - '', '', '' - ); - - $className = ltrim($class->name, '\\'); - $proxyClassName = ClassUtils::generateProxyClassName($class->name, $this->_proxyNamespace); - $parts = explode('\\', strrev($proxyClassName), 2); - $proxyClassNamespace = strrev($parts[1]); - $proxyClassName = strrev($parts[0]); - - $replacements = array( - $proxyClassNamespace, - $proxyClassName, - $className, - $methods, - $sleepImpl, - $cloneImpl - ); - - $file = str_replace($placeholders, $replacements, $file); - - $parentDirectory = dirname($fileName); - - if ( ! is_dir($parentDirectory)) { - if (false === @mkdir($parentDirectory, 0775, true)) { - throw ProxyException::proxyDirectoryNotWritable(); - } - } else if ( ! is_writable($parentDirectory)) { - throw ProxyException::proxyDirectoryNotWritable(); - } - - $tmpFileName = $fileName . '.' . uniqid("", true); - file_put_contents($tmpFileName, $file); - rename($tmpFileName, $fileName); + $this->proxyGenerator = $proxyGenerator; } /** - * Generates the methods of a proxy class. - * - * @param ClassMetadata $class - * - * @return string The code of the generated methods. + * @return ProxyGenerator */ - private function _generateMethods(ClassMetadata $class) + public function getProxyGenerator() { - $methods = ''; + if (null === $this->proxyGenerator) { + $this->proxyGenerator = new ProxyGenerator($this->proxyDir, $this->proxyNs); + $this->proxyGenerator->setPlaceholder('', 'Doctrine\ORM\Proxy\Proxy'); + } - $methodNames = array(); - foreach ($class->reflClass->getMethods() as $method) { - /* @var $method \ReflectionMethod */ - if ($method->isConstructor() || in_array(strtolower($method->getName()), array("__sleep", "__clone")) || isset($methodNames[$method->getName()])) { - continue; + return $this->proxyGenerator; + } + + /** + * @param string $className + */ + private function initProxyDefinitions($className) + { + $fqcn = ClassUtils::generateProxyClassName($className, $this->proxyNs); + $classMetadata = $this->em->getClassMetadata($className); + + if ( ! class_exists($fqcn, false)) { + $generator = $this->getProxyGenerator(); + $fileName = $generator->getProxyFileName($className); + + if ($this->autoGenerate) { + $generator->generateProxyClass($classMetadata); } - $methodNames[$method->getName()] = true; - if ($method->isPublic() && ! $method->isFinal() && ! $method->isStatic()) { - $methods .= "\n" . ' public function '; - if ($method->returnsReference()) { - $methods .= '&'; + require $fileName; + } + + $entityPersister = $this->uow->getEntityPersister($className); + + if ($classMetadata->getReflectionClass()->hasMethod('__wakeup')) { + $initializer = function (Proxy $proxy) use ($entityPersister, $classMetadata) { + $proxy->__setInitializer(null); + $proxy->__setCloner(null); + + if ($proxy->__isInitialized()) { + return; } - $methods .= $method->getName() . '('; - $firstParam = true; - $parameterString = $argumentString = ''; - foreach ($method->getParameters() as $param) { - if ($firstParam) { - $firstParam = false; - } else { - $parameterString .= ', '; - $argumentString .= ', '; - } + $properties = $proxy->__getLazyProperties(); - // We need to pick the type hint class too - if (($paramClass = $param->getClass()) !== null) { - $parameterString .= '\\' . $paramClass->getName() . ' '; - } else if ($param->isArray()) { - $parameterString .= 'array '; - } - - if ($param->isPassedByReference()) { - $parameterString .= '&'; - } - - $parameterString .= '$' . $param->getName(); - $argumentString .= '$' . $param->getName(); - - if ($param->isDefaultValueAvailable()) { - $parameterString .= ' = ' . var_export($param->getDefaultValue(), true); + foreach ($properties as $propertyName => $property) { + if (!isset($proxy->$propertyName)) { + $proxy->$propertyName = $properties[$propertyName]; } } - $methods .= $parameterString . ')'; - $methods .= "\n" . ' {' . "\n"; - if ($this->isShortIdentifierGetter($method, $class)) { - $identifier = lcfirst(substr($method->getName(), 3)); + $proxy->__setInitialized(true); + $proxy->__wakeup(); - $cast = in_array($class->fieldMappings[$identifier]['type'], array('integer', 'smallint')) ? '(int) ' : ''; - - $methods .= ' if ($this->__isInitialized__ === false) {' . "\n"; - $methods .= ' return ' . $cast . '$this->_identifier["' . $identifier . '"];' . "\n"; - $methods .= ' }' . "\n"; + if (null === $entityPersister->load($classMetadata->getIdentifierValues($proxy), $proxy)) { + throw new EntityNotFoundException(); } - $methods .= ' $this->__load();' . "\n"; - $methods .= ' return parent::' . $method->getName() . '(' . $argumentString . ');'; - $methods .= "\n" . ' }' . "\n"; - } - } - - return $methods; - } - - /** - * Checks if the method is a short identifier getter. - * - * What does this mean? For proxy objects the identifier is already known, - * however accessing the getter for this identifier usually triggers the - * lazy loading, leading to a query that may not be necessary if only the - * ID is interesting for the userland code (for example in views that - * generate links to the entity, but do not display anything else). - * - * @param \ReflectionMethod $method - * @param ClassMetadata $class - * - * @return bool - */ - private function isShortIdentifierGetter($method, ClassMetadata $class) - { - $identifier = lcfirst(substr($method->getName(), 3)); - $cheapCheck = ( - $method->getNumberOfParameters() == 0 && - substr($method->getName(), 0, 3) == "get" && - in_array($identifier, $class->identifier, true) && - $class->hasField($identifier) && - (($method->getEndLine() - $method->getStartLine()) <= 4) - && in_array($class->fieldMappings[$identifier]['type'], array('integer', 'bigint', 'smallint', 'string')) - ); - - if ($cheapCheck) { - $code = file($method->getDeclaringClass()->getFileName()); - $code = trim(implode(" ", array_slice($code, $method->getStartLine() - 1, $method->getEndLine() - $method->getStartLine() + 1))); - - $pattern = sprintf(self::PATTERN_MATCH_ID_METHOD, $method->getName(), $identifier); - - if (preg_match($pattern, $code)) { - return true; - } - } - return false; - } - - /** - * Generates the code for the __sleep method for a proxy class. - * - * @param ClassMetadata $class - * - * @return string - */ - private function _generateSleep(ClassMetadata $class) - { - $sleepImpl = ''; - - if ($class->reflClass->hasMethod('__sleep')) { - $sleepImpl .= "return array_merge(array('__isInitialized__'), parent::__sleep());"; + }; } else { - $sleepImpl .= "return array('__isInitialized__', "; - $first = true; + $initializer = function (Proxy $proxy) use ($entityPersister, $classMetadata) { + $proxy->__setInitializer(null); + $proxy->__setCloner(null); - foreach ($class->getReflectionProperties() as $name => $prop) { - if ($first) { - $first = false; - } else { - $sleepImpl .= ', '; + if ($proxy->__isInitialized()) { + return; } - $sleepImpl .= "'" . $name . "'"; - } + $properties = $proxy->__getLazyProperties(); - $sleepImpl .= ');'; + foreach ($properties as $propertyName => $property) { + if (!isset($proxy->$propertyName)) { + $proxy->$propertyName = $properties[$propertyName]; + } + } + + $proxy->__setInitialized(true); + + if (null === $entityPersister->load($classMetadata->getIdentifierValues($proxy), $proxy)) { + throw new EntityNotFoundException(); + } + }; } - return $sleepImpl; - } - - /** Proxy class code template */ - private static $_proxyClassTemplate = -'; - -/** - * THIS CLASS WAS GENERATED BY THE DOCTRINE ORM. DO NOT EDIT THIS FILE. - */ -class extends \ implements \Doctrine\ORM\Proxy\Proxy -{ - private $_entityPersister; - private $_identifier; - public $__isInitialized__ = false; - public function __construct($entityPersister, $identifier) - { - $this->_entityPersister = $entityPersister; - $this->_identifier = $identifier; - } - /** @private */ - public function __load() - { - if (!$this->__isInitialized__ && $this->_entityPersister) { - $this->__isInitialized__ = true; - - if (method_exists($this, "__wakeup")) { - // call this after __isInitialized__to avoid infinite recursion - // but before loading to emulate what ClassMetadata::newInstance() - // provides. - $this->__wakeup(); + $cloner = function (Proxy $proxy) use ($entityPersister, $classMetadata) { + if ($proxy->__isInitialized()) { + return; } - if ($this->_entityPersister->load($this->_identifier, $this) === null) { - throw new \Doctrine\ORM\EntityNotFoundException(); + $proxy->__setInitialized(true); + $proxy->__setInitializer(null); + $class = $entityPersister->getClassMetadata(); + $original = $entityPersister->load($classMetadata->getIdentifierValues($proxy)); + + if (null === $original) { + throw new EntityNotFoundException(); } - unset($this->_entityPersister, $this->_identifier); - } - } - /** @private */ - public function __isInitialized() - { - return $this->__isInitialized__; - } + foreach ($class->getReflectionClass()->getProperties() as $reflectionProperty) { + $propertyName = $reflectionProperty->getName(); - - - public function __sleep() - { - - } - - public function __clone() - { - if (!$this->__isInitialized__ && $this->_entityPersister) { - $this->__isInitialized__ = true; - $class = $this->_entityPersister->getClassMetadata(); - $original = $this->_entityPersister->load($this->_identifier); - if ($original === null) { - throw new \Doctrine\ORM\EntityNotFoundException(); + if ($class->hasField($propertyName) || $class->hasAssociation($propertyName)) { + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($proxy, $reflectionProperty->getValue($original)); + } } - foreach ($class->reflFields as $field => $reflProperty) { - $reflProperty->setValue($this, $reflProperty->getValue($original)); - } - unset($this->_entityPersister, $this->_identifier); - } - + }; + + $this->definitions[$className] = array( + 'fqcn' => $fqcn, + 'initializer' => $initializer, + 'cloner' => $cloner, + 'identifierFields' => $classMetadata->getIdentifierFieldNames(), + 'reflectionFields' => $classMetadata->getReflectionProperties(), + ); } -}'; } diff --git a/lib/Doctrine/ORM/Tools/DebugUnitOfWorkListener.php b/lib/Doctrine/ORM/Tools/DebugUnitOfWorkListener.php index c812ae942..2b6478be5 100644 --- a/lib/Doctrine/ORM/Tools/DebugUnitOfWorkListener.php +++ b/lib/Doctrine/ORM/Tools/DebugUnitOfWorkListener.php @@ -96,7 +96,7 @@ class DebugUnitOfWorkListener foreach ($cm->associationMappings as $field => $assoc) { fwrite($fh, " " . $field . " "); - $value = $cm->reflFields[$field]->getValue($entity); + $value = $cm->getFieldValue($entity, $field); if ($assoc['type'] & ClassMetadata::TO_ONE) { if ($value === null) { diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index cd1f69bf4..1fa7676bb 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -666,7 +666,9 @@ class UnitOfWork implements PropertyChangedListener // Look for changes in associations of the entity foreach ($class->associationMappings as $field => $assoc) { - if (($val = $class->reflFields[$field]->getValue($entity)) !== null) { + $val = $class->reflFields[$field]->getValue($entity); + + if (null !== $val) { $this->computeAssociationChanges($assoc, $val); if (!isset($this->entityChangeSets[$oid]) && $assoc['isOwningSide'] && @@ -1815,8 +1817,9 @@ class UnitOfWork implements PropertyChangedListener } if ($class->isVersioned) { - $managedCopyVersion = $class->reflFields[$class->versionField]->getValue($managedCopy); - $entityVersion = $class->reflFields[$class->versionField]->getValue($entity); + $reflField = $class->reflFields[$class->versionField]; + $managedCopyVersion = $reflField->getValue($managedCopy); + $entityVersion = $reflField->getValue($entity); // Throw exception if versions dont match. if ($managedCopyVersion != $entityVersion) { @@ -1828,14 +1831,17 @@ class UnitOfWork implements PropertyChangedListener foreach ($class->reflClass->getProperties() as $prop) { $name = $prop->name; $prop->setAccessible(true); + if ( ! isset($class->associationMappings[$name])) { if ( ! $class->isIdentifier($name)) { $prop->setValue($managedCopy, $prop->getValue($entity)); } } else { $assoc2 = $class->associationMappings[$name]; + if ($assoc2['type'] & ClassMetadata::TO_ONE) { $other = $prop->getValue($entity); + if ($other === null) { $prop->setValue($managedCopy, null); } else if ($other instanceof Proxy && !$other->__isInitialized__) { @@ -1857,6 +1863,7 @@ class UnitOfWork implements PropertyChangedListener } } else { $mergeCol = $prop->getValue($entity); + if ($mergeCol instanceof PersistentCollection && !$mergeCol->isInitialized()) { // do not merge fields marked lazy that have not been fetched. // keep the lazy persistent collection of the managed copy. @@ -1864,6 +1871,7 @@ class UnitOfWork implements PropertyChangedListener } $managedCol = $prop->getValue($managedCopy); + if (!$managedCol) { $managedCol = new PersistentCollection($this->em, $this->em->getClassMetadata($assoc2['targetEntity']), @@ -2468,8 +2476,26 @@ class UnitOfWork implements PropertyChangedListener $entity = $this->identityMap[$class->rootEntityName][$idHash]; $oid = spl_object_hash($entity); - if ($entity instanceof Proxy && ! $entity->__isInitialized__) { - $entity->__isInitialized__ = true; + if ( + isset($hints[Query::HINT_REFRESH]) + && isset($hints[Query::HINT_REFRESH_ENTITY]) + && ($unmanagedProxy = $hints[Query::HINT_REFRESH_ENTITY]) !== $entity + && $unmanagedProxy instanceof Proxy + ) { + // DDC-1238 - we have a managed instance, but it isn't the provided one. + // Therefore we clear its identifier. Also, we must re-fetch metadata since the + // refreshed object may be anything + $class = $this->em->getClassMetadata(get_class($unmanagedProxy)); + + foreach ($class->identifier as $fieldName) { + $class->reflFields[$fieldName]->setValue($unmanagedProxy, null); + } + + return $unmanagedProxy; + } + + if ($entity instanceof Proxy && ! $entity->__isInitialized()) { + $entity->__setInitialized(true); $overrideLocalValues = true; if ($entity instanceof NotifyPropertyChanged) { diff --git a/lib/vendor/doctrine-common b/lib/vendor/doctrine-common index d514e3920..c2b45fdb2 160000 --- a/lib/vendor/doctrine-common +++ b/lib/vendor/doctrine-common @@ -1 +1 @@ -Subproject commit d514e3920656921ba1148f16a4089222c58bc83a +Subproject commit c2b45fdb2757492e75abaab119164aaf311cd395 diff --git a/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php b/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php index 52ac8f591..2ca5cc289 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php @@ -2,9 +2,8 @@ namespace Doctrine\Tests\ORM\Functional; -use Doctrine\Common\Util\ClassUtils, - Doctrine\Tests\Models\CMS\CmsUser, - Doctrine\Tests\Proxies\__CG__\Doctrine\Tests\Models\CMS\CmsUser as Proxy; +use Doctrine\Common\Util\ClassUtils; +use Doctrine\Tests\Models\CMS\CmsUser; /** * Test that Doctrine ORM correctly works with proxy instances exactly like with ordinary Entities @@ -12,7 +11,7 @@ use Doctrine\Common\Util\ClassUtils, * The test considers two possible cases: * a) __initialized__ = true and no identifier set in proxy * b) __initialized__ = false and identifier set in proxy and in property - * @todo All other cases would cause lazy loading issues + * @todo All other cases would cause lazy loading */ class ProxiesLikeEntitiesTest extends \Doctrine\Tests\OrmFunctionalTestCase { @@ -27,6 +26,11 @@ class ProxiesLikeEntitiesTest extends \Doctrine\Tests\OrmFunctionalTestCase try { $this->_schemaTool->createSchema(array( $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsPhonenumber'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsArticle'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsAddress'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsEmail'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsGroup'), )); } catch (\Exception $e) { } @@ -44,8 +48,7 @@ class ProxiesLikeEntitiesTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testPersistUpdate() { // Considering case (a) - $persister = $this->_em->getUnitOfWork()->getEntityPersister('Doctrine\Tests\Models\CMS\CmsUser'); - $proxy = new Proxy($persister, array()); + $proxy = $this->_em->getProxyFactory()->getProxy('Doctrine\Tests\Models\CMS\CmsUser', array('id' => null)); $proxy->__isInitialized__ = true; $proxy->username = 'ocra'; $proxy->name = 'Marco'; @@ -65,15 +68,15 @@ class ProxiesLikeEntitiesTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testEntityWithIdentifier() { - // Considering case (b) - $persister = $this->_em->getUnitOfWork()->getEntityPersister('Doctrine\Tests\Models\CMS\CmsUser'); - $uninitializedProxy = new Proxy($persister, array('id' => $this->user->getId())); - $uninitializedProxy->id = $this->user->getId(); - $uninitializedProxy->username = 'ocra'; - $uninitializedProxy->name = 'Marco Pivetta'; + $userId = $this->user->getId(); + /* @var $uninitializedProxy \Doctrine\Tests\Proxies\__CG__\Doctrine\Tests\Models\CMS\CmsUser */ + $uninitializedProxy = $this->_em->getReference('Doctrine\Tests\Models\CMS\CmsUser', $userId); + $this->assertInstanceOf('Doctrine\Tests\Proxies\__CG__\Doctrine\Tests\Models\CMS\CmsUser', $uninitializedProxy); + $this->_em->persist($uninitializedProxy); - $this->_em->flush(); - $this->assertEquals($this->user->getId(), $uninitializedProxy->getId()); + $this->_em->flush($uninitializedProxy); + $this->assertFalse($uninitializedProxy->__isInitialized(), 'Proxy didn\'t get initialized during flush operations'); + $this->assertEquals($userId, $uninitializedProxy->getId()); $this->_em->remove($uninitializedProxy); $this->_em->flush(); } @@ -83,8 +86,7 @@ class ProxiesLikeEntitiesTest extends \Doctrine\Tests\OrmFunctionalTestCase */ public function testProxyAsDqlParameterPersist() { - $persister = $this->_em->getUnitOfWork()->getEntityPersister('Doctrine\Tests\Models\CMS\CmsUser'); - $proxy = new Proxy($persister, array('id' => $this->user->getId())); + $proxy = $this->_em->getProxyFactory()->getProxy('Doctrine\Tests\Models\CMS\CmsUser', array('id' => $this->user->getId())); $proxy->id = $this->user->getId(); $result = $this ->_em diff --git a/tests/Doctrine/Tests/ORM/Performance/ProxyPerformanceTest.php b/tests/Doctrine/Tests/ORM/Performance/ProxyPerformanceTest.php new file mode 100644 index 000000000..b94fd812e --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Performance/ProxyPerformanceTest.php @@ -0,0 +1,165 @@ +. + */ + +namespace Doctrine\Tests\ORM\Performance; + +use Doctrine\Tests\OrmPerformanceTestCase; +use Doctrine\Common\Proxy\Proxy; +use Doctrine\ORM\EntityManager; +use Doctrine\ORM\UnitOfWork; +use Doctrine\ORM\Proxy\ProxyFactory; +use Doctrine\ORM\Persisters\BasicEntityPersister; + +/** + * Performance test used to measure performance of proxy instantiation + * + * @author Marco Pivetta + * @group performance + */ +class ProxyPerformanceTest extends OrmPerformanceTestCase +{ + /** + * @return array + */ + public function entitiesProvider() + { + return array( + array('Doctrine\Tests\Models\CMS\CmsEmployee'), + array('Doctrine\Tests\Models\CMS\CmsUser'), + ); + } + + /** + * @dataProvider entitiesProvider + */ + public function testProxyInstantiationPerformance($entityName) + { + $proxyFactory = $this->_getEntityManager()->getProxyFactory(); + $this->setMaxRunningTime(5); + $start = microtime(true); + + for ($i = 0; $i < 100000; $i += 1) { + $user = $proxyFactory->getProxy($entityName, array('id' => $i)); + } + + echo __FUNCTION__ . " - " . (microtime(true) - $start) . " seconds with " . $entityName . PHP_EOL; + } + + /** + * @dataProvider entitiesProvider + */ + public function testProxyForcedInitializationPerformance($entityName) + { + $em = new MockEntityManager($this->_getEntityManager()); + $proxyFactory = $em->getProxyFactory(); + /* @var $user \Doctrine\Common\Proxy\Proxy */ + $user = $proxyFactory->getProxy($entityName, array('id' => 1)); + $initializer = $user->__getInitializer(); + + $this->setMaxRunningTime(5); + $start = microtime(true); + + for ($i = 0; $i < 100000; $i += 1) { + $user->__setInitialized(false); + $user->__setInitializer($initializer); + $user->__load(); + $user->__load(); + } + + echo __FUNCTION__ . " - " . (microtime(true) - $start) . " seconds with " . $entityName . PHP_EOL; + } +} + +/** + * Mock entity manager to fake `getPersister()` + */ +class MockEntityManager extends EntityManager +{ + /** @var EntityManager */ + private $em; + + /** @param EntityManager $em */ + public function __construct(EntityManager $em) + { + $this->em = $em; + } + + /** {@inheritDoc} */ + public function getProxyFactory() + { + $config = $this->em->getConfiguration(); + + return new ProxyFactory( + $this, + $config->getProxyDir(), + $config->getProxyNamespace(), + $config->getAutoGenerateProxyClasses() + ); + } + + /** {@inheritDoc} */ + public function getClassMetadata($className) + { + return $this->em->getClassMetadata($className); + } + + /** {@inheritDoc} */ + public function getUnitOfWork() + { + return new MockUnitOfWork(); + } +} + +/** + * Mock UnitOfWork manager to fake `getPersister()` + */ +class MockUnitOfWork extends UnitOfWork +{ + /** @var PersisterMock */ + private $entityPersister; + + /** */ + public function __construct() + { + $this->entityPersister = new PersisterMock(); + } + + /** {@inheritDoc} */ + public function getEntityPersister($entityName) + { + return $this->entityPersister; + } +} + +/** + * Mock persister (we don't want PHPUnit comparator API to play a role in here) + */ +class PersisterMock extends BasicEntityPersister +{ + /** */ + public function __construct() + { + } + + /** {@inheritDoc} */ + public function load(array $criteria, $entity = null, $assoc = null, array $hints = array(), $lockMode = 0, $limit = null, array $orderBy = null) + { + return $entity; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Proxy/AutoloaderTest.php b/tests/Doctrine/Tests/ORM/Proxy/AutoloaderTest.php deleted file mode 100644 index af0afec35..000000000 --- a/tests/Doctrine/Tests/ORM/Proxy/AutoloaderTest.php +++ /dev/null @@ -1,62 +0,0 @@ -. - */ - -namespace Doctrine\Tests\ORM\Proxy; - -use Doctrine\Tests\OrmTestCase; -use Doctrine\ORM\Proxy\Autoloader; - -/** - * @group DDC-1698 - */ -class AutoloaderTest extends OrmTestCase -{ - static public function dataResolveFile() - { - return array( - array('/tmp', 'MyProxy', 'MyProxy\__CG__\RealClass', '/tmp' . DIRECTORY_SEPARATOR . '__CG__RealClass.php'), - array('/tmp', 'MyProxy\Subdir', 'MyProxy\Subdir\__CG__\RealClass', '/tmp' . DIRECTORY_SEPARATOR . '__CG__RealClass.php'), - array('/tmp', 'MyProxy', 'MyProxy\__CG__\Other\RealClass', '/tmp' . DIRECTORY_SEPARATOR . '__CG__OtherRealClass.php'), - ); - } - - /** - * @dataProvider dataResolveFile - */ - public function testResolveFile($proxyDir, $proxyNamespace, $className, $expectedProxyFile) - { - $actualProxyFile = Autoloader::resolveFile($proxyDir, $proxyNamespace, $className); - $this->assertEquals($expectedProxyFile, $actualProxyFile); - } - - public function testAutoload() - { - if (file_exists(sys_get_temp_dir() ."/AutoloaderTestClass.php")) { - unlink(sys_get_temp_dir() ."/AutoloaderTestClass.php"); - } - - $autoloader = Autoloader::register(sys_get_temp_dir(), 'ProxyAutoloaderTest', function($proxyDir, $proxyNamespace, $className) { - file_put_contents(sys_get_temp_dir() . "/AutoloaderTestClass.php", "assertTrue(class_exists('ProxyAutoloaderTest\AutoloaderTestClass', true)); - unlink(sys_get_temp_dir() ."/AutoloaderTestClass.php"); - } -} - diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php new file mode 100644 index 000000000..2dbd6b816 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php @@ -0,0 +1,89 @@ + + */ +class ProxtFactoryTest extends \Doctrine\Tests\OrmTestCase +{ + /** + * @var ConnectionMock + */ + private $connectionMock; + + /** + * @var UnitOfWorkMock + */ + private $uowMock; + + /** + * @var EntityManagerMock + */ + private $emMock; + + /** + * @var \Doctrine\ORM\Proxy\ProxyFactory + */ + private $proxyFactory; + + /** + * {@inheritDoc} + */ + protected function setUp() + { + parent::setUp(); + $this->connectionMock = new ConnectionMock(array(), new DriverMock()); + $this->emMock = EntityManagerMock::create($this->connectionMock); + $this->uowMock = new UnitOfWorkMock($this->emMock); + $this->emMock->setUnitOfWork($this->uowMock); + $this->proxyFactory = new ProxyFactory($this->emMock, sys_get_temp_dir(), 'Proxies', true); + } + + public function testReferenceProxyDelegatesLoadingToThePersister() + { + $identifier = array('id' => 42); + $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; + $persister = $this->getMock('Doctrine\ORM\Persisters\BasicEntityPersister', array('load'), array(), '', false); + $this->uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); + + $proxy = $this->proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $identifier); + + $persister + ->expects($this->atLeastOnce()) + ->method('load') + ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) + ->will($this->returnValue(new \stdClass())); + + $proxy->getDescription(); + } + + /** + * @group DDC-1771 + */ + public function testSkipAbstractClassesOnGeneration() + { + $cm = new \Doctrine\ORM\Mapping\ClassMetadata(__NAMESPACE__ . '\\AbstractClass'); + $cm->initializeReflection(new \Doctrine\Common\Persistence\Mapping\RuntimeReflectionService); + $this->assertNotNull($cm->reflClass); + + $num = $this->proxyFactory->generateProxyClasses(array($cm)); + + $this->assertEquals(0, $num, "No proxies generated."); + } +} + +abstract class AbstractClass +{ + +} diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php deleted file mode 100644 index 894d500fe..000000000 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php +++ /dev/null @@ -1,204 +0,0 @@ - - */ -class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase -{ - private $_connectionMock; - private $_uowMock; - private $_emMock; - - /** - * @var \Doctrine\ORM\Proxy\ProxyFactory - */ - private $_proxyFactory; - - protected function setUp() - { - parent::setUp(); - $this->_connectionMock = new ConnectionMock(array(), new \Doctrine\Tests\Mocks\DriverMock()); - $this->_emMock = EntityManagerMock::create($this->_connectionMock); - $this->_uowMock = new UnitOfWorkMock($this->_emMock); - $this->_emMock->setUnitOfWork($this->_uowMock); - // SUT - $this->_proxyFactory = new ProxyFactory($this->_emMock, __DIR__ . '/generated', 'Proxies', true); - } - - protected function tearDown() - { - foreach (new \DirectoryIterator(__DIR__ . '/generated') as $file) { - if (strstr($file->getFilename(), '.php')) { - unlink($file->getPathname()); - } - } - } - - public function testReferenceProxyDelegatesLoadingToThePersister() - { - $identifier = array('id' => 42); - $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; - $persister = $this->_getMockPersister(); - $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); - - $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $identifier); - - $persister->expects($this->atLeastOnce()) - ->method('load') - ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) - ->will($this->returnValue(new \stdClass())); // fake return of entity instance - - $proxy->getDescription(); - } - - public function testReferenceProxyExecutesLoadingOnlyOnce() - { - $identifier = array('id' => 42); - $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; - $persister = $this->_getMockPersister(); - $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); - $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $identifier); - - $persister->expects($this->atLeastOnce()) - ->method('load') - ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) - ->will($this->returnValue(new \stdClass())); // fake return of entity instance - $proxy->getDescription(); - $proxy->getProduct(); - } - - public function testReferenceProxyRespectsMethodsParametersTypeHinting() - { - $proxyClass = 'Proxies\DoctrineTestsModelsECommerceECommerceFeatureProxy'; - $persister = $this->_getMockPersister(); - $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); - $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', null); - - $method = new \ReflectionMethod(get_class($proxy), 'setProduct'); - $params = $method->getParameters(); - - $this->assertEquals(1, count($params)); - $this->assertEquals('Doctrine\Tests\Models\ECommerce\ECommerceProduct', $params[0]->getClass()->getName()); - } - - /** - * Test that the proxy behaves in regard to methods like &foo() correctly - */ - public function testProxyRespectsMethodsWhichReturnValuesByReference() { - $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\Forum\ForumEntry', null); - $method = new \ReflectionMethod(get_class($proxy), 'getTopicByReference'); - - $this->assertTrue($method->returnsReference()); - } - - public function testCreatesAssociationProxyAsSubclassOfTheOriginalOne() - { - $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; - $this->assertTrue(is_subclass_of($proxyClass, 'Doctrine\Tests\Models\ECommerce\ECommerceFeature')); - } - - - public function testAllowsConcurrentCreationOfBothProxyTypes() - { - $referenceProxyClass = 'Proxies\DoctrineTestsModelsECommerceECommerceFeatureProxy'; - $associationProxyClass = 'Proxies\DoctrineTestsModelsECommerceECommerceFeatureAProxy'; - $this->assertNotEquals($referenceProxyClass, $associationProxyClass); - } - - public function testNonNamespacedProxyGeneration() - { - require_once dirname(__FILE__)."/fixtures/NonNamespacedProxies.php"; - - $className = "\DoctrineOrmTestEntity"; - $proxyName = "DoctrineOrmTestEntity"; - $classMetadata = new \Doctrine\ORM\Mapping\ClassMetadata($className); - $classMetadata->initializeReflection(new \Doctrine\Common\Persistence\Mapping\RuntimeReflectionService); - $classMetadata->mapField(array('fieldName' => 'id', 'type' => 'integer')); - $classMetadata->setIdentifier(array('id')); - - $this->_proxyFactory->generateProxyClasses(array($classMetadata)); - - $classCode = file_get_contents(dirname(__FILE__)."/generated/__CG__".$proxyName.".php"); - - $this->assertNotContains("class DoctrineOrmTestEntity extends \\\\DoctrineOrmTestEntity", $classCode); - $this->assertContains("class DoctrineOrmTestEntity extends \\DoctrineOrmTestEntity", $classCode); - } - - public function testClassWithSleepProxyGeneration() - { - $className = "\Doctrine\Tests\ORM\Proxy\SleepClass"; - $proxyName = "DoctrineTestsORMProxySleepClass"; - $classMetadata = new \Doctrine\ORM\Mapping\ClassMetadata($className); - $classMetadata->initializeReflection(new \Doctrine\Common\Persistence\Mapping\RuntimeReflectionService); - $classMetadata->mapField(array('fieldName' => 'id', 'type' => 'integer')); - $classMetadata->setIdentifier(array('id')); - - $this->_proxyFactory->generateProxyClasses(array($classMetadata)); - - $classCode = file_get_contents(dirname(__FILE__)."/generated/__CG__".$proxyName.".php"); - - $this->assertEquals(1, substr_count($classCode, 'function __sleep')); - } - - /** - * @group DDC-1771 - */ - public function testSkipAbstractClassesOnGeneration() - { - $cm = new \Doctrine\ORM\Mapping\ClassMetadata(__NAMESPACE__ . '\\AbstractClass'); - $cm->initializeReflection(new \Doctrine\Common\Persistence\Mapping\RuntimeReflectionService); - $this->assertNotNull($cm->reflClass); - - $num = $this->_proxyFactory->generateProxyClasses(array($cm)); - - $this->assertEquals(0, $num, "No proxies generated."); - } - - public function testNoConfigDir_ThrowsException() - { - $this->setExpectedException('Doctrine\ORM\Proxy\ProxyException'); - new ProxyFactory($this->_getTestEntityManager(), null, null); - } - - public function testNoNamespace_ThrowsException() - { - $this->setExpectedException('Doctrine\ORM\Proxy\ProxyException'); - new ProxyFactory($this->_getTestEntityManager(), __DIR__ . '/generated', null); - } - - protected function _getMockPersister() - { - $persister = $this->getMock('Doctrine\ORM\Persisters\BasicEntityPersister', array('load'), array(), '', false); - return $persister; - } -} - -class SleepClass -{ - public $id; - - public function __sleep() - { - return array('id'); - } -} - -abstract class AbstractClass -{ - -} diff --git a/tests/Doctrine/Tests/ORM/Proxy/fixtures/NonNamespacedProxies.php b/tests/Doctrine/Tests/ORM/Proxy/fixtures/NonNamespacedProxies.php deleted file mode 100644 index 88c06e449..000000000 --- a/tests/Doctrine/Tests/ORM/Proxy/fixtures/NonNamespacedProxies.php +++ /dev/null @@ -1,13 +0,0 @@ - Date: Sun, 6 Jan 2013 14:56:11 +0100 Subject: [PATCH 2/3] Adding fix and tests for DDC-1734 --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 29 ++++++- lib/Doctrine/ORM/UnitOfWork.php | 3 +- lib/vendor/doctrine-common | 2 +- .../Tests/Models/DDC1734/DDC1734Article.php | 37 +++++++++ .../ORM/Functional/Ticket/DDC1734Test.php | 78 +++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 3 + 6 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index de67e2f86..aa1e6a8e8 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -25,6 +25,7 @@ use Doctrine\ORM\EntityNotFoundException; use Doctrine\Common\Util\ClassUtils; use Doctrine\Common\Proxy\Proxy; use Doctrine\Common\Proxy\ProxyGenerator; +use Doctrine\ORM\ORMInvalidArgumentException; /** * This factory is used to create proxy objects for entities at runtime. @@ -123,6 +124,31 @@ class ProxyFactory return $proxy; } + /** + * @param \Doctrine\Common\Proxy\Proxy $proxy + * + * @return \Doctrine\Common\Proxy\Proxy + * + * @throws \Doctrine\ORM\ORMInvalidArgumentException + */ + public function resetUninitializedProxy(Proxy $proxy) + { + if ($proxy->__isInitialized()) { + throw new ORMInvalidArgumentException('Provided proxy must not be initialized'); + } + + $className = $this->em->getClassMetadata(get_class($proxy))->getName(); + + if ( ! isset($this->definitions[$className])) { + $this->initProxyDefinitions($className); + } + + $proxy->__setInitializer($this->definitions[$className]['initializer']); + $proxy->__setCloner($this->definitions[$className]['cloner']); + + return $proxy; + } + /** * Generates proxy classes for all given classes. * @@ -179,8 +205,9 @@ class ProxyFactory */ private function initProxyDefinitions($className) { - $fqcn = ClassUtils::generateProxyClassName($className, $this->proxyNs); $classMetadata = $this->em->getClassMetadata($className); + $className = $classMetadata->getName(); + $fqcn = ClassUtils::generateProxyClassName($className, $this->proxyNs); if ( ! class_exists($fqcn, false)) { $generator = $this->getProxyGenerator(); diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 1fa7676bb..35f677fc5 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1769,7 +1769,8 @@ class UnitOfWork implements PropertyChangedListener $managedCopy = $entity; if ($this->getEntityState($entity, self::STATE_DETACHED) !== self::STATE_MANAGED) { - if ($entity instanceof Proxy && ! $entity->__isInitialized__) { + if ($entity instanceof Proxy && ! $entity->__isInitialized()) { + $this->em->getProxyFactory()->resetUninitializedProxy($entity); $entity->__load(); } diff --git a/lib/vendor/doctrine-common b/lib/vendor/doctrine-common index c2b45fdb2..0aa165610 160000 --- a/lib/vendor/doctrine-common +++ b/lib/vendor/doctrine-common @@ -1 +1 @@ -Subproject commit c2b45fdb2757492e75abaab119164aaf311cd395 +Subproject commit 0aa165610e2fdd6617e0e2b91c35fedf92aea2f7 diff --git a/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php b/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php new file mode 100644 index 000000000..b018da840 --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php @@ -0,0 +1,37 @@ +name = $name; + } + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php new file mode 100644 index 000000000..d5748a5aa --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php @@ -0,0 +1,78 @@ +useModelSet('ddc1734'); + parent::setUp(); + } + + /** + * This test is DDC-1734 minus the serialization, i.e. it works + * @group DDC-1734 + */ + public function testMergeWorksOnNonSerializedProxies() + { + $article = new DDC1734Article("Foo"); + $this->_em->persist($article); + $this->_em->flush(); + // Get a proxy of the entity + $this->_em->clear(); + $proxy = $this->getProxy($article); + $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $proxy); + $this->assertFalse($proxy->__isInitialized__); + // Detach + $this->_em->detach($proxy); + $this->_em->clear(); + // Merge + $proxy = $this->_em->merge($proxy); + $this->assertEquals("Foo", $proxy->getName(), "The entity is broken"); + } + + /** + * This test reproduces DDC-1734 which is: + * - A non-initialized proxy is detached and serialized (the identifier of the proxy is *not* serialized) + * - the object is deserialized and merged (to turn into an entity) + * - the entity is broken because it has no identifier and no field defined + * @group DDC-1734 + */ + public function testMergeWorksOnSerializedProxies() + { + $article = new DDC1734Article("Foo"); + $this->_em->persist($article); + $this->_em->flush(); + // Get a proxy of the entity + $this->_em->clear(); + $proxy = $this->getProxy($article); + $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $proxy); + $this->assertFalse($proxy->__isInitialized()); + // Detach and serialize + $this->_em->detach($proxy); + $serializedProxy = serialize($proxy); + $this->_em->clear(); + // Unserialize and merge + /** @var $unserializedProxy DDC1734Article */ + $unserializedProxy = unserialize($serializedProxy); + // Merge + $unserializedProxy = $this->_em->merge($unserializedProxy); + $this->assertEquals("Foo", $unserializedProxy->getName(), "The entity is broken"); + } + + private function getProxy($object) + { + $metadataFactory = $this->_em->getMetadataFactory(); + $identifier = $metadataFactory->getMetadataFor(get_class($object))->getIdentifierValues($object); + $proxyFactory = $this->_em->getProxyFactory(); + + return $proxyFactory->getProxy(get_class($object), $identifier); + } + +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 715c32a5f..a16cba368 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -134,6 +134,9 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\DDC117\DDC117Editor', 'Doctrine\Tests\Models\DDC117\DDC117Link', ), + 'ddc1734' => array( + 'Doctrine\Tests\Models\DDC1734\DDC1734Article', + ), 'stockexchange' => array( 'Doctrine\Tests\Models\StockExchange\Bond', 'Doctrine\Tests\Models\StockExchange\Stock', From a58d4ae46292cc9d07cc9976477156611820ce9c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 6 Jan 2013 15:11:30 +0100 Subject: [PATCH 3/3] Cleaning up logic of the proxy factory by moving closure generation to own private methods --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 278 ++++++------------ lib/Doctrine/ORM/UnitOfWork.php | 14 +- lib/vendor/doctrine-common | 2 +- .../Tests/Models/DDC1734/DDC1734Article.php | 37 --- .../ORM/Functional/Ticket/DDC1734Test.php | 65 ++-- ...xtFactoryTest.php => ProxyFactoryTest.php} | 7 +- .../Doctrine/Tests/OrmFunctionalTestCase.php | 3 - 7 files changed, 134 insertions(+), 272 deletions(-) delete mode 100644 tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php rename tests/Doctrine/Tests/ORM/Proxy/{ProxtFactoryTest.php => ProxyFactoryTest.php} (92%) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index aa1e6a8e8..f23b1063f 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -19,26 +19,29 @@ namespace Doctrine\ORM\Proxy; -use Doctrine\ORM\EntityManager; -use Doctrine\ORM\EntityNotFoundException; - +use Doctrine\Common\Persistence\Mapping\ClassMetadata; +use Doctrine\Common\Proxy\AbstractProxyFactory; +use Doctrine\Common\Proxy\ProxyDefinition; use Doctrine\Common\Util\ClassUtils; use Doctrine\Common\Proxy\Proxy; use Doctrine\Common\Proxy\ProxyGenerator; use Doctrine\ORM\ORMInvalidArgumentException; +use Doctrine\ORM\Persisters\BasicEntityPersister; +use Doctrine\ORM\EntityManager; +use Doctrine\ORM\EntityNotFoundException; /** * This factory is used to create proxy objects for entities at runtime. * * @author Roman Borschel * @author Giorgio Sironi - * @author Marco Pivetta + * @author Marco Pivetta * @since 2.0 */ -class ProxyFactory +class ProxyFactory extends AbstractProxyFactory { /** - * @var EntityManager The EntityManager this factory is bound to. + * @var \Doctrine\ORM\EntityManager The EntityManager this factory is bound to. */ private $em; @@ -47,183 +50,73 @@ class ProxyFactory */ private $uow; - /** - * @var ProxyGenerator the proxy generator responsible for creating the proxy classes/files. - */ - private $proxyGenerator; - - /** - * @var bool Whether to automatically (re)generate proxy classes. - */ - private $autoGenerate; - /** * @var string */ private $proxyNs; - /** - * @var string - */ - private $proxyDir; - - /** - * @var array definitions (indexed by requested class name) for the proxy classes. - * Each element is an array containing following items: - * "fqcn" - FQCN of the proxy class - * "initializer" - Closure to be used as proxy __initializer__ - * "cloner" - Closure to be used as proxy __cloner__ - * "identifierFields" - list of field names for the identifiers - * "reflectionFields" - ReflectionProperties for the fields - */ - private $definitions = array(); - /** * Initializes a new instance of the ProxyFactory class that is * connected to the given EntityManager. * - * @param EntityManager $em The EntityManager the new factory works for. - * @param string $proxyDir The directory to use for the proxy classes. It must exist. - * @param string $proxyNs The namespace to use for the proxy classes. - * @param boolean $autoGenerate Whether to automatically generate proxy classes. + * @param \Doctrine\ORM\EntityManager $em The EntityManager the new factory works for. + * @param string $proxyDir The directory to use for the proxy classes. It must exist. + * @param string $proxyNs The namespace to use for the proxy classes. + * @param boolean $autoGenerate Whether to automatically generate proxy classes. */ public function __construct(EntityManager $em, $proxyDir, $proxyNs, $autoGenerate = false) { - $this->em = $em; - $this->uow = $em->getUnitOfWork(); - $this->proxyDir = $proxyDir; - $this->proxyNs = $proxyNs; - $this->autoGenerate = $autoGenerate; + $proxyGenerator = new ProxyGenerator($proxyDir, $proxyNs); + + $proxyGenerator->setPlaceholder('baseProxyInterface', 'Doctrine\ORM\Proxy\Proxy'); + parent::__construct($proxyGenerator, $em->getMetadataFactory(), $autoGenerate); + + $this->em = $em; + $this->uow = $em->getUnitOfWork(); + $this->proxyNs = $proxyNs; + } /** - * Gets a reference proxy instance for the entity of the given type and identified by - * the given identifier. - * - * @param string $className - * @param mixed $identifier - * @return object + * {@inheritDoc} */ - public function getProxy($className, $identifier) + protected function skipClass(ClassMetadata $metadata) { - if ( ! isset($this->definitions[$className])) { - $this->initProxyDefinitions($className); - } - - $definition = $this->definitions[$className]; - $fqcn = $definition['fqcn']; - $identifierFields = $definition['identifierFields']; - /* @var $reflectionFields \ReflectionProperty[] */ - $reflectionFields = $definition['reflectionFields']; - $proxy = new $fqcn($definition['initializer'], $definition['cloner']); - - foreach ($identifierFields as $idField) { - $reflectionFields[$idField]->setValue($proxy, $identifier[$idField]); - } - - return $proxy; + /* @var $metadata \Doctrine\ORM\Mapping\ClassMetadataInfo */ + return $metadata->isMappedSuperclass || $metadata->getReflectionClass()->isAbstract(); } /** - * @param \Doctrine\Common\Proxy\Proxy $proxy - * - * @return \Doctrine\Common\Proxy\Proxy - * - * @throws \Doctrine\ORM\ORMInvalidArgumentException + * {@inheritDoc} */ - public function resetUninitializedProxy(Proxy $proxy) + protected function createProxyDefinition($className) { - if ($proxy->__isInitialized()) { - throw new ORMInvalidArgumentException('Provided proxy must not be initialized'); - } - - $className = $this->em->getClassMetadata(get_class($proxy))->getName(); - - if ( ! isset($this->definitions[$className])) { - $this->initProxyDefinitions($className); - } - - $proxy->__setInitializer($this->definitions[$className]['initializer']); - $proxy->__setCloner($this->definitions[$className]['cloner']); - - return $proxy; - } - - /** - * Generates proxy classes for all given classes. - * - * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata[] $classes The classes (ClassMetadata instances) - * for which to generate proxies. - * @param string $proxyDir The target directory of the proxy classes. If not specified, the - * directory configured on the Configuration of the EntityManager used - * by this factory is used. - * @return int Number of generated proxies. - */ - public function generateProxyClasses(array $classes, $proxyDir = null) - { - $generated = 0; - - foreach ($classes as $class) { - /* @var $class \Doctrine\ORM\Mapping\ClassMetadataInfo */ - if ($class->isMappedSuperclass || $class->getReflectionClass()->isAbstract()) { - continue; - } - - $generator = $this->getProxyGenerator(); - - $proxyFileName = $generator->getProxyFileName($class->getName(), $proxyDir); - $generator->generateProxyClass($class, $proxyFileName); - $generated += 1; - } - - return $generated; - } - - /** - * @param ProxyGenerator $proxyGenerator - */ - public function setProxyGenerator(ProxyGenerator $proxyGenerator) - { - $this->proxyGenerator = $proxyGenerator; - } - - /** - * @return ProxyGenerator - */ - public function getProxyGenerator() - { - if (null === $this->proxyGenerator) { - $this->proxyGenerator = new ProxyGenerator($this->proxyDir, $this->proxyNs); - $this->proxyGenerator->setPlaceholder('', 'Doctrine\ORM\Proxy\Proxy'); - } - - return $this->proxyGenerator; - } - - /** - * @param string $className - */ - private function initProxyDefinitions($className) - { - $classMetadata = $this->em->getClassMetadata($className); - $className = $classMetadata->getName(); - $fqcn = ClassUtils::generateProxyClassName($className, $this->proxyNs); - - if ( ! class_exists($fqcn, false)) { - $generator = $this->getProxyGenerator(); - $fileName = $generator->getProxyFileName($className); - - if ($this->autoGenerate) { - $generator->generateProxyClass($classMetadata); - } - - require $fileName; - } - + $classMetadata = $this->em->getClassMetadata($className); $entityPersister = $this->uow->getEntityPersister($className); + return new ProxyDefinition( + ClassUtils::generateProxyClassName($className, $this->proxyNs), + $classMetadata->getIdentifierFieldNames(), + $classMetadata->getReflectionProperties(), + $this->createInitializer($classMetadata, $entityPersister), + $this->createCloner($classMetadata, $entityPersister) + ); + } + + /** + * Creates a closure capable of initializing a proxy + * + * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata $classMetadata + * @param \Doctrine\ORM\Persisters\BasicEntityPersister $entityPersister + * + * @return \Closure + * + * @throws \Doctrine\ORM\EntityNotFoundException + */ + private function createInitializer(ClassMetadata $classMetadata, BasicEntityPersister $entityPersister) + { if ($classMetadata->getReflectionClass()->hasMethod('__wakeup')) { - $initializer = function (Proxy $proxy) use ($entityPersister, $classMetadata) { + return function (Proxy $proxy) use ($entityPersister, $classMetadata) { $proxy->__setInitializer(null); $proxy->__setCloner(null); @@ -242,36 +135,49 @@ class ProxyFactory $proxy->__setInitialized(true); $proxy->__wakeup(); - if (null === $entityPersister->load($classMetadata->getIdentifierValues($proxy), $proxy)) { - throw new EntityNotFoundException(); - } - }; - } else { - $initializer = function (Proxy $proxy) use ($entityPersister, $classMetadata) { - $proxy->__setInitializer(null); - $proxy->__setCloner(null); - - if ($proxy->__isInitialized()) { - return; - } - - $properties = $proxy->__getLazyProperties(); - - foreach ($properties as $propertyName => $property) { - if (!isset($proxy->$propertyName)) { - $proxy->$propertyName = $properties[$propertyName]; - } - } - - $proxy->__setInitialized(true); - if (null === $entityPersister->load($classMetadata->getIdentifierValues($proxy), $proxy)) { throw new EntityNotFoundException(); } }; } - $cloner = function (Proxy $proxy) use ($entityPersister, $classMetadata) { + return function (Proxy $proxy) use ($entityPersister, $classMetadata) { + $proxy->__setInitializer(null); + $proxy->__setCloner(null); + + if ($proxy->__isInitialized()) { + return; + } + + $properties = $proxy->__getLazyProperties(); + + foreach ($properties as $propertyName => $property) { + if (!isset($proxy->$propertyName)) { + $proxy->$propertyName = $properties[$propertyName]; + } + } + + $proxy->__setInitialized(true); + + if (null === $entityPersister->load($classMetadata->getIdentifierValues($proxy), $proxy)) { + throw new EntityNotFoundException(); + } + }; + } + + /** + * Creates a closure capable of finalizing state a cloned proxy + * + * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata $classMetadata + * @param \Doctrine\ORM\Persisters\BasicEntityPersister $entityPersister + * + * @return \Closure + * + * @throws \Doctrine\ORM\EntityNotFoundException + */ + private function createCloner(ClassMetadata $classMetadata, BasicEntityPersister $entityPersister) + { + return function (Proxy $proxy) use ($entityPersister, $classMetadata) { if ($proxy->__isInitialized()) { return; } @@ -294,13 +200,5 @@ class ProxyFactory } } }; - - $this->definitions[$className] = array( - 'fqcn' => $fqcn, - 'initializer' => $initializer, - 'cloner' => $cloner, - 'identifierFields' => $classMetadata->getIdentifierFieldNames(), - 'reflectionFields' => $classMetadata->getReflectionProperties(), - ); } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 35f677fc5..9848499d9 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -666,9 +666,7 @@ class UnitOfWork implements PropertyChangedListener // Look for changes in associations of the entity foreach ($class->associationMappings as $field => $assoc) { - $val = $class->reflFields[$field]->getValue($entity); - - if (null !== $val) { + if (($val = $class->reflFields[$field]->getValue($entity)) !== null) { $this->computeAssociationChanges($assoc, $val); if (!isset($this->entityChangeSets[$oid]) && $assoc['isOwningSide'] && @@ -1818,9 +1816,9 @@ class UnitOfWork implements PropertyChangedListener } if ($class->isVersioned) { - $reflField = $class->reflFields[$class->versionField]; + $reflField = $class->reflFields[$class->versionField]; $managedCopyVersion = $reflField->getValue($managedCopy); - $entityVersion = $reflField->getValue($entity); + $entityVersion = $reflField->getValue($entity); // Throw exception if versions dont match. if ($managedCopyVersion != $entityVersion) { @@ -1832,17 +1830,14 @@ class UnitOfWork implements PropertyChangedListener foreach ($class->reflClass->getProperties() as $prop) { $name = $prop->name; $prop->setAccessible(true); - if ( ! isset($class->associationMappings[$name])) { if ( ! $class->isIdentifier($name)) { $prop->setValue($managedCopy, $prop->getValue($entity)); } } else { $assoc2 = $class->associationMappings[$name]; - if ($assoc2['type'] & ClassMetadata::TO_ONE) { $other = $prop->getValue($entity); - if ($other === null) { $prop->setValue($managedCopy, null); } else if ($other instanceof Proxy && !$other->__isInitialized__) { @@ -1864,7 +1859,6 @@ class UnitOfWork implements PropertyChangedListener } } else { $mergeCol = $prop->getValue($entity); - if ($mergeCol instanceof PersistentCollection && !$mergeCol->isInitialized()) { // do not merge fields marked lazy that have not been fetched. // keep the lazy persistent collection of the managed copy. @@ -1872,7 +1866,6 @@ class UnitOfWork implements PropertyChangedListener } $managedCol = $prop->getValue($managedCopy); - if (!$managedCol) { $managedCol = new PersistentCollection($this->em, $this->em->getClassMetadata($assoc2['targetEntity']), @@ -2497,6 +2490,7 @@ class UnitOfWork implements PropertyChangedListener if ($entity instanceof Proxy && ! $entity->__isInitialized()) { $entity->__setInitialized(true); + $overrideLocalValues = true; if ($entity instanceof NotifyPropertyChanged) { diff --git a/lib/vendor/doctrine-common b/lib/vendor/doctrine-common index 0aa165610..d5843a462 160000 --- a/lib/vendor/doctrine-common +++ b/lib/vendor/doctrine-common @@ -1 +1 @@ -Subproject commit 0aa165610e2fdd6617e0e2b91c35fedf92aea2f7 +Subproject commit d5843a462a4dfe4a42daf4645fc867c431a4170e diff --git a/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php b/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php deleted file mode 100644 index b018da840..000000000 --- a/tests/Doctrine/Tests/Models/DDC1734/DDC1734Article.php +++ /dev/null @@ -1,37 +0,0 @@ -name = $name; - } - - public function getId() - { - return $this->id; - } - - public function getName() - { - return $this->name; - } - -} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php index d5748a5aa..f0abda67d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1734Test.php @@ -2,39 +2,44 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; -use Doctrine\Tests\Models\DDC1734\DDC1734Article; - -require_once __DIR__ . '/../../../TestInit.php'; +use Doctrine\Tests\Models\CMS\CmsGroup; class DDC1734Test extends \Doctrine\Tests\OrmFunctionalTestCase { - + /** + * {@inheritDoc} + */ protected function setUp() { - $this->useModelSet('ddc1734'); + $this->useModelSet('cms'); parent::setUp(); } /** * This test is DDC-1734 minus the serialization, i.e. it works + * * @group DDC-1734 */ public function testMergeWorksOnNonSerializedProxies() { - $article = new DDC1734Article("Foo"); - $this->_em->persist($article); + $group = new CmsGroup(); + + $group->setName('Foo'); + $this->_em->persist($group); $this->_em->flush(); - // Get a proxy of the entity $this->_em->clear(); - $proxy = $this->getProxy($article); + + $proxy = $this->getProxy($group); + $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $proxy); - $this->assertFalse($proxy->__isInitialized__); - // Detach + $this->assertFalse($proxy->__isInitialized()); + $this->_em->detach($proxy); $this->_em->clear(); - // Merge + $proxy = $this->_em->merge($proxy); - $this->assertEquals("Foo", $proxy->getName(), "The entity is broken"); + + $this->assertEquals('Foo', $proxy->getName(), 'The entity is broken'); } /** @@ -42,37 +47,43 @@ class DDC1734Test extends \Doctrine\Tests\OrmFunctionalTestCase * - A non-initialized proxy is detached and serialized (the identifier of the proxy is *not* serialized) * - the object is deserialized and merged (to turn into an entity) * - the entity is broken because it has no identifier and no field defined + * * @group DDC-1734 */ public function testMergeWorksOnSerializedProxies() { - $article = new DDC1734Article("Foo"); - $this->_em->persist($article); + $group = new CmsGroup(); + + $group->setName('Foo'); + $this->_em->persist($group); $this->_em->flush(); - // Get a proxy of the entity $this->_em->clear(); - $proxy = $this->getProxy($article); + + $proxy = $this->getProxy($group); + $this->assertInstanceOf('Doctrine\ORM\Proxy\Proxy', $proxy); $this->assertFalse($proxy->__isInitialized()); - // Detach and serialize + $this->_em->detach($proxy); $serializedProxy = serialize($proxy); $this->_em->clear(); - // Unserialize and merge - /** @var $unserializedProxy DDC1734Article */ - $unserializedProxy = unserialize($serializedProxy); - // Merge - $unserializedProxy = $this->_em->merge($unserializedProxy); - $this->assertEquals("Foo", $unserializedProxy->getName(), "The entity is broken"); + + $unserializedProxy = $this->_em->merge(unserialize($serializedProxy)); + $this->assertEquals('Foo', $unserializedProxy->getName(), 'The entity is broken'); } + /** + * @param object $object + * + * @return \Doctrine\Common\Proxy\Proxy + */ private function getProxy($object) { $metadataFactory = $this->_em->getMetadataFactory(); - $identifier = $metadataFactory->getMetadataFor(get_class($object))->getIdentifierValues($object); - $proxyFactory = $this->_em->getProxyFactory(); + $className = get_class($object); + $identifier = $metadataFactory->getMetadataFor($className)->getIdentifierValues($object); - return $proxyFactory->getProxy(get_class($object), $identifier); + return $this->_em->getProxyFactory()->getProxy($className, $identifier); } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php similarity index 92% rename from tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php rename to tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php index 2dbd6b816..90f1d7ef7 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxtFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -2,6 +2,7 @@ namespace Doctrine\Tests\ORM\Proxy; +use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Proxy\ProxyFactory; use Doctrine\Common\Proxy\ProxyGenerator; use Doctrine\Tests\Mocks\ConnectionMock; @@ -9,13 +10,11 @@ use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Mocks\UnitOfWorkMock; use Doctrine\Tests\Mocks\DriverMock; -require_once __DIR__ . '/../../TestInit.php'; - /** * Test the proxy generator. Its work is generating on-the-fly subclasses of a given model, which implement the Proxy pattern. * @author Giorgio Sironi */ -class ProxtFactoryTest extends \Doctrine\Tests\OrmTestCase +class ProxyFactoryTest extends \Doctrine\Tests\OrmTestCase { /** * @var ConnectionMock @@ -73,7 +72,7 @@ class ProxtFactoryTest extends \Doctrine\Tests\OrmTestCase */ public function testSkipAbstractClassesOnGeneration() { - $cm = new \Doctrine\ORM\Mapping\ClassMetadata(__NAMESPACE__ . '\\AbstractClass'); + $cm = new ClassMetadata(__NAMESPACE__ . '\\AbstractClass'); $cm->initializeReflection(new \Doctrine\Common\Persistence\Mapping\RuntimeReflectionService); $this->assertNotNull($cm->reflClass); diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index a16cba368..715c32a5f 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -134,9 +134,6 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\DDC117\DDC117Editor', 'Doctrine\Tests\Models\DDC117\DDC117Link', ), - 'ddc1734' => array( - 'Doctrine\Tests\Models\DDC1734\DDC1734Article', - ), 'stockexchange' => array( 'Doctrine\Tests\Models\StockExchange\Bond', 'Doctrine\Tests\Models\StockExchange\Stock',