Issue #3093700 by heddn, ressa, rfay, rkoller: Improve requirements failures for composer workflow

automatic_updates.install

@@ -6,6 +6,7 @@
  */
 
 use Drupal\automatic_updates\ReadinessChecker\ReadinessCheckerManagerInterface;
+use Drupal\automatic_updates\ReadinessChecker\Vendor;
 use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
 use Drupal\Core\Url;
 
@@ -13,6 +14,17 @@ use Drupal\Core\Url;
  * Implements hook_requirements().
  */
 function automatic_updates_requirements($phase) {
+  $vendor_checker = new Vendor(\Drupal::getContainer()->get('app.root'));
+  if (!empty($vendor_checker->run())) {
+    return [
+      'not installable' => [
+        'title' => t('Automatic Updates'),
+        'severity' => REQUIREMENT_ERROR,
+        'value' => '1.x',
+        'description' => t('This module does not currently support relocated vendor folder and composer-based workflows.'),
+      ],
+    ];
+  }
   if ($phase !== 'runtime') {
     return;
   }

automatic_updates.services.yml

@@ -64,7 +64,6 @@ services:
   automatic_updates.disk_space_checker:
     class: Drupal\automatic_updates\ReadinessChecker\DiskSpace
     arguments:
-      - '@logger.channel.automatic_updates'
       - '@app.root'
     tags:
       - { name: readiness_checker, category: error}
@@ -120,3 +119,9 @@ services:
       - '@module_handler'
     tags:
       - { name: readiness_checker, category: warning}
+  automatic_updates.vendor:
+    class: Drupal\automatic_updates\ReadinessChecker\Vendor
+    arguments:
+      - '@app.root'
+    tags:
+      - { name: readiness_checker, category: error}

src/ReadinessChecker/DiskSpace.php

@@ -2,14 +2,10 @@
 
 namespace Drupal\automatic_updates\ReadinessChecker;
 
-use Drupal\Core\StringTranslation\StringTranslationTrait;
-use Psr\Log\LoggerInterface;
-
 /**
  * Disk space checker.
  */
 class DiskSpace extends Filesystem {
-  use StringTranslationTrait;
 
   /**
    * Minimum disk space (in bytes) is 10mb.
@@ -21,26 +17,6 @@ class DiskSpace extends Filesystem {
    */
   const MEGABYTE_DIVISOR = 1000000;
 
-  /**
-   * The logger.
-   *
-   * @var \Psr\Log\LoggerInterface
-   */
-  protected $logger;
-
-  /**
-   * ReadOnlyFilesystem constructor.
-   *
-   * @param \Psr\Log\LoggerInterface $logger
-   *   The logger.
-   * @param string $app_root
-   *   The app root.
-   */
-  public function __construct(LoggerInterface $logger, $app_root) {
-    $this->logger = $logger;
-    $this->rootPath = (string) $app_root;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -63,7 +39,7 @@ class DiskSpace extends Filesystem {
           '@space' => static::MINIMUM_DISK_SPACE / static::MEGABYTE_DIVISOR,
         ]);
       }
-      if (disk_free_space($this->getVendorPath()) < static::MINIMUM_DISK_SPACE) {
+      if (is_dir($this->getVendorPath()) && disk_free_space($this->getVendorPath()) < static::MINIMUM_DISK_SPACE) {
         $messages[] = $this->t('Vendor filesystem "@vendor" has insufficient space. There must be at least @space megabytes free.', [
           '@vendor' => $this->getVendorPath(),
           '@space' => static::MINIMUM_DISK_SPACE / static::MEGABYTE_DIVISOR,

src/ReadinessChecker/FileOwnership.php

@@ -2,29 +2,16 @@
 
 namespace Drupal\automatic_updates\ReadinessChecker;
 
-use Drupal\Core\StringTranslation\StringTranslationTrait;
-
 /**
  * File ownership checker.
  */
 class FileOwnership extends Filesystem {
-  use StringTranslationTrait;
-
-  /**
-   * FileOwnership constructor.
-   *
-   * @param string $app_root
-   *   The app root.
-   */
-  public function __construct($app_root) {
-    $this->rootPath = (string) $app_root;
-  }
 
   /**
    * {@inheritdoc}
    */
   protected function doCheck() {
-    $file_path = $this->getRootPath() . '/core/core.api.php';
+    $file_path = $this->getRootPath() . DIRECTORY_SEPARATOR . implode(DIRECTORY_SEPARATOR, ['core', 'core.api.php']);
     return $this->ownerIsScriptUser($file_path);
   }
 

src/ReadinessChecker/Filesystem.php

@@ -2,10 +2,13 @@
 
 namespace Drupal\automatic_updates\ReadinessChecker;
 
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+
 /**
  * Base class for filesystem checkers.
  */
 abstract class Filesystem implements ReadinessCheckerInterface {
+  use StringTranslationTrait;
 
   /**
    * The root file path.
@@ -21,11 +24,21 @@ abstract class Filesystem implements ReadinessCheckerInterface {
    */
   protected $vendorPath;
 
+  /**
+   * Filesystem constructor.
+   *
+   * @param string $app_root
+   *   The app root.
+   */
+  public function __construct($app_root) {
+    $this->rootPath = (string) $app_root;
+  }
+
   /**
    * {@inheritdoc}
    */
   public function run() {
-    if (!$this->exists($this->getRootPath() . '/core/core.api.php')) {
+    if (!$this->exists($this->getRootPath() . DIRECTORY_SEPARATOR . implode(DIRECTORY_SEPARATOR, ['core', 'core.api.php']))) {
       return [$this->t('The web root could not be located.')];
     }
 

src/ReadinessChecker/ReadOnlyFilesystem.php

@@ -5,14 +5,12 @@ namespace Drupal\automatic_updates\ReadinessChecker;
 use Drupal\Component\Render\MarkupInterface;
 use Drupal\Core\File\Exception\FileException;
 use Drupal\Core\File\FileSystemInterface;
-use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Psr\Log\LoggerInterface;
 
 /**
  * Read only filesystem checker.
  */
 class ReadOnlyFilesystem extends Filesystem {
-  use StringTranslationTrait;
 
   /**
    * The logger.
@@ -65,7 +63,7 @@ class ReadOnlyFilesystem extends Filesystem {
     }
     else {
       $error = $this->t('Drupal core filesystem at "@path" is read only. Updates to Drupal core cannot be applied against a read only file system.', ['@path' => $this->rootPath . '/core']);
-      $this->doReadOnlyCheck($this->getRootPath(), 'core.api.php', $messages, $error);
+      $this->doReadOnlyCheck($this->getRootPath(), implode(DIRECTORY_SEPARATOR, ['core', 'core.api.php']), $messages, $error);
       $error = $this->t('Vendor filesystem at "@path" is read only. Updates to the site\'s code base cannot be applied against a read only file system.', ['@path' => $this->vendorPath]);
       $this->doReadOnlyCheck($this->getVendorPath(), 'composer/LICENSE', $messages, $error);
     }
@@ -85,12 +83,16 @@ class ReadOnlyFilesystem extends Filesystem {
    *   The error message to add if the file is read only.
    */
   protected function doReadOnlyCheck($file_path, $file, array &$messages, MarkupInterface $message) {
+    // Ignore check if the path doesn't exit.
+    if (!is_file($file_path . DIRECTORY_SEPARATOR . $file)) {
+      return;
+    }
     try {
       // If we can copy and delete a file, then we don't have a read only
       // file system.
-      if ($this->fileSystem->copy("$file_path/$file", "$file_path/$file.automatic_updates", FileSystemInterface::EXISTS_REPLACE)) {
+      if ($this->fileSystem->copy($file_path . DIRECTORY_SEPARATOR . $file, $file_path . DIRECTORY_SEPARATOR . "$file.automatic_updates", FileSystemInterface::EXISTS_REPLACE)) {
         // Delete it after copying.
-        $this->fileSystem->delete("$file_path/$file.automatic_updates");
+        $this->fileSystem->delete($file_path . DIRECTORY_SEPARATOR . "$file.automatic_updates");
       }
       else {
         $this->logger->error($message);

src/ReadinessChecker/Vendor.php

+<?php
+
+namespace Drupal\automatic_updates\ReadinessChecker;
+
+/**
+ * Error if site is managed via composer instead of via tarballs.
+ */
+class Vendor extends Filesystem {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function doCheck() {
+    if (!$this->exists($this->getVendorPath() . DIRECTORY_SEPARATOR . 'autoload.php')) {
+      return [$this->t('The vendor folder could not be located.')];
+    }
+    return [];
+  }
+
+}

tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php

@@ -26,17 +26,17 @@ class DiskSpaceTest extends KernelTestBase {
    */
   public function testDiskSpace() {
     // No disk space issues.
-    $disk_space = new DiskSpace($this->container->get('logger.channel.automatic_updates'), $this->container->get('app.root'));
+    $disk_space = new DiskSpace($this->container->get('app.root'));
     $messages = $disk_space->run();
     $this->assertEmpty($messages);
 
     // Out of space.
-    $disk_space = new TestDiskSpace($this->container->get('logger.channel.automatic_updates'), $this->container->get('app.root'));
+    $disk_space = new TestDiskSpace($this->container->get('app.root'));
     $messages = $disk_space->run();
     $this->assertCount(1, $messages);
 
     // Out of space not the same logical disk.
-    $disk_space = new TestDiskSpaceNonSameDisk($this->container->get('logger.channel.automatic_updates'), $this->container->get('app.root'));
+    $disk_space = new TestDiskSpaceNonSameDisk($this->container->get('app.root'));
     $messages = $disk_space->run();
     $this->assertCount(2, $messages);
   }

tests/src/Kernel/ReadinessChecker/ReadOnlyTest.php

@@ -34,7 +34,7 @@ class ReadOnlyTest extends KernelTestBase {
     $this->assertEmpty($messages);
 
     $filesystem = $this->createMock(FileSystemInterface::class);
-    $filesystem->expects($this->any())
+    $filesystem
       ->method('copy')
       ->withAnyParameters()
       ->will($this->onConsecutiveCalls(
@@ -45,7 +45,7 @@ class ReadOnlyTest extends KernelTestBase {
         'full/file/path'
       )
     );
-    $filesystem->expects($this->any())
+    $filesystem
       ->method('delete')
       ->withAnyParameters()
       ->will($this->onConsecutiveCalls(
@@ -54,10 +54,11 @@ class ReadOnlyTest extends KernelTestBase {
       )
     );
 
+    $app_root = $this->container->get('app.root');
     $readonly = $this->getMockBuilder(ReadOnlyFilesystem::class)
       ->setConstructorArgs([
         $this->createMock(LoggerInterface::class),
-        '/var/www/html',
+        $app_root,
         $filesystem,
       ])
       ->setMethods([
@@ -65,7 +66,7 @@ class ReadOnlyTest extends KernelTestBase {
         'exists',
       ])
       ->getMock();
-    $readonly->expects($this->any())
+    $readonly
       ->method('areSameLogicalDisk')
       ->withAnyParameters()
       ->will($this->onConsecutiveCalls(
@@ -74,7 +75,7 @@ class ReadOnlyTest extends KernelTestBase {
         FALSE
       )
     );
-    $readonly->expects($this->any())
+    $readonly
       ->method('exists')
       ->withAnyParameters()
       ->will($this->onConsecutiveCalls(
@@ -91,21 +92,22 @@ class ReadOnlyTest extends KernelTestBase {
 
     // Test same logical disk.
     $expected_messages = [];
-    $expected_messages[] = $this->t('Logical disk at "/var/www/html" is read only. Updates to Drupal cannot be applied against a read only file system.');
+    $expected_messages[] = $this->t('Logical disk at "@app_root" is read only. Updates to Drupal cannot be applied against a read only file system.', ['@app_root' => $app_root]);
     $messages = $readonly->run();
     $this->assertEquals($expected_messages, $messages);
 
     // Test read-only.
     $expected_messages = [];
-    $expected_messages[] = $this->t('Drupal core filesystem at "/var/www/html/core" is read only. Updates to Drupal core cannot be applied against a read only file system.');
-    $expected_messages[] = $this->t('Vendor filesystem at "/var/www/html/vendor" is read only. Updates to the site\'s code base cannot be applied against a read only file system.');
+    $expected_messages[] = $this->t('Drupal core filesystem at "@core" is read only. Updates to Drupal core cannot be applied against a read only file system.', [
+      '@core' => $app_root . DIRECTORY_SEPARATOR . 'core',
+    ]);
+    $expected_messages[] = $this->t('Vendor filesystem at "@vendor" is read only. Updates to the site\'s code base cannot be applied against a read only file system.', [
+      '@vendor' => $app_root . DIRECTORY_SEPARATOR . 'vendor',
+    ]);
     $messages = $readonly->run();
     $this->assertEquals($expected_messages, $messages);
 
     // Test delete fails.
-    $expected_messages = [];
-    $expected_messages[] = $this->t('Drupal core filesystem at "/var/www/html/core" is read only. Updates to Drupal core cannot be applied against a read only file system.');
-    $expected_messages[] = $this->t('Vendor filesystem at "/var/www/html/vendor" is read only. Updates to the site\'s code base cannot be applied against a read only file system.');
     $messages = $readonly->run();
     $this->assertEquals($expected_messages, $messages);
   }

tests/src/Kernel/ReadinessChecker/VendorTest.php

+<?php
+
+namespace Drupal\Tests\automatic_updates\Kernel\ReadinessChecker;
+
+use Drupal\automatic_updates\ReadinessChecker\Vendor;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests locating vendor folder.
+ *
+ * @group automatic_updates
+ */
+class VendorTest extends KernelTestBase {
+  use StringTranslationTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = [
+    'automatic_updates',
+  ];
+
+  /**
+   * Tests vendor folder existing.
+   */
+  public function testVendor() {
+    $vendor = $this->container->get('automatic_updates.vendor');
+    $this->assertEmpty($vendor->run());
+
+    $missing_vendor = $this->getMockBuilder(Vendor::class)
+      ->setConstructorArgs([
+        $this->container->get('app.root'),
+      ])
+      ->setMethods([
+        'exists',
+      ])
+      ->getMock();
+    $missing_vendor
+      ->method('exists')
+      ->withAnyParameters()
+      ->will($this->onConsecutiveCalls(
+        TRUE,
+        FALSE
+      )
+    );
+    $expected_messages = [];
+    $expected_messages[] = $this->t('The vendor folder could not be located.');
+    $this->assertEquals($expected_messages, $missing_vendor->run());
+  }
+
+}