Skip to content

Commit

Permalink
[DependencyInjection] added a way to automatically update scoped serv…
Browse files Browse the repository at this point in the history
…ices

A service can now be marked as synchronized; when set, all method calls
involving this service will be called each time this service is set.
When in a scope, methods are also called to restore the previous version of the
service.
  • Loading branch information
fabpot committed Mar 20, 2013
1 parent 469330d commit ec1e7ca
Show file tree
Hide file tree
Showing 22 changed files with 366 additions and 24 deletions.
19 changes: 13 additions & 6 deletions src/Symfony/Component/DependencyInjection/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ public function set($id, $service, $scope = self::SCOPE_CONTAINER)
}

$this->services[$id] = $service;

if (method_exists($this, $method = 'synchronize'.strtr($id, array('_' => '', '.' => '_')).'Service')) {
$this->$method();
}
}

/**
Expand All @@ -221,7 +225,7 @@ public function has($id)
{
$id = strtolower($id);

return isset($this->services[$id]) || method_exists($this, 'get'.strtr($id, array('_' => '', '.' => '_')).'Service');
return array_key_exists($id, $this->services) || method_exists($this, 'get'.strtr($id, array('_' => '', '.' => '_')).'Service');
}

/**
Expand All @@ -247,7 +251,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
{
$id = strtolower($id);

if (isset($this->services[$id])) {
if (array_key_exists($id, $this->services)) {

This comment has been minimized.

Copy link
@stof

stof Jun 24, 2013

Member

@fabpot This broke the implementation of Doctrine\Common\Persistence\AbstractManagerRegistry::resetService in Symfony\Bridge\Doctrine\Registry.
This class is setting the service to null, relying on the fact that it was triggering the instantiation of a new entity manager in Symfony 2.2 and below when calling get again later. Now, calling getManager after resetting the manager in the registry is returning null, which breaks the code (as the method should never return null) and thus makes the resetting unusable.

With the new logic, I don't see any way to fix the registry implementation, and this is clearly an undocumented BC break in 2.3.

This comment has been minimized.

Copy link
@beberlei

beberlei Jun 24, 2013

Contributor

we could check for setting null in set and then unset the id

This comment has been minimized.

Copy link
@stof

stof Jun 24, 2013

Member

well, the question is whether switching from isset to array_key_exists was needed to implement the feature

return $this->services[$id];
}

Expand All @@ -263,7 +267,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
} catch (\Exception $e) {
unset($this->loading[$id]);

if (isset($this->services[$id])) {
if (array_key_exists($id, $this->services)) {
unset($this->services[$id]);
}

Expand All @@ -289,7 +293,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
*/
public function initialized($id)
{
return isset($this->services[strtolower($id)]);
return array_key_exists(strtolower($id), $this->services);
}

/**
Expand Down Expand Up @@ -393,8 +397,11 @@ public function leaveScope($name)
$services = $this->scopeStacks[$name]->pop();
$this->scopedServices += $services;

array_unshift($services, $this->services);
$this->services = call_user_func_array('array_merge', $services);
foreach ($services as $array) {
foreach ($array as $id => $service) {
$this->set($id, $service, $name);
}
}
}
}

Expand Down
74 changes: 60 additions & 14 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
*/
private $definitions = array();

/**
* @var Definition[]
*/
private $obsoleteDefinitions = array();

/**
* @var Alias[]
*/
Expand Down Expand Up @@ -351,14 +356,28 @@ public function set($id, $service, $scope = self::SCOPE_CONTAINER)

if ($this->isFrozen()) {
// setting a synthetic service on a frozen container is alright
if (!isset($this->definitions[$id]) || !$this->definitions[$id]->isSynthetic()) {
if (
(!isset($this->definitions[$id]) && !isset($this->obsoleteDefinitions[$id]))
||
(isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())
||
(isset($this->obsoleteDefinitions[$id]) && !$this->obsoleteDefinitions[$id]->isSynthetic())
) {
throw new BadMethodCallException(sprintf('Setting service "%s" on a frozen container is not allowed.', $id));
}
}

if (isset($this->definitions[$id])) {
$this->obsoleteDefinitions[$id] = $this->definitions[$id];
}

unset($this->definitions[$id], $this->aliases[$id]);

parent::set($id, $service, $scope);

if (isset($this->obsoleteDefinitions[$id]) && $this->obsoleteDefinitions[$id]->isSynchronized()) {
$this->synchronize($id);
}
}

/**
Expand Down Expand Up @@ -885,19 +904,7 @@ private function createService(Definition $definition, $id)
}

foreach ($definition->getMethodCalls() as $call) {
$services = self::getServiceConditionals($call[1]);

$ok = true;
foreach ($services as $s) {
if (!$this->has($s)) {
$ok = false;
break;
}
}

if ($ok) {
call_user_func_array(array($service, $call[0]), $this->resolveServices($parameterBag->resolveValue($call[1])));
}
$this->callMethod($service, $call);
}

$properties = $this->resolveServices($parameterBag->resolveValue($definition->getProperties()));
Expand Down Expand Up @@ -999,4 +1006,43 @@ public static function getServiceConditionals($value)

return $services;
}

/**
* Synchronizes a service change.
*
* This method updates all services that depend on the given
* service by calling all methods referencing it.
*
* @param string $id A service id
*/
private function synchronize($id)
{
foreach ($this->definitions as $definitionId => $definition) {
// only check initialized services
if (!$this->initialized($definitionId)) {
continue;
}

foreach ($definition->getMethodCalls() as $call) {
foreach ($call[1] as $argument) {
if ($argument instanceof Reference && $id == (string) $argument) {
$this->callMethod($this->get($definitionId), $call);
}
}
}
}
}

private function callMethod($service, $call)
{
$services = self::getServiceConditionals($call[1]);

foreach ($services as $s) {
if (!$this->has($s)) {
return;
}
}

call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->resolveValue($call[1])));
}
}
30 changes: 30 additions & 0 deletions src/Symfony/Component/DependencyInjection/Definition.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Definition
private $public;
private $synthetic;
private $abstract;
private $synchronized;

protected $arguments;

Expand All @@ -56,6 +57,7 @@ public function __construct($class = null, array $arguments = array())
$this->tags = array();
$this->public = true;
$this->synthetic = false;
$this->synchronized = false;
$this->abstract = false;
$this->properties = array();
}
Expand Down Expand Up @@ -569,6 +571,34 @@ public function isPublic()
return $this->public;
}

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

return $this;
}

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

/**
* Sets whether this definition is synthetic, that is not constructed by the
* container, but dynamically injected.
Expand Down
59 changes: 57 additions & 2 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ protected function get{$name}Service()
*/
private function addServices()
{
$publicServices = $privateServices = $aliasServices = '';
$publicServices = $privateServices = $aliasServices = $synchronizers = '';
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
Expand All @@ -576,6 +576,8 @@ private function addServices()
} else {
$privateServices .= $this->addService($id, $definition);
}

$synchronizers .= $this->addServiceSynchronizer($id, $definition);
}

$aliases = $this->container->getAliases();
Expand All @@ -584,7 +586,60 @@ private function addServices()
$aliasServices .= $this->addServiceAlias($alias, $id);
}

return $publicServices.$aliasServices.$privateServices;
return $publicServices.$aliasServices.$synchronizers.$privateServices;
}

/**
* Adds synchronizer methods.
*
* @param string $id A service identifier
* @param Definition $definition A Definition instance
*/
private function addServiceSynchronizer($id, Definition $definition)
{
if (!$definition->isSynchronized()) {
return;
}

$code = '';
foreach ($this->container->getDefinitions() as $definitionId => $definition) {
foreach ($definition->getMethodCalls() as $call) {
foreach ($call[1] as $argument) {
if ($argument instanceof Reference && $id == (string) $argument) {
$arguments = array();
foreach ($call[1] as $value) {
$arguments[] = $this->dumpValue($value);
}

$call = $this->wrapServiceConditionals($call[1], sprintf("\$this->get('%s')->%s(%s);", $definitionId, $call[0], implode(', ', $arguments)));

$code .= <<<EOF
if (\$this->initialized('$definitionId')) {
$call
}
EOF;
}
}
}
}

if (!$code) {
return;
}

$name = Container::camelize($id);

return <<<EOF
/**
* Updates the '$id' service.
*/
protected function synchronize{$name}Service()
{
$code }
EOF;
}

private function addNewInstance($id, Definition $definition, $return, $instantiation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ private function addService($definition, $id, \DOMElement $parent)
if (!$definition->isPublic()) {
$service->setAttribute('public', 'false');
}
if ($definition->isSynthetic()) {
$service->setAttribute('synthetic', 'true');
}
if ($definition->isSynchronized()) {
$service->setAttribute('synchronized', 'true');
}

foreach ($definition->getTags() as $name => $tags) {
foreach ($tags as $attributes) {
Expand Down Expand Up @@ -239,6 +245,9 @@ private function convertParameters($parameters, $type, \DOMElement $parent, $key
} elseif ($behaviour == ContainerInterface::IGNORE_ON_INVALID_REFERENCE) {
$element->setAttribute('on-invalid', 'ignore');
}
if (!$value->isStrict()) {
$element->setAttribute('strict', 'false');
}
} elseif ($value instanceof Definition) {
$element->setAttribute('type', 'service');
$this->addService($value, null, $element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ private function addService($id, $definition)
$code .= sprintf(" file: %s\n", $definition->getFile());
}

if ($definition->isSynthetic()) {
$code .= sprintf(" synthetic: true\n");
}

if ($definition->isSynchronized()) {
$code .= sprintf(" synchronized: true\n");
}

if ($definition->getFactoryMethod()) {
$code .= sprintf(" factory_method: %s\n", $definition->getFactoryMethod());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private function parseDefinition($id, $service, $file)
$definition = new Definition();
}

foreach (array('class', 'scope', 'public', 'factory-class', 'factory-method', 'factory-service', 'synthetic', 'abstract') as $key) {
foreach (array('class', 'scope', 'public', 'factory-class', 'factory-method', 'factory-service', 'synthetic', 'synchronized', 'abstract') as $key) {
if (isset($service[$key])) {
$method = 'set'.str_replace('-', '', $key);
$definition->$method((string) $service->getAttributeAsPhp($key));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ private function parseDefinition($id, $service, $file)
$definition->setSynthetic($service['synthetic']);
}

if (isset($service['synchronized'])) {
$definition->setSynchronized($service['synchronized']);
}

if (isset($service['public'])) {
$definition->setPublic($service['public']);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<xsd:attribute name="scope" type="xsd:string" />
<xsd:attribute name="public" type="boolean" />
<xsd:attribute name="synthetic" type="boolean" />
<xsd:attribute name="synchronized" type="boolean" />
<xsd:attribute name="abstract" type="boolean" />
<xsd:attribute name="factory-class" type="xsd:string" />
<xsd:attribute name="factory-method" type="xsd:string" />
Expand Down
Loading

0 comments on commit ec1e7ca

Please sign in to comment.