diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md index 6369ea3634..8e3f148448 100644 --- a/src/Symfony/Bridge/Doctrine/CHANGELOG.md +++ b/src/Symfony/Bridge/Doctrine/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +2.1.5 +----- + + * fixed caching of choice lists when EntityType is used with the "query_builder" option + 2.1.0 ----- diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 78594ed8ba..2e385a343f 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -392,7 +392,10 @@ class EntityChoiceList extends ObjectChoiceList private function getIdentifierValues($entity) { if (!$this->em->contains($entity)) { - throw new FormException('Entities passed to the choice field must be managed'); + throw new FormException( + 'Entities passed to the choice field must be managed. Maybe ' . + 'persist them in the entity manager?' + ); } $this->em->initializeObject($entity); diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php index 14d88e3403..5ac205a1d7 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php @@ -12,6 +12,7 @@ namespace Symfony\Bridge\Doctrine\Form\Type; use Doctrine\Common\Persistence\ManagerRegistry; +use Symfony\Component\Form\Exception\FormException; use Doctrine\Common\Persistence\ObjectManager; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList; @@ -130,7 +131,17 @@ abstract class DoctrineType extends AbstractType return $registry->getManager($em); } - return $registry->getManagerForClass($options['class']); + $em = $registry->getManagerForClass($options['class']); + + if (null === $em) { + throw new FormException(sprintf( + 'Class "%s" seems not to be a managed Doctrine entity. ' . + 'Did you forget to map it?', + $options['class'] + )); + } + + return $em; }; $resolver->setDefaults(array( diff --git a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php index 51b47be4dc..c9ffb95e9b 100644 --- a/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php +++ b/src/Symfony/Bridge/Doctrine/Form/Type/EntityType.php @@ -11,26 +11,53 @@ namespace Symfony\Bridge\Doctrine\Form\Type; -use Doctrine\Common\Persistence\ObjectManager; +use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader; +use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\ORM\QueryBuilder; class EntityType extends DoctrineType { + /** + * @var array + */ + private $loaderCache = array(); + /** * Return the default loader object. * - * @param ObjectManager $manager - * @param mixed $queryBuilder - * @param string $class + * @param ObjectManager $manager + * @param QueryBuilder|\Closure $queryBuilder + * @param string $class + * * @return ORMQueryBuilderLoader + * + * @throws UnexpectedTypeException If the passed $queryBuilder is no \Closure + * and no QueryBuilder or if the closure + * does not return a QueryBuilder. */ public function getLoader(ObjectManager $manager, $queryBuilder, $class) { - return new ORMQueryBuilderLoader( - $queryBuilder, - $manager, - $class - ); + if ($queryBuilder instanceof \Closure) { + $queryBuilder = $queryBuilder($manager->getRepository($class)); + + if (!$queryBuilder instanceof QueryBuilder) { + throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder'); + } + } elseif (!$queryBuilder instanceof QueryBuilder) { + throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure'); + } + + // It is important to return the same loader for identical queries, + // otherwise the caching mechanism in DoctrineType does not work + // (which expects identical loaders for the cache to work) + $hash = md5($queryBuilder->getQuery()->getDQL()); + + if (!isset($this->loaderCache[$hash])) { + $this->loaderCache[$hash] = new ORMQueryBuilderLoader($queryBuilder); + } + + return $this->loaderCache[$hash]; } public function getName() diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypePerformanceTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypePerformanceTest.php index ea068c24f3..e73dd11be0 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypePerformanceTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypePerformanceTest.php @@ -12,6 +12,7 @@ namespace Symfony\Bridge\Doctrine\Tests\Form\Type; use Symfony\Component\Form\Tests\FormPerformanceTestCase; +use Doctrine\ORM\EntityRepository; use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIdentEntity; use Doctrine\ORM\Tools\SchemaTool; use Symfony\Bridge\Doctrine\Tests\DoctrineOrmTestCase; @@ -36,6 +37,9 @@ class EntityTypePerformanceTest extends FormPerformanceTestCase $manager->expects($this->any()) ->method('getManager') ->will($this->returnValue($this->em)); + $manager->expects($this->any()) + ->method('getManagerForClass') + ->will($this->returnValue($this->em)); return array( new CoreExtension(), @@ -109,4 +113,21 @@ class EntityTypePerformanceTest extends FormPerformanceTestCase $form->createView(); } } + + public function testCollapsedEntityFieldWithQueryBuilder() + { + $this->setMaxRunningTime(1); + + for ($i = 0; $i < 20; ++$i) { + $form = $this->factory->create('entity', null, array( + 'class' => self::ENTITY_CLASS, + 'query_builder' => function (EntityRepository $repo) { + return $repo->createQueryBuilder('e')->addOrderBy('e.id', 'DESC'); + } + )); + + // force loading of the choice list + $form->createView(); + } + } } diff --git a/src/Symfony/Component/HttpFoundation/File/File.php b/src/Symfony/Component/HttpFoundation/File/File.php index d3d20f583a..729d870cfc 100644 --- a/src/Symfony/Component/HttpFoundation/File/File.php +++ b/src/Symfony/Component/HttpFoundation/File/File.php @@ -106,6 +106,20 @@ class File extends \SplFileInfo * @api */ public function move($directory, $name = null) + { + $target = $this->getTargetFile($directory, $name); + + if (!@rename($this->getPathname(), $target)) { + $error = error_get_last(); + throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); + } + + @chmod($target, 0666 & ~umask()); + + return $target; + } + + protected function getTargetFile($directory, $name = null) { if (!is_dir($directory)) { if (false === @mkdir($directory, 0777, true)) { @@ -117,14 +131,7 @@ class File extends \SplFileInfo $target = $directory.DIRECTORY_SEPARATOR.(null === $name ? $this->getBasename() : $this->getName($name)); - if (!@rename($this->getPathname(), $target)) { - $error = error_get_last(); - throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); - } - - @chmod($target, 0666 & ~umask()); - - return new File($target); + return new File($target, false); } /** diff --git a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php index 4bf920ef0f..63c5386a11 100644 --- a/src/Symfony/Component/HttpFoundation/File/UploadedFile.php +++ b/src/Symfony/Component/HttpFoundation/File/UploadedFile.php @@ -202,8 +202,21 @@ class UploadedFile extends File */ public function move($directory, $name = null) { - if ($this->isValid() && ($this->test || is_uploaded_file($this->getPathname()))) { - return parent::move($directory, $name); + if ($this->isValid()) { + if ($this->test) { + return parent::move($directory, $name); + } elseif (is_uploaded_file($this->getPathname())) { + $target = $this->getTargetFile($directory, $name); + + if (!@move_uploaded_file($this->getPathname(), $target)) { + $error = error_get_last(); + throw new FileException(sprintf('Could not move the file "%s" to "%s" (%s)', $this->getPathname(), $target, strip_tags($error['message']))); + } + + @chmod($target, 0666 & ~umask()); + + return $target; + } } throw new FileException(sprintf('The file "%s" has not been uploaded via Http', $this->getPathname())); diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index a0f02a49a3..492675c03e 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -989,6 +989,8 @@ class Request * * @return string * + * @throws \UnexpectedValueException when the host name is invalid + * * @api */ public function getHost() @@ -996,20 +998,24 @@ class Request if (self::$trustProxy && self::$trustedHeaders[self::HEADER_CLIENT_HOST] && $host = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_HOST])) { $elements = explode(',', $host); - $host = trim($elements[count($elements) - 1]); - } else { - if (!$host = $this->headers->get('HOST')) { - if (!$host = $this->server->get('SERVER_NAME')) { - $host = $this->server->get('SERVER_ADDR', ''); - } + $host = $elements[count($elements) - 1]; + } elseif (!$host = $this->headers->get('HOST')) { + if (!$host = $this->server->get('SERVER_NAME')) { + $host = $this->server->get('SERVER_ADDR', ''); } } - // Remove port number from host - $host = preg_replace('/:\d+$/', '', $host); - + // trim and remove port number from host // host is lowercase as per RFC 952/2181 - return trim(strtolower($host)); + $host = strtolower(trim(preg_replace('/:\d+$/', '', $host))); + + // as the host can come from the user (HTTP_HOST and depending on the configuration, SERVER_NAME too can come from the user) + // check that it does not contain forbidden characters (see RFC 952 and RFC 2181) + if ($host && !preg_match('/^\[?(?:[a-zA-Z0-9-:\]_]+\.?)+$/', $host)) { + throw new \UnexpectedValueException('Invalid Host'); + } + + return $host; } /** diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index cb57208255..0276786430 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -609,9 +609,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase $this->assertNull($request->getQueryString(), '->getQueryString() returns null for empty query string'); } - /** - * @covers Symfony\Component\HttpFoundation\Request::getHost - */ public function testGetHost() { $request = new Request(); @@ -635,6 +632,16 @@ class RequestTest extends \PHPUnit_Framework_TestCase $this->stopTrustingProxyData(); } + /** + * @expectedException RuntimeException + */ + public function testGetHostWithFakeHttpHostValue() + { + $request = new Request(); + $request->initialize(array(), array(), array(), array(), array(), array('HTTP_HOST' => 'www.host.com?query=string')); + $request->getHost(); + } + /** * @covers Symfony\Component\HttpFoundation\Request::setMethod * @covers Symfony\Component\HttpFoundation\Request::getMethod