Orm: Exception caused by paginating queries with WHERE IN on a custom type field

Created on 24 Sep 2019 · 7Comments · Source: doctrine/orm

BC Break Report

| Q | A
|------------ | ------
| BC Break | yes
| Version | 2.6.4

Summary

In 2.6.4 an exception is thrown using the paginator on queries with a WHERE IN clause on a field with a custom type. This worked correctly in 2.6.3.

Previous behavior

The query ran as expected.

Current behavior

An exception is thrown as a value already in database format is passed to the custom type class to be converted again.

How to reproduce

I'm struggling a little for a concise reproducer, but we use the ramsey/uuid-doctrine package and the UuidBinaryType from it. This stores a Ramsey\Uuid\Uuid object into a binary field in the database.

Example query builder usage in a repository class

In this example, p.user is a many-to-one relationship to the User entity, which has a binary UUID field as its primary key. Because Doctrine (AFAIK) can only handle array parameters of either strings or ints, we need to manually pass an array of binary strings into the parameter.

$qb = $this->createQueryBuilder('p')
    ->where('p.user IN (:users)')
    ->setParameter(
        'users',
        \array_map(
            function (User $user): string {
                return $user->getId()->getBytes();
            },
            $users
        )
    );

This query works correctly when run as-is, but when it's passed to the paginator, the exception below occurs…

Example stack trace

Doctrine\DBAL\Types\ConversionException: Could not convert database value "??p??tHh?)*?Oh}?" to Doctrine Type uuid_binary /var/www/symfony/vendor/doctrine/dbal/lib/Doctrine/DBAL/Types/ConversionException.php:33 /var/www/symfony/vendor/ramsey/uuid-doctrine/src/UuidBinaryType.php:101 /var/www/symfony/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1499 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php:181 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php:182 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php:119 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Query/TreeWalkerChain.php:113 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php:389 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Query.php:286 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Query.php:298 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:967 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:922 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php:726 /var/www/symfony/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php:170 /var/www/symfony/vendor/knplabs/knp-components/src/Knp/Component/Pager/Event/Subscriber/Paginate/Doctrine/ORM/QuerySubscriber.php:48 /var/www/symfony/vendor/symfony/event-dispatcher/Debug/WrappedListener.php:126 /var/www/symfony/vendor/symfony/event-dispatcher/EventDispatcher.php:247 /var/www/symfony/vendor/symfony/event-dispatcher/EventDispatcher.php:73 /var/www/symfony/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:168 /var/www/symfony/vendor/knplabs/knp-components/src/Knp/Component/Pager/Paginator.php:181 /var/www/symfony/vendor/knplabs/knp-components/src/Knp/Component/Pager/Paginator.php:123 /var/www/symfony/src/Controller/Api/RedactedController.php:46 /var/www/symfony/vendor/symfony/http-kernel/HttpKernel.php:151 /var/www/symfony/vendor/symfony/http-kernel/HttpKernel.php:68 /var/www/symfony/vendor/symfony/http-kernel/Kernel.php:198 /var/www/symfony/vendor/symfony/http-kernel/Client.php:68 /var/www/symfony/vendor/symfony/framework-bundle/Client.php:131 /var/www/symfony/vendor/symfony/browser-kit/Client.php:407 /var/www/symfony/tests/Controller/Api/RedactedControllerTest.php:709

Bug Missing Tests

Source

jaikdean

👍3

Most helpful comment

As @Wirone mentioned I'll try to provide all I know to help to reproduce error. First of all we've noticed problem is somehow connected with relations between entities. We were using combination of Doctrine, ApiPlatform and Ramsey\Uuid. There were two similar scenarios where only difference was how entites are related. Below you can see mapping

Product entity

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd" xmlns:gedmo="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping">
  <entity name="App\Entity\Product" table="products">
    <id name="id" type="uuid_binary_ordered_time" column="id">
      <generator strategy="CUSTOM"/>
      <custom-id-generator class="Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator"/>
    </id>
    <one-to-many field="statusHistory" target-entity="App\Entity\Status" mapped-by="product" fetch="LAZY">
      <cascade>
        <cascade-persist/>
      </cascade>
    </one-to-many>
    <one-to-many field="variants" target-entity="App\Entity\Variant" mapped-by="product" fetch="LAZY" orphan-removal="true">
      <cascade>
        <cascade-all/>
      </cascade>
    </one-to-many>
  </entity>
</doctrine-mapping>

Status entity

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
  <entity name="App\Entity\Status" table="product_status_history">
    <id name="id" type="integer" column="id">
      <generator strategy="IDENTITY"/>
    </id>
    <many-to-one field="product" target-entity="App\Entity\Product" inversed-by="statusHistory" fetch="LAZY">
      <join-columns>
        <join-column name="product_id" referenced-column-name="id"/>
      </join-columns>
    </many-to-one>
  </entity>
</doctrine-mapping>

Variant entity

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
    <entity name="App\Entity\Variant" inheritance-type="SINGLE_TABLE">
        <id name="id" column="id" type="uuid_binary_ordered_time">
            <generator strategy="CUSTOM"/>
            <custom-id-generator class="Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator"/>
        </id>
        <many-to-one field="product" target-entity="App\Entity\Product" inversed-by="variants"/>
        <discriminator-column name="category" type="string" />
        <discriminator-map>
            <discriminator-mapping value="color" class="App\Entity\Variant\Color" />
        </discriminator-map>
    </entity>
</doctrine-mapping>

Variant.Color entity

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
    <entity name="App\Entity\Variant\Color">
        <field name="hue" type="text" nullable="false"/>
    </entity>
</doctrine-mapping>

With these mappings and defining ApiPlatoform resources when you fetch paginated data using api (GET /api/products and GET /api/variants) of Product and Variant entities you receive completly different result.

Fetching Variant cause no errors at all and works just fine, however fetching Product crashes due to inconsistency of id.

Paginator fetches $ids at
api/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php:154 _{doctrine/orm:v2.4.6}

    public function getIterator()
    {
...
            $foundIdRows = $subQuery->getScalarResult();

            // don't do this for an empty id array
            if ($foundIdRows === []) {
                return new \ArrayIterator([]);
            }

            $whereInQuery = $this->cloneQuery($this->query);
            $ids          = array_map('current', $foundIdRows);
...

$ids contains binary string of uuids and this type of data is not acceptable by \Ramsey\Uuid\Doctrine\UuidBinaryOrderedTimeType::convertToDatabaseValue _{ramsey/uuid-doctrine:1.5.0} which causes at the end throwing \Doctrine\DBAL\Types\ConversionException.

In this particular case as we can see there are two ways to solve the problem

Modify convertToDatabaseValue to accept binary data and do not try to convert it again. (in my opinion not the best way)
Quering for ids inPaginator should denormalize binary data to acceptably by convertToDatabaseValue

Also during fetching Variant entities debuger never stopped at breakpoint in \Doctrine\ORM\Tools\Pagination\Paginator::getIterator. The two things which are different in maping are discriminator mapping way how relations are created. That's why I think focusing on entities relations is the key to problem.

I'm not sure if pattern above can be applied to all kinds of custom types but I hope it will helps solving the issue.

specific packages version we use
api-platform/api-pack:v1.2.0
api-platform/core:v2.4.7
doctrine/orm:v2.6.4
ramsey/uuid-doctrine:1.5.0

patrykmartynowicz on 11 Oct 2019

👍2

All 7 comments

This is an interesting one since we only convert the type of the parameter :dpid and it shouldn't influence the original query.

Would you send as a PR (targeting 2.6) with a failing test that reproduces this? You can use https://github.com/doctrine/orm/blob/b52ef5a1002f99ab506a5a2d6dba5a2c236c5f43/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7820Test.php as example for a custom type similar to the UuidBinaryType.

lcobucci on 24 Sep 2019

@jaikdean I've tried to reproduce this in a failing test and failed. I ask you again to send us a failing test otherwise it will be pretty hard to get it fixed (perhaps the sample test case below might help you).

Sample test case

<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\Common\Collections\Criteria;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\StringType;
use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\Tools\Pagination\Paginator;
use Doctrine\Tests\OrmFunctionalTestCase;
use function is_string;
use function iterator_to_array;

/**
 * @group GH7830
 */
final class GH7830Test extends OrmFunctionalTestCase
{
    private const SONG = [
        'What is this song all about?',
        'Can\'t figure any lyrics out',
        'How do the words to it go?',
        'I wish you\'d tell me, I don\'t know',
        'Don\'t know, don\'t know, don\'t know, I don\'t know!',
        'Don\'t know, don\'t know, don\'t know...',
    ];

    protected function setUp() : void
    {
        parent::setUp();

        if (! Type::hasType(GH7830LineTextType::class)) {
            Type::addType(GH7830LineTextType::class, GH7830LineTextType::class);
        }

        $this->setUpEntitySchema([GH7830Line::class, GH7830LineAuthor::class]);

        foreach (self::SONG as $index => $line) {
            $this->_em->persist(new GH7830Line(GH7830LineText::fromText($line), $index));
        }

        $this->_em->flush();
    }

    public function testWillFindAuthorsInPaginator() : void
    {
        $line1 = $this->_em->find(GH7830Line::class, GH7830LineText::fromText(self::SONG[0]));
        $line2 = $this->_em->find(GH7830Line::class, GH7830LineText::fromText(self::SONG[1]));

        $this->_em->persist(new GH7830LineAuthor(1, 'Test 1', $line1));
        $this->_em->persist(new GH7830LineAuthor(2, 'Test 2', $line1));
        $this->_em->persist(new GH7830LineAuthor(3, 'Test 3', $line2));
        $this->_em->flush();
        $this->_em->clear();

        $query = $this->_em->getRepository(GH7830LineAuthor::class)
            ->createQueryBuilder('author')
            ->where('author.line IN (:lines)')
            ->setParameter('lines', [self::SONG[0], self::SONG[1]])
            ->orderBy('author.name', Criteria::ASC);

        $result = iterator_to_array(new Paginator($query));

        self::assertCount(3, $result);
    }
}

/** @Entity */
class GH7830LineAuthor
{
    /**
     * @Id
     * @Column(type="integer")
     *
     * @var int
     */
    public $id;

    /**
     * @Column
     *
     * @var string
     */
    public $name;

    /**
     * @ManyToOne(targetEntity=GH7830Line::class)
     * @JoinColumn(referencedColumnName="text")
     *
     * @var GH7830Line
     */
    public $line;

    public function __construct(int $id, string $name, GH7830Line $line)
    {
        $this->id = $id;
        $this->name = $name;
        $this->line = $line;
    }
}

/** @Entity */
class GH7830Line
{
    /**
     * @var GH7830LineText
     * @Id()
     * @Column(type="Doctrine\Tests\ORM\Functional\Ticket\GH7830LineTextType")
     */
    private $text;

    /**
     * @var int
     * @Column(type="integer")
     */
    private $lineNumber;

    public function __construct(GH7830LineText $text, int $index)
    {
        $this->text       = $text;
        $this->lineNumber = $index;
    }

    public function toString() : string
    {
        return $this->text->getText();
    }
}

final class GH7830LineText
{
    /** @var string */
    private $text;

    private function __construct(string $text)
    {
        $this->text = $text;
    }

    public static function fromText(string $text) : self
    {
        return new self($text);
    }

    public function getText() : string
    {
        return $this->text;
    }

    public function __toString() : string
    {
        return 'Line: ' . $this->text;
    }
}

final class GH7830LineTextType extends StringType
{
    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        $text = parent::convertToPHPValue($value, $platform);

        if (! is_string($text)) {
            return $text;
        }

        return GH7830LineText::fromText($text);
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        if (! $value instanceof GH7830LineText) {
            return parent::convertToDatabaseValue($value, $platform);
        }

        return parent::convertToDatabaseValue($value->getText(), $platform);
    }

    /** {@inheritdoc} */
    public function getName() : string
    {
        return self::class;
    }
}

lcobucci on 2 Oct 2019

We encountered this problem too in 2.6.4, were struggling with this for few days. Thanks to @jaikdean's comment we tried 2.6.3 and it works. I'll ask my teammate, who've been working on it, to provide some details about issue.

Wirone on 11 Oct 2019