Issue #3069046 by mikelutz, init90, voleger, imalabya, andypost, catch, Berdir: Properly manage newly required parameter of FileUsageBase::__construct()

core/modules/file/file.services.yml

@@ -1,6 +1,6 @@
 services:
   file.usage:
     class: Drupal\file\FileUsage\DatabaseFileUsageBackend
-    arguments: ['@database', 'file_usage', '@config.factory']
+    arguments: ['@config.factory', '@database', 'file_usage']
     tags:
       - { name: backend_overridable }

core/modules/file/src/FileUsage/DatabaseFileUsageBackend.php

@@ -28,18 +28,39 @@ class DatabaseFileUsageBackend extends FileUsageBase {
   /**
    * Construct the DatabaseFileUsageBackend.
    *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory.
    * @param \Drupal\Core\Database\Connection $connection
    *   The database connection which will be used to store the file usage
    *   information.
    * @param string $table
    *   (optional) The table to store file usage info. Defaults to 'file_usage'.
-   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
-   *   (optional) The config factory.
+   *
+   * @todo Properly type-hint the constructor arguments in
+   *   https://www.drupal.org/project/drupal/issues/3070114 when the
+   *   drupal:9.0.x branch is opened.
    */
-  public function __construct(Connection $connection, $table = 'file_usage', ConfigFactoryInterface $config_factory = NULL) {
+  // @codingStandardsIgnoreLine
+  public function __construct($config_factory, $connection = NULL, $table = 'file_usage') {
+
+    // @todo Remove below conditional when the drupal:9.0.x branch is opened.
+    // @see https://www.drupal.org/project/drupal/issues/3070114
+    if (!$config_factory instanceof ConfigFactoryInterface) {
+      @trigger_error('Passing the database connection as the first argument to ' . __METHOD__ . ' is deprecated in drupal:8.8.0 and will throw a fatal error in drupal:9.0.0. Pass the config factory first. See https://www.drupal.org/node/3070148', E_USER_DEPRECATED);
+      if (!$config_factory instanceof Connection) {
+        throw new \InvalidArgumentException("The first argument to " . __METHOD__ . " should be an instance of \Drupal\Core\Config\ConfigFactoryInterface, " . gettype($config_factory) . " given.");
+      }
+      list($connection, $table, $config_factory) = array_pad(func_get_args(), 3, NULL);
+      if (NULL === $table) {
+        $table = 'file_usage';
+      }
+      if (!$config_factory instanceof ConfigFactoryInterface) {
+        $config_factory = \Drupal::configFactory();
+      }
+    }
+
     parent::__construct($config_factory);
     $this->connection = $connection;
-
     $this->tableName = $table;
   }
 

core/modules/file/src/FileUsage/FileUsageBase.php

@@ -21,14 +21,22 @@ abstract class FileUsageBase implements FileUsageInterface {
    * Creates a FileUsageBase object.
    *
    * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
-   *   (optional) The config factory. Defaults to NULL and will use
-   *   \Drupal::configFactory() instead.
+   *   The config factory. This parameter is required as of drupal:8.4.0 and
+   *   trigger a fatal error if not passed in drupal:9.0.0.
    *
-   * @deprecated The $config_factory parameter will become required in Drupal
-   *   9.0.0.
+   * @todo Update the docblock and make $config_factory required in
+   *   https://www.drupal.org/project/drupal/issues/3070114 when the
+   *   drupal:9.0.x branch is opened.
    */
   public function __construct(ConfigFactoryInterface $config_factory = NULL) {
-    $this->configFactory = $config_factory ?: \Drupal::configFactory();
+    // @todo Remove below conditional when the drupal:9.0.x branch is opened.
+    // @see https://www.drupal.org/project/drupal/issues/3070114
+    if (empty($config_factory)) {
+      @trigger_error('Not passing the $config_factory parameter to ' . __METHOD__ . ' is deprecated in drupal:8.4.0 and will trigger a fatal error in drupal:9.0.0. See https://www.drupal.org/project/drupal/issues/2801777', E_USER_DEPRECATED);
+      $config_factory = \Drupal::configFactory();
+    }
+
+    $this->configFactory = $config_factory;
   }
 
   /**

core/modules/file/tests/src/Unit/LegacyFileTest.php

@@ -0,0 +1,107 @@
+<?php
+
+namespace Drupal\Tests\file\Unit;
+
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Database\Connection;
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\file\FileInterface;
+use Drupal\file\FileUsage\DatabaseFileUsageBackend;
+use Drupal\file\FileUsage\FileUsageBase;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Provides unit tests for file module deprecation errors.
+ *
+ * @group file
+ * @group legacy
+ */
+class LegacyFileTest extends TestCase {
+
+  /**
+   * A mocked ConfigFactoryInterface.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    parent::setUp();
+    $this->configFactory = $this->prophesize(ConfigFactoryInterface::class)->reveal();
+    $container = new ContainerBuilder();
+    $container->set('config.factory', $this->configFactory);
+    \Drupal::setContainer($container);
+  }
+
+  /**
+   * Tests passing legacy arguments to FileUsageBase::__construct().
+   *
+   * @expectedDeprecation Not passing the $config_factory parameter to Drupal\file\FileUsage\FileUsageBase::__construct is deprecated in drupal:8.4.0 and will trigger a fatal error in drupal:9.0.0. See https://www.drupal.org/project/drupal/issues/2801777
+   *
+   * @throws \ReflectionException
+   */
+  public function testFileUsageBaseConstruct() {
+    $test_file_usage = new TestFileUsage();
+    $reflection = new \ReflectionObject($test_file_usage);
+    $config = $reflection->getProperty('configFactory');
+    $config->setAccessible(TRUE);
+    $this->assertSame($this->configFactory, $config->getValue($test_file_usage));
+  }
+
+  /**
+   * Tests passing legacy arguments to DatabaseFileUsageBackend::__construct().
+   *
+   * @expectedDeprecation Passing the database connection as the first argument to Drupal\file\FileUsage\DatabaseFileUsageBackend::__construct is deprecated in drupal:8.8.0 and will throw a fatal error in drupal:9.0.0. Pass the config factory first. See https://www.drupal.org/node/3070148
+   *
+   * @throws \ReflectionException
+   */
+  public function testDatabaseFileUsageBackendConstruct() {
+    $connection = $this->prophesize(Connection::class)->reveal();
+    $database_file_usage = new DatabaseFileUsageBackend($connection);
+    $reflection = new \ReflectionObject($database_file_usage);
+    $reflection_config = $reflection->getProperty('configFactory');
+    $reflection_config->setAccessible(TRUE);
+    $reflection_connection = $reflection->getProperty('connection');
+    $reflection_connection->setAccessible(TRUE);
+    $reflection_table_name = $reflection->getProperty('tableName');
+    $reflection_table_name->setAccessible(TRUE);
+    $this->assertSame($this->configFactory, $reflection_config->getValue($database_file_usage));
+    $this->assertSame($connection, $reflection_connection->getValue($database_file_usage));
+    $this->assertSame('file_usage', $reflection_table_name->getValue($database_file_usage));
+
+    $database_file_usage_test_table = new DatabaseFileUsageBackend($connection, 'test_table');
+    $this->assertSame('test_table', $reflection_table_name->getValue($database_file_usage_test_table));
+
+    $this->expectException(\InvalidArgumentException::class);
+    $database_file_usage_exception = new DatabaseFileUsageBackend('Invalid Argument');
+  }
+
+}
+
+/**
+ * Provides a pass through to the abstract FileUsageBase() constructor.
+ */
+class TestFileUsage extends FileUsageBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function add(FileInterface $file, $module, $type, $id, $count = 1) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function delete(FileInterface $file, $module, $type = NULL, $id = NULL, $count = 1) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function listUsage(FileInterface $file) {
+  }
+
+}