Convert file SQL queries to the Entity Query API

Working on this.

Go for it, testbot!

Let's try what happens w/o change in function system_update_8024().

1. +++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
   @@ -23,13 +24,14 @@ class Fid extends Numeric {
   +    $titles = array();
   +    foreach ($fids as &$fid) {
   +      $file = $controller->load($fid);
   

   No need to have $fid be by reference.

2. +++ b/core/modules/user/user.install
   @@ -636,22 +636,31 @@ function user_update_8011() {
   +    );
   +    if (empty($fid)) {
   +      $file = $controller->create($values);
   

   No need for empty(). Switch the conditions.

3. +++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
   @@ -23,13 +24,14 @@ class Fid extends Numeric {
   +    $controller = \Drupal::entityManager()->getStorageController('file');
   +    $titles = array();
   +    foreach ($fids as &$fid) {
   +      $file = $controller->load($fid);
   +      $titles[] = String::checkPlain($file->getFilename());
        }
   

   These files should be bulk-loaded before the foreach. $controller->loadMultiple()

Can we convert update functions to entity query?

Edit: Leaving at NR for testbot.

Fixed comments. chx confirmed on IRC that we do not touch update hooks, so I removed changes to user_update_8011().

#7: 2068343_7.patch queued for re-testing.

#7: 2068343_7.patch queued for re-testing.

+++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
@@ -23,13 +24,14 @@ class Fid extends Numeric {
+    $controller = \Drupal::entityManager()->getStorageController('file');

This plugin needs injection @see Plugins can receive injected dependencies from the container

Good point.

1. +++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
   @@ -17,19 +22,64 @@
   +  /**
   +   * Constructs a Drupal\Component\Plugin\PluginBase object.
   

   Seems to be just a copy and paste of some other code.

2. +++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
   @@ -17,19 +22,64 @@
   +   * @param \Drupal\Core\Entity\EntityManager $entity_manager
   +   *   The entity manager.
   +   */
   +  public function __construct(array $configuration, $plugin_id, array $plugin_definition, EntityManager $entity_manager, QueryFactory $entityQuery) {
   

   You missed to document the QueryFactory.

3. +++ b/core/modules/file/lib/Drupal/file/Plugin/views/argument/Fid.php
   @@ -17,19 +22,64 @@
   +    parent::__construct($configuration, $plugin_id, $plugin_definition);
   +    $this->entityManager = $entity_manager;
   +  }
   

   ... so we needs tests? This does NOT set $entityQuery so it should not work.

I'm ashamed... However, all comments from #13 should be fixed in attached patch. The only view in core that currently uses this argument plugin is file listing (detailed usage info part). I added some more tests to that, that should also catch non-working Fid plugin.

I have tested the patch and it works, also there's no coding standard problems; all seem good for me.

Committed 0ff5e6d and pushed to 8.x. Thanks!

YAY!