diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 228f9aad2f..9719462f19 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -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. diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index d5de61cd10..3aaf22a623 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -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()); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 4df785d6c7..c302c4c697 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -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))); } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index e92960e21e..919f0798cc 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -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); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index e7acee4451..77fe5e4937 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -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(); + } } diff --git a/src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php b/src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php index f8e3b5bfcb..4f6b12275b 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php +++ b/src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php @@ -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.'); } diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php index 4f6238aee0..a1217783d1 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php @@ -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)); } /** diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php index ee00fbb665..992c564aa7 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php @@ -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'; diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php index d1e825a824..4901098d0b 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -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 diff --git a/src/Symfony/Component/Intl/ResourceBundle/LanguageBundle.php b/src/Symfony/Component/Intl/ResourceBundle/LanguageBundle.php index c449f9c199..6b98a29e39 100644 --- a/src/Symfony/Component/Intl/ResourceBundle/LanguageBundle.php +++ b/src/Symfony/Component/Intl/ResourceBundle/LanguageBundle.php @@ -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(); } diff --git a/src/Symfony/Component/Intl/ResourceBundle/RegionBundle.php b/src/Symfony/Component/Intl/ResourceBundle/RegionBundle.php index a3cd9bd39a..bbfbedeed9 100644 --- a/src/Symfony/Component/Intl/ResourceBundle/RegionBundle.php +++ b/src/Symfony/Component/Intl/ResourceBundle/RegionBundle.php @@ -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(); } diff --git a/src/Symfony/Component/Intl/Tests/ResourceBundle/RegionBundleTest.php b/src/Symfony/Component/Intl/Tests/ResourceBundle/RegionBundleTest.php index ccdf904c91..8155040806 100644 --- a/src/Symfony/Component/Intl/Tests/ResourceBundle/RegionBundleTest.php +++ b/src/Symfony/Component/Intl/Tests/ResourceBundle/RegionBundleTest.php @@ -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')); diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 708eeb920b..ea6aa5245b 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -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; diff --git a/src/Symfony/Component/Process/ProcessPipes.php b/src/Symfony/Component/Process/ProcessPipes.php index 3b29ae9e51..a0fe71c8c1 100644 --- a/src/Symfony/Component/Process/ProcessPipes.php +++ b/src/Symfony/Component/Process/ProcessPipes.php @@ -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(); } /** diff --git a/src/Symfony/Component/Validator/Tests/Constraints/CountryValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/CountryValidatorTest.php index 3eedaa3264..95851e8097 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/CountryValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/CountryValidatorTest.php @@ -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()); + } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/LanguageValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/LanguageValidatorTest.php index 1230e3f322..3588887d74 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/LanguageValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/LanguageValidatorTest.php @@ -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' + ))); + } }