EagerLoadingExtension does not properly eager-load relations for which there is an already existing JOIN with conditions

[nbitouze]

API Platform version(s) affected

2.6.8 (probably any version with EagerLoadingExtension?)

Description

ApiPlatform's EagerLoadingExtension is used to add the necessary JOINs to the querybuilder that loads a collection, so that relations that are supposed to be serialized can be serialized without Doctrine having to run additional queries.

If a relation to eager-load already has a JOIN in the querybuilder, then EagerLoadingExtension chooses not to add a "copy" of that same JOIN, presumably for optimization purposes.

However if the existing JOIN (of a *ToMany relation) has a condition (like JOIN author.books book WITH book.isPublished = true), EagerLoadingExtension still does not add a JOIN. As a result, instead of author.books being serialized as an array containing every Book related to an Author, it is serialized as an array containing only the Book instances where isPublished is true.

How to reproduce

We start with two "classic" entities, Author and Book:

• Authors can have Books.
• Books can be published or not.
• Getting Authors eagerly fetch and serialize their Books.

#[ORM\Entity(repositoryClass: AuthorRepository::class)]
#[ApiResource(
    normalizationContext: ['groups' => ['author']],
)]
#[ApiFilter(PublishedAuthorFilter::class, properties: ['published' => true])]
class Author
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    private $id;

    #[ORM\OneToMany(mappedBy: 'author', targetEntity: Book::class)]
    #[Groups('author')]
    private $books;

   // ...
}

#[ORM\Entity(repositoryClass: BookRepository::class)]
#[ApiResource]
class Book
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    #[Groups('author')]
    private $id;

    #[ORM\Column(type: 'boolean')]
    #[Groups('author')]
    private $isPublished;

    #[ORM\ManyToOne(targetEntity: Author::class, inversedBy: 'books')]
    private $author;

    // ...
}

We define a filter to return only Authors that have at least one published Book.

final class PublishedAuthorFilter extends AbstractContextAwareFilter
{
    protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null)
    {
        $alias = $queryBuilder->getRootAliases()[0];
        $queryBuilder->leftJoin("{$alias}.books", 'book', Join::WITH, 'book.isPublished = true');
        $queryBuilder->andHaving("COUNT(book) > 0");
        $queryBuilder->groupBy("{$alias}.id", "book.id");
    }

   // ...
}

We query a database with two authors and three books:

dev=# select * from author;
 id 
----
  1
  2

dev=# select * from book;
 id | author_id | is_published 
----+-----------+--------------
  1 |         1 | t
  2 |         1 | f
  3 |         2 | f

The request on the whole collection of authors yields every author with every one of their books:

GET /api/authors:

[
        {
            "id": 1,
            "books": [
                {
                    "id": 1,
                    "isPublished": true
                },
                {
                    "id": 2,
                    "isPublished": false
                }
            ]
        },
        {
            "id": 2,
            "books": [
                {
                    "id": 3,
                    "isPublished": false
                }
            ]
        }
    ]

Now the request that filters only published authors yields the following:

GET /api/authors?published=true:

[
        {
            "id": 1,
            "books": [
                {
                    "id": 1,
                    "isPublished": true
                }
            ]
        }
    ]

⇨ We wanted to filter authors with at least one published book, but the filter does not show the non-published books of this author anymore.

The reason for this is that in the non-filtered request, EagerLoadingExtension notices that no JOIN exists on the books relation, and decides to create one for eager loading purposes. However in the filtered request, EagerLoadingExtension notices an existing JOIN on the books relation, does not care that this JOIN has a condition, and lets doctrine use this conditioned JOIN to eager-load the books.

Possible Solution

Assuming this behavior is indeed incorrect, a fix would be in QueryBuilderHelper::getExistingJoin. In the following block

        foreach ($parts[$rootAlias] as $join) {
            /** @var Join $join */
            if (sprintf('%s.%s', $alias, $association) === $join->getJoin()) {
                return $join;
            }
        }

the test for existing joins should be

            if (sprintf('%s.%s', $alias, $association) === $join->getJoin() && $join->getConditionType() === null) {

I have not checked whether this breaks other uses of this method.

Notice that there is an additional issue pertaining to the use of a GROUP BY clause in a filter when eager loading is active, which is that the additional SELECTs added by EagerLoadingExtension do not appear in the GROUP BY clause and must be added "manually". This is a different problem (even though it is related), we usually solve it by defining our own EntityCollectionDataProvider and adding some tracking of the additional selects when EagerLoadingExtension triggers. Here, a simple way to solve the issue is to "cheat" and hardcore $queryBuilder->groupBy("{$alias}.id", "book.id", "books_a1.id"); instead of the $queryBuilder->groupBy("{$alias}.id", "book.id"); of the PublishedAuthorFilter.