diff --git a/.travis.yml b/.travis.yml index 1073a487..4995221f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,10 +13,6 @@ sudo: false matrix: include: - - php: 5.3 - env: SYMFONY_VERSION=2.3.* COMPOSER_FLAGS="--prefer-lowest" - - php: 5.6 - env: SYMFONY_VERSION=2.3.* SYMFONY_DEPRECATIONS_HELPER=weak - php: 5.6 env: SYMFONY_VERSION=2.8.* - php: 5.6 diff --git a/Form/ChoiceList/DoctrineChoiceLoader.php b/Form/ChoiceList/DoctrineChoiceLoader.php new file mode 100644 index 00000000..a8748f14 --- /dev/null +++ b/Form/ChoiceList/DoctrineChoiceLoader.php @@ -0,0 +1,124 @@ + + */ +class DoctrineChoiceLoader extends BaseDoctrineChoiceLoader +{ + /** + * @var ObjectManager + */ + private $manager; + + /** + * @var ClassMetadata + */ + private $classMetadata; + + /** + * @var IdReader + */ + private $idReader; + + /** + * @var null|EntityLoaderInterface + */ + private $objectLoader; + + /** + * @var ChoiceListInterface + */ + private $choiceList; + + /** + * @var PropertyAccessor + */ + private $propertyAccessor; + + /** + * {@inheritdoc} + */ + public function __construct(ChoiceListFactoryInterface $factory, ObjectManager $manager, $class, IdReader $idReader = null, EntityLoaderInterface $objectLoader = null) + { + parent::__construct($factory, $manager, $class, $idReader, $objectLoader); + + $classMetadata = $manager->getClassMetadata($class); + + $this->manager = $manager; + $this->classMetadata = $classMetadata; + $this->idReader = $idReader ?: new IdReader($manager, $classMetadata); + $this->objectLoader = $objectLoader; + $this->propertyAccessor = PropertyAccess::createPropertyAccessor(); + } + + /** + * {@inheritdoc} + */ + public function loadChoicesForValues(array $values, $value = null) + { + // Performance optimization + // Also prevents the generation of "WHERE id IN ()" queries through the + // object 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(); + } + + $uuidFieldName = null; + if ($this->classMetadata instanceof PHPCRClassMetadata && $this->classMetadata->referenceable) { + $uuidFieldName = $this->classMetadata->getUuidFieldName(); + } + + // Optimize performance in case we have an object loader and + // a single-field identifier + if (!$this->choiceList && $this->objectLoader && $this->idReader->isSingleId()) { + $unorderedObjects = $this->objectLoader->getEntitiesByIds($this->idReader->getIdField(), $values); + $objectsById = array(); + $objects = array(); + + // Maintain order and indices from the given $values + // An alternative approach to the following loop is to add the + // "INDEX BY" clause to the Doctrine query in the loader, + // but I'm not sure whether that's doable in a generic fashion. + foreach ($unorderedObjects as $object) { + $objectsById[$this->idReader->getIdValue($object)] = $object; + } + + foreach ($values as $i => $id) { + if (null !== $uuidFieldName && UUIDHelper::isUUID($id)) { + foreach ($unorderedObjects as $object) { + if ($id === $this->propertyAccessor->getValue($object, $uuidFieldName)) { + $objects[$i] = $object; + break; + } + } + } elseif (isset($objectsById[$id])) { + $objects[$i] = $objectsById[$id]; + } + } + + return $objects; + } + + return $this->loadChoiceList($value)->getChoicesForValues($values); + } +} diff --git a/Form/ChoiceList/PhpcrOdmQueryBuilderLoader.php b/Form/ChoiceList/PhpcrOdmQueryBuilderLoader.php index 7a46d782..e5b053e5 100644 --- a/Form/ChoiceList/PhpcrOdmQueryBuilderLoader.php +++ b/Form/ChoiceList/PhpcrOdmQueryBuilderLoader.php @@ -3,7 +3,9 @@ namespace Doctrine\Bundle\PHPCRBundle\Form\ChoiceList; use Doctrine\ODM\PHPCR\DocumentManager; +use Doctrine\ODM\PHPCR\Mapping\ClassMetadata; use Doctrine\ODM\PHPCR\Query\Builder\QueryBuilder; +use PHPCR\Util\UUIDHelper; use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface; use Symfony\Component\Form\Exception\UnexpectedTypeException; @@ -26,6 +28,11 @@ class PhpcrOdmQueryBuilderLoader implements EntityLoaderInterface */ private $queryBuilder; + /** + * @var ClassMetadata + */ + private $classMetadata; + /** * Construct a PHPCR-ODM Query Builder Loader. * @@ -52,6 +59,13 @@ public function __construct($queryBuilder, DocumentManager $manager = null, $cla $this->manager = $manager; $this->queryBuilder = $queryBuilder; + + if (null !== $class) { + $classMetadata = $this->manager->getClassMetadata($class); + if ($classMetadata instanceof ClassMetadata) { + $this->classMetadata = $classMetadata; + } + } } /** @@ -93,8 +107,13 @@ public function getEntitiesByIds($identifier, array $values) $qb = clone $this->queryBuilder; $alias = $qb->getPrimaryAlias(); $where = $qb->andWhere()->orX(); + foreach ($values as $val) { - $where->same($val, $alias); + if ($this->classMetadata && $this->classMetadata->referenceable && UUIDHelper::isUUID($val)) { + $where->eq()->field($alias.'.'.$this->classMetadata->getUuidFieldName())->literal($val)->end(); + } else { + $where->same($val, $alias); + } } return $this->getResult($qb); diff --git a/Form/Type/DocumentType.php b/Form/Type/DocumentType.php index 53643523..3712630d 100644 --- a/Form/Type/DocumentType.php +++ b/Form/Type/DocumentType.php @@ -16,12 +16,34 @@ namespace Doctrine\Bundle\PHPCRBundle\Form\Type; +use Doctrine\Common\Persistence\ManagerRegistry; use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\Bundle\PHPCRBundle\Form\ChoiceList\DoctrineChoiceLoader; +use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator; +use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface; +use Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory; +use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator; +use Symfony\Component\OptionsResolver\Options; +use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\PropertyAccess\PropertyAccessorInterface; use Doctrine\Bundle\PHPCRBundle\Form\ChoiceList\PhpcrOdmQueryBuilderLoader; use Symfony\Bridge\Doctrine\Form\Type\DoctrineType; class DocumentType extends DoctrineType { + private $choiceListFactory; + + /** + * @var DoctrineChoiceLoader[] + */ + private $choiceLoaders = array(); + + public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null) + { + parent::__construct($registry, $propertyAccessor, $choiceListFactory); + $this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory(), $propertyAccessor); + } + /** * {@inheritdoc} */ @@ -34,6 +56,74 @@ public function getLoader(ObjectManager $manager, $queryBuilder, $class) ); } + /** + * We need to overrive the `choice_loader`, so that we can use our own DoctrineChoiceLoader + * class that includes support for UUIDs. + * + * @param OptionsResolver $resolver + */ + public function configureOptions(OptionsResolver $resolver) + { + parent::configureOptions($resolver); + + $choiceListFactory = $this->choiceListFactory; + $choiceLoaders = &$this->choiceLoaders; + $type = $this; + + $choiceLoader = function (Options $options) use ($choiceListFactory, &$choiceLoaders, $type) { + // Unless the choices are given explicitly, load them on demand + if (null === $options['choices']) { + $hash = null; + $qbParts = null; + + // If there is no QueryBuilder we can safely cache DoctrineChoiceLoader, + // also if concrete Type can return important QueryBuilder parts to generate + // hash key we go for it as well + if (!$options['query_builder'] || false !== ($qbParts = $type->getQueryBuilderPartsForCachingHash($options['query_builder']))) { + $hash = CachingFactoryDecorator::generateHash(array( + $options['em'], + $options['class'], + $qbParts, + $options['loader'], + )); + + if (isset($choiceLoaders[$hash])) { + return $choiceLoaders[$hash]; + } + } + + if ($options['loader']) { + $entityLoader = $options['loader']; + } elseif (null !== $options['query_builder']) { + $entityLoader = $type->getLoader($options['em'], $options['query_builder'], $options['class']); + } else { + $queryBuilder = $options['em']->getRepository($options['class'])->createQueryBuilder('e'); + $entityLoader = $type->getLoader($options['em'], $queryBuilder, $options['class']); + } + + // this line is different from the original choice loader, we use our own DoctrineChoiceLoader, + // were we have our changes to support UUIDs + $doctrineChoiceLoader = new DoctrineChoiceLoader( + $choiceListFactory, + $options['em'], + $options['class'], + $options['id_reader'], + $entityLoader + ); + + if ($hash !== null) { + $choiceLoaders[$hash] = $doctrineChoiceLoader; + } + + return $doctrineChoiceLoader; + } + }; + + $resolver->setDefaults(array( + 'choice_loader' => $choiceLoader, + )); + } + /** * {@inheritdoc} */ diff --git a/Tests/Functional/Form/ChoiceList/PhpcrOdmQueryBuilderLoaderTest.php b/Tests/Functional/Form/ChoiceList/PhpcrOdmQueryBuilderLoaderTest.php index 80c9674c..be4a7736 100644 --- a/Tests/Functional/Form/ChoiceList/PhpcrOdmQueryBuilderLoaderTest.php +++ b/Tests/Functional/Form/ChoiceList/PhpcrOdmQueryBuilderLoaderTest.php @@ -24,8 +24,9 @@ public function setUp() public function testGetByIds() { - $qb = $this->dm->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')->createQueryBuilder('e'); - $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm); + $class = 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument'; + $qb = $this->dm->getRepository($class)->createQueryBuilder('e'); + $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm, $class); $ids = array('/test/doc', '/test/doc2', '/test/doc3'); $documents = $loader->getEntitiesByIds('id', $ids); $this->assertCount(2, $documents); @@ -35,19 +36,38 @@ public function testGetByIds() } } + public function testGetByIdsWithUuid() + { + $class = 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument'; + $qb = $this->dm->getRepository($class)->createQueryBuilder('e'); + $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm, $class); + + $doc = $this->dm->find(null, '/test/doc'); + $uuids = array($doc->uuid, 'non-existing-uuid'); + + $documents = $loader->getEntitiesByIds('uuid', $uuids); + $this->assertCount(1, $documents); + foreach ($documents as $i => $document) { + $this->assertInstanceOf('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument', $document); + $this->assertTrue(in_array($document->uuid, $uuids)); + } + } + public function testGetByIdsNotFound() { - $qb = $this->dm->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')->createQueryBuilder('e'); - $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm); + $class = 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument'; + $qb = $this->dm->getRepository($class)->createQueryBuilder('e'); + $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm, $class); $documents = $loader->getEntitiesByIds('id', array('/foo/bar')); $this->assertCount(0, $documents); } public function testGetByIdsFilter() { + $class = 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument'; $qb = $this->dm->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')->createQueryBuilder('e'); $qb->where()->eq()->field('e.text')->literal('thiswillnotmatch'); - $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm); + $loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm, $class); $documents = $loader->getEntitiesByIds('id', array('/test/doc')); $this->assertCount(0, $documents); } diff --git a/Tests/Functional/Form/Type/DocumentTypeTest.php b/Tests/Functional/Form/Type/DocumentTypeTest.php index 6626c719..4871f86e 100644 --- a/Tests/Functional/Form/Type/DocumentTypeTest.php +++ b/Tests/Functional/Form/Type/DocumentTypeTest.php @@ -54,6 +54,34 @@ private function renderForm(FormBuilderInterface $formBuilder) return $templating->render('::form.html.twig', array('form' => $formView)); } + public function testUuid() + { + $document = $this->dm->find(null, '/test/doc'); + $uuid = $document->uuid; + + $formBuilder = $this->createFormBuilder($this->referrer); + + $formBuilder + ->add('single', $this->legacy ? 'phpcr_document' : 'Doctrine\Bundle\PHPCRBundle\Form\Type\DocumentType', array( + 'class' => 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument', + 'choice_value' => 'uuid', + )) + ; + + $form = $formBuilder->getForm(); + + $form->submit(array( + 'single' => $uuid, + )); + + $this->assertInstanceOf('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument', $this->referrer->getSingle()); + $this->assertEquals('doc', $this->referrer->getSingle()->nodename); + + $html = $this->renderForm($formBuilder); + $this->assertContains('single; } + /** + * @param $doc + */ + public function setSingle($doc) + { + $this->single = $doc; + } + /** * @return mixed */ diff --git a/Tests/Resources/app/config/config.php b/Tests/Resources/app/config/config.php index 9d4fb196..76276445 100644 --- a/Tests/Resources/app/config/config.php +++ b/Tests/Resources/app/config/config.php @@ -5,3 +5,10 @@ $loader->import(CMF_TEST_CONFIG_DIR.'/default.php'); $loader->import(CMF_TEST_CONFIG_DIR.'/phpcr_odm.php'); + +$config = array( + 'form' => array( + 'csrf_protection' => false, + ), +); +$container->loadFromExtension('framework', $config); diff --git a/composer.json b/composer.json index 3008f808..59a849cd 100644 --- a/composer.json +++ b/composer.json @@ -19,15 +19,15 @@ "prefer-stable": true, "require": { "php": ">=5.3.9", - "symfony/framework-bundle": "~2.3.27 || ^2.6.6 || ~3.0", - "symfony/doctrine-bridge": "~2.3 || ~3.0", + "symfony/framework-bundle": "~2.7 || ~3.0", + "symfony/doctrine-bridge": "~2.7 || ~3.0", "phpcr/phpcr-implementation": "2.1.*", "phpcr/phpcr-utils": "^1.2.7" }, "require-dev": { "phpunit/phpunit": "~4.5", "phpunit/php-code-coverage": "@stable", - "symfony/form": "~2.3 || ~3.0", + "symfony/form": "~2.7 || ~3.0", "doctrine/phpcr-odm": "~1.3,>1.3.0-rc3", "symfony-cmf/testing": "^1.3.0", "matthiasnoback/symfony-dependency-injection-test": "~0.7",