From c3a13b6aaa2da45769d32b8aa9af74ca178c1088 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 11 Sep 2024 14:16:57 -0700
Subject: [PATCH 1/7] full coverage for DrupalFiles::retrieveFile()

---
 modules/common/src/Util/DrupalFiles.php       | 28 ++++++-
 .../tests/src/Kernel/Util/DrupalFilesTest.php | 76 +++++++++++++++++++
 .../tests/src/Unit/Util/DrupalFilesTest.php   |  4 +-
 3 files changed, 106 insertions(+), 2 deletions(-)
 create mode 100644 modules/common/tests/src/Kernel/Util/DrupalFilesTest.php

diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index eda67b6a0a..f4839c4315 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -94,10 +94,36 @@ public function retrieveFile($url, $destination) {
       return $this->fileCreateUrl("{$destination}/{$filename}");
     }
     else {
-      return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE);
+      return $this->systemRetrieveFile($url, $destination);
     }
   }
 
+  /**
+   * Attempts to get a file using Guzzle HTTP client and to store it locally.
+   *
+   * The destination file will never be a managed file.
+   *
+   * @param string $url
+   *   The URL of the file to grab.
+   * @param string $destination
+   *   Stream wrapper URI specifying where the file should be placed. If a
+   *   directory path is provided, the file is saved into that directory under
+   *   its original name. If the path contains a filename as well, that one will
+   *   be used instead.
+   *   If this value is omitted, the site's default files scheme will be used,
+   *   usually "public://".
+   *
+   * @return mixed
+   *   One of these possibilities:
+   *   - If it succeeds the location where the file was saved.
+   *   - If it fails, FALSE.
+   *
+   * @see \system_retrieve_file()
+   */
+  protected function systemRetrieveFile($url, $destination = NULL) {
+    return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE);
+  }
+
   /**
    * Given a URI like public:// retrieve the URL.
    */
diff --git a/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php b/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php
new file mode 100644
index 0000000000..a280e26b5a
--- /dev/null
+++ b/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php
@@ -0,0 +1,76 @@
+<?php
+
+namespace Drupal\Tests\common\Kernel\Util;
+
+use Drupal\common\Util\DrupalFiles;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * @covers \Drupal\common\Util\DrupalFiles
+ * @coversDefaultClass \Drupal\common\Util\DrupalFiles
+ *
+ * @group dkan
+ * @group common
+ * @group kernel
+ */
+class DrupalFilesTest extends KernelTestBase {
+
+  protected static $modules = [
+    'common',
+  ];
+
+  public function provideExceptions() {
+    return [
+      ['Only file:// and http(s) urls are supported', 'badscheme://', 'any_destination'],
+      ["Only moving files to Drupal's public directory (public://) is supported", 'file://', 'badscheme://'],
+    ];
+  }
+
+  /**
+   * @covers ::retrieveFile
+   *
+   * @dataProvider provideExceptions
+   */
+  public function testExceptions($exception_message, $url, $destination) {
+    /** @var \Drupal\common\Util\DrupalFiles $drupal_files */
+    $drupal_files = $this->container->get('dkan.common.drupal_files');
+    $this->expectException(\Exception::class);
+    $this->expectExceptionMessage($exception_message);
+    $drupal_files->retrieveFile($url, $destination);
+  }
+
+  public function provideRetrieve() {
+    return [
+      ['http://'],
+      ['https://'],
+    ];
+  }
+
+  /**
+   * @covers ::retrieveFile
+   *
+   * @dataProvider provideRetrieve
+   */
+  public function testHttpSource($url) {
+    // We're checking the internal logic of retrieveFile(), to make sure it
+    // calls systemRetrieveFile() given the inputs, and not testing whether the
+    // file is successfully retrieved.
+    // Mock a DrupalFiles object so that we can mock systemRetrieveFile().
+    $drupal_files = $this->getMockBuilder(DrupalFiles::class)
+      ->setConstructorArgs([
+        $this->container->get('file_system'),
+        $this->container->get('stream_wrapper_manager'),
+      ])
+      ->onlyMethods(['systemRetrieveFile'])
+      ->getMock();
+    $drupal_files->expects($this->once())
+      ->method('systemRetrieveFile')
+      ->willReturn('/your/fake/path');
+
+    $this->assertEquals(
+      '/your/fake/path',
+      $drupal_files->retrieveFile($url, 'public://')
+    );
+  }
+
+}
diff --git a/modules/common/tests/src/Unit/Util/DrupalFilesTest.php b/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
index f100596fbe..8f4e2b7dd7 100644
--- a/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
+++ b/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
@@ -12,7 +12,9 @@
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
- *
+ * @group dkan
+ * @group common
+ * @group unit
  */
 class DrupalFilesTest extends TestCase {
 

From 159ae179000063d71151ecb199a9738c991bd25f Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 11 Sep 2024 14:44:25 -0700
Subject: [PATCH 2/7] some work

---
 modules/common/common.services.yml            |  2 +
 modules/common/src/Util/DrupalFiles.php       | 48 +++++++++++++++++--
 .../tests/src/Kernel/Util/DrupalFilesTest.php |  2 +
 .../tests/src/Unit/Util/DrupalFilesTest.php   |  4 ++
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/modules/common/common.services.yml b/modules/common/common.services.yml
index 24c4f7f0ad..b8a6ff7a0f 100644
--- a/modules/common/common.services.yml
+++ b/modules/common/common.services.yml
@@ -18,6 +18,8 @@ services:
     arguments:
       - '@file_system'
       - '@stream_wrapper_manager'
+      - '@http_client_factory'
+      - '@messenger'
 
   dkan.common.node_storage:
     class: Drupal\node\NodeStorage
diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index f4839c4315..84553d93af 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -3,8 +3,13 @@
 namespace Drupal\common\Util;
 
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\Core\File\Exception\FileException;
+use Drupal\Core\File\Exception\InvalidStreamWrapperException;
 use Drupal\Core\File\FileSystemInterface;
+use Drupal\Core\Http\ClientFactory;
+use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
+use GuzzleHttp\Exception\TransferException;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -32,6 +37,20 @@ class DrupalFiles implements ContainerInjectionInterface {
    */
   private $streamWrapperManager;
 
+  /**
+   * HTTP client factory service.
+   *
+   * @var \Drupal\Core\Http\ClientFactory
+   */
+  private ClientFactory $httpClientFactory;
+
+  /**
+   * Messenger service.
+   *
+   * @var \Drupal\Core\Messenger\MessengerInterface
+   */
+  private MessengerInterface $messenger;
+
   /**
    * Inherited.
    *
@@ -40,16 +59,25 @@ class DrupalFiles implements ContainerInjectionInterface {
   public static function create(ContainerInterface $container) {
     return new static(
       $container->get('file_system'),
-      $container->get('stream_wrapper_manager')
+      $container->get('stream_wrapper_manager'),
+      $container->get('http_client_factory'),
+      $container->get('messenger')
     );
   }
 
   /**
    * Constructor.
    */
-  public function __construct(FileSystemInterface $filesystem, StreamWrapperManager $streamWrapperManager) {
+  public function __construct(
+    FileSystemInterface $filesystem,
+    StreamWrapperManager $streamWrapperManager,
+    ClientFactory $httpClientFactory,
+    MessengerInterface $messenger
+  ) {
     $this->filesystem = $filesystem;
     $this->streamWrapperManager = $streamWrapperManager;
+    $this->httpClientFactory = $httpClientFactory;
+    $this->messenger = $messenger;
   }
 
   /**
@@ -119,9 +147,23 @@ public function retrieveFile($url, $destination) {
    *   - If it fails, FALSE.
    *
    * @see \system_retrieve_file()
+   * @see https://www.drupal.org/node/3223362
    */
   protected function systemRetrieveFile($url, $destination = NULL) {
-    return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE);
+    try {
+      $data = (string) $this->httpClientFactory
+        ->fromOptions()
+        ->get($url)
+        ->getBody();
+      return $this->filesystem->saveData($data, $destination, FileSystemInterface::EXISTS_REPLACE);
+    }
+    catch (TransferException $exception) {
+      $this->messenger->addError(t('Failed to fetch file due to error "%error"', ['%error' => $exception->getMessage()]));
+    }
+    catch (FileException | InvalidStreamWrapperException $e) {
+      $this->messenger->addError(t('Failed to save file due to error "%error"', ['%error' => $e->getMessage()]));
+    }
+    return FALSE;
   }
 
   /**
diff --git a/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php b/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php
index a280e26b5a..d7900492fc 100644
--- a/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php
+++ b/modules/common/tests/src/Kernel/Util/DrupalFilesTest.php
@@ -60,6 +60,8 @@ public function testHttpSource($url) {
       ->setConstructorArgs([
         $this->container->get('file_system'),
         $this->container->get('stream_wrapper_manager'),
+        $this->container->get('http_client_factory'),
+        $this->container->get('messenger'),
       ])
       ->onlyMethods(['systemRetrieveFile'])
       ->getMock();
diff --git a/modules/common/tests/src/Unit/Util/DrupalFilesTest.php b/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
index 8f4e2b7dd7..408021c14b 100644
--- a/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
+++ b/modules/common/tests/src/Unit/Util/DrupalFilesTest.php
@@ -4,6 +4,8 @@
 
 use Drupal\common\Util\DrupalFiles;
 use Drupal\Core\File\FileSystemInterface;
+use Drupal\Core\Http\ClientFactory;
+use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
 use MockChain\Chain;
@@ -36,6 +38,8 @@ private function getContainer(): ContainerInterface {
     $options = (new Options())
       ->add('file_system', FileSystemInterface::class)
       ->add('stream_wrapper_manager', StreamWrapperManager::class)
+      ->add('http_client_factory', ClientFactory::class)
+      ->add('messenger', MessengerInterface::class)
       ->index(0);
 
     return (new Chain($this))

From 070d548240d45175367f1bb3067fe616a64b30d9 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Tue, 4 Feb 2025 13:38:45 -0800
Subject: [PATCH 3/7] cs

---
 modules/common/src/Util/DrupalFiles.php | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index 4dd993ecf3..1ceeafd35f 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -52,9 +52,7 @@ class DrupalFiles implements ContainerInjectionInterface {
   private MessengerInterface $messenger;
 
   /**
-   * Inherited.
-   *
-   * @inheritdoc
+   * {@inheritDoc}
    */
   public static function create(ContainerInterface $container) {
     return new static(
@@ -72,7 +70,7 @@ public function __construct(
     FileSystemInterface $filesystem,
     StreamWrapperManager $streamWrapperManager,
     ClientFactory $httpClientFactory,
-    MessengerInterface $messenger
+    MessengerInterface $messenger,
   ) {
     $this->filesystem = $filesystem;
     $this->streamWrapperManager = $streamWrapperManager;

From 6ba8982a2b3eaad91cdbe20898ff6818266a3426 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 5 Feb 2025 07:32:17 -0800
Subject: [PATCH 4/7] fix test

---
 .../tests/src/Kernel/Transform/ResourceImporterTest.php       | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php b/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php
index 432057e42d..82833d7c2a 100644
--- a/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php
+++ b/modules/harvest/tests/src/Kernel/Transform/ResourceImporterTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Drupal\Tests\harvest\Kernel\Transform;
 
 use Drupal\common\Util\DrupalFiles;
@@ -85,6 +87,8 @@ public function testSaveFile() {
       ->setConstructorArgs([
         $this->container->get('file_system'),
         $this->container->get('stream_wrapper_manager'),
+        $this->container->get('http_client_factory'),
+        $this->container->get('messenger'),
       ])
       ->onlyMethods(['retrieveFile'])
       ->getMock();

From 43df67afbc5e7fd89bc273fc5bd91b4cffc06895 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 5 Feb 2025 08:03:49 -0800
Subject: [PATCH 5/7] updates DrupalFiles and test

---
 modules/common/src/Util/DrupalFiles.php       | 51 ++++++++++--
 .../src/Functional/Util/DrupalFilesTest.php   | 78 +++++++++++++++++++
 2 files changed, 121 insertions(+), 8 deletions(-)
 create mode 100644 modules/common/tests/src/Functional/Util/DrupalFilesTest.php

diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index 1ceeafd35f..bced8a4b38 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -9,7 +9,9 @@
 use Drupal\Core\Http\ClientFactory;
 use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
 use GuzzleHttp\Exception\TransferException;
+use Psr\Http\Client\ClientExceptionInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -23,6 +25,8 @@
  */
 class DrupalFiles implements ContainerInjectionInterface {
 
+  use StringTranslationTrait;
+
   /**
    * Drupal file system service.
    *
@@ -132,6 +136,8 @@ public function retrieveFile($url, $destination) {
    *
    * The destination file will never be a managed file.
    *
+   * This is a copy of system_retrieve_file().
+   *
    * @param string $url
    *   The URL of the file to grab.
    * @param string $destination
@@ -150,21 +156,50 @@ public function retrieveFile($url, $destination) {
    * @see \system_retrieve_file()
    * @see https://www.drupal.org/node/3223362
    */
-  protected function systemRetrieveFile($url, $destination = NULL) {
+  protected function systemRetrieveFile($url, $destination = NULL, $managed = FALSE, $replace = FileSystemInterface::EXISTS_RENAME) {
+    $parsed_url = parse_url($url);
+    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
+    $file_system = \Drupal::service('file_system');
+    if (!isset($destination)) {
+      $path = $file_system->basename($parsed_url['path']);
+      $path = \Drupal::config('system.file')->get('default_scheme') . '://' . $path;
+      $path = \Drupal::service('stream_wrapper_manager')->normalizeUri($path);
+    }
+    else {
+      if (is_dir($file_system->realpath($destination))) {
+        // Prevent URIs with triple slashes when glueing parts together.
+        $path = str_replace('///', '//', "$destination/") . \Drupal::service('file_system')->basename($parsed_url['path']);
+      }
+      else {
+        $path = $destination;
+      }
+    }
     try {
-      $data = (string) $this->httpClientFactory
-        ->fromOptions()
+      $data = (string) \Drupal::httpClient()
         ->get($url)
         ->getBody();
-      return $this->filesystem->saveData($data, $destination, FileSystemInterface::EXISTS_REPLACE);
+      if ($managed) {
+        /** @var \Drupal\file\FileRepositoryInterface $file_repository */
+        $file_repository = \Drupal::service('file.repository');
+        $local = $file_repository->writeData($data, $path, $replace);
+      }
+      else {
+        $local = $file_system->saveData($data, $path, $replace);
+      }
     }
-    catch (TransferException $exception) {
-      $this->messenger->addError(t('Failed to fetch file due to error "%error"', ['%error' => $exception->getMessage()]));
+    catch (ClientExceptionInterface $exception) {
+      \Drupal::messenger()->addError(t('Failed to fetch file due to error "%error"', ['%error' => $exception->getMessage()]));
+      return FALSE;
     }
     catch (FileException | InvalidStreamWrapperException $e) {
-      $this->messenger->addError(t('Failed to save file due to error "%error"', ['%error' => $e->getMessage()]));
+      \Drupal::messenger()->addError(t('Failed to save file due to error "%error"', ['%error' => $e->getMessage()]));
+      return FALSE;
+    }
+    if (!$local) {
+      \Drupal::messenger()->addError(t('@remote could not be saved to @path.', ['@remote' => $url, '@path' => $path]));
     }
-    return FALSE;
+
+    return $local;
   }
 
   /**
diff --git a/modules/common/tests/src/Functional/Util/DrupalFilesTest.php b/modules/common/tests/src/Functional/Util/DrupalFilesTest.php
new file mode 100644
index 0000000000..b423361155
--- /dev/null
+++ b/modules/common/tests/src/Functional/Util/DrupalFilesTest.php
@@ -0,0 +1,78 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\common\Functional\Util;
+
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * This is a copy of \Drupal\Tests\system\Functional\System\RetrieveFileTest.
+ *
+ * @covers \Drupal\common\Util\DrupalFiles
+ * @coversDefaultClass \Drupal\common\Util\DrupalFiles
+ *
+ * @group dkan
+ * @group common
+ * @group functional
+ *
+ * @see \Drupal\Tests\system\Functional\System\RetrieveFileTest
+ */
+class DrupalFilesTest extends BrowserTestBase {
+
+  protected static $modules = [
+    'common',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * @covers ::systemRetrieveFile
+   */
+  public function testFileRetrieving(): void {
+    /** @var \Drupal\common\Util\DrupalFiles $drupal_files */
+    $drupal_files = \Drupal::service('dkan.common.drupal_files');
+    $ref_system_retrieve_file = new \ReflectionMethod($drupal_files, 'systemRetrieveFile');
+    $ref_system_retrieve_file->setAccessible(TRUE);
+
+    // Test 404 handling by trying to fetch a randomly named file.
+    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
+    $file_system = \Drupal::service('file_system');
+    $file_system->mkdir($source_dir = 'public://' . $this->randomMachineName());
+    // cSpell:disable-next-line
+    $filename = 'Файл для тестирования ' . $this->randomMachineName();
+    $url = \Drupal::service('file_url_generator')->generateAbsoluteString($source_dir . '/' . $filename);
+    $retrieved_file = $ref_system_retrieve_file->invokeArgs($drupal_files, [$url]);
+    $this->assertFalse($retrieved_file, 'Non-existent file not fetched.');
+
+    // Actually create that file, download it via HTTP and test the returned path.
+    file_put_contents($source_dir . '/' . $filename, 'testing');
+    $retrieved_file = $ref_system_retrieve_file->invokeArgs($drupal_files, [$url]);
+
+    // URLs could not contains characters outside the ASCII set so $filename
+    // has to be encoded.
+    $encoded_filename = rawurlencode($filename);
+
+    $this->assertEquals('public://' . $encoded_filename, $retrieved_file, 'Sane path for downloaded file returned (public:// scheme).');
+    $this->assertFileExists($retrieved_file);
+    $this->assertEquals(7, filesize($retrieved_file), 'File size of downloaded file is correct (public:// scheme).');
+    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
+    $file_system = \Drupal::service('file_system');
+    $file_system->delete($retrieved_file);
+
+    // Test downloading file to a different location.
+    $file_system->mkdir($target_dir = 'temporary://' . $this->randomMachineName());
+    $retrieved_file = $ref_system_retrieve_file->invokeArgs($drupal_files, [$url, $target_dir]);
+    $this->assertEquals("{$target_dir}/{$encoded_filename}", $retrieved_file, 'Sane path for downloaded file returned (temporary:// scheme).');
+    $this->assertFileExists($retrieved_file);
+    $this->assertEquals(7, filesize($retrieved_file), 'File size of downloaded file is correct (temporary:// scheme).');
+    $file_system->delete($retrieved_file);
+
+    $file_system->deleteRecursive($source_dir);
+    $file_system->deleteRecursive($target_dir);
+  }
+
+}

From 3427dff9e762d75c79664029aa75c9642bbf628e Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 5 Feb 2025 08:25:41 -0800
Subject: [PATCH 6/7] services

---
 modules/common/src/Util/DrupalFiles.php | 48 ++++++++++++++++---------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index bced8a4b38..535cc247c9 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -10,7 +10,6 @@
 use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\StreamWrapper\StreamWrapperManager;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
-use GuzzleHttp\Exception\TransferException;
 use Psr\Http\Client\ClientExceptionInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
@@ -94,6 +93,8 @@ public function getFilesystem(): FileSystemInterface {
 
   /**
    * Getter.
+   *
+   * @deprecated
    */
   public function getStreamWrapperManager(): StreamWrapperManager {
     return $this->streamWrapperManager;
@@ -134,10 +135,6 @@ public function retrieveFile($url, $destination) {
   /**
    * Attempts to get a file using Guzzle HTTP client and to store it locally.
    *
-   * The destination file will never be a managed file.
-   *
-   * This is a copy of system_retrieve_file().
-   *
    * @param string $url
    *   The URL of the file to grab.
    * @param string $destination
@@ -147,10 +144,22 @@ public function retrieveFile($url, $destination) {
    *   be used instead.
    *   If this value is omitted, the site's default files scheme will be used,
    *   usually "public://".
+   * @param bool $managed
+   *   If this is set to TRUE, the file API hooks will be invoked and the file is
+   *   registered in the database.
+   * @param int $replace
+   *   Replace behavior when the destination file already exists:
+   *   - FileSystemInterface::EXISTS_REPLACE: Replace the existing file.
+   *   - FileSystemInterface::EXISTS_RENAME: Append _{incrementing number} until
+   *   the filename is unique.
+   *   - FileSystemInterface::EXISTS_ERROR: Do nothing and return FALSE.
    *
    * @return mixed
    *   One of these possibilities:
-   *   - If it succeeds the location where the file was saved.
+   *   - If it succeeds and $managed is FALSE, the location where the file was
+   *   saved.
+   *   - If it succeeds and $managed is TRUE, a \Drupal\file\FileInterface
+   *   object which describes the file.
    *   - If it fails, FALSE.
    *
    * @see \system_retrieve_file()
@@ -158,15 +167,13 @@ public function retrieveFile($url, $destination) {
    */
   protected function systemRetrieveFile($url, $destination = NULL, $managed = FALSE, $replace = FileSystemInterface::EXISTS_RENAME) {
     $parsed_url = parse_url($url);
-    /** @var \Drupal\Core\File\FileSystemInterface $file_system */
-    $file_system = \Drupal::service('file_system');
     if (!isset($destination)) {
-      $path = $file_system->basename($parsed_url['path']);
+      $path = $this->filesystem->basename($parsed_url['path']);
       $path = \Drupal::config('system.file')->get('default_scheme') . '://' . $path;
-      $path = \Drupal::service('stream_wrapper_manager')->normalizeUri($path);
+      $path = $this->streamWrapperManager->normalizeUri($path);
     }
     else {
-      if (is_dir($file_system->realpath($destination))) {
+      if (is_dir($this->filesystem->realpath($destination))) {
         // Prevent URIs with triple slashes when glueing parts together.
         $path = str_replace('///', '//', "$destination/") . \Drupal::service('file_system')->basename($parsed_url['path']);
       }
@@ -175,7 +182,7 @@ protected function systemRetrieveFile($url, $destination = NULL, $managed = FALS
       }
     }
     try {
-      $data = (string) \Drupal::httpClient()
+      $data = (string) $this->httpClientFactory->fromOptions()
         ->get($url)
         ->getBody();
       if ($managed) {
@@ -184,19 +191,26 @@ protected function systemRetrieveFile($url, $destination = NULL, $managed = FALS
         $local = $file_repository->writeData($data, $path, $replace);
       }
       else {
-        $local = $file_system->saveData($data, $path, $replace);
+        $local = $this->filesystem->saveData($data, $path, $replace);
       }
     }
     catch (ClientExceptionInterface $exception) {
-      \Drupal::messenger()->addError(t('Failed to fetch file due to error "%error"', ['%error' => $exception->getMessage()]));
+      \Drupal::messenger()->addError($this->t('Failed to fetch file due to error "%error"', [
+        '%error' => $exception->getMessage(),
+      ]));
       return FALSE;
     }
     catch (FileException | InvalidStreamWrapperException $e) {
-      \Drupal::messenger()->addError(t('Failed to save file due to error "%error"', ['%error' => $e->getMessage()]));
+      \Drupal::messenger()->addError($this->t('Failed to save file due to error "%error"', [
+        '%error' => $e->getMessage(),
+      ]));
       return FALSE;
     }
     if (!$local) {
-      \Drupal::messenger()->addError(t('@remote could not be saved to @path.', ['@remote' => $url, '@path' => $path]));
+      \Drupal::messenger()->addError($this->t('@remote could not be saved to @path.', [
+        '@remote' => $url,
+        '@path' => $path,
+      ]));
     }
 
     return $local;
@@ -212,7 +226,7 @@ public function fileCreateUrl($uri) : string {
     if (substr_count($uri, 'http') > 0) {
       return $uri;
     }
-    elseif ($wrapper = $this->getStreamWrapperManager()->getViaUri($uri)) {
+    elseif ($wrapper = $this->streamWrapperManager->getViaUri($uri)) {
       return $wrapper->getExternalUrl();
     }
     throw new \Exception("No stream wrapper available for {$uri}");

From 0121923628e5a49d67785c3b305b434bcf5791f6 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Wed, 5 Feb 2025 08:35:50 -0800
Subject: [PATCH 7/7] even more services

---
 modules/common/src/Util/DrupalFiles.php | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/modules/common/src/Util/DrupalFiles.php b/modules/common/src/Util/DrupalFiles.php
index 535cc247c9..7b8f0c23b1 100644
--- a/modules/common/src/Util/DrupalFiles.php
+++ b/modules/common/src/Util/DrupalFiles.php
@@ -145,8 +145,8 @@ public function retrieveFile($url, $destination) {
    *   If this value is omitted, the site's default files scheme will be used,
    *   usually "public://".
    * @param bool $managed
-   *   If this is set to TRUE, the file API hooks will be invoked and the file is
-   *   registered in the database.
+   *   If this is set to TRUE, the file API hooks will be invoked and the file
+   *   is registered in the database.
    * @param int $replace
    *   Replace behavior when the destination file already exists:
    *   - FileSystemInterface::EXISTS_REPLACE: Replace the existing file.
@@ -195,19 +195,19 @@ protected function systemRetrieveFile($url, $destination = NULL, $managed = FALS
       }
     }
     catch (ClientExceptionInterface $exception) {
-      \Drupal::messenger()->addError($this->t('Failed to fetch file due to error "%error"', [
+      $this->messenger->addError($this->t('Failed to fetch file due to error "%error"', [
         '%error' => $exception->getMessage(),
       ]));
       return FALSE;
     }
     catch (FileException | InvalidStreamWrapperException $e) {
-      \Drupal::messenger()->addError($this->t('Failed to save file due to error "%error"', [
+      $this->messenger->addError($this->t('Failed to save file due to error "%error"', [
         '%error' => $e->getMessage(),
       ]));
       return FALSE;
     }
     if (!$local) {
-      \Drupal::messenger()->addError($this->t('@remote could not be saved to @path.', [
+      $this->messenger->addError($this->t('@remote could not be saved to @path.', [
         '@remote' => $url,
         '@path' => $path,
       ]));