Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DoctrineBridge] Fixed submitting invalid ids when using queries with limit #34900

Merged
merged 1 commit into from Dec 15, 2019
Merged

[DoctrineBridge] Fixed submitting invalid ids when using queries with limit #34900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/Symfony/Bridge/Doctrine/Form/ChoiceList/ORMQueryBuilderLoader.php
View file
Open in desktop
Expand Up @@ -55,6 +55,21 @@ public function getEntities()
*/
public function getEntitiesByIds($identifier, array $values)
{
if (null !== $this->queryBuilder->getMaxResults() || null !== $this->queryBuilder->getFirstResult()) {
// an offset or a limit would apply on results including the where clause with submitted id values
// that could make invalid choices valid
$choices = [];
$metadata = $this->queryBuilder->getEntityManager()->getClassMetadata(current($this->queryBuilder->getRootEntities()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other way I see to fix this is to throw an exception here and to catch it in the DoctrineChoiceLoader to let it fall back on loading the list. But I would prefer to have it part of the EntityLoaderInterface contract rather than leaking too much in there.


foreach ($this->getEntities() as $entity) {
if (\in_array(current($metadata->getIdentifierValues($entity)), $values, true)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok with strict in_array? There is comming string variable from EntityType, but getIdentifierValues return integer from entity (if I am using strict_types). Then I get always error by comparing string vs integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jirinapravnik, I've opened #35609 to fix this.

$choices[] = $entity;
}
}

return $choices;
}

$qb = clone $this->queryBuilder;
$alias = current($qb->getRootAliases());
$parameter = 'ORMQueryBuilderLoader_getEntitiesByIds_'.$identifier;
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
View file
Open in desktop
Expand Up @@ -955,6 +955,31 @@ public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleIdentifie
$this->assertNull($field->getData());
}

public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleIdentifierWithLimit()
{
$entity1 = new SingleIntIdEntity(1, 'Foo');
$entity2 = new SingleIntIdEntity(2, 'Bar');
$entity3 = new SingleIntIdEntity(3, 'Baz');

$this->persist([$entity1, $entity2, $entity3]);

$repository = $this->em->getRepository(self::SINGLE_IDENT_CLASS);

$field = $this->factory->createNamed('name', static::TESTED_TYPE, null, [
'em' => 'default',
'class' => self::SINGLE_IDENT_CLASS,
'query_builder' => $repository->createQueryBuilder('e')
->where('e.id IN (1, 2, 3)')
->setMaxResults(1),
'choice_label' => 'name',
]);

$field->submit('3');

$this->assertFalse($field->isSynchronized());
$this->assertNull($field->getData());
}

public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleAssocIdentifier()
{
$innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo');
Expand Down