Skip to content

Lazy services - service proxies #7527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 21 commits into from
Closed

Lazy services - service proxies #7527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f503ad8
First implementation of lazy services via proxy manager
Ocramius Mar 29, 2013
67aef45
Adding basic logic to generate proxy instantiation into a php dumped …
Ocramius Mar 30, 2013
f4a19c7
Compiling proxies into the generated DIC file
Ocramius Mar 30, 2013
d2760f1
Upgrading dependency to ProxyManager 0.3.*
Ocramius Mar 30, 2013
4a13f82
Suggesting ProxyManager in composer.json, removing useless calls
Ocramius Mar 31, 2013
a6a6572
Adding failing test to demonstrate that proxy initialization breaks s…
Ocramius Mar 31, 2013
35fdded
Fixing shared service instance
Ocramius Mar 31, 2013
bec7774
Sharing services in the container should only happen when proxying fa…
Ocramius Mar 31, 2013
468e92e
Adding tests for proxy sharing within dumped containers
Ocramius Mar 31, 2013
4ecd5ad
Fixing shared proxies into the container
Ocramius Mar 31, 2013
11a1da9
Bumping required version of ProxyManager
Ocramius Mar 31, 2013
5870fed
Docblock for ContainerBuilder#shareService
Ocramius Mar 31, 2013
695e3c5
Adding `ContainerBuilder#addClassResource`
Ocramius Mar 31, 2013
c5a5af0
Adding test to check that class resources are registered for lazy ser…
Ocramius Apr 1, 2013
29899ec
Fixing tests, registering class resources for lazy services
Ocramius Apr 1, 2013
b5d0298
Reverting import of global namespace classes
Ocramius Apr 2, 2013
cda390b
Lazier checks on the proxy structure (avoiding whitespace-based test …
Ocramius Apr 25, 2013
1eb4cf7
Getters for proxied services are public for 5.3.3 compatibility
Ocramius Apr 25, 2013
b417969
Enforcing soft dependency to ProxyManager
Ocramius Apr 25, 2013
1e24767
Reverting documentation changes, adding exception types description i…
Ocramius Apr 25, 2013
450635a
Lazier checks on the proxy structure
Ocramius Apr 27, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
"doctrine/orm": "~2.2,>=2.2.3",
"monolog/monolog": "~1.3",
"propel/propel1": "1.6.*",
"ircmaxell/password-compat": "1.0.*"
"ircmaxell/password-compat": "1.0.*",
"ocramius/proxy-manager": ">=0.3.1,<0.4-dev"
},
"autoload": {
"psr-0": { "Symfony\\": "src/" },
Expand Down
109 changes: 93 additions & 16 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

namespace Symfony\Component\DependencyInjection;

use ProxyManager\Configuration;
use ProxyManager\Factory\LazyLoadingValueHolderFactory;
use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy;
use ProxyManager\Proxy\LazyLoadingInterface;
use Symfony\Component\DependencyInjection\Compiler\Compiler;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
Expand Down Expand Up @@ -222,15 +226,32 @@ public function setResources(array $resources)
* @api
*/
public function addObjectResource($object)
{
if ($this->trackResources) {
$this->addClassResource(new \ReflectionClass($object));
}

return $this;
}

/**
* Adds the given class hierarchy as resources.
*
* @param \ReflectionClass $class
*
* @return ContainerBuilder The current instance
*
* @api
*/
public function addClassResource(\ReflectionClass $class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not passing a class name and building the ReflectionClass internally ? addObjectResource expects an object, not a ReflectionObject

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strict typing. This disallows unexisting classes upfront

{
if (!$this->trackResources) {
return $this;
}

$parent = new \ReflectionObject($object);
do {
$this->addResource(new FileResource($parent->getFileName()));
} while ($parent = $parent->getParentClass());
$this->addResource(new FileResource($class->getFileName()));
} while ($class = $class->getParentClass());

return $this;
}
Expand Down Expand Up @@ -417,8 +438,10 @@ public function has($id)
*
* @return object The associated service
*
* @throws InvalidArgumentException if the service is not defined
* @throws LogicException if the service has a circular reference to itself
* @throws InvalidArgumentException when no definitions are available
* @throws InactiveScopeException when the current scope is not active
* @throws LogicException when a circular dependency is detected
* @throws \Exception
*
* @see Reference
*
Expand Down Expand Up @@ -584,6 +607,12 @@ public function compile()
foreach ($this->compiler->getPassConfig()->getPasses() as $pass) {
$this->addObjectResource($pass);
}

foreach ($this->definitions as $definition) {
if ($definition->isLazy() && ($class = $definition->getClass()) && class_exists($class)) {
$this->addClassResource(new \ReflectionClass($class));
}
}
}

$this->compiler->compile($this);
Expand Down Expand Up @@ -865,20 +894,51 @@ public function findDefinition($id)
*
* @param Definition $definition A service definition instance
* @param string $id The service identifier
* @param Boolean $tryProxy Whether to try proxying the service with a lazy proxy
*
* @return object The service described by the service definition
*
* @throws RuntimeException When the scope is inactive
* @throws RuntimeException When the factory definition is incomplete
* @throws RuntimeException When the service is a synthetic service
* @throws InvalidArgumentException When configure callable is not callable
*
* @internal this method is public because of PHP 5.3 limitations, do not use it explicitly in your code
*/
private function createService(Definition $definition, $id)
public function createService(Definition $definition, $id, $tryProxy = true)
{
if ($definition->isSynthetic()) {
throw new RuntimeException(sprintf('You have requested a synthetic service ("%s"). The DIC does not know how to construct this service.', $id));
}

if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this whole block of code into a new service? This adds a hidden dependency to ZendFramework/ThirdParty code inside the ContainerBuilder that is not replaceable. This violates both Single Responsibilty and Dependency Inversion Principle here, not keeping the code against abstract concepts.

I say introduce a LazyLoad something interface, optionally inject it into the ContainerBuilder and then use the ProxyManager as one possible implementation there.

$tryProxy
&& ($className = $definition->getClass())
&& $definition->isLazy()
&& class_exists('ProxyManager\\Factory\\LazyLoadingValueHolderFactory')
) {
$config = new Configuration();

$config->setGeneratorStrategy(new EvaluatingGeneratorStrategy());

$factory = new LazyLoadingValueHolderFactory($config);
$container = $this;
$proxy = $factory->createProxy(
$className,
function (& $wrappedInstance, LazyLoadingInterface $proxy) use ($container, $definition, $id) {
$proxy->setProxyInitializer(null);

$wrappedInstance = $container->createService($definition, $id, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the call to createService replace the shared instance ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this has also a test for it, see ContainerBuilderTest:276, where I explicitly check:

$this->assertSame($foo1, $builder->get('foo1'), 'The same proxy is retrieved after initialization');


return true;
}
);

$this->shareService($definition, $proxy, $id);

return $proxy;
}

$parameterBag = $this->getParameterBag();

if (null !== $definition->getFile()) {
Expand All @@ -903,16 +963,9 @@ private function createService(Definition $definition, $id)
$service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments);
}

if (self::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
if (self::SCOPE_CONTAINER !== $scope && !isset($this->scopedServices[$scope])) {
throw new InactiveScopeException($id, $scope);
}

$this->services[$lowerId = strtolower($id)] = $service;

if (self::SCOPE_CONTAINER !== $scope) {
$this->scopedServices[$scope][$lowerId] = $service;
}
if ($tryProxy || !$definition->isLazy()) {
// share only if proxying failed, or if not a proxy
$this->shareService($definition, $service, $id);
}

foreach ($definition->getMethodCalls() as $call) {
Expand Down Expand Up @@ -1057,4 +1110,28 @@ private function callMethod($service, $call)

call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->resolveValue($call[1])));
}

/**
* Shares a given service in the container
*
* @param Definition $definition
* @param mixed $service
* @param string $id
*
* @throws InactiveScopeException
*/
private function shareService(Definition $definition, $service, $id)
{
if (self::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
if (self::SCOPE_CONTAINER !== $scope && !isset($this->scopedServices[$scope])) {
throw new InactiveScopeException($id, $scope);
}

$this->services[$lowerId = strtolower($id)] = $service;

if (self::SCOPE_CONTAINER !== $scope) {
$this->scopedServices[$scope][$lowerId] = $service;
}
}
}
}
30 changes: 30 additions & 0 deletions src/Symfony/Component/DependencyInjection/Definition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Definition
private $synthetic;
private $abstract;
private $synchronized;
private $lazy;

protected $arguments;

Expand All @@ -58,6 +59,7 @@ public function __construct($class = null, array $arguments = array())
$this->public = true;
$this->synthetic = false;
$this->synchronized = false;
$this->lazy = false;
$this->abstract = false;
$this->properties = array();
}
Expand Down Expand Up @@ -599,6 +601,34 @@ public function isSynchronized()
return $this->synchronized;
}

/**
* Sets the lazy flag of this service.
*
* @param Boolean $lazy
*
* @return Definition The current instance
*
* @api
*/
public function setLazy($lazy)
{
$this->lazy = (Boolean) $lazy;

return $this;
}

/**
* Whether this service is lazy.
*
* @return Boolean
*
* @api
*/
public function isLazy()
{
return $this->lazy;
}

/**
* Sets whether this definition is synthetic, that is not constructed by the
* container, but dynamically injected.
Expand Down
Loading