[DDC-2226] addFilterConstraint is called with wrong ClassMetadata, if Entity uses inheritance Created: 09/Jan/13  Updated: 13/Jan/13  Resolved: 13/Jan/13

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.3.2
Fix Version/s: None
Security Level: All

Type: Bug Priority: Major
Reporter: Philipp Dobrigkeit Assignee: Benjamin Eberlei
Resolution: Invalid Votes: 0
Labels: filtering


 Description   

When using the filter system to modify the generated SQL the addFilterConstraint method always receives the metadata of the parent object. Both for Joined and Single Table Inheritance, which is not helpful if you want to do something based on the entity, but for example the parent entity does not fulfill the prequisites.



 Comments   
Comment by Marco Pivetta [ 09/Jan/13 ]

Can you add some example code?

Comment by Philipp Dobrigkeit [ 09/Jan/13 ]
ParentClass.php
<?php
namespace Entities;

/**
 * @Entity @Table(name="parent_class")
 * @InheritanceType("SINGLE_TABLE")
 * @DiscriminatorColumn(name="discr", type="string")
 * @DiscriminatorMap({"parentclass" = "ParentClass", "childclass" = "ChildClass"})
 **/
class ParentClass {

    /**
     * @Id @GeneratedValue @Column(type="integer")
     * @var int
     **/
    protected $id;

}
ChildClass.php
<?php
namespace Entities;

/**
 * @Entity
 **/
class ChildClass extends ParentClass {

    /**
     * @Column(type="string")
     * @var string
     **/
    protected $foo;

}
filter.php
<?php
use Doctrine\ORM\Mapping\ClassMetaData,
    Doctrine\ORM\Query\Filter\SQLFilter;

class TestFilter extends SQLFilter
{
    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias)
    {
        print_r($targetEntity);
        exit;

        return "";
    }
}
index.php
<?php

require_once "vendor/autoload.php";

// Create a simple "default" Doctrine ORM configuration for XML Mapping
$isDevMode = true;

$config = Setup::createAnnotationMetadataConfiguration(array(__DIR__."/Entities"), $isDevMode);
//$config = Setup::createYAMLMetadataConfiguration(array(__DIR__."/config/yaml"), $isDevMode);

// database configuration parameters
$conn = array(
    'driver' => 'pdo_sqlite',
    'path' => __DIR__ . '/db.sqlite',
);

// obtaining the entity manager
$entityManager = \Doctrine\ORM\EntityManager::create($conn, $config);

require_once "Entities/ParentClass.php";
require_once "Entities/ChildClass.php";
require_once "filter.php";

$config->addFilter("test", "TestFilter");

$filter = $entityManager->getFilters()->enable("test");

$entityManager->find('Entities\ChildClass', 1);

When doing this the filter is called, but with the ParentClass metadata. Looking into the source-code, both the SingleTablePersister:158 as well as the JoinedSubclassPersister:386 explicity change the metadata for the filter to the RootEntitiy, but there is no indication why this is done?! If there is a reason for doing so, it should also pass the child metadata, so the filter can actually work based on the queried entity, not the root entity only.

Comment by Benjamin Eberlei [ 12/Jan/13 ]

This is not a bug, its by design, see the documentation of filters.

We cannot pass the child class, since there are common cases where using fields of the children will break the complete query.

Comment by Philipp Dobrigkeit [ 12/Jan/13 ]

I just read the documentation again.

Throughout this document the example MyLocaleFilter class will be used to illustrate how the filter feature works. A filter class must extend the base Doctrine\ORM\Query\Filter\SQLFilter class and implement the addFilterConstraint method. The method receives the ClassMetadata of the filtered entity and the table alias of the SQL table of the entity.

it states that the method receives the ClassMetadata of the filtered entity, which the above shows is not true. My Filter is supposed to add security constraints and it is not possible to have explicit differences between for example the different "Roles": "Admin" and "User", because I would only get the ClassMetadata of "Role".

Would you be ok if there was an optional third parameter to the addFilterConstraint that passes the actual metadata and then I can check if that parameter is present?

Comment by Philipp Dobrigkeit [ 12/Jan/13 ]

See my comment

Comment by Benjamin Eberlei [ 13/Jan/13 ]

No that is not possible.

As for the documentation, i fixed it in the following commit:

https://github.com/doctrine/orm-documentation/commit/47043a54a535941f5cdcdacb4a9dd2bb5de887c1

I was pretty sure Alexander added that paragraph before, sorry to point you to the docs without rechecking first.

Comment by Stefan Kleff [ 13/Jan/13 ]

Can you please be more specific or provide an example? Saying "there are some edge cases" does not clarify things. There seems to be a valid reason why the root is used instead of the entity, but it would be very helpful to know this for writing custom filters.

Comment by Stefan Kleff [ 13/Jan/13 ]

And just to emphasise the importance: Afaik all filters (which I know) rely on the entity and not the root entity, like f.ex. the commonly used DoctrineExtensions: https://github.com/l3pp4rd/DoctrineExtensions/blob/master/lib/Gedmo/SoftDeleteable/Filter/SoftDeleteableFilter.php

Comment by Alexander [ 13/Jan/13 ]

The problem is that with for example Joined Table Inheritance the tables of the child classes are outer/left joined. Adding a filter for those tables will result in filtering away the data of the joined table, not of the whole entity. Based on this we decided to implement only filtering on the root entity of the inheritance tree.

Does this answer your question?

Comment by Philipp Dobrigkeit [ 13/Jan/13 ]

At least to some bit. But aren't the filters completely in user-land code and I would be responsible for making sure what I am doing? I. e. checking if I have a child entity at hand? You guys cannot be responsible for every mistake your users make and limiting the functionality in one of the (already) few extension points makes things harder for the developer.

I understand that there might be cases when people implement a bad filter and things break down, but that can happen.

For us this thing is related to the security of our complete application and it makes a huge difference if a user must be allowed to view every entity of a inheritance hierarchy or I can define on a much more granular level. And for using a filter to limit the result returned from the DB is the logical place to do so.

I also would be happy to hear some kind of suggestion what I could do instead, not only a "will not fix, not my problem" kind of response

Are there any filters implemented in the main ORM project?

Comment by Benjamin Eberlei [ 13/Jan/13 ]

No, that is not our opinion. The ORM is complicated and we don't make it even more complicated by allowing users to practically do everything they want.

Filters and inheritance are common enough problems that we think this would occur relatively often and by the way this bug occurs it wouldn't even be apparent to probably most people. Thats a very dangerous setup we avoid by restricing the usage of this feature.

Inheritance is overused in OOP applications and composition is much better in most cases anyways. With a model that is not using inheritance, you would not have this problem. With inheritance, you can still do filtering as long as the filtered properties are on the root of the inheritance hierachy.

There are no filters in the main ORM.

Generated at Tue Jul 29 17:15:31 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.