Skip to content

Commit

Permalink
[Forms] Fix PhpcrOdmQueryBuilderLoader and depend on symfony version …
Browse files Browse the repository at this point in the history
…that fixed DoctrineType
  • Loading branch information
dbu authored and lsmith77 committed May 21, 2015
1 parent 23a4960 commit f393c06
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 5 deletions.
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ matrix:
env: SYMFONY_VERSION=2.6.*

before_install:
- composer self-update
- composer self-update || true
- sh -c 'if [ "${TRAVIS_PHP_VERSION}" != "hhvm" ]; then echo "memory_limit = -1" >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini; fi;'
- COMPOSER_ROOT_VERSION=dev-master composer require symfony/symfony:${SYMFONY_VERSION}
- vendor/symfony-cmf/testing/bin/travis/phpcr_odm_doctrine_dbal.sh

Expand Down
15 changes: 12 additions & 3 deletions Form/ChoiceList/PhpcrOdmQueryBuilderLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\Bundle\PHPCRBundle\Form\ChoiceList;

use Doctrine\Common\Persistence\ObjectManager;
use Doctrine\ODM\PHPCR\DocumentManager;
use Doctrine\ODM\PHPCR\Query\Builder\QueryBuilder;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Component\Form\Exception\FormException;
Expand Down Expand Up @@ -32,10 +33,10 @@ class PhpcrOdmQueryBuilderLoader implements EntityLoaderInterface
* Construct a PHPCR-ODM Query Builder Loader
*
* @param QueryBuilder|\Closure $queryBuilder
* @param ObjectManager $manager
* @param DocumentManager $manager
* @param string $class
*/
public function __construct($queryBuilder, ObjectManager $manager = null, $class = null)
public function __construct($queryBuilder, DocumentManager $manager = null, $class = null)
{
// If a query builder was passed, it must be a closure or QueryBuilder
// instance
Expand All @@ -51,6 +52,8 @@ public function __construct($queryBuilder, ObjectManager $manager = null, $class
}
}

$this->manager = $manager;

This comment has been minimized.

Copy link
@stof

stof Sep 30, 2015

Member

why storing the manager while you don't use it ?


$this->queryBuilder = $queryBuilder;
}

Expand All @@ -77,10 +80,16 @@ public function getEntities()
*/
public function getEntitiesByIds($identifier, array $values)
{
/* performance: if we could figure out whether the query builder is "
* empty" (that is only checking for the class) we could optimize this
* to a $this->dm->findMany(null, $values)
*/

$qb = clone $this->queryBuilder;
$alias = $qb->getPrimaryAlias();
$where = $qb->andWhere()->orX();
foreach ($values as $val) {
$qb->orWhere()->eq()->field($alias.'.'.$identifier)->literal($val);
$where->same($val, $alias);

This comment has been minimized.

Copy link
@uwej711

uwej711 Sep 30, 2015

Contributor

this change leads to an invalid query when you pass an empty value in values, this worked before (used in a form which is optional. We need to check $val for emptyness

This comment has been minimized.

Copy link
@stof

stof Sep 30, 2015

Member

the ORM implementation bypasses the query in such case btw, so it would be a valid implementation

This comment has been minimized.

Copy link
@uwej711

uwej711 Sep 30, 2015

Contributor

We might filter the array for the empty ids and only query if there is something left (that's what the ORM does in some way)

}

return $this->getResult($qb);
Expand Down
1 change: 0 additions & 1 deletion Form/Type/DocumentType.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

use Doctrine\Common\Persistence\ObjectManager;
use Doctrine\Bundle\PHPCRBundle\Form\ChoiceList\PhpcrOdmQueryBuilderLoader;
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\Type\DoctrineType;

class DocumentType extends DoctrineType
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

namespace Doctrine\Bundle\PHPCRBundle\Tests\Functional\Form\ChoiceList;

use Doctrine\Bundle\PHPCRBundle\Form\ChoiceList\PhpcrOdmQueryBuilderLoader;
use Doctrine\ODM\PHPCR\DocumentManager;
use Symfony\Cmf\Component\Testing\Functional\BaseTestCase;

class PhpcrOdmQueryBuilderLoaderTest extends BaseTestCase
{
/**
* @var DocumentManager
*/
private $dm;

public function setUp()
{
$this->db('PHPCR')->loadFixtures(array(
'Doctrine\Bundle\PHPCRBundle\Tests\Resources\DataFixtures\PHPCR\LoadData',
));

$this->dm = $this->getContainer()->get('doctrine_phpcr.odm.default_document_manager');
}

public function testGetByIds()
{
$qb = $this->dm->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')->createQueryBuilder('e');
$loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm);
$ids = array('/test/doc', '/test/doc2', '/test/doc3');
$documents = $loader->getEntitiesByIds('id', $ids);
$this->assertCount(2, $documents);
foreach ($documents as $i => $document) {
$this->assertInstanceOf('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument', $document);
$this->assertTrue(in_array($document->id, $ids));
}
}

public function testGetByIdsNotFound()
{
$qb = $this->dm->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')->createQueryBuilder('e');
$loader = new PhpcrOdmQueryBuilderLoader($qb, $this->dm);
$documents = $loader->getEntitiesByIds('id', array('/foo/bar'));
$this->assertCount(0, $documents);
}

public function testGetByIdsFilter()
{
$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);
$documents = $loader->getEntitiesByIds('id', array('/test/doc'));
$this->assertCount(0, $documents);
}
}
7 changes: 7 additions & 0 deletions Tests/Functional/Form/PHPCRTypeGuesserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Doctrine\Common\Util\Debug;
use Symfony\Component\Form\FormBuilderInterface;

use Symfony\Component\Form\Extension\Core\Type\CheckboxType;

use Symfony\Cmf\Component\Testing\Functional\BaseTestCase;
Expand All @@ -13,10 +14,16 @@

class PhpcrOdmTypeGuesserTest extends BaseTestCase
{
/**
* @var DocumentManager
*/
private $dm;

/**
* @var TestDocument
*/
private $document;

/**
* @var ReferrerDocument
*/
Expand Down
88 changes: 88 additions & 0 deletions Tests/Functional/Form/Type/DocumentTypeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php

namespace Doctrine\Bundle\PHPCRBundle\Tests\Functional\Form\Type;

use Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\ReferrerDocument;
use Doctrine\ODM\PHPCR\DocumentManager;
use Symfony\Cmf\Component\Testing\Functional\BaseTestCase;
use Symfony\Component\Form\FormBuilderInterface;

class DocumentTypeTest extends BaseTestCase
{
/**
* @var DocumentManager
*/
private $dm;

/**
* @var ReferrerDocument
*/
private $referrer;

public function setUp()
{
$this->db('PHPCR')->loadFixtures(array(
'Doctrine\Bundle\PHPCRBundle\Tests\Resources\DataFixtures\PHPCR\LoadData',
));
$this->dm = $this->db('PHPCR')->getOm();
$document = $this->dm->find(null, '/test/doc');
$this->assertNotNull($document, 'fixture loading not working');
$this->referrer = $this->dm->find(null, '/test/ref');
$this->assertNotNull($this->referrer, 'fixture loading not working');
}

/**
* @return FormBuilderInterface
*/
private function createFormBuilder($data, $options = array())
{
return $this->container->get('form.factory')->createBuilder('form', $data, $options);
}

/**
* Render a form and return the HTML
*/
private function renderForm(FormBuilderInterface $formBuilder)
{
$formView = $formBuilder->getForm()->createView();
$templating = $this->getContainer()->get('templating');

return $templating->render('::form.html.twig', array('form' => $formView));
}

public function testUnfiltered()
{
$formBuilder = $this->createFormBuilder($this->referrer);

$formBuilder
->add('single', 'phpcr_document', array(
'class' => 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument',
))
;

$html = $this->renderForm($formBuilder);
$this->assertContains('<select id="form_single" name="form[single]"', $html);
$this->assertContains('<option value="/test/doc"', $html);
}

public function testFiltered()
{
$qb = $this->dm
->getRepository('Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument')
->createQueryBuilder('e')
;
$qb->where()->eq()->field('e.text')->literal('thiswillnotmatch');
$formBuilder = $this->createFormBuilder($this->referrer);

$formBuilder
->add('single', 'phpcr_document', array(
'class' => 'Doctrine\Bundle\PHPCRBundle\Tests\Resources\Document\TestDocument',
'query_builder' => $qb,
))
;

$html = $this->renderForm($formBuilder);
$this->assertContains('<select id="form_single" name="form[single]"', $html);
$this->assertNotContains('<option', $html);
}
}
11 changes: 11 additions & 0 deletions Tests/Resources/DataFixtures/PHPCR/LoadData.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ public function load(ObjectManager $manager)

$manager->persist($doc);

$doc = new TestDocument();
$doc->id = '/test/doc2';
$doc->bool = true;
$doc->date = new \DateTime('2014-01-14');
$doc->integer = 42;
$doc->long = 24;
$doc->number = 3.14;
$doc->text = 'text content';

$manager->persist($doc);

$ref = new ReferrerDocument();
$ref->id = '/test/ref';
$ref->addDocument($doc);
Expand Down

0 comments on commit f393c06

Please sign in to comment.