Controller __get() and getDI()->get() return different results.

[Bellardia]

It seems that the service returned using magic methods inside controllers are cached.

If I replace a default service inside a module, it won't be referenced properly inside controllers.

public function defaultAction($campaign, $pixel) {
        $di = \Phalcon\DI\FactoryDefault::getDefault();
        var_dump($di === $this->getDI(), $this->getDI()->get('request') === $this->request);
}

returns

bool(true)
bool(false)

This doesn't make sense: the internal DI is clearly the same as DI that is statically resolved, so I'm not sure where $this->request is being retrieved from, but it isn't being taken from the DI at call time.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[Bellardia]

After a bit of investigation, it looks like it's caused by the inability to clear a shared service.

class Module extends Phalcon\Mvc\ModuleDefinitionInterface {

    public function registerServices($di) {

        // Custom HTTP Request Wrapper
        // Make sure we remove the old one first (if it exists)
        $di->remove('request');
        $di->set('request', new Request);
    }
}

Calling $di->get('request') will resolve the updated service, but the Controller::__get() function resolves via $di->getShared('request'), which I can't seem to modify.

Further: If I simply remove the service and don't re-instantiate it, calls to $di->getShared() throws a Phalcon\DI\Exception exception, saying the service doesn't exist.

class Module extends Phalcon\Mvc\ModuleDefinitionInterface {

    public function registerServices($di) {

        // Custom HTTP Request Wrapper
        // Make sure we remove the old one first (if it exists)
        $di->remove('request');
        $di->getShared('request');
    }
}

class Phalcon\DI\Exception#95 (8) {
  protected $message =>
  string(69) "Service 'request' was not found in the dependency injection container"
  ...

So why do we ensure that the service is currently defined before attempting to return it when its current definition doesn't matter anyways? The cached service should continue to be returned, or remove() should destroy the shared object as well.

Related: http://forum.phalconphp.com/discussion/1476/-feature-bug-clear-di-shared-services-already-instantiated-is-ac

So is there any way to clear/update a shared service?

[bios-ben]

Note this cached __get is the same in views as well. So when inside a view doing $this->service is not the same as doing $this->getDI()->get('service') when you have updated a service in the DI. The injections seem to be copy rather than reference.

[Bellardia]

@bios-ben I believe it still returns a reference, just an out-dated reference (it will never get updated during subsequent calls to ->set().)

I wrote a wrapper to the DI class which solves these issues if you'd like it. It sits on top of the Phalcon implementation and invalidates references when they get re-assigned. It's in pure PHP, so it's not as performant as Phalcon, but at least you can override shared services. Let me know if you'd like the source.

[bios-ben]

Sure, I'd appreciate the source for that. I'm currently hacking away in PHP working around broken Phalcon things ... when I'm complete my project I'll be posting items like these to the team so they can be corrected in core, or at the very least translate it into Zephir and use as a patch for the core in my projects.

[Bellardia]

I never implemented setRaw(), but I fixed the other functions to behave how I'd expect. Let me know if you think anything needs changing.

https://gist.github.com/Bellardia/09a69565a8d363577ff9

[bios-ben]

An update for you: this fixed the issue perfectly for me in both view and controller, just plug and play. Thank you! Hopefully the team gets this fixed in the core so we don't have to work around with PHP.

[bios-ben]

@Bellardia does your code fix the issue for phalcon/cphalcon#3293 as well? In my case it fixed some of my problem, but the __get in a controller still differs from the $this->getDI() call.

[Bellardia]

@bios-ben I did run into an issue if Phalcon initializes a class itself through the DI using getShared() that's defined with a closure. It's really specific, but what happens is the first invocation of Phalcons default getShared() returns null, the second returns the class as expected.

Even my override class relies on get() and getShared() to work correctly the first time they're called, but I don't think they do. I haven't investigated it far enough to decide yet though.

[bios-ben]

Well in my situation, I've instead created a wrapper as my service that just updates its internal object and automatically forwards all call, get, and __set to said internal object I want to change. It's a workaround that works fine for me so I can at least keep going on my project. I'll wait for the Phalcon team to get back to us on these issues.

[Jurigag]

Services once got from any class extending Injectable using magic method are added as property to object causing accessing them again no longer use magic method, i think this is exepcted behaviour just to reduce method calls, sure in some scenarios it can cause problem that you will have other object in di and other in controller, it could be changed but it will introduce performance slowdown on multiple service access in controller.

[Bellardia]

@Jurigag it's a tough trade off... In my view a shared component should be a singleton and when the class is re-instantiated that promise is broken. The performance is negligible since you do very few DI calls directly in the controller.

[Jurigag]

Yea, i agree. this should be refactored:

https://github.com/phalcon/cphalcon/blob/master/phalcon/di/injectable.zep#L134

Like we should use getService, check if it's shared(singleton) - if not, don't add instance as property to controller.

It will still not solve problems in 100%, but at least setting shared/not shared service will have effect on injectable class.

[dschissler]

@sergeyklay Maybe this should be fixed in version 4.

[Jurigag]

Yea but what exactly you mean by fixed? The behavior i wrote above?

[niden]

Moving this to 4.1 - The DI service needs to be heavily refactored to address issues like this one.