doctrine/mongodb-odm

The implementation of Countable on the CachingIterator is causing the UniqueEntityValidator to always fail

jasonw-i6 opened this issue · 2 comments

BC Break Report

Q A
BC Break yes
Version 2.5.0

Summary

In the 2.5.0 release the CachingIterator was improved to implement the Countable interface. However, the implementation is such that the internal pointer of $this->items will be at the end after counting due to the call to $this->exhaustIterator();. The means that calls to valid will fail after counting.

This is problematic as it's tripping up the UniqueEntityValidator from the Symfony Bridge. See https://github.com/symfony/symfony/blob/6.4/src/Symfony/Bridge/Doctrine/Validator/Constraints/UniqueEntityValidator.php. The check for valid on line 166 will always fail given the count on line 163.

Either there should be a reset after counting or alternatively a bug would have to be submitted to the Symfony project. Given the inbuilt ArrayIterator remains valid after after counting, I'd suggest that counting should leave a valid iterator as valid and the bug is here.

Previous behavior

The CachingIterator worked fine with the UniqueEntityValidator from the Symfony Bridge

Current behavior

The CachingIterator causes the UniqueEntityValidator to always validate without error when there is exactly one existing matching entity.

How to reproduce

use Doctrine\ODM\MongoDB\Iterator\CachingIterator;
use PHPUnit\Framework\TestCase;

class CachingIteratorTest extends TestCase 
{
    public function testCountCausesValidToFail(): void 
    {
        $iterator = new CachingIterator(new \ArrayIterator([1,2,3]));
        $this->assertTrue($iterator->valid());
        count($iterator);
        $this->assertTrue($iterator->valid());
    }
}

Thanks for the report @jasonw-i6! Adding this to next patch release's backlog.

Ideally we should also add a test to https://github.com/doctrine/DoctrineMongoDBBundle

+1 for maintaining the exact current position rather than just resetting as I suggested!