Issue #3092777 by heddn: Fail in place update if modified files hash is not available

config/optional/views.view.automatic_updates_log.yml

@@ -239,21 +239,21 @@ display:
           value:
             6: '6'
           group: 1
-          exposed: false
+          exposed: true
           expose:
-            operator_id: ''
-            label: ''
+            operator_id: severity_op
+            label: 'Severity level'
             description: ''
             use_operator: false
-            operator: ''
-            operator_limit_selection: false
-            operator_list: {  }
-            identifier: ''
+            operator: severity_op
+            identifier: severity
             required: false
             remember: false
-            multiple: false
+            multiple: true
             remember_roles:
               authenticated: authenticated
+              anonymous: '0'
+              administrator: '0'
             reduce: false
           is_grouped: false
           group_info:
@@ -305,6 +305,7 @@ display:
       max-age: -1
       contexts:
         - 'languages:language_interface'
+        - url
         - url.query_args
         - user.permissions
       tags: {  }
@@ -329,6 +330,7 @@ display:
       max-age: -1
       contexts:
         - 'languages:language_interface'
+        - url
         - url.query_args
         - user.permissions
       tags: {  }

src/Controller/InPlaceUpdateController.php

@@ -8,7 +8,7 @@ use Drupal\Core\Messenger\MessengerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
- * Returns responses for Test Automatic Updates routes.
+ * Returns responses for Automatic Updates routes.
  */
 class InPlaceUpdateController extends ControllerBase {
 
@@ -47,7 +47,7 @@ class InPlaceUpdateController extends ControllerBase {
     $message = $this->t('Update successful');
     if (!$updated) {
       $message_type = MessengerInterface::TYPE_ERROR;
-      $message = $this->t('Update Failed');
+      $message = $this->t('Update failed. Please review logs to determine the cause.');
     }
     $this->messenger()->addMessage($message, $message_type);
     return $this->redirect('automatic_updates.settings');

src/ProjectInfoTrait.php

@@ -46,7 +46,7 @@ trait ProjectInfoTrait {
       $info['version'] = $this->getExtensionVersion($info);
     });
     $system = isset($infos['system']) ? $infos['system'] : NULL;
-    $infos = array_filter($infos, function (array $info, $project_name) {
+    $infos = array_filter($infos, static function (array $info, $project_name) {
       return $info && $info['project'] === $project_name;
     }, ARRAY_FILTER_USE_BOTH);
     if ($system) {
@@ -70,7 +70,7 @@ trait ProjectInfoTrait {
       return $info['version'];
     }
     // Handle experimental modules from core.
-    if (substr($info['install path'], 0, 4) === "core") {
+    if (strpos($info['install path'], "core") === 0) {
       return $this->getExtensionList('module')->get('system')->info['version'];
     }
     \Drupal::logger('automatic_updates')->error('Version cannot be located for @extension', ['@extension' => $extension_name]);
@@ -98,7 +98,7 @@ trait ProjectInfoTrait {
         $project_name = $this->getSuffix($composer_json['name'], '/', $extension_name);
       }
     }
-    if (substr($info['install path'], 0, 4) === "core") {
+    if (strpos($info['install path'], 'core') === 0) {
       $project_name = 'drupal';
     }
     return $project_name;

src/Services/InPlaceUpdate.php

@@ -196,15 +196,20 @@ class InPlaceUpdate implements UpdateInterface {
    *   Return TRUE if modified files exist, FALSE otherwise.
    */
   protected function checkModifiedFiles($project_name, $project_type, ArchiverInterface $archive) {
-    $extensions = [];
-    foreach (['module', 'profile', 'theme'] as $extension_type) {
-      $extensions[] = $this->getInfos($extension_type);
+    if ($project_type === 'core') {
+      $project_type = 'module';
     }
-    $extensions = array_merge(...$extensions);
-
+    $extensions = $this->getInfos($project_type);
     /** @var \Drupal\automatic_updates\Services\ModifiedFilesInterface $modified_files */
     $modified_files = \Drupal::service('automatic_updates.modified_files');
-    $files = iterator_to_array($modified_files->getModifiedFiles([$extensions[$project_name]]));
+    try {
+      $files = iterator_to_array($modified_files->getModifiedFiles([$extensions[$project_name]], TRUE));
+    }
+    catch (RequestException $exception) {
+      // While not strictly true that there are modified files, we can't be sure
+      // there aren't any. So assume the worst.
+      return TRUE;
+    }
     $files = array_unique($files);
     $archive_files = $archive->listContents();
     foreach ($archive_files as $index => &$archive_file) {
@@ -247,8 +252,9 @@ class InPlaceUpdate implements UpdateInterface {
       ]);
     }
     catch (RequestException $exception) {
-      if ($exception->getResponse()->getStatusCode() === 429 && ($retry = $exception->getResponse()->getHeader('Retry-After'))) {
-        $this->doGetArchive($url, $destination, $retry[0] * 1000);
+      $response = $exception->getResponse();
+      if (!$response || ($response->getStatusCode() === 429 && ($retry = $response->getHeader('Retry-After')))) {
+        $this->doGetArchive($url, $destination, $retry[0] ?? 10 * 1000);
       }
       else {
         $this->logger->error('Retrieval of "@url" failed with: @message', [
@@ -371,7 +377,7 @@ class InPlaceUpdate implements UpdateInterface {
    *   TRUE if path was removed, else FALSE.
    */
   protected function stripFileDirectoryPath(&$file) {
-    if (substr($file, 0, 6) === self::ARCHIVE_DIRECTORY) {
+    if (strpos($file, self::ARCHIVE_DIRECTORY) === 0) {
       $file = substr($file, 6);
       return TRUE;
     }
@@ -440,11 +446,11 @@ class InPlaceUpdate implements UpdateInterface {
    *   The iterator of SplFileInfos.
    */
   protected function getFilesList($directory) {
-    $filter = function ($file, $file_name, $iterator) {
+    $filter = static function ($file, $file_name, $iterator) {
       /** @var \SplFileInfo $file */
       /** @var string $file_name */
       /** @var \RecursiveDirectoryIterator $iterator */
-      if ($iterator->hasChildren() && !in_array($file->getFilename(), ['.git'], TRUE)) {
+      if ($iterator->hasChildren() && $file->getFilename() !== '.git') {
         return TRUE;
       }
       $skipped_files = [
@@ -471,7 +477,7 @@ class InPlaceUpdate implements UpdateInterface {
    */
   protected function buildUrl($project_name, $file_name) {
     $uri = $this->configFactory->get('automatic_updates.settings')->get('download_uri');
-    return Url::fromUri($uri . "/$project_name/" . $file_name)->toString();
+    return Url::fromUri("$uri/$project_name/$file_name")->toString();
   }
 
   /**

src/Services/ModifiedFiles.php

@@ -10,6 +10,7 @@ use Drupal\Signify\ChecksumList;
 use Drupal\Signify\FailedCheckumFilter;
 use Drupal\Signify\Verifier;
 use GuzzleHttp\ClientInterface;
+use GuzzleHttp\Exception\RequestException;
 use GuzzleHttp\Promise\EachPromise;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Log\LoggerInterface;
@@ -43,7 +44,7 @@ class ModifiedFiles implements ModifiedFilesInterface {
   protected $configFactory;
 
   /**
-   * ModifiedCode constructor.
+   * ModifiedFiles constructor.
    *
    * @param \Psr\Log\LoggerInterface $logger
    *   The logger.
@@ -63,16 +64,19 @@ class ModifiedFiles implements ModifiedFilesInterface {
   /**
    * {@inheritdoc}
    */
-  public function getModifiedFiles(array $extensions = []) {
+  public function getModifiedFiles(array $extensions = [], $exception_on_failure = FALSE) {
     $modified_files = new \ArrayIterator();
     /** @var \GuzzleHttp\Promise\PromiseInterface[] $promises */
     $promises = $this->getHashRequests($extensions);
     // Wait until all the requests are finished.
     (new EachPromise($promises, [
       'concurrency' => 4,
-      'fulfilled' => function ($resource) use ($modified_files) {
+      'fulfilled' => function (array $resource) use ($modified_files) {
         $this->processHashes($resource, $modified_files);
       },
+      'rejected' => function (RequestException $exception) use ($exception_on_failure) {
+        $this->processFailures($exception, $exception_on_failure);
+      },
     ]))->promise()->wait();
     return $modified_files;
   }
@@ -80,14 +84,16 @@ class ModifiedFiles implements ModifiedFilesInterface {
   /**
    * Process checking hashes of files from external URL.
    *
-   * @param array $resource
+   * @param array $hash
    *   An array of http response and project info.
    * @param \ArrayIterator $modified_files
    *   The list of modified files.
+   *
+   * @throws \SodiumException
    */
-  protected function processHashes(array $resource, \ArrayIterator $modified_files) {
-    $contents = $resource['contents'];
-    $info = $resource['info'];
+  protected function processHashes(array $hash, \ArrayIterator $modified_files) {
+    $contents = $hash['contents'];
+    $info = $hash['info'];
     $directory_root = $info['install path'];
     if ($info['project'] === 'drupal') {
       $directory_root = '';
@@ -113,6 +119,21 @@ class ModifiedFiles implements ModifiedFilesInterface {
     }
   }
 
+  /**
+   * Handle HTTP failures.
+   *
+   * @param \GuzzleHttp\Exception\RequestException $exception
+   *   The request exception.
+   * @param bool $exception_on_failure
+   *   Throw exception on HTTP failures, defaults to FALSE.
+   */
+  protected function processFailures(RequestException $exception, $exception_on_failure) {
+    if ($exception_on_failure) {
+      watchdog_exception('automatic_updates', $exception);
+      throw $exception;
+    }
+  }
+
   /**
    * Get an iterator of promises that return a resource stream.
    *
@@ -151,13 +172,14 @@ class ModifiedFiles implements ModifiedFilesInterface {
     return $this->httpClient->requestAsync('GET', $url, [
       'stream' => TRUE,
       'read_timeout' => 30,
-    ])
-      ->then(function (ResponseInterface $response) use ($info) {
+    ])->then(
+      static function (ResponseInterface $response) use ($info) {
         return [
           'contents' => $response->getBody()->getContents(),
           'info' => $info,
         ];
-      });
+      }
+    );
   }
 
   /**
@@ -174,7 +196,7 @@ class ModifiedFiles implements ModifiedFilesInterface {
     $project_name = $info['project'];
     $hash_name = $this->getHashName($info);
     $uri = ltrim($this->configFactory->get('automatic_updates.settings')->get('hashes_uri'), '/');
-    return Url::fromUri($uri . "/$project_name/$version/$hash_name")->toString();
+    return Url::fromUri("$uri/$project_name/$version/$hash_name")->toString();
   }
 
   /**

src/Services/ModifiedFilesInterface.php

@@ -13,10 +13,12 @@ interface ModifiedFilesInterface {
    * @param array $extensions
    *   The list of extensions, keyed by extension name with values an info
    *   array.
+   * @param bool $exception_on_failure
+   *   (optional) Throw exception on HTTP failures, defaults to FALSE.
    *
    * @return \Iterator
    *   The modified files.
    */
-  public function getModifiedFiles(array $extensions = []);
+  public function getModifiedFiles(array $extensions = [], $exception_on_failure = FALSE);
 
 }