DDC-3687: Entities part of a hierarchy seem not to inherit SLC configuration from 'root' Entity

[doctrinebot]

Jira issue originally created by user holtkamp:

When using the Second Level Cache on associations, the documentation states that "the target entity must also be marked as cacheable"

It seems that when this targetEntity is an (possibly abstract) 'root' Entity of an Entity hierarchy, all Entities of that hierarchy need to be marked as cacheable. I would expect that all Entities that are part of the Entity hierarchy, also inherit this cache configuration metadata of the root Entity...

Is this intended behavior?

For example, this does NOT work:

Entity
 - cacheable X-to-many association, targetEntity: RootEntity

RootEntity, SINGLE_TABLE (marked as cacheable)
 - SubEntity1
 - SubEntityN

The generated error:

Array
(
    [type] => 1
    [message] => Call to a member function resolveAssociationEntries() on a non-object
    [file] => /srv/www/code/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php
    [line] => 92
)

For example, this does DOES work

Entity
 - cacheable X-to-many association, targetEntity: RootEntity

RootEntity, SINGLE_TABLE 
 - SubEntity1 (marked as cacheable)
 - SubEntityN (marked as cacheable)

[doctrinebot]

• relates to DDC-3689: [GH-1382] Patch second level cache association hydration

[doctrinebot]

Comment created by @FabioBatSilva:

Hi [~holtkamp] All subclasses should already Inherit the cache configuration .

Can please attach your entities/mapping ?

[doctrinebot]

Comment created by holtkamp:

Mm, you are right, the cache configuration seems to be inherited. General idea of my Mapping is at the end of this comment.

The error occurs after the cache has been filled (2nd request). The following CollectionCacheEntry is used to lookup the EntityEntries:

identifiers => Array ( [0] => Doctrine\ORM\Cache\EntityCacheKey Object ( [identifier] => Array ( [id] => 357 )
entityClass => Project\Domain\Entity\Cms\Page\SectionAbstract
hash => project.domain.entity.cms.page.sectionabstract_357

This results in a $entityEntries collection with one entry, (which is empty!) as retrieved from cache at:
https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php#L84

array(1) {
  [0] =>
  NULL
}

Having a look at the actual content of the SLC cache shows that the entry exists, so the lookup goes wrong. It seems 'DefaultRegion::getMultiple()' contains a bug:

https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php#L122

$returnableItems[$index] = $items[$key];

Should be

$returnableItems[$index] = $items[$index];

Since the $items array uses the same keys as the $keysToRetrieve array, filled at https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php#L106

My mapping (pseudo-code)

/****
 * @Entity
 * @Cache
 */
class Page
{
    /****
     * @OneToMany
     * @Cache
     */
    private $translations;
}

/****
 * @Entity
 * @Cache
 */
class Translation
{
    /****
     * @ManyToOne
     */
    private $page;

    /****
     * @OneToMany
     * @Cache
     */
    private $sections;
}

/****
 * @Entity
 * @Cache
 */
abstract class SectionAbstract
{
    /****
     * @ManyToOne
     */
    private $pageTranslation;
}

/****
 * @Entity
 */
class SectionFaq extends SectionAbstract
{

    /****
     * @var string
     */
    private $answer;

    /****
     * @var string
     */
    private $question;
}

/****
 * @Entity
 */
class SectionText extends SectionAbstract
{
    /****
     * @var string
     */
    private $content;
}

[doctrinebot]

Comment created by @FabioBatSilva:

Yes, it looks like a bug on DefaultRegion::getMultiple()
Can you try to write a failing test case or/and maybe send a PR ?

[doctrinebot]

Comment created by holtkamp:

Mm, will try to find some time this week. Not very experienced in writing test cases though...

[doctrinebot]

Comment created by @FabioBatSilva:

Cool,
Take a look at : SecondLevelCacheTest and let me know if you need help..

[doctrinebot]

Comment created by holtkamp:

Ok, challenge accepted, got a test case running in this branch, I used:

php ./vendor/bin/phpunit -v --exclude-group performance,non-cacheable,locking_functional --filter DefaultRegionTest

It seems the way DefaultRegion::getMultiple() works is overcomplicated? Maybe just return the $items array as an end-result?

[doctrinebot]

Comment created by @FabioBatSilva:

If you want to refactory it some how, that will be great..
DefaultRegion::getMultiple tries to check if all keys are found in the cache, and then maps it back to the original key
If any of then are missing it will return null which will trigger a new database query to reload the entries and repopulate the cache.

Overall you branch seems good, Can you send a PR please ?

[doctrinebot]

Comment created by holtkamp:

Ok, I gave it a shot at #1382, AFAIK the index of the resulting array can just be numeric, right?

[doctrinebot]

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1382] was labeled:
#1382

[doctrinebot]

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1382] was assigned:
#1382

[doctrinebot]

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1382] was merged:
#1382

[doctrinebot]

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1382] was unassigned:
#1382

[doctrinebot]

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1382] was assigned:
#1382

[lcobucci]

Fixed by #1382