[DCOM-189] Doctrine Proxies may conflict with interfaced constructors Created: 03/May/13  Updated: 26/Mar/15

Status: Reopened
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: Harmen M Assignee: Marco Pivetta
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The Doctrine ProxyGenerator generates for a proxy a constructor. The documentation of Doctrine states that the constructor is never called. For a project, I created a group of entities with a interfaced constructor in order to enforce a common interface. This results in a incompatible proxy and so a fatal error.



 Comments   
Comment by Marco Pivetta [ 03/May/13 ]

Cannot fix this - the constructor is required to override instantiation logic

Comment by Harmen M [ 03/May/13 ]

Edit: added the correct description. I accidentially submitted the form before editing the description.

Comment by Marco Pivetta [ 03/May/13 ]

Harmen M why do you have a constructor in an interface? That's a very bad practice, and it makes things quite hard to handle.

I can think of a workaround, but I first want to be sure there's a real advantage in changing the current implementation to use

unserialize()

just to handle this specific use case.

Comment by Benjamin Eberlei [ 03/May/13 ]

Adding __construct to an interface is an anti pattern and shouldn't be done.

Comment by Harmen M [ 03/May/13 ]

Ok, then I change my implementation.

But, maybe it is an idea to update the documentation of the ORM and state that constructor interfacing is not possible?
http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/architecture.html

Comment by Marco Pivetta [ 18/Dec/13 ]

I actually had more requests for this feature on a similar project ( https://github.com/Ocramius/ProxyManager/issues/115 ).
I will mark this as feature request, but can't guarantee that it will get into Doctrine 2.x, since it may be a BC break.

Comment by Guilherme Blanco [ 24/Apr/14 ]

Creating interfaces with __construct ties your contract preventing extensibility points. This is nature of OOP and I do not consider this should be documented because it's (in-theory) expected that people have some level of knowledge in OO design when coding.

Closing this as invalid.

Comment by Marco Pivetta [ 24/Apr/14 ]

Re-opening.

While interfaced constructors are a known bad practice, changing constructor parameters is also a well known LSP violation (a minor one).

I'll keep tracking this, but I'm blocked by HHVM's missing Closure::bind() as of https://github.com/facebook/hhvm/issues/1203

Comment by Marco Pivetta [ 26/Mar/15 ]

Note: won't be solved in 2.5.0.

As a side-note, this was solved in ProxyManager meanwhile, but we can consider this change only for 3.0.x





[DCOM-272] Proxy generator doesn't understand splat operator (PHP 5.6 argument unpacking) Created: 09/Feb/15  Updated: 26/Mar/15  Resolved: 26/Mar/15

Status: Resolved
Project: Doctrine Common
Component/s: None
Affects Version/s: 2.4.1
Fix Version/s: 2.5.0

Type: Bug Priority: Major
Reporter: Ruud Kamphuis Assignee: Marco Pivetta
Resolution: Fixed Votes: 0
Labels: code, php-5.6, proxy, proxy-generator, splat

Issue Links:
Dependency
depends on DCOM-273 [GH-353] Support for Variadic argumen... Open
Reference
is referenced by DCOM-273 [GH-353] Support for Variadic argumen... Open

 Description   

I have a function inside my entity that uses variadic arguments (argument unpacking) using the PHP 5.6 splat operator:

public function addReference(...$references) {}

But the proxy generator doesn't take that into account when generating the proxies:

    /**
     * {@inheritDoc}
     */
    public function addReference($references)
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'addReference', array($references));

        return parent::addReference($references);
    }

Resulting in this error:

Runtime Notice: Declaration of Proxies\__CG__\AccountingBundle\Entity\Journal::addReference() should be compatible with AccountingBundle\Entity\Journal::addReference(...$references)

The solution would be to add this to the ProxyGenerator generateMethods method:

                if (method_exists($param, 'isVariadic')) {
                    if ($param->isVariadic()) {
                        $parameterString .= '...';
                        $argumentString .= '...';
                    }
                }


 Comments   
Comment by Marco Pivetta [ 09/Feb/15 ]

Ruud Kamphuis I knew this was coming, heh.

Can you actually provide the fix as a PR with a test?

Comment by Ruud Kamphuis [ 09/Feb/15 ]

haha I really want to create PR for it, but I'm just curious how we are going to test this on multiple PHP versions? Because isVariadic only works in PHP 5.6. Do the ProxyGenerator tests compare the output of the generated PHP scripts?

Comment by Marco Pivetta [ 09/Feb/15 ]

Ruud Kamphuis something like following:

Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
public function testZehSplatThing()
{
    if ( ! method_exists('ReflectionParameter', 'isVariadic')) {
        $this->markTestSkipped('PHP 5.6+ only test');
    }

    // ...
}
Comment by Ruud Kamphuis [ 09/Feb/15 ]

I created a PR for this https://github.com/doctrine/common/pull/353

Could you check my question regarding the __invoke call?

Comment by Doctrine Bot [ 09/Feb/15 ]

A related Github Pull-Request [GH-353] was assigned:
https://github.com/doctrine/common/pull/353

Comment by Marco Pivetta [ 09/Feb/15 ]

Ruud Kamphuis I think single parameter is fine for now, as it is consistent and easy to handle on the closure side

Comment by Doctrine Bot [ 26/Mar/15 ]

A related Github Pull-Request [GH-353] was closed:
https://github.com/doctrine/common/pull/353





[DCOM-273] [GH-353] Support for Variadic arguments (PHP 5.6 - Argument unpacking) Created: 09/Feb/15  Updated: 26/Mar/15

Status: Open
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Doctrine Bot Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: php-5.6, proxy, proxy-generator, splat

Issue Links:
Dependency
is required for DCOM-272 Proxy generator doesn't understand sp... Resolved
Reference
relates to DCOM-272 Proxy generator doesn't understand sp... Resolved

 Description   

This issue is created automatically through a Github pull request on behalf of ruudk:

Url: https://github.com/doctrine/common/pull/353

Message:

See http://www.doctrine-project.org/jira/browse/DCOM-272

It converts
```php
/**

  • @param ...$types
    */
    public function addType(...$types)
    {
    }
    ```

into
```php
/**

  • {@inheritDoc}

    */
    public function addType(...$types)

    { $this->__initializer__ && $this->__initializer__->__invoke($this, 'addType', array($types)); return parent::addType(...$types); }

    ```

This works but I don't know if the `__invoke` call is correct.



 Comments   
Comment by Doctrine Bot [ 09/Feb/15 ]

A related Github Pull-Request [GH-353] was assigned:
https://github.com/doctrine/common/pull/353

Comment by Doctrine Bot [ 26/Mar/15 ]

A related Github Pull-Request [GH-353] was closed:
https://github.com/doctrine/common/pull/353





[DCOM-251] [GH-337] added a test for identifier getter in a trait Created: 03/Sep/14  Updated: 25/Mar/15  Resolved: 25/Mar/15

Status: Resolved
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Doctrine Bot Assignee: Marco Pivetta
Resolution: Incomplete Votes: 0
Labels: getter, identifier, proxy, proxy-generator, traits


 Description   

This issue is created automatically through a Github pull request on behalf of dorongutman:

Url: https://github.com/doctrine/common/pull/337

Message:

When the identifier getter method is in a trait, the proxy generation ignores it and treats it as a lazy loading field, causing the $proxyInstance->getId() to initialize the object.

This is a test that proves that.



 Comments   
Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-337] was closed:
https://github.com/doctrine/common/pull/337

Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-337] was assigned:
https://github.com/doctrine/common/pull/337





[DCOM-271] [GH-352] SymfonyFileLocator: DIRECTORY_SEPARATOR instead of slash Created: 02/Feb/15  Updated: 25/Mar/15  Resolved: 25/Mar/15

Status: Resolved
Project: Doctrine Common
Component/s: Annotations
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Doctrine Bot Assignee: Marco Pivetta
Resolution: Incomplete Votes: 0
Labels: directory-separator, file-locator, windows


 Description   

This issue is created automatically through a Github pull request on behalf of bartekgruszka:

Url: https://github.com/doctrine/common/pull/352

Message:

It could case some odd behaviour especially on Windows.



 Comments   
Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-352] was assigned:
https://github.com/doctrine/common/pull/352

Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-352] was closed:
https://github.com/doctrine/common/pull/352





[DCOM-269] [GH-350] Made PATTERN_MATCH_ID_METHOD more flexible Created: 23/Jan/15  Updated: 25/Mar/15

Status: Open
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Doctrine Bot Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This issue is created automatically through a Github pull request on behalf of 4alexandr:

Url: https://github.com/doctrine/common/pull/350

Message:

PATTERN_MATCH_ID_METHOD match any valid function declaration, example:
function getId()

{return$this->id;}

 Comments   
Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-350] was assigned:
https://github.com/doctrine/common/pull/350





[DCOM-160] [GH-242] adds a simple manager registry Created: 10/Jan/13  Updated: 25/Mar/15  Resolved: 25/Mar/15

Status: Resolved
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Benjamin Eberlei Assignee: Marco Pivetta
Resolution: Won't Fix Votes: 0
Labels: object-manager, persistence


 Description   

This issue is created automatically through a Github pull request on behalf of schmittjoh:

Url: https://github.com/doctrine/common/pull/242

Message:

This registry adds some sane defaults and just requires a simple callable to be fully functional.



 Comments   
Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-242] was assigned:
https://github.com/doctrine/common/pull/242

Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-242] was closed:
https://github.com/doctrine/common/pull/242





[DCOM-74] Ideas for Event Manager Improvements Created: 31/Oct/11  Updated: 25/Mar/15  Resolved: 25/Mar/15

Status: Resolved
Project: Doctrine Common
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major
Reporter: Johannes Schmitt Assignee: Marco Pivetta
Resolution: Incomplete Votes: 0
Labels: None


 Description   

I have two ideas for improving the event manager.

1. Add a lazy-loading implementation as we discussed on IRC already (helpful for keeping the overhead of post generation schema listener down for example)
2. Providing better debugging information of what is going on (which listeners have been called for which event, similar to Symfony2's event dispatcher)



 Comments   
Comment by Guilherme Blanco [ 16/Jan/12 ]

I'm still a huge fan of DOM2 Events.

I even have the code somewhere here, but that would break BC.

Comment by Marijn Huizendveld [ 06/Feb/13 ]

Would you care to elaborate on your DOM2 Events implementation Guilherme?

Comment by Guilherme Blanco [ 21/Apr/14 ]

Marijn Huizendveld sure (like... 1 year later!)
DOM2 Events ( http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/ )
The idea is to create an event system where you can control the flow over the listeners, such as prevent the default operation or stop propagation.
I'm a big fan of this because that way any piece can be easily decoupled from the base system and become a specialized event if needed. It also can be controlled depending on the conditions to be introspected by an specific listener.

Comment by Guilherme Blanco [ 21/Apr/14 ]

I also provided a very initial draft 2 years ago, as of https://github.com/doctrine/common/pull/153

Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-153] was closed:
https://github.com/doctrine/common/pull/153

Comment by Doctrine Bot [ 25/Mar/15 ]

A related Github Pull-Request [GH-153] was assigned:
https://github.com/doctrine/common/pull/153





Generated at Fri Mar 27 02:09:35 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.