Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feature #19673 [DI] Deprecate Container::isFrozen and introduce isCom…
…piled (ro0NL)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate Container::isFrozen and introduce isCompiled

| Q | A |
| --- | --- |
| Branch? | "master" |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |

This deprecates the concept of freezing a container, implied by `Container::isFrozen`. However, freezing happens due compilation (`Container::compile`). So having just `isCompiled` instead seems more intuitive, and plays along well with `ContainerBuilder`.

Before/After;
- `Container::isFrozen`
  - Checks if the parameter bag is frozen, but is deprecated in 3.2
  - In 4.0 this methods does not exists and can be replaced with `getParameterBag() instanceof FrozenParameterBag` _or_ `isCompiled()`. Depending on what you want (to clarify; the behavior is different when passing a frozen bag to the constructor)
- `Container::isCompiled`
  - Truly checks if `compile()` has ran, and is a new feature
- `ContainerBuilder::merge` etc.
  - Now uses `isCompiled` instead of `isFrozen`, ie. we allow for it till compilation regarding the state of the paramater bag

Commits
-------

6abd312 [DI] Deprecate Container::isFrozen and introduce isCompiled
  • Loading branch information
fabpot committed Mar 22, 2017
2 parents c97407e + 6abd312 commit 7b60064
Show file tree
Hide file tree
Showing 25 changed files with 275 additions and 48 deletions.
17 changes: 17 additions & 0 deletions src/Symfony/Component/DependencyInjection/Container.php
Expand Up @@ -77,6 +77,7 @@ class Container implements ResettableContainerInterface

private $underscoreMap = array('_' => '', '.' => '_', '\\' => '_');
private $envCache = array();
private $compiled = false;

/**
* @param ParameterBagInterface $parameterBag A ParameterBagInterface instance
Expand All @@ -99,15 +100,31 @@ public function compile()
$this->parameterBag->resolve();

$this->parameterBag = new FrozenParameterBag($this->parameterBag->all());

$this->compiled = true;
}

/**
* Returns true if the container is compiled.
*
* @return bool
*/
public function isCompiled()
{
return $this->compiled;
}

/**
* Returns true if the container parameter bag are frozen.
*
* Deprecated since 3.3, to be removed in 4.0.
*
* @return bool true if the container parameter bag are frozen, false otherwise
*/
public function isFrozen()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);

return $this->parameterBag instanceof FrozenParameterBag;
}

Expand Down
24 changes: 12 additions & 12 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -416,13 +416,13 @@ public function fileExists($path, $trackContents = true)
*
* @return $this
*
* @throws BadMethodCallException When this ContainerBuilder is frozen
* @throws \LogicException if the container is frozen
* @throws BadMethodCallException When this ContainerBuilder is compiled
* @throws \LogicException if the extension is not registered
*/
public function loadFromExtension($extension, array $values = array())
{
if ($this->isFrozen()) {
throw new BadMethodCallException('Cannot load from an extension on a frozen container.');
if ($this->isCompiled()) {
throw new BadMethodCallException('Cannot load from an extension on a compiled container.');
}

$namespace = $this->getExtension($extension)->getAlias();
Expand Down Expand Up @@ -493,13 +493,13 @@ public function getCompiler()
* @param string $id The service identifier
* @param object $service The service instance
*
* @throws BadMethodCallException When this ContainerBuilder is frozen
* @throws BadMethodCallException When this ContainerBuilder is compiled
*/
public function set($id, $service)
{
$id = $this->normalizeId($id);

if ($this->isFrozen() && (isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())) {
if ($this->isCompiled() && (isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())) {
// setting a synthetic service on a frozen container is alright
throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a frozen container is not allowed.', $id));
}
Expand Down Expand Up @@ -601,12 +601,12 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
*
* @param ContainerBuilder $container The ContainerBuilder instance to merge
*
* @throws BadMethodCallException When this ContainerBuilder is frozen
* @throws BadMethodCallException When this ContainerBuilder is compiled
*/
public function merge(ContainerBuilder $container)
{
if ($this->isFrozen()) {
throw new BadMethodCallException('Cannot merge on a frozen container.');
if ($this->isCompiled()) {
throw new BadMethodCallException('Cannot merge on a compiled container.');
}

$this->addDefinitions($container->getDefinitions());
Expand Down Expand Up @@ -922,12 +922,12 @@ public function getDefinitions()
*
* @return Definition the service definition
*
* @throws BadMethodCallException When this ContainerBuilder is frozen
* @throws BadMethodCallException When this ContainerBuilder is compiled
*/
public function setDefinition($id, Definition $definition)
{
if ($this->isFrozen()) {
throw new BadMethodCallException('Adding definition to a frozen container is not allowed');
if ($this->isCompiled()) {
throw new BadMethodCallException('Adding definition to a compiled container is not allowed');
}

$id = $this->normalizeId($id);
Expand Down
34 changes: 22 additions & 12 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -83,7 +83,7 @@ class PhpDumper extends Dumper
*/
public function __construct(ContainerBuilder $container)
{
if (!$container->isFrozen()) {
if (!$container->isCompiled()) {
@trigger_error('Dumping an uncompiled ContainerBuilder is deprecated since version 3.3 and will not be supported anymore in 4.0. Compile the container beforehand.', E_USER_DEPRECATED);
}

Expand Down Expand Up @@ -162,10 +162,10 @@ public function dump(array $options = array())

$code = $this->startClass($options['class'], $options['base_class'], $options['namespace']);

if ($this->container->isFrozen()) {
if ($this->container->isCompiled()) {
$code .= $this->addFrozenConstructor();
$code .= $this->addFrozenCompile();
$code .= $this->addIsFrozenMethod();
$code .= $this->addFrozenIsCompiled();
} else {
$code .= $this->addConstructor();
}
Expand Down Expand Up @@ -893,7 +893,7 @@ private function addNewInstance(Definition $definition, $return, $instantiation,
*/
private function startClass($class, $baseClass, $namespace)
{
$bagClass = $this->container->isFrozen() ? 'use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;' : 'use Symfony\Component\DependencyInjection\ParameterBag\\ParameterBag;';
$bagClass = $this->container->isCompiled() ? 'use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;' : 'use Symfony\Component\DependencyInjection\ParameterBag\\ParameterBag;';
$namespaceLine = $namespace ? "namespace $namespace;\n" : '';

return <<<EOF
Expand Down Expand Up @@ -958,7 +958,7 @@ public function __construct()
}

/**
* Adds the constructor for a frozen container.
* Adds the constructor for a compiled container.
*
* @return string
*/
Expand Down Expand Up @@ -994,7 +994,7 @@ public function __construct()
}

/**
* Adds the constructor for a frozen container.
* Adds the compile method for a compiled container.
*
* @return string
*/
Expand All @@ -1007,26 +1007,36 @@ private function addFrozenCompile()
*/
public function compile()
{
throw new LogicException('You cannot compile a dumped frozen container.');
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}
EOF;
}

/**
* Adds the isFrozen method for a frozen container.
* Adds the isCompiled method for a compiled container.
*
* @return string
*/
private function addIsFrozenMethod()
private function addFrozenIsCompiled()
{
return <<<EOF
/*{$this->docStar}
* {@inheritdoc}
*/
public function isCompiled()
{
return true;
}
/*{$this->docStar}
* {@inheritdoc}
*/
public function isFrozen()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);
return true;
}
Expand Down Expand Up @@ -1112,7 +1122,7 @@ private function addPrivateServices()
private function addAliases()
{
if (!$aliases = $this->container->getAliases()) {
return $this->container->isFrozen() ? "\n \$this->aliases = array();\n" : '';
return $this->container->isCompiled() ? "\n \$this->aliases = array();\n" : '';
}

$code = " \$this->aliases = array(\n";
Expand Down Expand Up @@ -1158,7 +1168,7 @@ private function addDefaultParametersMethod()
$parameters = sprintf("array(\n%s\n%s)", implode("\n", $php), str_repeat(' ', 8));

$code = '';
if ($this->container->isFrozen()) {
if ($this->container->isCompiled()) {
$code .= <<<'EOF'
/**
Expand Down Expand Up @@ -1721,7 +1731,7 @@ private function dumpLiteralClass($class)
*/
private function dumpParameter($name)
{
if ($this->container->isFrozen() && $this->container->hasParameter($name)) {
if ($this->container->isCompiled() && $this->container->hasParameter($name)) {
return $this->dumpValue($this->container->getParameter($name), false);
}

Expand Down
Expand Up @@ -73,7 +73,7 @@ private function addParameters(\DOMElement $parent)
return;
}

if ($this->container->isFrozen()) {
if ($this->container->isCompiled()) {
$data = $this->escape($data);
}

Expand Down
Expand Up @@ -218,7 +218,7 @@ private function addParameters()
return '';
}

$parameters = $this->prepareParameters($this->container->getParameterBag()->all(), $this->container->isFrozen());
$parameters = $this->prepareParameters($this->container->getParameterBag()->all(), $this->container->isCompiled());

return $this->dumper->dump(array('parameters' => $parameters), 2);
}
Expand Down
Expand Up @@ -848,7 +848,7 @@ public function testPrivateServiceUser()
/**
* @expectedException \BadMethodCallException
*/
public function testThrowsExceptionWhenSetServiceOnAFrozenContainer()
public function testThrowsExceptionWhenSetServiceOnACompiledContainer()
{
$container = new ContainerBuilder();
$container->setResourceTracking(false);
Expand All @@ -857,15 +857,15 @@ public function testThrowsExceptionWhenSetServiceOnAFrozenContainer()
$container->set('a', new \stdClass());
}

public function testThrowsExceptionWhenAddServiceOnAFrozenContainer()
public function testThrowsExceptionWhenAddServiceOnACompiledContainer()
{
$container = new ContainerBuilder();
$container->compile();
$container->set('a', $foo = new \stdClass());
$this->assertSame($foo, $container->get('a'));
}

public function testNoExceptionWhenSetSyntheticServiceOnAFrozenContainer()
public function testNoExceptionWhenSetSyntheticServiceOnACompiledContainer()
{
$container = new ContainerBuilder();
$def = new Definition('stdClass');
Expand All @@ -879,7 +879,7 @@ public function testNoExceptionWhenSetSyntheticServiceOnAFrozenContainer()
/**
* @expectedException \BadMethodCallException
*/
public function testThrowsExceptionWhenSetDefinitionOnAFrozenContainer()
public function testThrowsExceptionWhenSetDefinitionOnACompiledContainer()
{
$container = new ContainerBuilder();
$container->setResourceTracking(false);
Expand Down
20 changes: 20 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;

class ContainerTest extends TestCase
{
Expand Down Expand Up @@ -82,6 +83,11 @@ public function testCompile()
$this->assertEquals(array('foo' => 'bar'), $sc->getParameterBag()->all(), '->compile() copies the current parameters to the new parameter bag');
}

/**
* @group legacy
* @expectedDeprecation The Symfony\Component\DependencyInjection\Container::isFrozen() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.
* @expectedDeprecation The Symfony\Component\DependencyInjection\Container::isFrozen() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.
*/
public function testIsFrozen()
{
$sc = new Container(new ParameterBag(array('foo' => 'bar')));
Expand All @@ -90,6 +96,20 @@ public function testIsFrozen()
$this->assertTrue($sc->isFrozen(), '->isFrozen() returns true if the parameters are frozen');
}

public function testIsCompiled()
{
$sc = new Container(new ParameterBag(array('foo' => 'bar')));
$this->assertFalse($sc->isCompiled(), '->isCompiled() returns false if the container is not compiled');
$sc->compile();
$this->assertTrue($sc->isCompiled(), '->isCompiled() returns true if the container is compiled');
}

public function testIsCompiledWithFrozenParameters()
{
$sc = new Container(new FrozenParameterBag(array('foo' => 'bar')));
$this->assertFalse($sc->isCompiled(), '->isCompiled() returns false if the container is not compiled but the parameter bag is already frozen');
}

public function testGetParameterBag()
{
$sc = new Container();
Expand Down
Expand Up @@ -41,14 +41,24 @@ public function __construct()
*/
public function compile()
{
throw new LogicException('You cannot compile a dumped frozen container.');
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}

/**
* {@inheritdoc}
*/
public function isCompiled()
{
return true;
}

/**
* {@inheritdoc}
*/
public function isFrozen()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);

return true;
}
}
Expand Up @@ -40,14 +40,24 @@ public function __construct()
*/
public function compile()
{
throw new LogicException('You cannot compile a dumped frozen container.');
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}

/**
* {@inheritdoc}
*/
public function isCompiled()
{
return true;
}

/**
* {@inheritdoc}
*/
public function isFrozen()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);

return true;
}
}
Expand Up @@ -45,14 +45,24 @@ public function __construct()
*/
public function compile()
{
throw new LogicException('You cannot compile a dumped frozen container.');
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}

/**
* {@inheritdoc}
*/
public function isCompiled()
{
return true;
}

/**
* {@inheritdoc}
*/
public function isFrozen()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);

return true;
}

Expand Down

0 comments on commit 7b60064

Please sign in to comment.