Parent Issue

#2359437: [meta] Remove entity_load* family of functions usage from the code base

Problem/Motivation

There are case where dynamically typed entities are retrieved using the functional loader, entity_load*()

Proposed resolution

Replace calls to entity_load*($entity, ...) by using \Drupal::entityTypeManager()->getStorage($entity)->loadByProperties(...)

CommentFileSizeAuthor
#22 remove_entity_load-2723607-22.patch65.16 KBdeepakaryan1988
#17 2723607-17.patch65.15 KBvalthebald
#17 interdiff-2723607-15-17.txt698 bytesvalthebald
#16 2723607-15.patch65.14 KBvalthebald
#15 interdiff-2723607-12-15.txt737 bytesvalthebald
#12 2723607-12.patch65.15 KBvalthebald
#12 interdiff-2723607-6-12.txt6.66 KBvalthebald
#6 2723607-6.patch65.13 KBvalthebald
#4 2723607-4.patch15.71 KBvalthebald
#4 interdiff-2723607-4.txt1.81 KBvalthebald
#2 2723607.patch15.73 KBvalthebald

Comments

valthebald created an issue. See original summary.

valthebald’s picture

valthebald
Location Sofia
commented
Status: Active » Needs review
StatusFileSize
new 2723607.patch15.73 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2723607.patch, failed testing.

valthebald’s picture

valthebald
Location Sofia
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2723607-4.txt1.81 KB
new 2723607-4.patch15.71 KB

Fix failing tests

john cook’s picture

john cook commented
Issue summary: View changes
Status: Needs review » Needs work

Running grep -irn "entity_load[^ (]*(\\$" * reveals a lot more calls to entity_load*() functions passing a parameter as the type.

Here's the list:

core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php:70:      $entity = entity_load($this->getTargetDefinition()->getEntityTypeId(), $this->id);
core/modules/block_content/src/Tests/BlockContentTranslationUITest.php:182:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/comment/src/Tests/CommentTranslationUITest.php:149:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/comment/src/Tests/CommentTranslationUITest.php:169:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/comment/src/Tests/CommentTranslationUITest.php:198:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:80:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:122:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:157:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:187:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:207:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:229:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:237:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:255:        $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:265:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:273:        $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:287:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:305:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:333:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:338:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:468:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:487:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:520:        $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:554:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:560:      $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php:571:      $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/content_translation/src/Tests/ContentTranslationWorkflowsTest.php:67:    $this->entity = entity_load($this->entityTypeId, $id, TRUE);
core/modules/field/src/Tests/FieldTestBase.php:51:    $e = entity_load($entity->getEntityTypeId(), $entity->id());
core/modules/field/src/Tests/FormTest.php:557:    $entity = entity_load($entity_type, $id);
core/modules/field/src/Tests/FormTest.php:570:    $entity = entity_load($entity_type, $id);
core/modules/field/src/Tests/FormTest.php:608:    $entity = entity_load($entity_type, $id);
core/modules/field/src/Tests/FormTest.php:631:    $entity = entity_load($entity_type, $id);
core/modules/field/src/Tests/FormTest.php:645:    $entity = entity_load($entity_type, $id);
core/modules/field/src/Tests/NestedFormTest.php:107:    $entity_1 = entity_load($entity_type, 1);
core/modules/field/src/Tests/NestedFormTest.php:108:    $entity_2 = entity_load($entity_type, 2);
core/modules/field/tests/src/Kernel/FieldKernelTestBase.php:192:    $e = entity_load($entity->getEntityTypeId(), $entity->id());
core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php:101:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:86:    $entity = entity_load($this->entityTypeId, $entity_id, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:101:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:151:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:167:      $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:182:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:207:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/node/src/Tests/NodeTranslationUITest.php:427:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/rest/src/Tests/DeleteTest.php:45:      $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/DeleteTest.php:62:      $this->assertNotIdentical(FALSE, entity_load($entity_type, $entity->id(), TRUE), 'The ' . $entity_type . ' entity is still in the database.');
core/modules/rest/src/Tests/RESTTestBase.php:380:    return entity_load($this->testEntityType, $id);
core/modules/rest/src/Tests/UpdateTest.php:60:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:71:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:83:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:97:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:108:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:124:    $entity = entity_load($entity_type, $entity->id(), TRUE);
core/modules/rest/src/Tests/UpdateTest.php:138:    $loaded_entity = entity_load($entity_type, 9999, TRUE);
core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php:68:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php:87:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php:110:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/system/src/Tests/Entity/EntityCacheTagsTestBase.php:518:      $bundle_entity = entity_load($bundle_entity_type_id, $this->entity->bundle());
core/modules/system/src/Tests/Entity/EntityRevisionsTest.php:81:      $entity = entity_load($entity_type, $entity->id->value);
core/modules/system/src/Tests/Entity/EntityRevisionsTest.php:110:    $entity = entity_load($entity_type, $entity->id->value);
core/modules/system/src/Tests/Entity/EntityWithUriCacheTagsTestBase.php:83:      $bundle_entity = entity_load($bundle_entity_type_id, $this->entity->bundle());
core/modules/taxonomy/src/Tests/TermTranslationUITest.php:142:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/modules/text/tests/src/Kernel/TextWithSummaryItemTest.php:71:    $entity = entity_load($entity_type, $entity->id());
core/modules/user/src/Tests/UserTranslationUITest.php:56:    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStaticCacheTest.php:53:    $entity_1 = entity_load($this->entityTypeId, $this->entityId);
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStaticCacheTest.php:54:    $entity_2 = entity_load($this->entityTypeId, $this->entityId);
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStaticCacheTest.php:65:    $entity = entity_load($this->entityTypeId, $this->entityId);
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStaticCacheTest.php:71:    $entity = entity_load($this->entityTypeId, $this->entityId);
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStaticCacheTest.php:77:    $this->assertNull(entity_load($this->entityTypeId, $this->entityId));
core/tests/Drupal/KernelTests/Core/Config/ConfigEntityStorageTest.php:34:    $entity = entity_load($entity_type, $id);
core/tests/Drupal/KernelTests/Core/Entity/EntityFieldTest.php:380:    $entity = entity_load($entity_type, $entity->id());
core/tests/Drupal/KernelTests/Core/Entity/EntityFieldTest.php:747:    $entity = entity_load($entity_type, $entity->id());
core/tests/Drupal/KernelTests/Core/Entity/EntityTranslationTest.php:171:    $entity = entity_load($entity_type, $entity->id());
core/tests/Drupal/KernelTests/Core/Entity/EntityTranslationTest.php:195:    $entity = entity_load($entity_type, $entity->id());
core/tests/Drupal/KernelTests/Core/Entity/EntityTranslationTest.php:227:    $entity = entity_load($entity_type, $entity->id());
valthebald’s picture

valthebald
Location Sofia
commented
StatusFileSize
new 2723607-6.patch65.13 KB

Let's try again (not adding interdiff, because there are too many changes)

valthebald’s picture

valthebald
Location Sofia
commented
Status: Needs work » Needs review
john cook’s picture

john cook commented
Status: Needs review » Reviewed & tested by the community

After applying the patch from #6 no more calls to the functional loads can be found.

There is no extra changes in the patch.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2723607-6.patch, failed testing.

The last submitted patch, 6: 2723607-6.patch, failed testing.

The last submitted patch, 6: 2723607-6.patch, failed testing.

valthebald’s picture

valthebald
Location Sofia
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2723607-6-12.txt6.66 KB
new 2723607-12.patch65.15 KB

Fix failing tests

Status: Needs review » Needs work

The last submitted patch, 12: 2723607-12.patch, failed testing.

The last submitted patch, 12: 2723607-12.patch, failed testing.

valthebald’s picture

valthebald
Location Sofia
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2723607-12-15.txt737 bytes

Fix misspelled variable name

valthebald’s picture

valthebald
Location Sofia
commented
StatusFileSize
new 2723607-15.patch65.14 KB
valthebald’s picture

valthebald
Location Sofia
commented
StatusFileSize
new interdiff-2723607-15-17.txt698 bytes
new 2723607-17.patch65.15 KB

Fix another typo

The last submitted patch, 16: 2723607-15.patch, failed testing.

marvin_b8’s picture

marvin_b8 commented
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll 
$ git apply 2723607-17.patch 
error: patch failed: core/modules/rest/src/Tests/UpdateTest.php:57
error: core/modules/rest/src/Tests/UpdateTest.php: patch does not apply
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
Assigned: Unassigned » deepakaryan1988
Status: Needs work » Active
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
StatusFileSize
new remove_entity_load-2723607-22.patch65.16 KB

Rerolled #17

deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
commented
Assigned: deepakaryan1988 » Unassigned
Status: Active » Needs review
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.2.x-dev » 8.3.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Patch applies to 8.3.x, removes usages of entity_load() with a variable entity type.

RTBC and re-running tests.

  • catch committed ad76a96 on 8.3.x 
    Issue #2723607 by valthebald, deepakaryan1988: Remove entity_load* usage...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed 
+++ b/core/modules/content_translation/src/Tests/ContentTranslationUITestBase.php
@@ -465,7 +499,10 @@ protected function getChangedFieldName($entity) {
   protected function doTestTranslationEdit() {
-    $entity = entity_load($this->entityTypeId, $this->entityId, TRUE);
+    $storage = $this->container->get('entity_type.manager')
+      ->getStorage($this->entityTypeId);
+    $storage->resetCache([$this->entityId]);
+    $entity = $storage->load($this->entityId);

So there's a lot of changes like this, which are much more verbose, but 99% of them are in tests, so I think the verbosity is fine. It's also a sign that the test could use refactoring (to be either 100% kernel or 100% browser - this is a symptom of mix and match tests usually).

Committed/pushed to 8.3.x and cherry-picked to 8.2.x - given this is 99% test changes better to have in both branches to keep tests in sync.

  • catch committed 82deced on 8.2.x 
    Issue #2723607 by valthebald, deepakaryan1988: Remove entity_load* usage...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.