Merge pull request #6640 from doctrine/fix/#6284-#6217-avoid-passing-l2-cache-information-internals-to-the-uow

#6217 #6284 when hydrating an entity with a composite primary key that is both an `EAGER` and a `LAZY` association and also cached, the `DefaultQueryCache` tries to pass L2 cache implementation detail objects to the `UnitOfWork`

lib/Doctrine/ORM/Cache/DefaultQueryCache.php

@@ -92,13 +92,13 @@ class DefaultQueryCache implements QueryCache
             return null;
         }
 
-        $entry = $this->region->get($key);
+        $cacheEntry = $this->region->get($key);
 
-        if ( ! $entry instanceof QueryCacheEntry) {
+        if ( ! $cacheEntry instanceof QueryCacheEntry) {
             return null;
         }
 
-        if ( ! $this->validator->isValid($key, $entry)) {
+        if ( ! $this->validator->isValid($key, $cacheEntry)) {
             $this->region->evict($key);
 
             return null;
@@ -117,11 +117,11 @@ class DefaultQueryCache implements QueryCache
             return new EntityCacheKey($cm->rootEntityName, $entry['identifier']);
         };
 
-        $cacheKeys = new CollectionCacheEntry(array_map($generateKeys, $entry->result));
+        $cacheKeys = new CollectionCacheEntry(array_map($generateKeys, $cacheEntry->result));
         $entries   = $region->getMultiple($cacheKeys);
 
         // @TODO - move to cache hydration component
-        foreach ($entry->result as $index => $entry) {
+        foreach ($cacheEntry->result as $index => $entry) {
             $entityEntry = is_array($entries) && array_key_exists($index, $entries) ? $entries[$index] : null;
 
             if ($entityEntry === null) {
@@ -210,6 +210,25 @@ class DefaultQueryCache implements QueryCache
                 $collection->setInitialized(true);
             }
 
+            foreach ($data as $fieldName => $unCachedAssociationData) {
+                // In some scenarios, such as EAGER+ASSOCIATION+ID+CACHE, the
+                // cache key information in `$cacheEntry` will not contain details
+                // for fields that are associations.
+                //
+                // This means that `$data` keys for some associations that may
+                // actually not be cached will not be converted to actual association
+                // data, yet they contain L2 cache AssociationCacheEntry objects.
+                //
+                // We need to unwrap those associations into proxy references,
+                // since we don't have actual data for them except for identifiers.
+                if ($unCachedAssociationData instanceof AssociationCacheEntry) {
+                    $data[$fieldName] = $this->em->getReference(
+                        $unCachedAssociationData->class,
+                        $unCachedAssociationData->identifier
+                    );
+                }
+            }
+
             $result[$index] = $this->uow->createEntity($entityEntry->class, $data, self::$hints);
         }
 
@@ -246,7 +265,6 @@ class DefaultQueryCache implements QueryCache
         $data        = [];
         $entityName  = reset($rsm->aliasMap);
         $rootAlias   = key($rsm->aliasMap);
-        $hasRelation = ( ! empty($rsm->relationMap));
         $persister   = $this->uow->getEntityPersister($entityName);
 
         if ( ! ($persister instanceof CachedPersister)) {
@@ -258,8 +276,6 @@ class DefaultQueryCache implements QueryCache
         foreach ($result as $index => $entity) {
             $identifier                     = $this->uow->getEntityIdentifier($entity);
             $entityKey                      = new EntityCacheKey($entityName, $identifier);
-            $data[$index]['identifier']     = $identifier;
-            $data[$index]['associations']   = [];
 
             if (($key->cacheMode & Cache::MODE_REFRESH) || ! $region->contains($entityKey)) {
                 // Cancel put result if entity put fail
@@ -268,9 +284,8 @@ class DefaultQueryCache implements QueryCache
                 }
             }
 
-            if ( ! $hasRelation) {
-                continue;
-            }
+            $data[$index]['identifier']   = $identifier;
+            $data[$index]['associations'] = [];
 
             // @TODO - move to cache hydration components
             foreach ($rsm->relationMap as $alias => $name) {

tests/Doctrine/Tests/ORM/Functional/Ticket/GH6217Test.php

@@ -0,0 +1,78 @@
+<?php
+namespace Doctrine\Tests\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group #6217
+ */
+final class GH6217Test extends OrmFunctionalTestCase
+{
+    public function setUp() : void
+    {
+        $this->enableSecondLevelCache();
+
+        parent::setUp();
+
+        $this->_schemaTool->createSchema([
+            $this->_em->getClassMetadata(GH6217AssociatedEntity::class),
+            $this->_em->getClassMetadata(GH6217FetchedEntity::class),
+        ]);
+    }
+
+    public function testLoadingOfSecondLevelCacheOnEagerAssociations() : void
+    {
+        $lazy = new GH6217AssociatedEntity();
+        $eager = new GH6217AssociatedEntity();
+        $fetched = new GH6217FetchedEntity($lazy, $eager);
+
+        $this->_em->persist($eager);
+        $this->_em->persist($lazy);
+        $this->_em->persist($fetched);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $repository = $this->_em->getRepository(GH6217FetchedEntity::class);
+        $filters    = ['eager' => $eager->id];
+
+        self::assertCount(1, $repository->findBy($filters));
+        $queryCount = $this->getCurrentQueryCount();
+
+        /* @var $found GH6217FetchedEntity[] */
+        $found = $repository->findBy($filters);
+
+        self::assertCount(1, $found);
+        self::assertInstanceOf(GH6217FetchedEntity::class, $found[0]);
+        self::assertSame($lazy->id, $found[0]->lazy->id);
+        self::assertSame($eager->id, $found[0]->eager->id);
+        self::assertEquals($queryCount, $this->getCurrentQueryCount(), 'No queries were executed in `findBy`');
+    }
+}
+
+/** @Entity @Cache(usage="NONSTRICT_READ_WRITE") */
+class GH6217AssociatedEntity
+{
+    /** @Id @Column(type="string") @GeneratedValue(strategy="NONE") */
+    public $id;
+
+    public function __construct()
+    {
+        $this->id = uniqid(self::class, true);
+    }
+}
+
+/** @Entity @Cache(usage="NONSTRICT_READ_WRITE") */
+class GH6217FetchedEntity
+{
+    /** @Id @Cache("NONSTRICT_READ_WRITE") @ManyToOne(targetEntity=GH6217AssociatedEntity::class) */
+    public $lazy;
+
+    /** @Id @Cache("NONSTRICT_READ_WRITE") @ManyToOne(targetEntity=GH6217AssociatedEntity::class, fetch="EAGER") */
+    public $eager;
+
+    public function __construct(GH6217AssociatedEntity $lazy, GH6217AssociatedEntity $eager)
+    {
+        $this->lazy  = $lazy;
+        $this->eager = $eager;
+    }
+}