[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.





[DDC-2220] Add joins to Collection Filtering API Created: 03/Jan/13  Updated: 11/Sep/13  Resolved: 11/Sep/13

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

Type: Improvement Priority: Major
Reporter: Oleg Namaka Assignee: Benjamin Eberlei
Resolution: Won't Fix Votes: 2
Labels: api, collection, filtering


 Description   

The recently added collection filtering API only goes half way in achieving a full fledged solution to filter huge collections. It still lacks joins. Look at the next two snippets:

    $criteria = Criteria::create()
        ->where(Criteria::expr()->eq('storeId', $store->getId()))
        ->andWhere(Criteria::expr()->eq('Category', 20))
        ->orderBy(array('popularity' => 'DESC'));
    return $this->BrandCategories->matching($criteria);

This piece of code works but what if there is a need to filter the BrandCategories collection by Categories with some extra criteria:

    $criteria = Criteria::create()
        ->where(Criteria::expr()->eq('storeId', $store->getId()))
        ->andWhere(Criteria::expr()->eq('Category', 20))
        ->andWhere(Criteria::expr()->eq('Category.name', 'Electronics'))
        ->orderBy(array('popularity' => 'DESC'));
    return $this->BrandCategories->matching($criteria);

That would not work.

Ideally we should have a possibility to join other entities, the Category entity in our case here:

    $criteria = Criteria::create()
        ->where(Criteria::expr()->eq('storeId', $store->getId()))
        ->andWhere(Criteria::expr()->eq('Category', 20))
        ->innerJoin(Criteria::expr()->field('Category', 'Category'))
        ->andWhere(Criteria::expr()->eq('Category.name', 'Electronics'))
        ->orderBy(array('popularity' => 'DESC'));
    return $this->BrandCategories->matching($criteria);

What do you think about it, does it make sense to add such functionality?



 Comments   
Comment by Benjamin Eberlei [ 11/Sep/13 ]

This is not a good idea, because the API has to be small to allow many different implementations, for example the in memory implementation on ArrayCollection, or the implementaiton on MongoDB ODM.





[DDC-2185] Better explain DQL "WITH" and implications for the collection filtering API Created: 04/Dec/12  Updated: 17/Dec/12

Status: Open
Project: Doctrine 2 - ORM
Component/s: Documentation, DQL
Affects Version/s: 2.2
Fix Version/s: None
Security Level: All

Type: Documentation Priority: Major
Reporter: Matthias Pigulla Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: collection, documentation, dql, filtering


 Description   

Available documentation is a bit thin regarding the "WITH" clause on JOIN expressions. Only a single example is provided in

http://docs.doctrine-project.org/en/2.1/reference/dql-doctrine-query-language.html#dql-select-examples

WITH seems to allow to only "partially" load a collection, so the collection in memory does not fully represent the associations available in the database.

The resulting collection is marked as "initialized" and it seems there is no way to tell later on whether/how (with which expression) the collection has been initialized.

When using the collection filtering API, the "initialized" flag on the collection will lead to in-memory processing. If a collection has been loaded WITH a restricting clause and another filter is applied later, results may not be what one might expect.

I assume this is by design (no idea how the collection could be "partially" loaded and behave correctly under all conditions), so filing it as a documentation issue.



 Comments   
Comment by Matthias Pigulla [ 17/Dec/12 ]

An additional observation:

If you eager-load a collection using WITH, for the resulting entities that collection is marked as initialized as described above.

Should you happen to come across the same entity during hydration in another (later) context where you explicitly eager load the same association without the WITH restriction (or with another one), the collection on that (existing) entity won't be re-initialized and still contains the associated objects found during the first query.





Generated at Tue Sep 02 06:44:00 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.