Skip to content

#14832 - Add the rebinder mechanism to DI#14834

Closed
TimurFlush wants to merge 31 commits intophalcon:4.1.xfrom
TimurFlush:rebinder-4.1.x
Closed

#14832 - Add the rebinder mechanism to DI#14834
TimurFlush wants to merge 31 commits intophalcon:4.1.xfrom
TimurFlush:rebinder-4.1.x

Conversation

@TimurFlush
Copy link
Copy Markdown

@TimurFlush TimurFlush commented Feb 10, 2020

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a PR for the documentation about this change

Small description of change:

Q1: What does that mean?
A1: This means that already existing services in the container can be modified.

Q2: Are there any differences in modification between shared and non shared services?
A2: Yes.

  1. Non shared services can be changed even after they have been resolved.

  2. Shared services cannot be changed even after they have been resolved. That is, when creating a shared service, we polls all registered binders and passes this shared service to them. Once a shared service has passed into the container cache, it can no longer be modified by the binders.

Q3: How it differs from the chain of methods ->remove(), ->get(), ->set().
A3: My implementation does not create/delete services, but only adds "handlers" that will be called when the service is created. In this way, we can avoid additional overhead.

Q4: Will it be useful in real projects
Q4: Yes.

Q5: How can I use this?
Q5: Very simply.

PersistentJsonResponseServiceProvider.php

use Phalcon\Di;
use Phalcon\Http\Response;

class PersistentJsonResponseBinder implements \Phalcon\Di\ServiceProviderInterface
{
    public function register(Di $di): void
    {
        $di->rebind('response', function (Di $di, Response $response) {
            $response->setJsonContent(['ok' => true]);
        });
    }
}

...I will not describe how to register a provider...

IndexController.php

class IndexController extends \Phalcon\Mvc\Controller
{
    public function indexAction()
    {
        return $this->response; // { "ok": true }
    }
}

Thanks
P.S I apologize for my English.

@TimurFlush TimurFlush changed the title Rebinder 4.1.x #14832 - Add the rebinder mechanism to DI Feb 10, 2020
@Jurigag
Copy link
Copy Markdown
Contributor

Jurigag commented Feb 10, 2020

Hmmm, why exactly we need this? I guess something new is always nice but I don't feel it, do you maybe have some examples of this feature existing in other frameworks or containers?

@TimurFlush
Copy link
Copy Markdown
Author

Hmmm, why exactly we need this? I guess something new is always nice but I don't feel it, do you maybe have some examples of this feature existing in other frameworks or containers?

Yes. Laravel for example.
https://github.com/laravel/framework/blob/6.x/src/Illuminate/Container/Container.php#L516

@TimurFlush
Copy link
Copy Markdown
Author

Oh, finally

@sergeyklay
Copy link
Copy Markdown
Contributor

In general I would like to see any real use case. Right now there is just example how to use it but nothing about why I should use it.

@TimurFlush
Copy link
Copy Markdown
Author

@sergeyklay

  1. You can install some specific service in the container, and in the needed HMVC module you can extend this service without creating it. (Setting the headers in the service response, сhanging the template directory of the module Phalcon/Mvc/View etc)
  2. When you have some resource dependent unresolved service and you do not know if it will be called in the needed module. In this case, it can be extended instead of the usual replacement.
  3. Some custom service provider can extend the module provided from the Phalcon box instead of recreating it

@TimurFlush
Copy link
Copy Markdown
Author

TimurFlush commented Feb 13, 2020

For example:

index.php:

$di = new Di();

//common DI
$di->setShared('volt', function () use ($di) {
    $view = new \Phalcon\Mvc\View\Simple();
    $view->setViewsDir('/path/to/common/template/directory');
    return $view;
});

AdminModule.php

class Module implements Phalcon\Mvc\ModuleDefinitionInterface
{
    public function registerServices($di)
    {
        // this will be done without resolving
        $di->rebind('view', function ($di, \Phalcon\Mvc\View\Simple $view) {
            $view->setViewsDir('/path/to/admin/templates/directory');
        });
    }
}

@CameronHall
Copy link
Copy Markdown
Contributor

I've run into a perfect use case for this today. So I have a shared user service in my DI. This service will return the user model of the current user, and this works well on the web front.

A long time ago, I wrote a bunch of business logic that relied upon this user service. Now we have new features/tools that require CLI tasks that also need to be able to execute this same business logic.

The trouble is, these CLI tasks need to work across multiple users at a time, not just one user per process like the web application. Naturally, you'd assume you could just create a new method that does something like this.

public static function setUser(int $user_id): void
{
    $app = self::getInstance();        
    $app->getDI()->setShared('user', function () use ($user_id) {
        return UserModel::findFirstById($user_id);
    });
}

However, this doesn't reset the DI's_sharedServices cache. Now I totally get that we can just call $app->getDI()->remove('user');, but if I could have called setShared in the first place without having to remove it first, that'd have been far more convenient from the start.

I've also only just gotten this far and read the code for this PR. I think calling this->remove(name) before we set the service (by calling this->set) is a far cleaner solution.

@zsilbi
Copy link
Copy Markdown
Contributor

zsilbi commented Oct 4, 2020

Thank you @TimurFlush but I have to close this for now as we won't be able to include this in 4.1
When we will refactor the DI this will be taken into consideration.

@zsilbi zsilbi closed this Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants