Filtering by a relation's property

[ymarillet]

Hi,

This is affecting version 2.0.1 to current (2.0.3)

Having the following entities:

<?php

namespace AppBundle\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;
use Symfony\Component\Serializer\Annotation as Serializer;

/**
 * @ApiResource(
 *     attributes={
 *          "normalization_context"={"groups"={"foo"}},
 *          "filters"={"foo.search_filter"}
 *     }
 * )
 * @ORM\Entity
 */
class Foo
{
    /**
     * @var int The entity Id
     *
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @var string Something else
     *
     * @ORM\OneToMany(targetEntity="Bar", mappedBy="foo")
     *
     * @Serializer\Groups({"foo"})
     */
    private $bars;

    public function __construct()
    {
        $this->bars = new ArrayCollection();
    }

    public function getId()
    {
        return $this->id;
    }

    /**
     * @return string
     */
    public function getBars()
    {
        return $this->bars;
    }

    /**
     * @param string $bars
     * @return static
     */
    public function setBars($bars)
    {
        $this->bars = $bars;
        return $this;
    }
}

<?php

namespace AppBundle\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Validator\Constraints as Assert;

/**
 * @ApiResource
 * @ORM\Entity
 */
class Bar
{
    /**
     * @var int The entity Id
     *
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @var Foo
     *
     * @ORM\ManyToOne(targetEntity="Foo", inversedBy="bars")
     * @ORM\JoinColumn(nullable=false, onDelete="CASCADE")
     * @Assert\NotBlank
     */
    private $foo;

    /**
     * @var string
     *
     * @ORM\Column(nullable=false)
     * @Assert\NotBlank
     *
     * @Serializer\Groups({"foo"})
     */
    private $prop = '';

    public function getId()
    {
        return $this->id;
    }

    /**
     * @return Foo|null
     */
    public function getFoo()
    {
        return $this->foo;
    }

    /**
     * @param Foo $foo
     * @return static
     */
    public function setFoo(Foo $foo)
    {
        $this->foo = $foo;
        return $this;
    }

    /**
     * @return string
     */
    public function getProp()
    {
        return $this->prop;
    }

    /**
     * @param string $prop
     * @return static
     */
    public function setProp($prop)
    {
        $this->prop = $prop;
        return $this;
    }
}

Services defined as:

services:
  app.entity.filter.foo:
      parent:    'api_platform.doctrine.orm.search_filter'
      arguments:
          - { bars.prop: 'ipartial' }
      tags:
          - { name: 'api_platform.filter', id: 'foo.search_filter' }

Having the following data:

AppBundle\Entity\Foo:
  foo1:
    id: 1
    bars: ["@bar1", "@bar2"]
  
AppBundle\Entity\Bar:
  bar1:
    id: 1
    prop: "toto"
    foo: "@foo1"
  bar2:
    id: 2
    prop: "tata"
    foo: "@foo1"

Having set the proper filters in order to run that simple query:
http://localhost/foos?bar.prop=toto

Will result:

{
  "@context": "/contexts/Foo",
  "@id": "/foos",
  "@type": "hydra:Collection",
  "hydra:member": [
    {
      "@id": "/foos/1",
      "@type": "Foo",
      "bars": [
        {
          "@id": "/bars/1",
          "@type": "Bar",
          "prop": "toto"
        }
      ]
    }
  ],
  "hydra:totalItems": 1,
  "...otherProps...": "...otherValues...."
}

I understand that the second Bar attached to foo1 gets filtered through the doctrine query, but since we're on the Foo endpoint:

• is it still a valid behaviour ?
• if so, how to tell api-platform/doctrine to fetch all relations upon filtering on them ?

Regards,
Yohann

[teohhanhui]

Filtering does not affect normalization. See https://api-platform.com/docs/core/serialization-groups-and-relations#embedding-relations

[ymarillet]

@teohhanhui I appreciate your quickness on this issue, however, it does not answer me at all. Are you saying that it's normal that the 2nd bar entity does not appear in the response ?

[dunglas]

IMO there is a problem: the list of foo.bars should not be filtered. The expected behavior is:

• Return only foos containing a bar with prop=toto
• But return the full list of bars for this foo

[soyuka]

@ymarillet is it possible for you to get the DQL from the profiler? Thanks!

[teohhanhui]

Uhh, sorry about that. I overlooked.

[ymarillet]

@teohhanhui no problem ;-)

@soyuka hmm I've only got sql in the profiler, any tip to show dql there ?

SELECT 
  f0_.id AS id_0, 
  b1_.id AS id_1, 
  b1_.prop AS prop_2, 
  b1_.foo_id AS foo_id_3 
FROM 
  foo f0_ 
  LEFT JOIN bar b1_ ON f0_.id = b1_.foo_id 
WHERE 
  LOWER(b1_.prop) LIKE LOWER('%' || 'toto' || '%') 
  AND f0_.id IN ('1')

[ymarillet]

I've also updated my original post in order to include normalization groups (so I posted more config/entities), without that it works like a charm

[soyuka]

Fine like this. This query will get only bars having the prop LIKE toto. To me, this is the intended behavior for the current filter implementation.

• Return only foos containing a bar with prop=toto
• But return the full list of bars for this foo

For this to work we need more work as we first need to find ids that have a such relation, and then do a SELECT IN (subquery) or something like that. Anyway, not an easy task, nor currently supported.

[ymarillet]

I think the problem got introduced in 2.0.1 with the use of the DQL "partial" keyword (EagerLoadingExtension:183). Would it be a pain to make it revertable as per the behaviour of 2.0.0 with a feature flag ?

[teohhanhui]

I'd consider it a bug. Partial embedded relation is unacceptable. Let's revert the commit which is the culprit if possible.

…

On Thu, Feb 16, 2017 at 12:28 AM, ymarillet ***@***.***> wrote: I think the problem got introduced in 2.0.1 with the use of the DQL "partial" keyword (EagerLoadingExtension:183). Would it be a pain to make it revertable as per the behaviour of 2.0.0 with a feature flag ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#944 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAhf6zr8C8QqFbEdpd13RaxwZdfgIzmdks5rcyeogaJpZM4MBxr_> .

[soyuka]

I'm 👎 to revert. I'd prefer if we either make it customizable or find a clean fix. I'll think about this.

Also, if I'm filtering on bars.prop I don't see the point in getting the bars that doesn't meet the filter requirements.

[dunglas]

I agree with @teohhanhui, this behavior is weird. Filtering the root collection should not impact embedded documents.