doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.94k stars 2.52k forks source link

PersistentCollection::matching uses getters #7573

Open arnaud-lb opened 5 years ago

arnaud-lb commented 5 years ago

Bug Report

Q A
BC Break no
Version 2.x

Summary

PersistentCollection::matching may use getters to fetch field values. This enforces the implementation of getters for every property that may be referenced in a criteria, and forces the getters to return a scalar (it is not possible to return a value object, for example).

This is troublesome for multiple reasons:

Current behaviour

PersistentCollection::matching may or may not use getters during execution, depending on the state of the collection.

How to reproduce

class Board
{
    /** @var Collection */
    private $issues;

    public function openIssues(): array
    {
        return $this->issues->matching(
            Criteria::create()->where(Criteria::expr()->eq('issueState', 'open'));
        );
    } 
}

class Issue
{
    /** @var string */
    private $issueState;

    public function getIssueState(): IssueState
    {
        return new IssueState($this->issueState);
    }
}

class IssueState
{
    /** @var string */
    private $state;
}

In this code, Board::openIssues() will find open issues when the $issues collection is not initialised, and will silently fail to find open issues when the collection is initialised.

Expected behaviour

PersistentCollection::matching must have predictable behaviour. Its behaviour should not depend on whether the collection is initialised or not.

Ocramius commented 5 years ago

Pretty much on board with this: the reliance on transparent getters is dangerous and un-productive.

The problem is that the ship already sailed here, so we can only change this behavior in a major version of the tools, and deprecate this behavior in doctrine/collections.