RFC: Recited: Give me my constructor back

[Pittiplatsch]

In addition to #112, I already discussed possible solutions to this problem with @Ocramius, and in coordination with him ask hereby for comments.

---

Given an architecture of entity models, which heavily uses entity's constructors:

• to fill initial data
• to do entity-related initialization,

this collides with the current ghost proxy implementation which uses constructor injection for passing the initalization closure, making the usage of the underlying object's constructor nearly impossible.

For the sake of simplicity and imagination I'll speak about "entities" being used as proxy's underlying object, although usage of ghost proxies is of course not limited to those!

These are the options we identified:

1. Entirely avoid entity constructors.
   This is a no-go, as at least you would have to largely change the entity (library) implementation, which - if it should be possible anyway - clearly clashes with the target of a ghost proxy acting completely transparently.

2. The ghost proxy provides access to entity's constructor via some helper accessor method to enable the initializer closure invoking entity's constructor if needed (see: Ghost Proxy: Constructor of base object #112 (comment)).
   This turned out kinda inconvenient, primarily to due the dynamic nature of entity constructor's interface. It could be solved by generating the accessor via (already existing) code generator explicitly, with the exact interface of the entity's constructor - just like it is done for all other entity methods. However, this seems unnecessarily long-winded to me...

3. The ghost proxy does not use the constructor at all - neither explicitly (by implementing an own constructor) nor implicitly (by NOT implementing an own constructor, thus implicitly invoking entity's constructor).
   That means, that the ghost proxy must not be explicitly instantiated via "new" operator, but via reflection.

   1. = PHP 5.4: ReflectionClass::newInstanceWithoutConstructor()

   2. < PHP 5.4: serialization hack also used in Doctrine

   $serialized = sprintf('O:%u:"%s":0:{}', strlen($proxyClassName), $proxyClassName);
   

   Technically, this could be implemented using some kind of proxy factory.

   In the initializing closure, we then have the trivial possibility to call the entity's constructor, which already has all data initialization logic implemented, besides helping to DRY our code :-)

@Ocramius: Please add aspects I might have forgotten...

[Ocramius]

Given an architecture of entity models, which heavily uses entity's constructors:

• to fill initial data
• to do entity-related initialization,

So far, the suggested solution is to manually call the parent class constructor:

$constructor = (new ReflectionClass($proxy)->getParentClass())->getMethod('__construct');

$constructor->invoke($proxy, ...);

That's the cleanest approach to resume constructor logic without emulating it outside the entities

this collides with the current ghost proxy implementation which uses constructor injection for passing the initalization closure, making the usage of the underlying object's constructor nearly impossible.

Agreed. Proxies currently override the constructor. That is a plus for performance, but causes other issues if you rely on child class implementations.

1. Entirely avoid entity constructors.
   This is a no-go, as at least you would have to largely change the entity (library) implementation, which - if it should be possible anyway - clearly clashes with the target of a ghost proxy acting completely transparently.

I'm not sure why your entities are initialized via constructor like that - that may be ok in an opinionated API, but is obviously causing issues in your case.

1. The ghost proxy provides access to entity's constructor via some helper accessor method to enable the initializer closure invoking entity's constructor if needed (see: Ghost Proxy: Constructor of base object #112 (comment)).
   This turned out kinda inconvenient, primarily to due the dynamic nature of entity constructor's interface. It could be solved by generating the accessor via (already existing) code generator explicitly, with the exact interface of the entity's constructor - just like it is done for all other entity methods. However, this seems unnecessarily long-winded to me...

Yes, a possible approach would be to clone the parent entity's constructor into a different method, but that still changes the interface of the parent class, which is the issue here.

1. The ghost proxy does not use the constructor at all - neither explicitly (by implementing an own constructor) nor implicitly (by NOT implementing an own constructor, thus implicitly invoking entity's constructor).
   That means, that the ghost proxy must not be explicitly instantiated via "new" operator, but via reflection.

   1. = PHP 5.4: ReflectionClass::newInstanceWithoutConstructor()

   2. < PHP 5.4: serialization hack also used in Doctrine

   $serialized = sprintf('O:%u:"%s":0:{}', strlen($proxyClassName), $proxyClassName);
   

   Technically, this could be implemented using some kind of proxy factory.

I actually prefer this way. No constructor, initialization being handled by some different API added to the proxy.
The negative side here is that we'd have 2 method calls per proxy instantiation (cloning a prototype obtained via serialization, and then calling something like a proxyInit() or such). That also makes the API of a proxy harder to use if you want to directly communicate with it. It's deceptive API that may cause headaches. Also, we'd have to track if proxyInit() was already called. Moving the proxy instantiation to a public static method may be the right way in order to avoid those problems.

This is how it could look like:

class SomeProxy extends Some
{
    private static $blueprint;

    public static function createProxyInstance(... params here depending on proxy type...)
    {
        // note: `sprintf` can be avoided since we generate this code
        $proxy = clone (self::$blueprint 
            ? self::$blueprint
            : self::$blueprint = unserialize(sprintf('O:%u:"%s":0:{}', strlen(__CLASS__), __CLASS__));

        $proxy->initializeProxy(... params here depending on proxy type...);

        return $proxy;
    }

    private function initializeProxy(... params here depending on proxy type...)
    {
        unset($this->foo, $this->bar);
        // ...
    }
}

There's more advantages as well:

1. can be handled silently within a proxy factory
2. createProxyInstance can be part of an interface for specific proxy types. That makes it self-documented and very simple to use

Since this is a huge BC break though, we'd have to move this to 1.x.

With 1.x, I also hope to drop PHP <5.4 support, which allows us to get rid of a lot of conditional code to be checked.

[Pittiplatsch]

So far, the suggested solution is to manually call the parent class constructor:

That's what I'm actually doing right now (almost).

1. Entirely avoid entity constructors. This is a no-go, as at least you would have to largely change the entity (library) implementation, which - if it should be possible anyway - clearly clashes with the target of a ghost proxy acting completely transparently.

I'm not sure why your entities are initialized via constructor like that - that may be ok in an opinionated API, but is obviously causing issues in your case.

It's not only that entities are pre-filled with data via their constructors, but additional initialization work is done.
For example, we have list objects to be proxied which do some work like prototyping a list item, handle configurations etc.

Of course, for concrete use-cases one could always find work-arounds to avoid the usage of object constructors. However, I'd like to abstract the circumstances as you not always have the possibility to change the objects' implementation you want to proxy!

Yes, a possible approach would be to clone the parent entity's constructor into a different method, but that still changes the interface of the parent class, which is the issue here.

You're right - I missed that...

The ghost proxy does not use the constructor at all - neither explicitly (by implementing an own constructor) nor implicitly (by NOT implementing an own constructor, thus implicitly invoking entity's constructor). That means, that the ghost proxy must not be explicitly instantiated via "new" operator, but via reflection.
>= PHP 5.4: ReflectionClass::newInstanceWithoutConstructor()
< PHP 5.4: serialization hack also used in Doctrine $serialized = sprintf('O:%u:"%s":0:{}', strlen($proxyClassName), $proxyClassName); Technically, this could be implemented using some kind of proxy factory.

I actually prefer this way. No constructor, initialization being handled by some different API added to the proxy.
The negative side here is that we'd have 2 method calls per proxy instantiation (cloning a prototype obtained via serialization, and then calling something like a proxyInit() or such). That also makes the API of a proxy harder to use if you want to directly communicate with it. It's deceptive API that may cause headaches. Also, we'd have to track if proxyInit() was already called. Moving the proxy instantiation to a public static method may be the right way in order to avoid those problems.

I would be absolutely fine with that :-)

There's more advantages as well:

• can be handled silently within a proxy factory
• createProxyInstance can be part of an interface for specific proxy types. That makes it self-documented and very simple to use

Since this is a huge BC break though, we'd have to move this to 1.x.

Not really sure about that BC being HUGE, as "normal users", i.e. plain consumers of this lib don't get in touch with these internals.
Actually, the biggest change is that AbstractLazyFactory::createProxy()'s implementation would have to change from instantiating the proxy via "new" operator to use the static factory instead - do I miss sth.?
So, BC only affects people with deep knowledge of PM lib, and do special kinds of extending.

With 1.x, I also hope to drop PHP <5.4 support, which allows us to get rid of a lot of conditional code to be checked.

Would love to use 5.4, but we are still caught in 5.3 due to quite complex server environments which keep us from "just upgrade while having a beer" :'(

[Ocramius]

Re-opening. The issue is still valid IMO

[Ocramius]

Not really sure about that BC being HUGE, as "normal users", i.e. plain consumers of this lib don't get in touch with these internals.

The proxy interface changes, therefore this is a break. Many tests and some client libraries already use the generated classes directly (symfony, for example)

Actually, the biggest change is that AbstractLazyFactory::createProxy()'s implementation would have to change from instantiating the proxy via "new" operator to use the static factory instead - do I miss sth.?
So, BC only affects people with deep knowledge of PM lib, and do special kinds of extending.

As said, not everybody goes through the factories

Would love to use 5.4, but we are still caught in 5.3 due to quite complex server environments which keep us from "just upgrade while having a beer" :'(

I see... Well, the patch can be applied even without a 5.4 minimum requirement for now. It just requires a major version bump. I can delay moving to 5.4 if there's still need for 5.3, but I really want to push forward, and the limitations of 5.3 start to bubble up...

[Pittiplatsch]

Sorry, didn't mean to close this issue - obviously pushed the wrong button...

The proxy interface changes, therefore this is a break. Many tests and some client libraries already use the generated classes directly (symfony, for example)

I of course agree with the tests. But who would want to explicitly create a proxy instance via "new" operator instead of calling AbstractLazyFactory::createProxy()? This would be a clear mis-use in my eyes, isn't it?

[Ocramius]

The proxy interface changes, therefore this is a break. Many tests and some client libraries already use the generated classes directly (symfony, for example)

I of course agree with the tests. But who would want to explicitly create a proxy instance via "new" operator instead of calling AbstractLazyFactory::createProxy()? This would be a clear mis-use in my eyes, isn't it?

No, symfony explicitly avoids the factories when compiling the proxies into the DIC. I wrote the implementation for that myself :-)

It uses the generators directly

[Ocramius]

Handled in #175