Di::get() overwrites Di container in instances before return about phalcon

[niden]

@Laxilef Actually you stepped on Pandora's box. I have spent a great amount of hours trying to fix the DI container, looking towards a PSR-11 implementation. I was unsuccessful because of all the Di::getDefault() calls around the components.

In 4.1 and further I was going to slowly implement a PSR-11 container which would be much more flexible and offer everything we need including lazy loading of components. To get there however we will need to rewrite or refactor a lot of components.

[Laxilef]

Working and tested code:

        /**
         * Pass the DI to the instance if it implements
         * \Phalcon\Di\InjectionAwareInterface
         */
        if typeof instance == "object" {
            if instance instanceof InjectionAwareInterface && !(instance->getDi() instanceof DiInterface) {
                instance->setDI(this);
            }
        }

php:

		/**
		 * Pass the DI to the instance if it implements
		 * \Phalcon\Di\InjectionAwareInterface
		 */
		if ( gettype($instance) == "object" ) {
			if ( $instance instanceof InjectionAwareInterface && !($instance->getDi() instanceof DiInterface) ) {
				$instance->setDI($this);
			}
		}

Method code in php:

	/**
	 * @param string $name
	 * @param null $parameters
	 * @return mixed
	 * @throws Exception
	 * @throws \ReflectionException
	 */
	public function get($name, $parameters = null) {
		/**
		 * If the service is shared and it already has a cached instance then
		 * immediately return it without triggering events.
		 */

		$service = $eventsManager = $isShared = $instance = null;

		if ( isset($this->services[$name]) ) {
			$service = $this->services[$name];
			$isShared = $service->isShared();

			if ( $isShared && isset($this->sharedInstances[$name]) ) {
				return $this->sharedInstances[$name];
			}
		}

		$eventsManager = $this->eventsManager;

		/**
		 * Allows for custom creation of instances through the
		 * "di:beforeServiceResolve" event.
		 */
		if ( gettype($eventsManager) == "object" ) {
			$instance = $eventsManager->fire(
				"di:beforeServiceResolve",
				$this,
				[
					"name" => $name,
					"parameters" => $parameters
				]
			);
		}

		if ( gettype($instance) != "object" ) {
			if ( $service !== null ) {
				// The service is registered in the DI.
				try {
					$instance = $service->resolve($parameters, $this);
				} catch ( ServiceResolutionException $e ) {
					throw new Exception(
						"Service '" . $name . "' cannot be resolved"
					);
				}

				// If the service is shared then we'll cache the instance.
				if ( $isShared ) {
					$this->sharedInstances[$name] = $instance;
				}
			} else {
				/**
				 * The DI also acts as builder for any class even if it isn't
				 * defined in the DI
				 */
				if ( !class_exists($name) ) {
					throw new Exception(
						"Service '" . $name . "' wasn't found in the dependency injection container"
					);
				}

				if ( gettype($parameters) == "array" && count($parameters) ) {
					//$instance = call_user_func_array($name, $parameters);
					$reflect  = new \ReflectionClass($name);
					$instance = $reflect->newInstanceArgs($parameters);

				} else {
					//$instance = call_user_func($name);
					$reflect  = new \ReflectionClass($name);
					$instance = $reflect->newInstance();
				}
			}
		}

		/**
		 * Pass the DI to the instance if it implements
		 * \Phalcon\Di\InjectionAwareInterface
		 */
		if ( gettype($instance) == "object" ) {
			if ( $instance instanceof InjectionAwareInterface && !($instance->getDi() instanceof DiInterface) ) {
				$instance->setDI($this);
			}
		}

		/**
		 * Allows for post creation instance configuration through the
		 * "di:afterServiceResolve" event.
		 */
		if ( gettype($eventsManager) == "object" ) {
			$eventsManager->fire(
				"di:afterServiceResolve",
				$this,
				[
					"name" => $name,
					"parameters" => $parameters,
					"instance" => $instance
				]
			);
		}

		return $instance;
	}

[Laxilef]

Sorry.
This will not work: many Di::getDefault() in the Phalcon source code.

p.s. Only if the code is completely ugly:

		/**
		 * Pass the DI to the instance if it implements
		 * \Phalcon\Di\InjectionAwareInterface
		 */
		if ( gettype($instance) == "object" ) {
			self::setDefault($this); // important for Di::getDefault()
			if ( $instance instanceof InjectionAwareInterface /*&& !($instance->getDi() instanceof DiInterface)*/ ) {
				$reflection = new \ReflectionClass($instance);
				$property = $reflection->getProperty('container');
				$property->setAccessible(true);
				$_container = $property->getValue($instance);

				if ( !$_container ) {
					$instance->setDI($this);
				}
			}
		}

Maybe there are other methods for implementing nested Di for different Applications?

[sergeyklay]

@Laxilef Could you send a patch?

[Laxilef]

@sergeyklay I think that this is impossible. The current Phalcon architecture implies a single Di container due to static method calls. For example, Phalcon\Mvc\Model::find() calls Phalcon\Mvc\Model::getPreparedQuery(), which calls Di::getDefault():

        let container = Di::getDefault();
        let manager = <ManagerInterface> container->getShared("modelsManager");

This could be solved using the setDi() static methods, which sets Di to static protected $container, but this will affect all calls to this model from all other containers. And this is wrong, as it will confuse developers and spawn ugly/redundant code.

Maybe you have ideas on how to implement this? Or ideas on how to use the MVC App and Cli App at the same time?

Now this is not possible due to the fact that the MVC App and Cli App use $di>getShared('router'), that is, the same router service name.

[Laxilef]

@niden Thank you for the answer!
We will wait for the implementation of this functionality :)

[diplopito]

Out of curiosity: which is the benefit of using this declaration rather than the standard one, i.e.:

$di = new Di();
$app = new MvcApplication($di);
$app->handle($_GET['_url']);

Why declaring a new Di inside the Di?

[Laxilef]

@diplopito Almost the same as namespace.
I wrote an extension for Di to access the parent Di. It looks something like this:

$di = new Di();
$di->setShared('goodService', function() {
	return new GoodService;
});
$di->setShared('app', function() {
	$di = new Di();
	$di->setParent($this);
	/* some code... */
	$app = new MvcApplication($di);
	/* some code... */
	$di->get('goodService')->... /* obj phalcon/cphalcon#1 */
	/* some code... */
	return $app;
});
$di->setShared('cli', function() {
	$di = new Di();
	$di->setParent($this);
	
	$di->setShared('otherGoodService', function() {
		return new GoodService;
	});
	
	/* some code... */
	$app = new CliApplication($di);
	/* some code... */
	$di->get('goodService')->... /* obj phalcon/cphalcon#1 */
	/* some code... */
	return $app;
});

$di->setShared('altCli', function() {
	$di = new Di();
	$di->setParent($this);
	
	$di->setShared('goodService', function() {
		return new GoodService;
	});
	
	/* some code... */
	$app = new CliApplication($di);
	/* some code... */
	$di->get('goodService')->... /* obj phalcon/cphalcon#2 */
	$di->get('otherGoodService')->... /* NOT FOUND, ONLY FOR cli */
	/* some code... */
	return $app;
});

$di->get('app');
$di->get('cli');
$di->get('altCli');

Advantages: app has access to cli, cli has access to app. Each service can redefine within itself any service that exists in parentDi.

And that should have worked too:
$di->get('app')->get('cli')->get('altCli')

P.S. You don’t need to do this, this is an example :)

[diplopito]

Thank you @Laxilef