Merge branch '2.3'

* 2.3:
  [HttpKernel] added a check for private event listeners/subscribers
  [FrameworkBundle] fixed registration of the register listener pass
  [Form] Fixed regression causing invalid "WHERE id IN ()" statements
  [DependencyInjection] fixed a non-detected circular reference in PhpDumper (closes #8425)
  [Form] Fixed regression in BooleanToStringTransformer from ed83752
  [FrameworkBundle] removed obsolete code
  [Process] Close unix pipes before calling `proc_close` to avoid a deadlock
  [Process] Fix process merge in 2.3
  [Intl] made RegionBundle and LanguageBundle merge fallback data when using a country-specific locale
This commit is contained in:
Fabien Potencier 2013-09-12 14:59:51 +02:00
commit 5b71e61d15
16 changed files with 168 additions and 39 deletions

View File

@ -204,6 +204,16 @@ class EntityChoiceList extends ObjectChoiceList
*/
public function getChoicesForValues(array $values)
{
// Performance optimization
// Also prevents the generation of "WHERE id IN ()" queries through the
// entity loader. At least with MySQL and on the development machine
// this was tested on, no exception was thrown for such invalid
// statements, consequently no test fails when this code is removed.
// https://github.com/symfony/symfony/pull/8981#issuecomment-24230557
if (empty($values)) {
return array();
}
if (!$this->loaded) {
// Optimize performance in case we have an entity loader and
// a single-field identifier
@ -247,6 +257,11 @@ class EntityChoiceList extends ObjectChoiceList
*/
public function getValuesForChoices(array $entities)
{
// Performance optimization
if (empty($entities)) {
return array();
}
if (!$this->loaded) {
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as values
@ -284,6 +299,11 @@ class EntityChoiceList extends ObjectChoiceList
*/
public function getIndicesForChoices(array $entities)
{
// Performance optimization
if (empty($entities)) {
return array();
}
if (!$this->loaded) {
// Optimize performance for single-field identifiers. We already
// know that the IDs are used as indices
@ -321,6 +341,11 @@ class EntityChoiceList extends ObjectChoiceList
*/
public function getIndicesForValues(array $values)
{
// Performance optimization
if (empty($values)) {
return array();
}
if (!$this->loaded) {
// Optimize performance for single-field identifiers.

View File

@ -65,7 +65,9 @@ class FrameworkBundle extends Bundle
$container->addCompilerPass(new RoutingResolverPass());
$container->addCompilerPass(new ProfilerPass());
$container->addCompilerPass(new RegisterListenersPass(), PassConfig::TYPE_AFTER_REMOVING);
// must be registered before removing private services as some might be listeners/subscribers
// but as late as possible to get resolved parameters
$container->addCompilerPass(new RegisterListenersPass(), PassConfig::TYPE_BEFORE_REMOVING);
$container->addCompilerPass(new TemplatingPass());
$container->addCompilerPass(new AddConstraintValidatorsPass());
$container->addCompilerPass(new AddValidatorInitializersPass());

View File

@ -283,13 +283,12 @@ abstract class FrameworkExtensionTest extends TestCase
protected function createContainer(array $data = array())
{
return new ContainerBuilder(new ParameterBag(array_merge(array(
'kernel.bundles' => array('FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'),
'kernel.cache_dir' => __DIR__,
'kernel.compiled_classes' => array(),
'kernel.debug' => false,
'kernel.environment' => 'test',
'kernel.name' => 'kernel',
'kernel.root_dir' => __DIR__,
'kernel.bundles' => array('FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'),
'kernel.cache_dir' => __DIR__,
'kernel.debug' => false,
'kernel.environment' => 'test',
'kernel.name' => 'kernel',
'kernel.root_dir' => __DIR__,
), $data)));
}

View File

@ -443,6 +443,12 @@ class PhpDumper extends Dumper
continue;
}
// if the instance is simple, the return statement has already been generated
// so, the only possible way to get there is because of a circular reference
if ($this->isSimpleInstance($id, $definition)) {
throw new ServiceCircularReferenceException($id, array($id));
}
$name = (string) $this->definitionVariables->offsetGet($iDefinition);
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
$code .= $this->addServiceProperties(null, $iDefinition, $name);

View File

@ -181,4 +181,19 @@ class PhpDumperTest extends \PHPUnit_Framework_TestCase
$this->assertSame($bar, $container->get('foo')->bar, '->set() overrides an already defined service');
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
*/
public function testCircularReference()
{
$container = new ContainerBuilder();
$container->register('foo', 'stdClass')->addArgument(new Reference('bar'));
$container->register('bar', 'stdClass')->setPublic(false)->addMethodCall('setA', array(new Reference('baz')));
$container->register('baz', 'stdClass')->addMethodCall('setA', array(new Reference('foo')));
$container->compile();
$dumper = new PhpDumper($container);
$dumper->dump();
}
}

View File

@ -49,6 +49,10 @@ class BooleanToStringTransformer implements DataTransformerInterface
*/
public function transform($value)
{
if (null === $value) {
return null;
}
if (!is_bool($value)) {
throw new TransformationFailedException('Expected a Boolean.');
}

View File

@ -38,12 +38,10 @@ class BooleanToStringTransformerTest extends \PHPUnit_Framework_TestCase
$this->assertNull($this->transformer->transform(false));
}
/**
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
*/
public function testTransformFailsIfNull()
// https://github.com/symfony/symfony/issues/8989
public function testTransformAcceptsNull()
{
$this->transformer->transform(null);
$this->assertNull($this->transformer->transform(null));
}
/**

View File

@ -57,6 +57,11 @@ class RegisterListenersPass implements CompilerPassInterface
$definition = $container->getDefinition($this->dispatcherService);
foreach ($container->findTaggedServiceIds($this->listenerTag) as $id => $events) {
$def = $container->getDefinition($id);
if (!$def->isPublic()) {
throw new \InvalidArgumentException(sprintf('The service "%s" must be public as event listeners are lazy-loaded.', $id));
}
foreach ($events as $event) {
$priority = isset($event['priority']) ? $event['priority'] : 0;
@ -77,8 +82,13 @@ class RegisterListenersPass implements CompilerPassInterface
}
foreach ($container->findTaggedServiceIds($this->subscriberTag) as $id => $attributes) {
$def = $container->getDefinition($id);
if (!$def->isPublic()) {
throw new \InvalidArgumentException(sprintf('The service "%s" must be public as event subscribers are lazy-loaded.', $id));
}
// We must assume that the class value has been correctly filled, even if the service is created by a factory
$class = $container->getDefinition($id)->getClass();
$class = $def->getClass();
$refClass = new \ReflectionClass($class);
$interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface';

View File

@ -31,6 +31,9 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase
);
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
$definition->expects($this->atLeastOnce())
->method('isPublic')
->will($this->returnValue(true));
$definition->expects($this->atLeastOnce())
->method('getClass')
->will($this->returnValue('stdClass'));
@ -60,6 +63,9 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase
);
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
$definition->expects($this->atLeastOnce())
->method('isPublic')
->will($this->returnValue(true));
$definition->expects($this->atLeastOnce())
->method('getClass')
->will($this->returnValue('Symfony\Component\HttpKernel\Tests\DependencyInjection\SubscriberService'));
@ -81,6 +87,34 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase
$registerListenersPass = new RegisterListenersPass();
$registerListenersPass->process($builder);
}
/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "foo" must be public as event listeners are lazy-loaded.
*/
public function testPrivateEventListener()
{
$container = new ContainerBuilder();
$container->register('foo', 'stdClass')->setPublic(false)->addTag('kernel.event_listener', array());
$container->register('event_dispatcher', 'stdClass');
$registerListenersPass = new RegisterListenersPass();
$registerListenersPass->process($container);
}
/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The service "foo" must be public as event subscribers are lazy-loaded.
*/
public function testPrivateEventSubscriber()
{
$container = new ContainerBuilder();
$container->register('foo', 'stdClass')->setPublic(false)->addTag('kernel.event_subscriber', array());
$container->register('event_dispatcher', 'stdClass');
$registerListenersPass = new RegisterListenersPass();
$registerListenersPass->process($container);
}
}
class SubscriberService implements \Symfony\Component\EventDispatcher\EventSubscriberInterface

View File

@ -27,7 +27,7 @@ class LanguageBundle extends AbstractBundle implements LanguageBundleInterface
$locale = \Locale::getDefault();
}
if (null === ($languages = $this->readEntry($locale, array('Languages')))) {
if (null === ($languages = $this->readEntry($locale, array('Languages'), true))) {
return null;
}
@ -49,7 +49,7 @@ class LanguageBundle extends AbstractBundle implements LanguageBundleInterface
$locale = \Locale::getDefault();
}
if (null === ($languages = $this->readEntry($locale, array('Languages')))) {
if (null === ($languages = $this->readEntry($locale, array('Languages'), true))) {
return array();
}
@ -102,7 +102,7 @@ class LanguageBundle extends AbstractBundle implements LanguageBundleInterface
$locale = \Locale::getDefault();
}
if (null === ($scripts = $this->readEntry($locale, array('Scripts')))) {
if (null === ($scripts = $this->readEntry($locale, array('Scripts'), true))) {
return array();
}

View File

@ -27,7 +27,7 @@ class RegionBundle extends AbstractBundle implements RegionBundleInterface
$locale = \Locale::getDefault();
}
return $this->readEntry($locale, array('Countries', $country));
return $this->readEntry($locale, array('Countries', $country), true);
}
/**
@ -39,7 +39,7 @@ class RegionBundle extends AbstractBundle implements RegionBundleInterface
$locale = \Locale::getDefault();
}
if (null === ($countries = $this->readEntry($locale, array('Countries')))) {
if (null === ($countries = $this->readEntry($locale, array('Countries'), true))) {
return array();
}

View File

@ -40,7 +40,7 @@ class RegionBundleTest extends \PHPUnit_Framework_TestCase
{
$this->reader->expects($this->once())
->method('readEntry')
->with(self::RES_DIR, 'en', array('Countries', 'AT'))
->with(self::RES_DIR, 'en', array('Countries', 'AT'), true)
->will($this->returnValue('Austria'));
$this->assertSame('Austria', $this->bundle->getCountryName('AT', 'en'));
@ -55,7 +55,7 @@ class RegionBundleTest extends \PHPUnit_Framework_TestCase
$this->reader->expects($this->once())
->method('readEntry')
->with(self::RES_DIR, 'en', array('Countries'))
->with(self::RES_DIR, 'en', array('Countries'), true)
->will($this->returnValue($sortedCountries));
$this->assertSame($sortedCountries, $this->bundle->getCountryNames('en'));

View File

@ -308,18 +308,6 @@ class Process
}
$this->updateStatus(false);
while ($this->processPipes->hasOpenHandles()) {
usleep(100);
foreach ($this->processPipes->readAndCloseHandles(true) as $type => $data) {
if (3 == $type) {
$this->fallbackExitcode = (int) $data;
} else {
call_user_func($this->callback, $type === self::STDOUT ? self::OUT : self::ERR, $data);
}
}
}
$this->close();
if ($this->processInformation['signaled']) {
if ($this->isSigchildEnabled()) {
throw new RuntimeException('The process has been signaled.');
@ -1111,13 +1099,29 @@ class Process
*/
private function close()
{
$this->processPipes->close();
$exitcode = -1;
if (is_resource($this->process)) {
// Unix pipes must be closed before calling proc_close to void deadlock
// see manual http://php.net/manual/en/function.proc-close.php
$this->processPipes->closeUnixPipes();
$exitcode = proc_close($this->process);
}
// Windows only : when using file handles, some activity may occur after
// calling proc_close
while ($this->processPipes->hasOpenHandles()) {
usleep(100);
foreach ($this->processPipes->readAndCloseHandles(true) as $type => $data) {
if (3 == $type) {
$this->fallbackExitcode = (int) $data;
} else {
call_user_func($this->callback, $type === self::STDOUT ? self::OUT : self::ERR, $data);
}
}
}
$this->processPipes->close();
$this->exitcode = $this->exitcode !== null ? $this->exitcode : -1;
$this->exitcode = -1 != $exitcode ? $exitcode : $this->exitcode;

View File

@ -77,14 +77,24 @@ class ProcessPipes
*/
public function close()
{
foreach ($this->pipes as $offset => $pipe) {
fclose($pipe);
}
$this->closeUnixPipes();
foreach ($this->fileHandles as $offset => $handle) {
fclose($handle);
}
$this->fileHandles = $this->pipes = array();
$this->fileHandles = array();
}
/**
* Closes unix pipes.
*
* Nothing happens in case file handles are used.
*/
public function closeUnixPipes()
{
foreach ($this->pipes as $pipe) {
fclose($pipe);
}
$this->pipes = array();
}
/**

View File

@ -104,4 +104,14 @@ class CountryValidatorTest extends \PHPUnit_Framework_TestCase
array('EN'),
);
}
public function testValidateUsingCountrySpecificLocale()
{
\Locale::setDefault('en_GB');
$existingCountry = 'GB';
$this->context->expects($this->never())
->method('addViolation');
$this->validator->validate($existingCountry, new Country());
}
}

View File

@ -104,4 +104,16 @@ class LanguageValidatorTest extends \PHPUnit_Framework_TestCase
array('foobar'),
);
}
public function testValidateUsingCountrySpecificLocale()
{
\Locale::setDefault('fr_FR');
$existingLanguage = 'en';
$this->context->expects($this->never())
->method('addViolation');
$this->validator->validate($existingLanguage, new Language(array(
'message' => 'aMessage'
)));
}
}