[DCOM-96] ProxyFactory logic moved to doctrine common #168

Merged
merged 1 commit into from Jan 10, 2013

Projects

None yet

10 participants

@Ocramius
Doctrine member

Initial requirements

  1. Common proxy generation for:
  2. Generic implementation of proxies: it is now possible to proxy any object as long as a ClassMetadata definition is provided
  3. Public properties lazy loading (yes we can!)

Sample Proxy

See this example generated proxy class

Public properties lazy loading

Public properties lazy loading has been implemented, and I've opened php/php-src#228 to protect the trick discovered by @lsmith77.

Mapped public fields and associations will trigger lazy loading through __get, __set and __isset.

Transient properties and identifier fields won't trigger lazy loading. Same applies for simple identifier getters (as was already happening in ORM since 2.2). I've used the ClassMetadata interface to determine fields and associations to lazy load.

New Proxy interface (not a BC Break)

interface Proxy
{
    const MARKER = '__CG__';
    const MARKER_LENGTH = 6;
    public function __load();
    public function __isInitialized();
    public function __setInitialized($initialized);
    public function __setInitializer(Closure $initializer = null);
    public function __getInitializer();
    public function __setCloner(Closure $cloner = null);
    public function __getCloner();
    public function __getLazyProperties(); // retrieves default properties values - useful when restoring properties
}

Lazy loading via closures

I came up with the idea of using closures to lazy load proxies. This allows removal of any reference to the OjectManager, and makes it possible to dump Proxy objects as long as there are not circular references between them (XDebug solves this).

The default constructor as of the current proxy generator implementation has following signature:

public function __construct(Closure $initializer = null, Closure $cloner = null);

When used by the Proxy, the initializer will be called with object itself as first parameter, the called method name (the one that triggered lazy loading) as second parameter and the parameters that were passed to that method as third parameter.

This allows any of the current implementation to define an initialization logic (even outside the scope of persistence), and to eventually define some finer grained logic to be dispatched for LOB fields or getters/setters.

Proxy generation

Generating a proxy requires that a valid instance of Doctrine\Common\Persistence\Mapping\ClassMetadata is passed to the Doctrine\Common\Proxy\ProxyGenerator. A proxy namespace and a proxy directory are also required to build the ProxyGenerator itself.

The generator uses a template and a list of placeholders mapped to either strings or callables. Those placeholders are used in the template while generating the Proxy class. Every callable mapped as a placeholder is called with the class metadata as first parameter. This should allow some fine-grained customization when the defaults of the proxy generator are not enough.

Autoloading

Autoloader for proxies has been simply copied from the ORM.

Testing

I tested each case that I could come up with. I need suggestions on edge cases that should be documented (like above, when I mentioned setting public properties and the PHP bug).

Directions for MongoDB ODM/PHPCR implementors

I've opened following related PRs:

Performance

  • When having identifier fields being private or protected, the proxy factory is twice as fast in instantiating a proxy compared to before (proxy factory to be seen in doctrine/doctrine2#406)
  • When having identifier fields being public, the proxy factory is twice as slow in instantiating a proxy compared to before. Hydration is also slower on public properties. This can be fixed by excluding the identifier fields from being associated to a Doctrine\Common\Reflection\RuntimePublicReflectionProperty, which is a workaround for PHP Bug 63463. It could also be possible to speed up hydration of proxies by using bulk reflection operation.

That said, allover performance and memory usage is kept almost at same levels (sometimes even faster), but the new feature is introduced.

@schmittjoh
Doctrine member

Just curious, have you thought about using my cg-library? This should allow you to save a lot of code.

I'm using it for generating all sorts of proxies in Symfony2.

@Ocramius
Doctrine member

@schmittjoh never tried it, but I don't know if we can include it here as a dependency...

@stof
Doctrine member

@beberlei's goal is to replace the code generation in Doctrine to use https://github.com/beberlei/DoctrineCodeGenerator so it could probably be used for proxies too

@stof stof commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyException.php
+
+/**
+ * Proxy Exceptions
+ *
+ * @license MIT
+ * @link http://www.doctrine-project.com
+ * @since 2.3
+ * @author Benjamin Eberlei <kontakt@beberlei.de>
+ */
+class ProxyException extends CommonException
+{
+ /**
+ * @static
+ * @return self
+ */
+ public static function proxyDirectoryRequired() {
@stof
stof Jul 22, 2012

CS issue :)

@stof stof and 1 other commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyException.php
+namespace Doctrine\Common\Persistence;
+
+use Doctrine\Common\CommonException;
+
+/**
+ * Proxy Exceptions
+ *
+ * @license MIT
+ * @link http://www.doctrine-project.com
+ * @since 2.3
+ * @author Benjamin Eberlei <kontakt@beberlei.de>
+ */
+class ProxyException extends CommonException
+{
+ /**
+ * @static
@stof
stof Jul 22, 2012

no need for @static. The code already tell us it is static

@guilhermeblanco
guilhermeblanco Jul 23, 2012

@stof some API generators require this. It doesn't hurt.

@stof stof commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * @var string The namespace that contains all proxy classes.
+ */
+ private $proxyNamespace;
+
+ /**
+ * @var string The directory that contains all proxy classes.
+ */
+ private $proxyDir;
+
+ /**
+ * Used to match very simple id methods that don't need
+ * to be proxied since the identifier is known.
+ *
+ * @var string
+ */
+ 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';
@stof
stof Jul 22, 2012

constants should be declared before properties

@Ocramius
Doctrine member

@stof not sure it should be achieved immediately, but yes, that would be a nice one :)

@stof stof commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $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 $class
+ * @return string
+ */
+ private function _generateSleep(ClassMetadata $class)
@stof
stof Jul 22, 2012

please remove the _. It was part of the doctrine1 CS (used for the early development of doctrine2 too) but not of the current ones anymore (new code does not use it anymore, and private methods are renamed when the code is modified in existing classes.

@stof stof and 2 others commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ return false;
+ }
+
+ /**
+ * Generates the code for the __sleep method for a proxy class.
+ *
+ * @param $class
+ * @return string
+ */
+ private function _generateSleep(ClassMetadata $class)
+ {
+ $sleepImpl = '';
+
+ if ($class->getReflectionClass()->hasMethod('__sleep')) {
+ $sleepImpl .= "return array_merge(array('__isInitialized__'), parent::__sleep());";
+ } else {
@stof
stof Jul 22, 2012

you should return early instead of using else (otherwise @guilhermeblanco will be mad 😄)

@Ocramius
Ocramius Jul 22, 2012

His stuff :P I just copied over

@stof stof commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ }
+
+ $sleepImpl .= ');';
+ }
+
+ return $sleepImpl;
+ }
+
+ /** Proxy class code template */
+ protected static $proxyClassTemplate =
+'<?php
+
+namespace <namespace>;
+
+/**
+ * THIS CLASS WAS GENERATED BY THE DOCTRINE ORM. DO NOT EDIT THIS FILE.
@stof
stof Jul 22, 2012

the ORM ? not necessarely

@stof stof commented on an outdated diff Jul 22, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $sleepImpl .= ');';
+ }
+
+ return $sleepImpl;
+ }
+
+ /** Proxy class code template */
+ protected static $proxyClassTemplate =
+'<?php
+
+namespace <namespace>;
+
+/**
+ * THIS CLASS WAS GENERATED BY THE DOCTRINE ORM. DO NOT EDIT THIS FILE.
+ */
+class <proxyClassName> extends \<className> implements \Doctrine\ORM\Proxy\Proxy
@stof
stof Jul 22, 2012

this looks like the wrong interface

@stof
Doctrine member

@Ocramius my link to the CodeGenerator was mainly a reply to the CG suggestion

@lsmith77
Doctrine member

like i said on IRC yesterday .. i would really like to retain the ability to map public properties:
https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Proxy/ProxyFactory.php#L366

@beberlei
Doctrine member
@beberlei
Doctrine member
@Ocramius
Doctrine member

@lsmith77 I am quite against public properties myself in my entities. Having __set (and it's siblings in the PHP docs) implemented by default would be wrong in my use case. Anyway, I'm thinking of allowing to override the template set in the generator, which would add a nice degree of flexibility.
Also it is quite unclear how this public properties mapping would be achieved, but I'm pinging you on IRC for that :)

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyAutoloader.php
+ * 3. Return PHP filename from proxy-dir with the result from 2.
+ *
+ * @param string $proxyDir
+ * @param string $proxyNamespace
+ * @param string $className
+ * @return string
+ */
+ 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';
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing spaces around the last .

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyAutoloader.php
+ {
+ $proxyNamespace = ltrim($proxyNamespace, "\\");
+
+ $loader = function($className) use ($proxyDir, $proxyNamespace, $notFoundCallback) {
+ if (0 === strpos($className, $proxyNamespace)) {
+ $file = ProxyAutoloader::resolveFile($proxyDir, $proxyNamespace, $className);
+
+ if ($notFoundCallback && ! file_exists($file)) {
+ $notFoundCallback($proxyDir, $proxyNamespace, $className);
+ }
+
+ require_once $file;
+ }
+ };
+
+ spl_autoload_register($loader);
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break after this line

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyAutoloader.php
+
+ /**
+ * Register and return 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, "\\");
+
+ $loader = function($className) use ($proxyDir, $proxyNamespace, $notFoundCallback) {
+ if (0 === strpos($className, $proxyNamespace)) {
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Can't we have with normal flow as primary one? We use an alternative flow for entire code execution.
All you need to do is return; and negate this condition.

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * @var \Doctrine\Common\Persistence\ProxyInitializer
+ */
+ private $__proxyInitializer;
+
+ /**
+ * @var mixed
+ */
+ private $__identifier;
+
+ /**
+ * @var bool
+ */
+ public $__isInitialized__ = false;
+
+ public function __construct(\Doctrine\Common\Persistence\ProxyInitializer $proxyInitializer, $identifier)
+ {
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * @var bool
+ */
+ public $__isInitialized__ = false;
+
+ public function __construct(\Doctrine\Common\Persistence\ProxyInitializer $proxyInitializer, $identifier)
+ {
+ $this->__proxyInitializer = $proxyInitializer;
+ $this->__identifier = $identifier;
+ }
+
+ /**
+ * @private
+ */
+ public function __load()
+ {
+ if (!$this->__isInitialized__ && $this->__proxyInitializer) {
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing spaces around !.

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ */
+ public function __isInitialized()
+ {
+ return $this->__isInitialized__;
+ }
+
+ <methods>
+
+ public function __sleep()
+ {
+ <sleepImpl>
+ }
+
+ public function __clone()
+ {
+ if (!$this->__isInitialized__ && $this->__proxyInitializer) {
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing spaces around !.

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ public function __isInitialized()
+ {
+ return $this->__isInitialized__;
+ }
+
+ <methods>
+
+ public function __sleep()
+ {
+ <sleepImpl>
+ }
+
+ public function __clone()
+ {
+ if (!$this->__isInitialized__ && $this->__proxyInitializer) {
+ $this->__isInitialized__ = true;
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Line break missing

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ return $this->__isInitialized__;
+ }
+
+ <methods>
+
+ public function __sleep()
+ {
+ <sleepImpl>
+ }
+
+ public function __clone()
+ {
+ if (!$this->__isInitialized__ && $this->__proxyInitializer) {
+ $this->__isInitialized__ = true;
+ $class = $this->__proxyInitializer->getClassMetadata();
+ $original = $this->__proxyInitializer->initializeProxy($this->__identifier);
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * @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($proxyDir, $proxyNs, $autoGenerate = false)
+ {
+ if ( ! $proxyDir) {
+ throw ProxyException::proxyDirectoryRequired();
+ }
+
+ if ( ! $proxyNs) {
+ throw ProxyException::proxyNamespaceRequired();
+ }
+
+ $this->proxyDir = $proxyDir;
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ throw ProxyException::proxyDirectoryRequired();
+ }
+
+ if ( ! $proxyNs) {
+ throw ProxyException::proxyNamespaceRequired();
+ }
+
+ $this->proxyDir = $proxyDir;
+ $this->autoGenerate = $autoGenerate;
+ $this->proxyNamespace = $proxyNs;
+ }
+
+ /**
+ * Generates the Proxy and retrieves its FQCN
+ *
+ * @param ClassMetadata $class
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing lie break between param and return

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ }
+
+ $this->proxyDir = $proxyDir;
+ $this->autoGenerate = $autoGenerate;
+ $this->proxyNamespace = $proxyNs;
+ }
+
+ /**
+ * Generates the Proxy and retrieves its FQCN
+ *
+ * @param ClassMetadata $class
+ * @return string FQCN of the proxy
+ */
+ public function getProxyClass(ClassMetadata $class)
+ {
+ $className = $class->getName();
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+
+ $this->proxyDir = $proxyDir;
+ $this->autoGenerate = $autoGenerate;
+ $this->proxyNamespace = $proxyNs;
+ }
+
+ /**
+ * Generates the Proxy and retrieves its FQCN
+ *
+ * @param ClassMetadata $class
+ * @return string FQCN of the proxy
+ */
+ public function getProxyClass(ClassMetadata $class)
+ {
+ $className = $class->getName();
+ $fqn = ClassUtils::generateProxyClassName($className, $this->proxyNamespace);
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Rule #5: Do not abbreviate

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $this->autoGenerate = $autoGenerate;
+ $this->proxyNamespace = $proxyNs;
+ }
+
+ /**
+ * Generates the Proxy and retrieves its FQCN
+ *
+ * @param ClassMetadata $class
+ * @return string FQCN of the proxy
+ */
+ public function getProxyClass(ClassMetadata $class)
+ {
+ $className = $class->getName();
+ $fqn = ClassUtils::generateProxyClassName($className, $this->proxyNamespace);
+
+ if (! class_exists($fqn, false)) {
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing space before !.

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ /**
+ * Generates the Proxy and retrieves its FQCN
+ *
+ * @param ClassMetadata $class
+ * @return string FQCN of the proxy
+ */
+ public function getProxyClass(ClassMetadata $class)
+ {
+ $className = $class->getName();
+ $fqn = ClassUtils::generateProxyClassName($className, $this->proxyNamespace);
+
+ if (! class_exists($fqn, false)) {
+ $fileName = $this->getProxyFileName($className);
+
+ if ($this->autoGenerate) {
+ $this->generateProxyClass(
@guilhermeblanco
guilhermeblanco Jul 23, 2012

This line is not long enough to be split into multiple lines

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ }
+
+ /**
+ * Generates proxy classes for all given classes.
+ *
+ * @param ClassMetadata[] $classes The classes (ClassMetadata instances) for which to generate proxies.
+ * @param string $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.
+ * @return int Number of generated proxies.
+ */
+ public function generateProxyClasses(array $classes, $toDir = null)
+ {
+ $proxyDir = $toDir ?: $this->proxyDir;
+ $proxyDir = rtrim($proxyDir, DIRECTORY_SEPARATOR);
+ $num = 0;
@guilhermeblanco guilhermeblanco and 1 other commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * Generates proxy classes for all given classes.
+ *
+ * @param ClassMetadata[] $classes The classes (ClassMetadata instances) for which to generate proxies.
+ * @param string $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.
+ * @return int Number of generated proxies.
+ */
+ public function generateProxyClasses(array $classes, $toDir = null)
+ {
+ $proxyDir = $toDir ?: $this->proxyDir;
+ $proxyDir = rtrim($proxyDir, DIRECTORY_SEPARATOR);
+ $num = 0;
+
+ foreach ($classes as $class) {
+ /* @var $class ClassMetadata */
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Why you need this comment here?

@Ocramius
Ocramius Jul 23, 2012

@guilhermeblanco IDE happiness :) Otherwise I can mark the parameter passed to generateProxyClasses as ClassMetadata[]

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * @return int Number of generated proxies.
+ */
+ public function generateProxyClasses(array $classes, $toDir = null)
+ {
+ $proxyDir = $toDir ?: $this->proxyDir;
+ $proxyDir = rtrim($proxyDir, DIRECTORY_SEPARATOR);
+ $num = 0;
+
+ foreach ($classes as $class) {
+ /* @var $class ClassMetadata */
+ if ($class->getReflectionClass()->isAbstract()) {
+ continue;
+ }
+
+ $proxyFileName = $this->getProxyFileName($class->getName(), $proxyDir);
+ $this->generateProxyClass($class, $proxyFileName, $this->proxyClassTemplate);
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break before and after. They are 3 independent logical blocks.

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ }
+
+ require_once $fileName;
+ }
+
+ return $fqn;
+ }
+
+ /**
+ * Generates proxy classes for all given classes.
+ *
+ * @param ClassMetadata[] $classes The classes (ClassMetadata instances) for which to generate proxies.
+ * @param string $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.
+ * @return int Number of generated proxies.
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $proxyFileName = $this->getProxyFileName($class->getName(), $proxyDir);
+ $this->generateProxyClass($class, $proxyFileName, $this->proxyClassTemplate);
+ $num++;
+ }
+
+ return $num;
+ }
+
+ /**
+ * Generate the Proxy file name
+ *
+ * @param string $className
+ * @param string $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
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ {
+ $proxyDir = $baseDir ?: $this->proxyDir;
+
+ return $proxyDir . DIRECTORY_SEPARATOR . Proxy::MARKER . str_replace('\\', '', $className) . '.php';
+ }
+
+ /**
+ * 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
+ */
+ private function generateProxyClass(ClassMetadata $class, $fileName, $file)
+ {
+ $methods = $this->generateMethods($class);
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+
+ return $proxyDir . DIRECTORY_SEPARATOR . Proxy::MARKER . str_replace('\\', '', $className) . '.php';
+ }
+
+ /**
+ * 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
+ */
+ private function generateProxyClass(ClassMetadata $class, $fileName, $file)
+ {
+ $methods = $this->generateMethods($class);
+ $sleepImpl = $this->generateSleep($class);
+ // hasMethod() checks case-insensitive
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Inline comment at the end of line

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ {
+ $methods = $this->generateMethods($class);
+ $sleepImpl = $this->generateSleep($class);
+ // hasMethod() checks case-insensitive
+ $cloneImpl = $class->getReflectionClass()->hasMethod('__clone') ? 'parent::__clone();' : '';
+
+ $placeholders = array(
+ '<namespace>',
+ '<proxyClassName>',
+ '<className>',
+ '<methods>',
+ '<sleepImpl>',
+ '<cloneImpl>'
+ );
+
+ $className = ltrim($class->getName(), '\\');
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $className = ltrim($class->getName(), '\\');
+ $proxyClassName = ClassUtils::generateProxyClassName($class->getName(), $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);
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $sleepImpl,
+ $cloneImpl
+ );
+
+ $file = str_replace($placeholders, $replacements, $file);
+ $parentDirectory = dirname($fileName);
+
+ if ( ! is_dir($parentDirectory)) {
+ if (false === @mkdir($parentDirectory, 0775, true)) {
+ throw ProxyException::proxyDirectoryNotWritable();
+ }
+ } elseif ( ! is_writable($parentDirectory)) {
+ throw ProxyException::proxyDirectoryNotWritable();
+ }
+
+ $tmpFileName = $fileName . '.' . uniqid("", true);
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break after this line

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ throw ProxyException::proxyDirectoryNotWritable();
+ }
+ } elseif ( ! is_writable($parentDirectory)) {
+ throw ProxyException::proxyDirectoryNotWritable();
+ }
+
+ $tmpFileName = $fileName . '.' . uniqid("", true);
+ file_put_contents($tmpFileName, $file);
+ rename($tmpFileName, $fileName);
+ }
+
+ /**
+ * Generates the methods of a proxy class.
+ *
+ * @param ClassMetadata $class
+ * @return string The code of the generated methods.
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ rename($tmpFileName, $fileName);
+ }
+
+ /**
+ * Generates the methods of a proxy class.
+ *
+ * @param ClassMetadata $class
+ * @return string The code of the generated methods.
+ */
+ private function generateMethods(ClassMetadata $class)
+ {
+ $methods = '';
+ $methodNames = array();
+
+ foreach ($class->getReflectionClass()->getMethods() as $method) {
+ /* @var $method \ReflectionMethod */
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+
+ return $methods;
+ }
+
+ /**
+ * Check 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
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $class->hasField($identifier) &&
+ (($method->getEndLine() - $method->getStartLine()) <= 4)
+ && in_array($class->getTypeOfField($identifier), 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;
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ }
+
+ $parameterString .= '$' . $param->getName();
+ $argumentString .= '$' . $param->getName();
+
+ if ($param->isDefaultValueAvailable()) {
+ $parameterString .= ' = ' . var_export($param->getDefaultValue(), true);
+ }
+ }
+
+ $methods .= $parameterString . ')';
+ $methods .= "\n" . ' {' . "\n";
+
+ // @todo make it possible to skip/override method generation via some parameters?
+ if ($this->isShortIdentifierGetter($method, $class)) {
+ $identifier = lcfirst(substr($method->getName(), 3));
@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $parameterString .= '$' . $param->getName();
+ $argumentString .= '$' . $param->getName();
+
+ if ($param->isDefaultValueAvailable()) {
+ $parameterString .= ' = ' . var_export($param->getDefaultValue(), true);
+ }
+ }
+
+ $methods .= $parameterString . ')';
+ $methods .= "\n" . ' {' . "\n";
+
+ // @todo make it possible to skip/override method generation via some parameters?
+ if ($this->isShortIdentifierGetter($method, $class)) {
+ $identifier = lcfirst(substr($method->getName(), 3));
+ $cast = in_array($class->getTypeOfField($identifier), array('integer', 'smallint')) ? '(int) ' : '';
+ $methods .= ' if ($this->__isInitialized__ === false) {' . "\n";
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Missing line break before this line

@guilhermeblanco guilhermeblanco commented on an outdated diff Jul 23, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ * Generates the code for the __sleep method for a proxy class.
+ *
+ * @param $class
+ * @return string
+ */
+ private function generateSleep(ClassMetadata $class)
+ {
+ $sleepImpl = '';
+
+ if ($class->getReflectionClass()->hasMethod('__sleep')) {
+ $sleepImpl .= "return array_merge(array('__isInitialized__'), parent::__sleep());";
+ } else {
+ $sleepImpl .= "return array('__isInitialized__', ";
+ $first = true;
+
+ foreach ($class->getReflectionClass()->getProperties() as $property) {
@guilhermeblanco
guilhermeblanco Jul 23, 2012

Instead of using and if...else, why don't you build and array of properties and then just implode?

@Ocramius
Doctrine member

There's still some additions to be made to make all this code compatible with what @lsmith77 needs to achieve, then I'll apply the CS fixes about alignment&co (CS fixer FTW!)

@beberlei
Doctrine member

test

@beberlei
Doctrine member

test2

@stof stof commented on an outdated diff Sep 16, 2012
lib/Doctrine/Common/Persistence/ProxyGenerator.php
+ $methods .= ' }' . "\n";
+ }
+
+ $methods .= ' $this->__load();' . "\n";
+ $methods .= ' return parent::' . $method->getName() . '(' . $argumentString . ');';
+ $methods .= "\n" . ' }' . "\n";
+ }
+ }
+
+ return $methods;
+ }
+
+ /**
+ * Check if the method is a short identifier getter.
+ *
+ * What does this mean? For proxy objects the identifier is already known,
@stof
stof Sep 16, 2012

you should remove What does this mean?

@Ocramius Ocramius referenced this pull request in php/php-src Nov 8, 2012
Closed

Regression/unset properties behavior #228

@stof stof and 1 other commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+
+ $className = str_replace('\\', '', substr($className, strlen($proxyNamespace) +1));
+
+ return $proxyDir . DIRECTORY_SEPARATOR . $className.'.php';
+ }
+
+ /**
+ * Register and return 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
+ */
+ public static function register($proxyDir, $proxyNamespace, \Closure $notFoundCallback = null)
@stof
stof Nov 8, 2012

Please don't typehint the closure. Forbidding other callable types does not make much sense (we have this issue in the Collection interface which cannot be changed until 3.0 unfortunately)

@Ocramius
Ocramius Nov 8, 2012

Will duck-type :)

@stof stof commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+
+/**
+ * DO NOT EDIT THIS FILE - IT WAS CREATED BY DOCTRINE\'S PROXY GENERATOR
+ */
+class <proxyClassName> extends \<className> implements \<baseProxyInterface>
+{
+ /**
+ * @var Callable the callback responsible for loading properties in the proxy object. This callback is called with
+ * three parameters, being respectively the proxy object to be initialized, the method that triggered the
+ * initialization process and an array of ordered parameters that were passed to that method.
+ * @see \Doctrine\Common\Persistence\Proxy::__setInitializer
+ */
+ public $__initializer__;
+
+ /**
+ * @var Callable the callback responsible of loading properties that need to be copied in the cloned object
@stof
stof Nov 8, 2012

callable is a type, nto a class, so it should be lowercased

@stof stof commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ * @see \Doctrine\Common\Persistence\Proxy::__isInitialized
+ */
+ public $__isInitialized__ = false;
+
+ /**
+ * @var array public properties to be lazy loaded (with their default values)
+ * @see \Doctrine\Common\Persistence\Proxy::__getLazyLoadedPublicProperties
+ */
+ public static $lazyPublicPropertiesDefaultValues = array(<lazyLoadedPublicPropertiesDefaultValues>);
+
+ /**
+ * @param \Closure $initializer
+ * @param \Closure $cloner
+ * @param mixed[] $identifier
+ */
+ public function __construct(\Closure $initializer, \Closure $cloner, array $identifier)
@stof
stof Nov 8, 2012

please remove the typehint (the right typehint would be callable if we were 5.4+ only)

@stof stof commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ */
+ public function __construct(\Closure $initializer, \Closure $cloner, array $identifier)
+ {
+ <ctorImpl>
+ $this->__initializer__ = $initializer;
+ $this->__cloner__ = $cloner;
+ }
+
+ /**
+ * @private
+ * @deprecated please do not call this method directly anymore.
+ */
+ public function __load()
+ {
+ $cb = $this->__initializer__;
+ $cb($this, \'__load\', array());
@stof
stof Nov 8, 2012

please use call_user_func to allow any callable type on 5.3

@stof stof commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ $values = array();
+
+ foreach ($lazyPublicProperties as $key => $value) {
+ $values[] = var_export($key, true) . ' => ' . var_export($value, true);
+ }
+
+ return implode(', ', $values);
+ }
+
+ /**
+ * Generates the constructor code (un-setting public lazy loaded properties, setting identifier field values)
+ *
+ * @param ClassMetadata $class
+ * @return string
+ */
+ public function generateCtorImpl(ClassMetadata $class)
@stof
stof Nov 8, 2012

Please write constructor, not ctor`

@stof stof and 1 other commented on an outdated diff Nov 8, 2012
...Common/Reflection/RuntimePublicReflectionProperty.php
+ *
+ * @author Marco Pivetta <ocramius@gmail.com>
+ * @since 2.4
+ */
+class RuntimePublicReflectionProperty extends ReflectionProperty
+{
+ /**
+ * {@inheritDoc}
+ *
+ * Checks is the value actually exist before fetching it. This is to avoid calling `__get` on the provided $object
+ */
+ public function getValue($object = null)
+ {
+ $name = $this->getName();
+
+ return isset($object->$name) ? $object->$name : null;
@stof
stof Nov 8, 2012

this will still call __isset, no ?

@Ocramius
Ocramius Nov 8, 2012

Yes, you're right. I considered that, and it's the least of all evils IMO...

@stof
Doctrine member

@Ocramius I think the proxy class should implement __isset, otherwise the magic public properties will not behave well in Twig (or any other place using isset to know if the property exists on the object)

@Ocramius
Doctrine member

@stof I can add __isset, but only if it triggers lazy loading obviously...

@Ocramius
Doctrine member

@stof no, nevermind. I cannot implement __isset, it would break the override I specifically wrote for reflection of public properties.

@stof
Doctrine member

@Ocramius then please check if your proxied public preperties still work in a Twig template.

@Ocramius
Doctrine member

@stof not using twig... Will try

@dbu
Doctrine member
dbu commented Nov 8, 2012
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ * 2. Remove namespace separators 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 InvalidArgumentException
+ */
+ public static function resolveFile($proxyDir, $proxyNamespace, $className)
+ {
+ if (0 !== strpos($className, $proxyNamespace)) {
+ throw InvalidArgumentException::notProxyClass($className, $proxyNamespace);
+ }
+
+ $className = str_replace('\\', '', substr($className, strlen($proxyNamespace) +1));
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing space after +.

@guilhermeblanco guilhermeblanco and 1 other commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ *
+ * @param string $proxyDir
+ * @param string $proxyNamespace
+ * @param string $className
+ * @return string
+ * @throws InvalidArgumentException
+ */
+ public static function resolveFile($proxyDir, $proxyNamespace, $className)
+ {
+ if (0 !== strpos($className, $proxyNamespace)) {
+ throw InvalidArgumentException::notProxyClass($className, $proxyNamespace);
+ }
+
+ $className = str_replace('\\', '', substr($className, strlen($proxyNamespace) +1));
+
+ return $proxyDir . DIRECTORY_SEPARATOR . $className.'.php';
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing space around . at the last concatenation.

@stof
stof Nov 8, 2012

and why not using / simply ? It is no an issue for filenames as we are on 5.3+

@guilhermeblanco
guilhermeblanco Nov 8, 2012

If we hardcode the /, then I'd suggest changing this line to a sprintf().

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+
+ /**
+ * Register and return autoloader callback for the given proxy dir and
+ * namespace.
+ *
+ * @param string $proxyDir
+ * @param string $proxyNamespace
+ * @param callable $notFoundCallback Invoked when the proxy file is not found.
+ * @return \Closure
+ * @throws InvalidArgumentException
+ */
+ public static function register($proxyDir, $proxyNamespace, $notFoundCallback = null)
+ {
+ $proxyNamespace = ltrim($proxyNamespace, '\\');
+
+ if (!(null === $notFoundCallback || is_callable($notFoundCallback))) {
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing space around !.

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ if (!(null === $notFoundCallback || is_callable($notFoundCallback))) {
+ throw InvalidArgumentException::invalidClassNotFoundCallback($notFoundCallback);
+ }
+
+ $autoloader = function ($className) use ($proxyDir, $proxyNamespace, $notFoundCallback) {
+ if (0 === strpos($className, $proxyNamespace)) {
+ $file = Autoloader::resolveFile($proxyDir, $proxyNamespace, $className);
+
+ if ($notFoundCallback && ! file_exists($file)) {
+ call_user_func($notFoundCallback, $proxyDir, $proxyNamespace, $className);
+ }
+
+ require $file;
+ }
+ };
+ spl_autoload_register($autoloader);
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing line break before the function call

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ public $__isInitialized__ = false;
+
+ /**
+ * @var array public properties to be lazy loaded (with their default values)
+ * @see \Doctrine\Common\Persistence\Proxy::__getLazyLoadedPublicProperties
+ */
+ public static $lazyPublicPropertiesDefaultValues = array(<lazyLoadedPublicPropertiesDefaultValues>);
+
+ /**
+ * @param callable $initializer
+ * @param callable $cloner
+ * @param mixed[] $identifier
+ */
+ public function __construct($initializer, $cloner, array $identifier)
+ {
+ <constructorImpl>
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ /*<magicSet>*/
+
+ public function __sleep()
+ {
+ <sleepImpl>
+ }
+
+ public function __wakeup()
+ {
+ <wakeupImpl>
+ }
+
+ public function __clone()
+ {
+ call_user_func($this->__cloner__, $this, \'__clone\', array());
+ <cloneImpl>
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+
+ <methods>
+}
+';
+
+ /**
+ * Initializes a new instance of the <tt>ProxyFactory</tt> class that is
+ * connected to the given <tt>EntityManager</tt>.
+ *
+ * @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.
+ * @throws InvalidArgumentException
+ */
+ public function __construct($proxyDir, $proxyNs)
+ {
+ if (! $proxyDir) {
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing space before !.

@guilhermeblanco guilhermeblanco and 1 other commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+
+ /**
+ * Initializes a new instance of the <tt>ProxyFactory</tt> class that is
+ * connected to the given <tt>EntityManager</tt>.
+ *
+ * @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.
+ * @throws InvalidArgumentException
+ */
+ public function __construct($proxyDir, $proxyNs)
+ {
+ if (! $proxyDir) {
+ throw InvalidArgumentException::proxyDirectoryRequired();
+ }
+
+ if (! $proxyNs) {
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing space before !.

@cordoval
cordoval Nov 8, 2012

please make more explicit that N

@stof stof and 2 others commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ *
+ * @author Benjamin Eberlei <kontakt@beberlei.de>
+ */
+class Autoloader
+{
+ /**
+ * Resolves proxy class name to a filename based on the following pattern.
+ *
+ * 1. Remove Proxy namespace from class name
+ * 2. Remove namespace separators 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
@stof
stof Nov 8, 2012

missing linebreak between the params and return

@cordoval
cordoval Nov 8, 2012

nice so this is PSR-0?

@guilhermeblanco
guilhermeblanco Nov 8, 2012

It's our awesome CS... not PSR-0. =)

@cordoval
cordoval Nov 8, 2012

tell me what level of php-cs-fixer are you applying?, our === doctrine?

@stof
stof Nov 8, 2012

@cordoval The php-cs-fixer does not enforce this blank line. but it will align the grouped annotations. Separating params and returns allow to align them separately (if you return \Doctrine\Common\Collections\Collection, it will avoid moving the param names far away on the right

@stof
stof Nov 8, 2012

@guilhermeblanco s/PSR-0/PSR1.5 (PSR-2 for the new code but there is a garbage of legacy which cannot be fully converted to PSR-2 without breaking BC for child classes)

@stof stof and 1 other commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Persistence/Proxy.php
*/
-interface Proxy
+interface Proxy extends BaseProxy
@stof
stof Nov 8, 2012

this is a BC break as the new method contains more methods, meaning all O*M projects will have to be updated before merging

@Ocramius
Ocramius Nov 9, 2012

I'm aware of that. I'm missing PHPCR and CouchDB ODM currently. MongoDB ODM has an open PR for this at doctrine/mongodb-odm#445

@stof
stof Nov 9, 2012

but anyway, breaking BC is a no-go until 3.0

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ throw InvalidArgumentException::notProxyClass($className, $proxyNamespace);
+ }
+
+ $className = str_replace('\\', '', substr($className, strlen($proxyNamespace) +1));
+
+ return $proxyDir . DIRECTORY_SEPARATOR . $className.'.php';
+ }
+
+ /**
+ * Register and return autoloader callback for the given proxy dir and
+ * namespace.
+ *
+ * @param string $proxyDir
+ * @param string $proxyNamespace
+ * @param callable $notFoundCallback Invoked when the proxy file is not found.
+ * @return \Closure
@guilhermeblanco
guilhermeblanco Nov 8, 2012

Missing line break before and after @return.

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 8, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+
+ /**
+ * @return self
+ */
+ public static function proxyNamespaceRequired()
+ {
+ return new self("You must configure a proxy namespace. See docs for details");
+ }
+
+ /**
+ * @param mixed $callback
+ * @return self
+ */
+ public static function invalidClassNotFoundCallback($callback)
+ {
+ $type = is_object($callback) ? get_class($callback) : getType($callback);
@cordoval cordoval commented on an outdated diff Nov 8, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ * @author Marco Pivetta <ocramius@gmail.com>
+ */
+class InvalidArgumentException extends BaseInvalidArgumentException implements ProxyException
+{
+ /**
+ * @return self
+ */
+ public static function proxyDirectoryRequired()
+ {
+ return new self("You must configure a proxy directory. See docs for details");
+ }
+
+ /**
+ * @param string $className
+ * @param string $proxyNamespace
+ * @return self
@cordoval
cordoval Nov 8, 2012

missing space before param section and return statement on docblock

@cordoval cordoval commented on an outdated diff Nov 8, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ return new self("You must configure a proxy directory. See docs for details");
+ }
+
+ /**
+ * @param string $className
+ * @param string $proxyNamespace
+ * @return self
+ */
+ public static function notProxyClass($className, $proxyNamespace)
+ {
+ return new self('The class "' . $className . '" is not part of the proxy namespace "' . $proxyNamespace . '"');
+ }
+
+ /**
+ * @param string $name
+ * @return self
@cordoval
cordoval Nov 8, 2012

missing line break

@cordoval cordoval commented on an outdated diff Nov 8, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ public static function invalidPlaceholder($name)
+ {
+ return new self('Provided placeholder for "' . $name . '" must be either a string or a valid callable');
+ }
+
+ /**
+ * @return self
+ */
+ public static function proxyNamespaceRequired()
+ {
+ return new self("You must configure a proxy namespace. See docs for details");
+ }
+
+ /**
+ * @param mixed $callback
+ * @return self
@cordoval
cordoval Nov 8, 2012

missing line break

@cordoval cordoval commented on an outdated diff Nov 8, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+
+ /**
+ * @param string $name
+ * @return self
+ */
+ public static function invalidPlaceholder($name)
+ {
+ return new self('Provided placeholder for "' . $name . '" must be either a string or a valid callable');
+ }
+
+ /**
+ * @return self
+ */
+ public static function proxyNamespaceRequired()
+ {
+ return new self("You must configure a proxy namespace. See docs for details");
@cordoval
cordoval Nov 8, 2012

See docs for details? if you are not giving an url i think this should be removed

@cordoval cordoval commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/Proxy.php
+ * @author Roman Borschel <roman@code-factory.org>
+ * @since 2.2
+ */
+interface Proxy
+{
+ /**
+ * Marker for Proxy class names.
+ *
+ * @var string
+ */
+ const MARKER = '__CG__';
+
+ /**
+ * Length of the proxy marker
+ *
+ * @var int
@cordoval cordoval commented on the diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ private $proxyNamespace;
+
+ /**
+ * @var string The directory that contains all proxy classes.
+ */
+ private $proxyDir;
+
+ /**
+ * @var string[]|callable[] map of callables used to fill in placeholders set in the template
+ */
+ protected $placeholders = array();
+
+ /**
+ * @var string template used as a blueprint to generate proxies
+ */
+ protected $proxyClassTemplate = '<?php
@cordoval
cordoval Nov 8, 2012

why not rather using

protected $proxyClassTemplate = <<<PHPTEMPLATE

<?php

here your php code

PHPTEMPLATE;

$moreCode = ...;
@Ocramius
Ocramius Nov 9, 2012

Screws with a lot of IDEs. I'd prefer not doing that.

@cordoval cordoval and 1 other commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ * {@inheritDoc}
+ * @private
+ */
+ public function __setCloner($cloner)
+ {
+ $this->__cloner__ = $cloner;
+ }
+
+ public function __getLazyLoadedPublicProperties()
+ {
+ return self::$lazyPublicPropertiesDefaultValues;
+ }
+
+ <magicGet>
+
+ /*<magicSet>*/
@cordoval
cordoval Nov 8, 2012

why commenting a placeholder?

@Ocramius
Ocramius Nov 9, 2012

That's because __set isn't active right now. I'm still thinking about it

@cordoval cordoval commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ foreach ($placeholders as $name => $value) {
+ $this->setPlaceholder($name, $value);
+ }
+ }
+
+ /**
+ * Set a placeholder to be replaced in the template
+ *
+ * @param string $name
+ * @param string|callable $placeholder
+ * @throws InvalidArgumentException
+ */
+ public function setPlaceholder($name, $placeholder)
+ {
+ if (!is_string($placeholder) && !is_callable($placeholder)) {
@cordoval
cordoval Nov 8, 2012

hmm should you need a space away from the !?

@cordoval cordoval commented on the diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ * @param string|callable $placeholder
+ * @throws InvalidArgumentException
+ */
+ public function setPlaceholder($name, $placeholder)
+ {
+ if (!is_string($placeholder) && !is_callable($placeholder)) {
+ throw InvalidArgumentException::invalidPlaceholder($name);
+ }
+
+ $this->placeholders[$name] = $placeholder;
+ }
+
+ /**
+ * Generate the Proxy class name
+ *
+ * @param string $originalClassName
@cordoval cordoval commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ /**
+ * Generate the Proxy class name
+ *
+ * @param string $originalClassName
+ * @return string the FQCN class name of the proxy. If the proxy does not exist, it is generated
+ */
+ public function getProxyClassName($originalClassName)
+ {
+ return ClassUtils::generateProxyClassName($originalClassName, $this->proxyNamespace);
+ }
+
+ /**
+ * Generates the proxy short class name
+ *
+ * @param ClassMetadata $class
+ * @return string
@cordoval cordoval commented on an outdated diff Nov 8, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ *
+ * @param ClassMetadata $class
+ * @return string
+ */
+ public function generateProxyClassName(ClassMetadata $class)
+ {
+ $proxyClassName = ClassUtils::generateProxyClassName($class->getName(), $this->proxyNamespace);
+ $parts = explode('\\', strrev($proxyClassName), 2);
+
+ return strrev($parts[0]);
+ }
+
+ /**
+ * Generates the proxy namespace
+ *
+ * @param ClassMetadata $class
@Ocramius
Doctrine member

@dbu yes, I saw that PHPCR ODM implements __isset, but it's not really applicable, since before loading you won't know which properties were nulled.

@cordoval cordoval commented on the diff Nov 8, 2012
.../Persistence/Mapping/RuntimeReflectionServiceTest.php
@@ -65,6 +70,9 @@ public function testGetAccessibleProperty()
{
$reflProp = $this->reflectionService->getAccessibleProperty(__CLASS__, "reflectionService");
$this->assertInstanceOf("ReflectionProperty", $reflProp);
+
+ $reflProp = $this->reflectionService->getAccessibleProperty(__CLASS__, "unusedPublicProperty");
+ $this->assertInstanceOf("Doctrine\Common\Reflection\RuntimePublicReflectionProperty", $reflProp);
@cordoval
cordoval Nov 8, 2012

why are you not using // //

@stof
Doctrine member

@Ocramius What Twig need to know is whether the property exists.

@cordoval cordoval commented on an outdated diff Nov 8, 2012
.gitignore
@@ -2,3 +2,4 @@ build/
logs/
reports/
dist/
+tests/Doctrine/Tests/Common/Proxy/generated/*
@cordoval
cordoval Nov 8, 2012

why the * if you are going to ignore everything in that folder?

@Ocramius Ocramius added a commit to Ocramius/mongodb-odm that referenced this pull request Nov 9, 2012
@Ocramius Ocramius DCOM-96 compliance as of doctrine/common#168 7721c21
@Ocramius Ocramius referenced this pull request in doctrine/mongodb-odm Nov 9, 2012
Closed

[DCOM-96] proxy generation as of doctrine/common#168 #445

@Ocramius
Doctrine member

@stof I'm aware of how twig would work with that. The problem is that by implementing __isset I can currently only write a version that does not trigger lazy loading (see https://github.com/doctrine/phpcr-odm/blob/c0664db84d956759533bedda803ead7254913eba/lib/Doctrine/ODM/PHPCR/Proxy/ProxyFactory.php#L363-366 ). This is broken anyway, since __isset will just report true without actually checking the field value. I also would break hydration since reflection would trigger lazy loading when checking properties.

@cordoval cordoval and 1 other commented on an outdated diff Nov 10, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ *
+ * @return string the FQCN class name of the proxy. If the proxy does not exist, it is generated
+ */
+ public function getProxyClassName($originalClassName)
+ {
+ return ClassUtils::generateProxyClassName($originalClassName, $this->proxyNamespace);
+ }
+
+ /**
+ * Generates the proxy short class name
+ *
+ * @param ClassMetadata $class
+ *
+ * @return string
+ */
+ public function generateProxyClassName(ClassMetadata $class)
@cordoval
cordoval Nov 10, 2012

@Ocramius just putting a note here for pending taking ClassMetadata to the next level to be independent of persistence and to be used outside doctrine such as e.g. Symfony2 👍

@stof
stof Nov 10, 2012

@Ocramius couldn't you pass the class name instead of the ClassMetadata (as all you do is $class->getName()) to be independant from Persistence ?

@stof
stof Nov 10, 2012

btw, the name is quite confusing compared with getProxyClassName. You should try to find a better name

@Ocramius
Doctrine member

@stof I removed the BC on the interface.

Found another (quite big) problem while trying to implement __set btw... looks like I'll have to restore the __identifier__ property, since setting the identifier directly does not work when it is private :(

@stof
Doctrine member

@Ocramius set it through reflection ?

@cordoval cordoval and 1 other commented on an outdated diff Nov 10, 2012
tests/Doctrine/Tests/Common/Proxy/ProxyLogicTest.php
+ }
+
+ $this->lazyObject = new LazyLoadableObjectProxy($initializer, $cloner, $identifier);
+
+ foreach ($metadata->getIdentifierFieldNames() as $idField) {
+ $prop = $reflClass->getProperty($idField);
+ $prop->setAccessible(true);
+ $prop->setValue($this->lazyObject, $identifier[$idField]);
+ }
+
+ $this->assertFalse($this->lazyObject->__isInitialized());
+ }
+
+ public function testFetchingPublicIdentifierDoesNotCauseLazyLoading()
+ {
+ $cb = $this->getMock('stdClass', array('cb'));
@cordoval
cordoval Nov 10, 2012

what cb stands for? maybe make a comment?

@Ocramius
Ocramius Nov 12, 2012

'callback' :) I think that one is clear enough

@cordoval cordoval and 1 other commented on an outdated diff Nov 10, 2012
lib/Doctrine/Common/Proxy/Proxy.php
+
+use Doctrine\Common\Persistence\Proxy as BaseProxy;
+
+/**
+ * Interface for proxy classes.
+ *
+ * @author Roman Borschel <roman@code-factory.org>
+ * @author Marco Pivetta <ocramius@gmail.com>
+ * @since 2.4
+ */
+interface Proxy extends BaseProxy
+{
+ /**
+ * Marks the proxy as initialized or not.
+ *
+ * @param bool $initialized
@cordoval cordoval and 1 other commented on an outdated diff Nov 10, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ /**
+ * @var callable the callback responsible for loading properties in the proxy object. This callback is called with
+ * three parameters, being respectively the proxy object to be initialized, the method that triggered the
+ * initialization process and an array of ordered parameters that were passed to that method.
+ * @see \Doctrine\Common\Persistence\Proxy::__setInitializer
+ */
+ public $__initializer__;
+
+ /**
+ * @var callable the callback responsible of loading properties that need to be copied in the cloned object
+ * @see \Doctrine\Common\Persistence\Proxy::__setCloner
+ */
+ public $__cloner__;
+
+ /**
+ * @var bool flag indicating if this object was already initialized
@cordoval cordoval commented on an outdated diff Nov 10, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ <sleepImpl>
+ }
+
+ public function __wakeup()
+ {
+ <wakeupImpl>
+ }
+
+ public function __clone()
+ {
+ call_user_func($this->__cloner__, $this, \'__clone\', array());
+
+ <cloneImpl>
+ }
+
@cordoval
cordoval Nov 10, 2012

maybe even remove the spaces or remove the spaces on the factory where the generator is configured because they there have already the "\n"

@cordoval
cordoval Nov 10, 2012

otherwise you create useless spaces

@cordoval cordoval commented on an outdated diff Nov 10, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ *
+ * @param ClassMetadata $class
+ *
+ * @return string
+ */
+ public function generateConstructorImpl(ClassMetadata $class)
+ {
+ $toUnset = array();
+
+ foreach ($this->getLazyLoadedPublicProperties($class) as $lazyPublicProperty => $unused) {
+ $toUnset[] = '$this->' . $lazyPublicProperty;
+ }
+
+ $constructorImpl = empty($toUnset) ? '' : 'unset(' . implode(', ', $toUnset) . ");\n";
+
+ /*foreach ($class->getIdentifierFieldNames() as $identifierField) {
@cordoval
cordoval Nov 10, 2012

make sure you don't leave this in

@beberlei beberlei and 1 other commented on an outdated diff Nov 12, 2012
tests/Doctrine/Tests/Common/Proxy/ProxyLogicTest.php
+ ->expects($this->once())
+ ->method('cb')
+ ->with($this->lazyObject, '__get', array('publicPersistentField'))
+ ->will($this->returnCallback(function () use ($lazyObject, $cb) {
+ $lazyObject->__setInitializer(function () {});
+ $lazyObject->publicPersistentField = 'loadedValue';
+ $lazyObject->publicAssociation = 'publicAssociationValue';
+ $lazyObject->__setInitializer(array($cb, 'cb'));
+ }));
+ $this->lazyObject->__setInitializer(array($cb, 'cb'));
+
+ $this->assertSame('loadedValue', $this->lazyObject->publicPersistentField);
+ $this->assertSame('publicAssociationValue', $this->lazyObject->publicAssociation);
+ }
+
+ public function testErrorWhenReadingNonExistentPublicProperties()
@beberlei
beberlei Nov 12, 2012

PHP behavior here is to throw a NOTICE, can we emulate that with E_USER_NOTICE instead?

@Ocramius
Ocramius Nov 12, 2012

Will be done :)

@Ocramius Ocramius commented on an outdated diff Nov 12, 2012
...ctrine/Tests/Common/Proxy/ProxyClassGeneratorTest.php
+ * @var string
+ */
+ protected $proxyClass = 'Doctrine\Tests\Common\ProxyProxy\__CG__\LazyLoadableObject';
+
+ /**
+ * {@inheritDoc}
+ */
+ protected function setUp()
+ {
+ if (class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\LazyLoadableObject', false)) {
+ return;
+ }
+
+ require_once __DIR__ . '/fixtures/LazyLoadableObject.php';
+
+ $metadata = $this->getMock('Doctrine\Common\Persistence\Mapping\ClassMetadata');
@Ocramius
Ocramius Nov 12, 2012

Reminder - to be moved to test asset instead of mock, since this creeps the hell out of @beberlei :P

@Ocramius
Doctrine member

@stof I'll be type-hinting Closure since there's performance implication related to using call_user_func. I think it's an acceptable limitation, since anybody wanting to pass in a callable can simply wrap it in a closure:


$callable = 'something::method';
$c = function () use ($callable) {
    call_user_func($callable);
}

$proxy->__setInitializer($c);

See test script at https://gist.github.com/72f95cc0801b4e2cb568, producing http://i.imgur.com/kfX8w.png

@schmittjoh
Doctrine member

I have to say this once more, it really hurts to see 2.6k lines of code dedicated to inventing yet another code generation library instead of re-using something that exists already.

And by something I don't necessarily mean my library, but the fact that there has not even been an attempt to re-use something is really sad :(

@Ocramius
Doctrine member

@schmittjoh perfectly understandable, but the code generation part is probably the smallest problem here, heh :|

@schmittjoh
Doctrine member

We can also talk more specifically about proxy generation as a subset of code generation.

If there just is no library for that, that's ok, but if there is not even an attempt to research a library, and try it, I don't understand that. imho it's a wrong signal, and it's just sad :/

@Ocramius
Doctrine member

@schmittjoh I was tempted to use another lib many many times, but the fact is that doctrine/common is a dependency of "world" currently

@stof stof and 3 others commented on an outdated diff Nov 15, 2012
lib/Doctrine/Common/Proxy/Proxy.php
+
+ /**
+ * Retrieves the initializer callback used to initialize the proxy.
+ * @see __setInitializer
+ *
+ * @return callable
+ */
+ public function __getInitializer();
+
+ /**
+ * Set the callback to be used when cloning the proxy. That initializer should accept
+ * a single parameter, which is the cloned proxy instance itself
+ *
+ * @param callable $cloner
+ */
+ public function __setCloner(Closure $cloner = null);
@stof
stof Nov 15, 2012

you should remove the typehint to allow any callable (the right typehint would be callable but it would bump the requirement to PHP 5.4)

@Ocramius
Ocramius Nov 15, 2012

Right, forgot about those :) Will be done!

@beberlei
beberlei Nov 15, 2012

No we want Closure here explicitly, because that makes all the code var_dumpable without problems.

@Ocramius
Ocramius Nov 15, 2012

Ah, sorry, I mean I'll fix the @param callable $cloner part :)

@stof the point in closures is exactly to avoid problems with dumping or accessing the objects in here...
Using call_user_func (for compatibility with 5.3) in 5.4 is also much slower than just invoking the closure, which is an additional point in favor of allowing closures only.

@cordoval
cordoval Nov 15, 2012

@Ocramius hmm could you explain me the use case with var_dumping?

@stof stof commented on an outdated diff Nov 15, 2012
lib/Doctrine/Common/Proxy/Proxy.php
+ /**
+ * Marks the proxy as initialized or not.
+ *
+ * @param bool $initialized
+ */
+ public function __setInitialized($initialized);
+
+ /**
+ * Set the initializer callback to be used when initializing the proxy. That
+ * initializer should accept 3 parameters: $proxy, $method and $params. Those
+ * are respectively the proxy object that is being initialized, the method name
+ * that triggered initialization and the parameters passed to that method
+ *
+ * @param callable $initializer
+ */
+ public function __setInitializer(Closure $initializer = null);
@stof
stof Nov 15, 2012

the typehint should be removed to allow any callable

@Ocramius
Doctrine member
@guilhermeblanco
Doctrine member

Let me double review first

@Ocramius Ocramius referenced this pull request in zendframework/zendframework Nov 17, 2012
Closed

service proxies / lazy services #2995

@Ocramius
Doctrine member
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...mmon/Persistence/Mapping/RuntimeReflectionService.php
@@ -41,32 +39,27 @@ public function getParentClasses($class)
}
/**
- * Return the shortname of a class.
- *
- * @param string $class
- * @return string
+ * {@inheritDoc}
*/
public function getClassShortName($class)
{
$r = new ReflectionClass($class);
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Never ever abbreviate variables, methods, etc. Use $reflection.

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...mmon/Persistence/Mapping/RuntimeReflectionService.php
return $r->getShortName();
}
/**
- * @param string $class
- * @return string
+ * {@inheritDoc}
*/
public function getClassNamespace($class)
{
$r = new ReflectionClass($class);
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Never ever abbreviate variables, methods, etc. Use $reflection.

@guilhermeblanco guilhermeblanco commented on the diff Nov 22, 2012
lib/Doctrine/Common/Proxy/Autoloader.php
+ * @param string $proxyNamespace
+ * @param callable $notFoundCallback Invoked when the proxy file is not found.
+ *
+ * @return \Closure
+ *
+ * @throws InvalidArgumentException
+ */
+ public static function register($proxyDir, $proxyNamespace, $notFoundCallback = null)
+ {
+ $proxyNamespace = ltrim($proxyNamespace, '\\');
+
+ if ( ! (null === $notFoundCallback || is_callable($notFoundCallback))) {
+ throw InvalidArgumentException::invalidClassNotFoundCallback($notFoundCallback);
+ }
+
+ $autoloader = function ($className) use ($proxyDir, $proxyNamespace, $notFoundCallback) {
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Curly brace should be on a new line.

@stof
stof Nov 22, 2012

not for closures, at least in PSR-2

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+
+/**
+ * Proxy Invalid Argument Exception
+ *
+ * @link www.doctrine-project.com
+ * @since 2.4
+ * @author Marco Pivetta <ocramius@gmail.com>
+ */
+class InvalidArgumentException extends BaseInvalidArgumentException implements ProxyException
+{
+ /**
+ * @return self
+ */
+ public static function proxyDirectoryRequired()
+ {
+ return new self("You must configure a proxy directory. See docs for details");
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Use single quotes.

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ * @return self
+ */
+ public static function proxyDirectoryRequired()
+ {
+ return new self("You must configure a proxy directory. See docs for details");
+ }
+
+ /**
+ * @param string $className
+ * @param string $proxyNamespace
+ *
+ * @return self
+ */
+ public static function notProxyClass($className, $proxyNamespace)
+ {
+ return new self('The class "' . $className . '" is not part of the proxy namespace "' . $proxyNamespace . '"');
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Use sprintf for this

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ *
+ * @return self
+ */
+ public static function notProxyClass($className, $proxyNamespace)
+ {
+ return new self('The class "' . $className . '" is not part of the proxy namespace "' . $proxyNamespace . '"');
+ }
+
+ /**
+ * @param string $name
+ *
+ * @return self
+ */
+ public static function invalidPlaceholder($name)
+ {
+ return new self('Provided placeholder for "' . $name . '" must be either a string or a valid callable');
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ /**
+ * @param string $name
+ *
+ * @return self
+ */
+ public static function invalidPlaceholder($name)
+ {
+ return new self('Provided placeholder for "' . $name . '" must be either a string or a valid callable');
+ }
+
+ /**
+ * @return self
+ */
+ public static function proxyNamespaceRequired()
+ {
+ return new self("You must configure a proxy namespace");
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
...e/Common/Proxy/Exception/InvalidArgumentException.php
+ */
+ public static function proxyNamespaceRequired()
+ {
+ return new self("You must configure a proxy namespace");
+ }
+
+ /**
+ * @param mixed $callback
+ *
+ * @return self
+ */
+ public static function invalidClassNotFoundCallback($callback)
+ {
+ $type = is_object($callback) ? get_class($callback) : gettype($callback);
+
+ return new self('Invalid \$notFoundCallback given: must be a callable, "' . $type . '" given');
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ * @var string template used as a blueprint to generate proxies
+ */
+ protected $proxyClassTemplate = '<?php
+
+namespace <namespace>;
+
+/**
+ * DO NOT EDIT THIS FILE - IT WAS CREATED BY DOCTRINE\'S PROXY GENERATOR
+ */
+class <proxyClassName> extends \<className> implements \<baseProxyInterface>
+{
+ /**
+ * @var \Closure the callback responsible for loading properties in the proxy object. This callback is called with
+ * three parameters, being respectively the proxy object to be initialized, the method that triggered the
+ * initialization process and an array of ordered parameters that were passed to that method.
+ * @see \Doctrine\Common\Persistence\Proxy::__setInitializer
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+/**
+ * DO NOT EDIT THIS FILE - IT WAS CREATED BY DOCTRINE\'S PROXY GENERATOR
+ */
+class <proxyClassName> extends \<className> implements \<baseProxyInterface>
+{
+ /**
+ * @var \Closure the callback responsible for loading properties in the proxy object. This callback is called with
+ * three parameters, being respectively the proxy object to be initialized, the method that triggered the
+ * initialization process and an array of ordered parameters that were passed to that method.
+ * @see \Doctrine\Common\Persistence\Proxy::__setInitializer
+ */
+ public $__initializer__;
+
+ /**
+ * @var \Closure the callback responsible of loading properties that need to be copied in the cloned object
+ * @see \Doctrine\Common\Persistence\Proxy::__setCloner
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+
+ /**
+ * @var \Closure the callback responsible of loading properties that need to be copied in the cloned object
+ * @see \Doctrine\Common\Persistence\Proxy::__setCloner
+ */
+ public $__cloner__;
+
+ /**
+ * @var bool flag indicating if this object was already initialized
+ * @see \Doctrine\Common\Persistence\Proxy::__isInitialized
+ */
+ public $__isInitialized__ = false;
+
+ /**
+ * @var array public properties to be lazy loaded (with their default values)
+ * @see \Doctrine\Common\Persistence\Proxy::__getLazyLoadedPublicProperties
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Missing line break

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+
+<magicGet>
+
+<magicSet>
+
+<magicIsset>
+
+<sleepImpl>
+
+<wakeupImpl>
+
+<cloneImpl>
+
+ /**
+ * Forces initialization of the proxy
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ $this->__initializer__ && $this->__initializer__->__invoke($this, \'__load\', array());
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __isInitialized()
+ {
+ return $this->__isInitialized__;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco
guilhermeblanco Nov 22, 2012

Remove. Again, add an internal message

@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ return $this->__isInitialized__;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __setInitialized($initialized)
+ {
+ $this->__isInitialized__ = $initialized;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ $this->__isInitialized__ = $initialized;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __setInitializer(\Closure $initializer = null)
+ {
+ $this->__initializer__ = $initializer;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ $this->__initializer__ = $initializer;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __getInitializer()
+ {
+ return $this->__initializer__;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ return $this->__initializer__;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __setCloner(\Closure $cloner = null)
+ {
+ $this->__cloner__ = $cloner;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ {
+ $this->__cloner__ = $cloner;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
+ */
+ public function __getCloner()
+ {
+ return $this->__cloner__;
+ }
+
+ /**
+ * {@inheritDoc}
+ * @private
@guilhermeblanco guilhermeblanco commented on an outdated diff Nov 22, 2012
lib/Doctrine/Common/Proxy/ProxyGenerator.php
+ public function __getLazyLoadedPublicProperties()
+ {
+ return self::$lazyPublicPropertiesDefaultValues;
+ }
+
+ <methods>
+}
+';
+
+ /**
+ * Initializes a new instance of the <tt>ProxyFactory</tt> class that is
+ * connected to the given <tt>EntityManager</tt>.
+ *
+ * @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.
+ * @throws InvalidArgumentException