From fe4246aaa0fb861e312ae1e6d756393ea2448dc5 Mon Sep 17 00:00:00 2001 From: Giorgio Premi Date: Fri, 10 Apr 2015 16:44:40 +0200 Subject: [PATCH 1/2] [DoctrineBridge][Form] Fix EntityChoiceList when indexing by primary foreign key --- .../Form/ChoiceList/EntityChoiceList.php | 95 ++++++++++++++++--- .../SingleAssociationToIntIdEntity.php | 38 ++++++++ ...ChoiceListSingleAssociationToIntIdTest.php | 86 +++++++++++++++++ .../AbstractEntityChoiceListTest.php | 7 +- ...ChoiceListSingleAssociationToIntIdTest.php | 32 +++++++ ...ChoiceListSingleAssociationToIntIdTest.php | 32 +++++++ ...AssociationToIntIdWithQueryBuilderTest.php | 33 +++++++ 7 files changed, 309 insertions(+), 14 deletions(-) create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php create mode 100644 src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php index 964fbdc935..976393e74b 100644 --- a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php +++ b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php @@ -40,6 +40,13 @@ class EntityChoiceList extends ObjectChoiceList */ private $classMetadata; + /** + * Metadata for target class of primary key association. + * + * @var ClassMetadata + */ + private $idClassMetadata; + /** * Contains the query builder that builds the query for fetching the * entities. @@ -107,16 +114,21 @@ class EntityChoiceList extends ObjectChoiceList $this->class = $this->classMetadata->getName(); $this->loaded = is_array($entities) || $entities instanceof \Traversable; $this->preferredEntities = $preferredEntities; + list( + $this->idAsIndex, + $this->idAsValue, + $this->idField + ) = $this->getIdentifierInfoForClass($this->classMetadata); - $identifier = $this->classMetadata->getIdentifierFieldNames(); + if (null !== $this->idField && $this->classMetadata->hasAssociation($this->idField)) { + $this->idClassMetadata = $this->em->getClassMetadata( + $this->classMetadata->getAssociationTargetClass($this->idField) + ); - if (1 === count($identifier)) { - $this->idField = $identifier[0]; - $this->idAsValue = true; - - if (in_array($this->classMetadata->getTypeOfField($this->idField), array('integer', 'smallint', 'bigint'))) { - $this->idAsIndex = true; - } + list( + $this->idAsIndex, + $this->idAsValue + ) = $this->getIdentifierInfoForClass($this->idClassMetadata); } if (!$this->loaded) { @@ -228,7 +240,7 @@ class EntityChoiceList extends ObjectChoiceList // "INDEX BY" clause to the Doctrine query in the loader, // but I'm not sure whether that's doable in a generic fashion. foreach ($unorderedEntities as $entity) { - $value = $this->fixValue(current($this->getIdentifierValues($entity))); + $value = $this->fixValue($this->getSingleIdentifierValue($entity)); $entitiesByValue[$value] = $entity; } @@ -274,7 +286,7 @@ class EntityChoiceList extends ObjectChoiceList foreach ($entities as $i => $entity) { if ($entity instanceof $this->class) { // Make sure to convert to the right format - $values[$i] = $this->fixValue(current($this->getIdentifierValues($entity))); + $values[$i] = $this->fixValue($this->getSingleIdentifierValue($entity)); } } @@ -314,7 +326,7 @@ class EntityChoiceList extends ObjectChoiceList foreach ($entities as $i => $entity) { if ($entity instanceof $this->class) { // Make sure to convert to the right format - $indices[$i] = $this->fixIndex(current($this->getIdentifierValues($entity))); + $indices[$i] = $this->fixIndex($this->getSingleIdentifierValue($entity)); } } @@ -372,7 +384,7 @@ class EntityChoiceList extends ObjectChoiceList protected function createIndex($entity) { if ($this->idAsIndex) { - return $this->fixIndex(current($this->getIdentifierValues($entity))); + return $this->fixIndex($this->getSingleIdentifierValue($entity)); } return parent::createIndex($entity); @@ -392,7 +404,7 @@ class EntityChoiceList extends ObjectChoiceList protected function createValue($entity) { if ($this->idAsValue) { - return (string) current($this->getIdentifierValues($entity)); + return (string) $this->getSingleIdentifierValue($entity); } return parent::createValue($entity); @@ -415,6 +427,36 @@ class EntityChoiceList extends ObjectChoiceList return $index; } + /** + * Get identifier information for a class. + * + * @param ClassMetadata $classMetadata The entity metadata + * + * @return array Return an array with idAsIndex, idAsValue and identifier + */ + private function getIdentifierInfoForClass(ClassMetadata $classMetadata) + { + $identifier = null; + $idAsIndex = false; + $idAsValue = false; + + $identifiers = $classMetadata->getIdentifierFieldNames(); + + if (1 === count($identifiers)) { + $identifier = $identifiers[0]; + + if (!$classMetadata->hasAssociation($identifier)) { + $idAsValue = true; + + if (in_array($classMetadata->getTypeOfField($identifier), array('integer', 'smallint', 'bigint'))) { + $idAsIndex = true; + } + } + } + + return array($idAsIndex, $idAsValue, $identifier); + } + /** * Loads the list with entities. * @@ -438,6 +480,33 @@ class EntityChoiceList extends ObjectChoiceList $this->loaded = true; } + /** + * Returns the first (and only) value of the identifier fields of an entity. + * + * Doctrine must know about this entity, that is, the entity must already + * be persisted or added to the identity map before. Otherwise an + * exception is thrown. + * + * @param object $entity The entity for which to get the identifier + * + * @return array The identifier values + * + * @throws RuntimeException If the entity does not exist in Doctrine's identity map + */ + private function getSingleIdentifierValue($entity) + { + $value = current($this->getIdentifierValues($entity)); + + if ($this->idClassMetadata) { + $class = $this->idClassMetadata->getName(); + if ($value instanceof $class) { + $value = current($this->idClassMetadata->getIdentifierValues($value)); + } + } + + return $value; + } + /** * Returns the values of the identifier fields of an entity. * diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php new file mode 100644 index 0000000000..954de338d3 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Fixtures; + +use Doctrine\ORM\Mapping\Id; +use Doctrine\ORM\Mapping\Column; +use Doctrine\ORM\Mapping\Entity; +use Doctrine\ORM\Mapping\OneToOne; + +/** @Entity */ +class SingleAssociationToIntIdEntity +{ + /** @Id @OneToOne(targetEntity="SingleIntIdNoToStringEntity", cascade={"ALL"}) */ + protected $entity; + + /** @Column(type="string", nullable=true) */ + public $name; + + public function __construct(SingleIntIdNoToStringEntity $entity, $name) + { + $this->entity = $entity; + $this->name = $name; + } + + public function __toString() + { + return (string) $this->name; + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php new file mode 100644 index 0000000000..80710828e6 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php @@ -0,0 +1,86 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList; + +use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity; +use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity; +use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList; + +/** + * Test choices generated from an entity with a primary foreign key. + * + * @author Premi Giorgio + * @author Bernhard Schussek + */ +abstract class AbstractEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListTest +{ + protected function getEntityClass() + { + return 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity'; + } + + protected function getClassesMetadata() + { + return array( + $this->em->getClassMetadata($this->getEntityClass()), + $this->em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity'), + ); + } + + protected function createChoiceList() + { + return new EntityChoiceList($this->em, $this->getEntityClass(), 'name'); + } + + /** + * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + */ + protected function createObjects() + { + $innerA = new SingleIntIdNoToStringEntity(-10, 'inner_A'); + $innerB = new SingleIntIdNoToStringEntity(10, 'inner_B'); + $innerC = new SingleIntIdNoToStringEntity(20, 'inner_C'); + $innerD = new SingleIntIdNoToStringEntity(30, 'inner_D'); + + $this->em->persist($innerA); + $this->em->persist($innerB); + $this->em->persist($innerC); + $this->em->persist($innerD); + + return array( + new SingleAssociationToIntIdEntity($innerA, 'A'), + new SingleAssociationToIntIdEntity($innerB, 'B'), + new SingleAssociationToIntIdEntity($innerC, 'C'), + new SingleAssociationToIntIdEntity($innerD, 'D'), + ); + } + + protected function getChoices() + { + return array('_10' => $this->obj1, 10 => $this->obj2, 20 => $this->obj3, 30 => $this->obj4); + } + + protected function getLabels() + { + return array('_10' => 'A', 10 => 'B', 20 => 'C', 30 => 'D'); + } + + protected function getValues() + { + return array('_10' => '-10', 10 => '10', 20 => '20', 30 => '30'); + } + + protected function getIndices() + { + return array('_10', 10, 20, 30); + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php index 2b8bedf15b..4f3d54a30f 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php @@ -39,7 +39,7 @@ abstract class AbstractEntityChoiceListTest extends AbstractChoiceListTest $this->em = DoctrineTestHelper::createTestEntityManager(); $schemaTool = new SchemaTool($this->em); - $classes = array($this->em->getClassMetadata($this->getEntityClass())); + $classes = $this->getClassesMetadata(); try { $schemaTool->dropSchema($classes); @@ -73,6 +73,11 @@ abstract class AbstractEntityChoiceListTest extends AbstractChoiceListTest abstract protected function createObjects(); + protected function getClassesMetadata() + { + return array($this->em->getClassMetadata($this->getEntityClass())); + } + /** * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface */ diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php new file mode 100644 index 0000000000..e16be91f46 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList; + +/** + * @author Premi Giorgio + * @author Bernhard Schussek + */ +class LoadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest +{ + /** + * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + */ + protected function createChoiceList() + { + $list = parent::createChoiceList(); + + // load list + $list->getChoices(); + + return $list; + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php new file mode 100644 index 0000000000..00a4f545aa --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList; + +/** + * @author Premi Giorgio + * @author Bernhard Schussek + */ +class UnloadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest +{ + public function testGetIndicesForValuesIgnoresNonExistingValues() + { + $this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.'); + } + + /** + * @group legacy + */ + public function testLegacyGetIndicesForValuesIgnoresNonExistingValues() + { + $this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.'); + } +} diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php new file mode 100644 index 0000000000..4cad380552 --- /dev/null +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Doctrine\Tests\Form\ChoiceList; + +use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList; +use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader; + +/** + * @author Premi Giorgio + * @author Bernhard Schussek + */ +class UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest extends UnloadedEntityChoiceListSingleAssociationToIntIdTest +{ + /** + * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface + */ + protected function createChoiceList() + { + $qb = $this->em->createQueryBuilder()->select('s')->from($this->getEntityClass(), 's'); + $loader = new ORMQueryBuilderLoader($qb); + + return new EntityChoiceList($this->em, $this->getEntityClass(), null, $loader); + } +} From 2856abe87f1086bfa563c770447cbd2d7c7875e0 Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 7 Aug 2015 15:14:23 +0200 Subject: [PATCH 2/2] Implement the support of timezone objects in the stub IntlDateFormatter As of PHP 5.5, the IntlDateFormatter accepts to use DateTimeZone or IntlTimeZone objects as timezone in the constructor (and in the new setTimeZone method) rather than timezone ids. This is even the proper way to pass a timezone from a DateTime object as DateTimeZone names are not all valid ICU identifiers. --- .../Intl/DateFormatter/IntlDateFormatter.php | 17 +++++++++-- .../AbstractIntlDateFormatterTest.php | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Intl/DateFormatter/IntlDateFormatter.php b/src/Symfony/Component/Intl/DateFormatter/IntlDateFormatter.php index f08ed8d8ed..3e0ac5de17 100644 --- a/src/Symfony/Component/Intl/DateFormatter/IntlDateFormatter.php +++ b/src/Symfony/Component/Intl/DateFormatter/IntlDateFormatter.php @@ -132,7 +132,7 @@ class IntlDateFormatter * @param string $locale The locale code. The only currently supported locale is "en" (or null using the default locale, i.e. "en"). * @param int $datetype Type of date formatting, one of the format type constants * @param int $timetype Type of time formatting, one of the format type constants - * @param string $timezone Timezone identifier + * @param mixed $timezone Timezone identifier * @param int $calendar Calendar to use for formatting or parsing. The only currently * supported value is IntlDateFormatter::GREGORIAN. * @param string $pattern Optional pattern to use when formatting @@ -157,7 +157,7 @@ class IntlDateFormatter $this->timetype = $timetype; $this->setPattern($pattern); - $this->setTimeZoneId($timezone); + $this->setTimeZone($timezone); } /** @@ -588,6 +588,19 @@ class IntlDateFormatter */ public function setTimeZone($timeZone) { + if ($timeZone instanceof \IntlTimeZone) { + $timeZone = $timeZone->getID(); + } + + if ($timeZone instanceof \DateTimeZone) { + $timeZone = $timeZone->getName(); + + // DateTimeZone returns the GMT offset timezones without the leading GMT, while our parsing requires it. + if (!empty($timeZone) && ('+' === $timeZone[0] || '-' === $timeZone[0])) { + $timeZone = 'GMT'.$timeZone; + } + } + return $this->setTimeZoneId($timeZone); } diff --git a/src/Symfony/Component/Intl/Tests/DateFormatter/AbstractIntlDateFormatterTest.php b/src/Symfony/Component/Intl/Tests/DateFormatter/AbstractIntlDateFormatterTest.php index 71ef473960..c17dca13fd 100644 --- a/src/Symfony/Component/Intl/Tests/DateFormatter/AbstractIntlDateFormatterTest.php +++ b/src/Symfony/Component/Intl/Tests/DateFormatter/AbstractIntlDateFormatterTest.php @@ -387,6 +387,36 @@ abstract class AbstractIntlDateFormatterTest extends \PHPUnit_Framework_TestCase ); } + public function testFormatWithDateTimeZone() + { + if (PHP_VERSION_ID < 50500) { + $this->markTestSkipped('Only in PHP 5.5+ IntlDateFormatter allows to use DateTimeZone objects.'); + } + + if (defined('HHVM_VERSION_ID')) { + $this->markTestSkipped('This test cannot work on HHVM. See https://github.com/facebook/hhvm/issues/5875 for the issue.'); + } + + $formatter = $this->getDateFormatter('en', IntlDateFormatter::MEDIUM, IntlDateFormatter::SHORT, new \DateTimeZone('GMT+03:00'), IntlDateFormatter::GREGORIAN, 'zzzz'); + + $this->assertEquals('GMT+03:00', $formatter->format(0)); + } + + public function testFormatWithIntlTimeZone() + { + if (PHP_VERSION_ID < 50500) { + $this->markTestSkipped('Only in PHP 5.5+ IntlDateFormatter allows to use DateTimeZone objects.'); + } + + if (!class_exists('IntlTimeZone')) { + $this->markTestSkipped('This test requires the IntlTimeZone class from the Intl extension.'); + } + + $formatter = $this->getDateFormatter('en', IntlDateFormatter::MEDIUM, IntlDateFormatter::SHORT, \IntlTimeZone::createTimeZone('GMT+03:00'), IntlDateFormatter::GREGORIAN, 'zzzz'); + + $this->assertEquals('GMT+03:00', $formatter->format(0)); + } + public function testFormatWithTimezoneFromEnvironmentVariable() { if (PHP_VERSION_ID >= 50500) {