Replace all instances of node_load(), node_load_multiple(), entity_load('node') and entity_load_multiple('node') with static method calls

I will recreate the patch.

Patch recreated.

Fixed the test that failed.

Fixed the file extension.

Rerolled

Rerolled

core//modules/file/src/Tests/FileListingTest.php: $node = entity_load('node', $node->id());
core//modules/quickedit/src/Tests/QuickEditLoadingTest.php: $node = entity_load('node', 1);

There are two that were missed.

I'm working on review of this issue.

Reviewing.

Patch looks ok for me (from static perspective). Changeset is huge (because it's major refactorization), but it's also very straightforwards (it's only replacement of node_load into Node::load, and in test cases, call to container of node entities in order to flush cache). Unfortunately, patch doesn't apply on latest version of Drupal, so it's require re-roll. I'm working on this.

I updated patch, now it should apply cleanly. (Ops, missing attachment).

Fixed the fatal error in the previous patch.

I can't see any issues with the code in this patch and tests are passing.

1. +++ b/core/modules/book/src/Plugin/Block/BookNavigationBlock.php
   @@ -132,7 +133,7 @@ public function build() {
   +          $book_node = Node::load($book['nid']);
   

   Need to inject the node storage here.

2. +++ b/core/modules/comment/src/Controller/CommentController.php
   @@ -285,7 +286,7 @@ public function renderNewCommentsNodeLinks(Request $request) {
   -      $node = node_load($nid);
   +      $node = Node::load($nid);
   

   Should inject the node storage

3. +++ b/core/modules/node/src/NodeViewBuilder.php
   @@ -108,7 +109,7 @@ public static function renderLinks(array $element, array $context) {
   -      $entity = entity_load('node', $context['node_entity_id'])->getTranslation($context['langcode']);
   +      $entity = Node::load($context['node_entity_id'])->getTranslation($context['langcode']);
   

   Should inject the node storage.

4. +++ b/core/modules/node/src/Plugin/views/argument/Nid.php
   @@ -23,7 +24,7 @@ class Nid extends Numeric {
   -    $nodes = node_load_multiple($this->value);
   +    $nodes = Node::loadMultiple($this->value);
   

   Should inject the node storage

5. +++ b/core/modules/node/src/Plugin/views/argument/Vid.php
   @@ -63,7 +64,7 @@ public function titleQuery() {
   -    $nodes = node_load_multiple(array_unique($nids));
   +    $nodes = Node::loadMultiple(array_unique($nids));
   

   Should inject thr node storage.

6. +++ b/core/modules/node/src/Plugin/views/row/Rss.php
   @@ -80,7 +81,7 @@ public function preRender($values) {
   -      $this->nodes = node_load_multiple($nids);
   +      $this->nodes = Node::loadMultiple($nids);
   

   Should inject the node storage

Thanks for the review @alexpott. I am working on #25.

This patch is for comments 1 and 2 from #25.

3, 4, 5, 6 are still undone.

Comment 3 concerns a static function so I am not clear how to inject things properly.

@alexpott: currently we have a big issue here. the commentController has some methods whose are strictly related to nodes. But the comments can be used on any entity. if we inject the node storage and we don't have the nodes activated we are getting these 4 tests failing.

the ideal solution would be to create a new service with the methods whose need the node storage injected. but I don't know if this is possible after the beta release.

@skipyT What if we inject the entity manager and check if the node storage exists?

@cilefen: yes, that would be the simplest solution. but i think it would be a hack. lets see what alexpott will reply.

Let's go with the idea in #30

Comment 3 from #25 is still undone.

Fixed the fatal error form #33

Fixed the fatal error from #37
Comment 3 from #25 is still undone.

Fixed the broken tests.

Rerolled

+++ b/core/modules/action/src/Tests/BulkFormTest.php
@@ -66,14 +69,14 @@ public function testBulkForm() {
+      $changed_node = Node::load($node->id());

use $node_storage->load instead of Node::load where you already have the node storage.

@prics: not sure the replace_all_instances-2322509-45.patch is the good file uploaded. The interdiff shows you modified the code, but the patch is the old one.

@skipyT, good file, did not notice i was changing in BulkFormTest.php from node module. So I replaced Node::load() with $node_storage->load() where we already have the storage.

Rerolled because of #2304969: Port private files access bypass from SA-CORE-2014-003.

Patch rerolled. Needs reroll tag removed.

Look fine for me :)

This change looks super disruptive but actually most of the changes are in tests and the other changes are making classes more honest by injecting the node storage as a dependency. This issue is a prioritized change (deprecated function removal) as per https://www.drupal.org/core/beta-changes and it's benefits outweigh any disruption. These type of changes will be even better once #2372323: Static loaders on entity types don't return a properly typed object lands. Committed 5c3a1c6 and pushed to 8.0.x. Thanks!

diff --git a/core/modules/file/src/Tests/FileFieldDisplayTest.php b/core/modules/file/src/Tests/FileFieldDisplayTest.php
index 3448f0c..68c1179 100644
--- a/core/modules/file/src/Tests/FileFieldDisplayTest.php
+++ b/core/modules/file/src/Tests/FileFieldDisplayTest.php
@@ -8,7 +8,6 @@
 namespace Drupal\file\Tests;
 
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
-use Drupal\node\Entity\Node;
 
 /**
  * Tests the display of file fields in node and views.
@@ -57,7 +56,7 @@ function testNodeDisplay() {
     // Check that the default formatter is displaying with the file name.
     $node_storage = $this->container->get('entity.manager')->getStorage('node');
     $node_storage->resetCache(array($nid));
-    $node = Node::load($nid);
+    $node = $node_storage->load($nid);
     $node_file = file_load($node->{$field_name}->target_id);
     $file_link = array(
       '#theme' => 'file_link',

Considering we're getting the node storage here we should just use it instead of the static loader.