From c86caa0a89b9c6b69caa6f6b80dd548e914fc458 Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 2 Sep 2019 15:36:48 +0100 Subject: [PATCH] Issue #3068733 by blazey, amateescu, pmelab, hchonov: EntityStorageBase::loadMultiple returns unwanted entities when the static cache is warm --- .../Drupal/Core/Entity/EntityStorageBase.php | 22 +++++++++++++------ .../src/Kernel/WorkspaceIntegrationTest.php | 12 ++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Entity/EntityStorageBase.php b/core/lib/Drupal/Core/Entity/EntityStorageBase.php index 7837a5b27979..dd6fda8dbc8c 100644 --- a/core/lib/Drupal/Core/Entity/EntityStorageBase.php +++ b/core/lib/Drupal/Core/Entity/EntityStorageBase.php @@ -256,6 +256,7 @@ abstract class EntityStorageBase extends EntityHandlerBase implements EntityStor */ public function loadMultiple(array $ids = NULL) { $entities = []; + $preloaded_entities = []; // Create a new variable which is either a prepared version of the $ids // array for later comparison with the entity cache, or FALSE if no $ids @@ -271,16 +272,23 @@ abstract class EntityStorageBase extends EntityHandlerBase implements EntityStor $ids = array_keys(array_diff_key($flipped_ids, $entities)); } - // Gather entities from a 'preload' method. This method can invoke a hook to - // be used by modules that need, for example, to swap the default revision - // of an entity with a different one. Even though the base entity storage - // class does not actually invoke any preload hooks, we need to call the - // method here so we can add the pre-loaded entity objects to the static - // cache below. - $preloaded_entities = $this->preLoad($ids); + // Try to gather any remaining entities from a 'preload' method. This method + // can invoke a hook to be used by modules that need, for example, to swap + // the default revision of an entity with a different one. Even though the + // base entity storage class does not actually invoke any preload hooks, we + // need to call the method here so we can add the pre-loaded entity objects + // to the static cache below. If all the entities were fetched from the + // static cache, skip this step. + if ($ids === NULL || $ids) { + $preloaded_entities = $this->preLoad($ids); + } if (!empty($preloaded_entities)) { $entities += $preloaded_entities; + // If any entities were pre-loaded, remove them from the IDs still to + // load. + $ids = array_keys(array_diff_key($flipped_ids, $entities)); + // Add pre-loaded entities to the cache. $this->setStaticCache($preloaded_entities); } diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php index 77c24efdcda8..20a7efd04e30 100644 --- a/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php +++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php @@ -708,6 +708,18 @@ class WorkspaceIntegrationTest extends KernelTestBase { $this->assertEquals($expected_default_revision[$published_key], $entities[$entity_id]->isPublished()); } + // Check loading entities one by one. It is important to do these checks + // after the "multiple load" ones above so we can test with a fully warmed + // static cache. + foreach ($expected_default_revisions as $expected_default_revision) { + $entity_id = $expected_default_revision[$id_key]; + $entities = $this->entityTypeManager->getStorage($entity_type_id)->loadMultiple([$entity_id]); + $this->assertCount(1, $entities); + $this->assertEquals($expected_default_revision[$revision_key], $entities[$entity_id]->getRevisionId()); + $this->assertEquals($expected_default_revision[$label_key], $entities[$entity_id]->label()); + $this->assertEquals($expected_default_revision[$published_key], $entities[$entity_id]->isPublished()); + } + // Check \Drupal\Core\Entity\EntityStorageInterface::loadUnchanged(). foreach ($expected_default_revisions as $expected_default_revision) { /** @var \Drupal\Core\Entity\RevisionableInterface|\Drupal\Core\Entity\EntityPublishedInterface $entity */