diff -u b/core/modules/auto_updates/auto_updates.info.yml b/core/modules/auto_updates/auto_updates.info.yml --- b/core/modules/auto_updates/auto_updates.info.yml +++ b/core/modules/auto_updates/auto_updates.info.yml @@ -1,6 +1,7 @@ name: 'Automatic Updates' type: module description: 'Experimental module to develop automatic updates. Currently the module does not provide update functionality.' +configure: auto_updates.settings package: Core (Experimental) version: VERSION hidden: true diff -u b/core/modules/auto_updates/auto_updates.install b/core/modules/auto_updates/auto_updates.install --- b/core/modules/auto_updates/auto_updates.install +++ b/core/modules/auto_updates/auto_updates.install @@ -5,9 +5,9 @@ * Contains install and update functions for Automatic Updates. */ -use Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface; use Drupal\Core\StringTranslation\PluralTranslatableMarkup; use Drupal\Core\Url; +use Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface; /** * Implements hook_requirements(). @@ -24,7 +24,7 @@ } $requirements['auto_updates_readiness']['title'] = t('Update readiness checks'); $readiness_check = Url::fromRoute('auto_updates.update_readiness'); - $last_check_timestamp = $checker_manager->timestamp(); + $last_check_timestamp = $checker_manager->getTimestamp(); if ($last_check_timestamp === NULL) { $requirements['auto_updates_readiness']['severity'] = REQUIREMENT_ERROR; $requirements['auto_updates_readiness']['value'] = t('Your site has never checked if it is ready to apply automatic updates.', ['@readiness_checks' => 'https://www.drupal.org/docs/8/update/automatic-updates#readiness-checks']); @@ -76,5 +76,3 @@ function auto_updates_install($is_syncing) { - /** @var \Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface $checker_manager */ - $checker_manager = \Drupal::service('auto_updates.readiness_checker_manager'); - $checker_manager->run(); + \Drupal::service('auto_updates.readiness_checker_manager')->run(); } diff -u b/core/modules/auto_updates/auto_updates.module b/core/modules/auto_updates/auto_updates.module --- b/core/modules/auto_updates/auto_updates.module +++ b/core/modules/auto_updates/auto_updates.module @@ -5,8 +5,8 @@ * Provides hook implementations for Automatic Updates. */ -use Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface; use Drupal\Core\Url; +use Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface; /** * Implements hook_page_top(). @@ -42,7 +42,7 @@ } $results = $checker_manager->getResults(ReadinessCheckerManagerInterface::ERROR); if ($results) { - \Drupal::messenger()->addError(t('Your site is currently failing readiness checks for automatic updates. It cannot be automatically updated until further action is performed:', [':readiness_checks' => 'https://www.drupal.org/docs/8/update/auto-updates#readiness-checks'])); + \Drupal::messenger()->addError(t('Your site is currently failing readiness checks for automatic updates. It cannot be automatically updated until further action is performed.', [':readiness_checks' => 'https://www.drupal.org/docs/8/update/auto-updates#readiness-checks'])); foreach ($results as $message) { \Drupal::messenger()->addError($message); } @@ -64,14 +64,11 @@ $request_time = \Drupal::time()->getRequestTime(); /** @var \Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface $checker_manager */ $checker_manager = \Drupal::service('auto_updates.readiness_checker_manager'); - $last_check = $checker_manager->timestamp(); + $last_check = $checker_manager->getTimestamp(); // Only allow cron to run once every hour. if ($last_check && ($request_time - $last_check) < 3600) { return; } - - /** @var \Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface $checker_manager */ - $checker_manager = \Drupal::service('auto_updates.readiness_checker_manager'); $checker_manager->run(); } diff -u b/core/modules/auto_updates/auto_updates.routing.yml b/core/modules/auto_updates/auto_updates.routing.yml --- b/core/modules/auto_updates/auto_updates.routing.yml +++ b/core/modules/auto_updates/auto_updates.routing.yml @@ -11,7 +11,7 @@ path: '/admin/config/auto_updates/readiness' defaults: _controller: '\Drupal\auto_updates\Controller\ReadinessCheckerController::run' - _title: 'Update readiness checking...' + _title: 'Update readiness checking' requirements: _permission: 'administer software updates' _custom_access: '\Drupal\auto_updates\Controller\ReadinessCheckerController::access' diff -u b/core/modules/auto_updates/src/Controller/ReadinessCheckerController.php b/core/modules/auto_updates/src/Controller/ReadinessCheckerController.php --- b/core/modules/auto_updates/src/Controller/ReadinessCheckerController.php +++ b/core/modules/auto_updates/src/Controller/ReadinessCheckerController.php @@ -17,7 +17,7 @@ class ReadinessCheckerController extends ControllerBase { /** - * The readiness checker. + * The readiness checker manager. * * @var \Drupal\auto_updates\ReadinessChecker\ReadinessCheckerManagerInterface */ diff -u b/core/modules/auto_updates/src/Form/SettingsForm.php b/core/modules/auto_updates/src/Form/SettingsForm.php --- b/core/modules/auto_updates/src/Form/SettingsForm.php +++ b/core/modules/auto_updates/src/Form/SettingsForm.php @@ -64,7 +64,7 @@ '#open' => TRUE, ]; - $last_check_timestamp = $this->checkerManager->timestamp(); + $last_check_timestamp = $this->checkerManager->getTimestamp(); $form['readiness']['enable_readiness_checks'] = [ '#type' => 'checkbox', '#title' => $this->t('Check the readiness of automatically updating the site.'), diff -u b/core/modules/auto_updates/src/ReadinessChecker/DiskSpace.php b/core/modules/auto_updates/src/ReadinessChecker/DiskSpace.php --- b/core/modules/auto_updates/src/ReadinessChecker/DiskSpace.php +++ b/core/modules/auto_updates/src/ReadinessChecker/DiskSpace.php @@ -7,7 +7,7 @@ /** * The disk space readiness checker. */ -class DiskSpace extends FilesystemBase { +class DiskSpace extends FileSystemBase { /** * Minimum disk space (in bytes) is 100mb. @@ -25,7 +25,7 @@ /** * {@inheritdoc} */ - protected function doCheck() { + protected function doCheck(): array { return $this->diskSpaceCheck(); } @@ -35,7 +35,7 @@ * @return \Drupal\Core\StringTranslation\TranslatableMarkup[] * An array of translatable strings if there is not sufficient space. */ - protected function diskSpaceCheck() { + protected function diskSpaceCheck(): array { $messages = []; $minimum_megabytes = static::MINIMUM_DISK_SPACE / static::MEGABYTE_DIVISOR; if (!$this->areSameLogicalDisk($this->getRootPath(), $this->getVendorPath())) { reverted: --- b/core/modules/auto_updates/src/ReadinessChecker/FilesystemBase.php +++ /dev/null @@ -1,96 +0,0 @@ -rootPath = $app_root; - } - - /** - * {@inheritdoc} - */ - public function run() { - $messages = []; - if (!file_exists($this->getRootPath() . DIRECTORY_SEPARATOR . implode(DIRECTORY_SEPARATOR, ['core', 'core.api.php']))) { - $messages[] = $this->t('The web root could not be located.'); - } - if (!is_dir($this->getVendorPath())) { - $messages[] = $this->t('Vendor folder "@vendor" is not a valid directory. Alternate vendor folder locations are not currently supported.', [ - '@vendor' => $this->getVendorPath(), - ]); - } - if ($messages) { - return $messages; - } - return $this->doCheck(); - } - - /** - * Perform checks. - * - * @return array - * An array of translatable strings if any checks fail. - */ - abstract protected function doCheck(); - - /** - * Get the root file path. - * - * @return string - * The root file path. - */ - protected function getRootPath() { - return $this->rootPath; - } - - /** - * Get the vendor file path. - * - * @return string - * The vendor file path. - */ - protected function getVendorPath() { - // @todo Support finding the 'vendor' directory dynamically in - // https://www.drupal.org/node/3166435. - return $this->getRootPath() . DIRECTORY_SEPARATOR . 'vendor'; - } - - /** - * Determine if the root and vendor file system are the same logical disk. - * - * @param string $root - * Root file path. - * @param string $vendor - * Vendor file path. - * - * @return bool - * TRUE if same file system, FALSE otherwise. - */ - protected function areSameLogicalDisk(string $root, string $vendor) { - $root_statistics = stat($root); - $vendor_statistics = stat($vendor); - return $root_statistics && $vendor_statistics && $root_statistics['dev'] === $vendor_statistics['dev']; - } - -} diff -u b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerInterface.php b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerInterface.php --- b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerInterface.php +++ b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerInterface.php @@ -9,10 +9,11 @@ /** - * Run check. + * Runs a check. * * @return array - * An array of translatable strings or an empty array. + * An array of translatable strings if any checks fail, otherwise an empty + * array. */ - public function run(); + public function run(): array; } diff -u b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManager.php b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManager.php --- b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManager.php +++ b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManager.php @@ -12,9 +12,11 @@ class ReadinessCheckerManager implements ReadinessCheckerManagerInterface { /** - * Last checked ago warning (in seconds). + * Time (in seconds) since the last check after which we generate a warning. + * + * Defaults to 1 day. */ - private const LAST_CHECKED_WARNING = 3600 * 24; + private const LAST_CHECKED_WARNING = 60 * 60 * 24; /** * The key/value storage. @@ -66,7 +68,7 @@ /** * {@inheritdoc} */ - public function addChecker(ReadinessCheckerInterface $checker, string $category = 'warning', $priority = 0) { + public function addChecker(ReadinessCheckerInterface $checker, string $category = 'warning', $priority = 0): ReadinessCheckerManagerInterface { if (!in_array($category, $this->getCategories(), TRUE)) { throw new \InvalidArgumentException(sprintf('Readiness checker category "%s" is invalid. Use "%s" instead.', $category, implode('" or "', $this->getCategories()))); } @@ -77,7 +79,7 @@ /** * {@inheritdoc} */ - public function run() { + public function run(): array { if (!$this->isEnabled()) { return []; } @@ -91,7 +93,7 @@ } } - $this->keyValue->set("readiness_check_results", + $this->keyValue->set('readiness_check_results', [ 'messages' => $messages_by_category, 'checkers' => $this->getCurrentCheckerIds(), @@ -104,9 +106,9 @@ /** * {@inheritdoc} */ - public function getResults($category) { + public function getResults($category): array { if ($this->isEnabled()) { - $results = $this->keyValue->get("readiness_check_results", []); + $results = $this->keyValue->get('readiness_check_results', []); $all_messages = $results['messages'] ?? []; return $all_messages[$category] ?? []; } @@ -116,7 +118,7 @@ /** * {@inheritdoc} */ - public function clearStaleResults() { + public function clearStaleResults(): bool { $results = $this->keyValue->get('readiness_check_results'); if (isset($results['checkers']) && $this->getCurrentCheckerIds() !== $results['checkers']) { $this->keyValue->delete('readiness_check_results'); @@ -128,21 +130,21 @@ /** * {@inheritdoc} */ - public function timestamp() { + public function getTimestamp(): int { return $this->keyValue->get('readiness_check_timestamp'); } /** * {@inheritdoc} */ - public function isEnabled() { + public function isEnabled(): bool { return $this->configFactory->get('auto_updates.settings')->get('enable_readiness_checks'); } /** * {@inheritdoc} */ - public function getCategories() { + public function getCategories(): array { return [self::ERROR, self::WARNING]; } @@ -152,9 +154,9 @@ * @return \Drupal\auto_updates\ReadinessChecker\ReadinessCheckerInterface[][] * A nested and sorted array of checker objects. The first level of the * array is keyed by checker categories. The second level array is checker - * objects in the category order by priority. + * objects in the category ordered by priority. */ - protected function getSortedCheckers() { + protected function getSortedCheckers(): array { $sorted = []; foreach ($this->checkers as $category => $priorities) { foreach ($priorities as $checkers) { @@ -170,7 +172,7 @@ * Gets the current checker service Ids. * * @return string - * A concatenated list of checker service Ids. + * A concatenated list of checker service Ids delimited by '::'. */ protected function getCurrentCheckerIds(): string { $service_ids = []; @@ -185,8 +187,8 @@ /** * {@inheritdoc} */ - public function hasRunRecently() { - return !($this->time->getRequestTime() > $this->timestamp() + self::LAST_CHECKED_WARNING); + public function hasRunRecently(): bool { + return $this->time->getRequestTime() <= $this->getTimestamp() + self::LAST_CHECKED_WARNING; } } diff -u b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManagerInterface.php b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManagerInterface.php --- b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManagerInterface.php +++ b/core/modules/auto_updates/src/ReadinessChecker/ReadinessCheckerManagerInterface.php @@ -24,75 +24,76 @@ * The checker interface to be appended to the checker chain. * @param string $category - * (optional) The category of check. + * (optional) The category of check. Defaults to 'warning' * @param int $priority - * (optional) The priority of the checker being added. If not provided the - * default priority is 0. Readiness checkers with larger priorities will run - * first within a category. + * (optional) The priority of the checker being added. Defaults to 0. + * Readiness checkers with larger priorities will run first within a + * category. * * @return $this */ - public function addChecker(ReadinessCheckerInterface $checker, string $category = 'warning', $priority = 0); + public function addChecker(ReadinessCheckerInterface $checker, string $category = 'warning', $priority = 0): ReadinessCheckerManagerInterface; /** - * Run readiness checks. + * Runs readiness checks. * * @return string[][] - * A nested array readiness check messages. The top level array is keyed by - * category and the next level array is is an array of translatable strings + * A nested array of readiness check messages. The top level array is keyed + * by category and the next level array is an array of translatable strings * for the category. */ - public function run(); + public function run(): array; /** - * Get results of most recent run. + * Gets the results of the most recent run. * * @param string $category * The category of check. * * @return array - * An array of translatable strings. + * An array of translatable messages if any checks fail, otherwise an empty + * array. */ - public function getResults($category); + public function getResults($category): array; /** - * Get timestamp of most recent run. + * Gets the timestamp of the most recent run. * * @return int|null - * A unix timestamp of most recent completed run if one exists or NULL if no - * run has been completed. + * The timestamp of the most recently completed run, or NULL if no run has + * been completed. */ - public function timestamp(); + public function getTimestamp(); /** - * Determine if readiness checks is enabled. + * Determines if readiness checks are enabled. * * @return bool * TRUE if enabled, otherwise FALSE. */ - public function isEnabled(); + public function isEnabled(): bool; /** - * Get the checker categories. + * Gets the checker categories. * * @return string[] * The checkers categories. */ - public function getCategories(); + public function getCategories(): array; /** * Clears readiness checker results if the available checkers have changed. * * @return bool - * Return TRUE if the results were cleared, otherwise returns FALSE. + * TRUE if the results were cleared, otherwise FALSE. */ - public function clearStaleResults(); + public function clearStaleResults(): bool; /** * Determines whether the readiness checkers have been run recently. * * @return bool - * True if the checkers have been run recently, otherwise false. + * TRUE if the checkers have been run recently, otherwise FALSE. */ - public function hasRunRecently(); + public function hasRunRecently(): bool; } reverted: --- b/core/modules/auto_updates/tests/modules/auto_updates_test/auto_updates_test.info.yml +++ a/core/modules/big_pipe/tests/modules/big_pipe_test/big_pipe_test.info.yml @@ -1,5 +1,5 @@ +name: 'BigPipe test' -name: 'Automatic Updates Test' type: module +description: 'Support module for BigPipe testing.' -description: 'Module for testing Automatic Updates.' package: Testing version: VERSION reverted: --- b/core/modules/auto_updates/tests/modules/auto_updates_test/src/Datetime/TestTime.php +++ a/core/modules/update/tests/modules/update_test/src/Datetime/TestTime.php @@ -1,6 +1,6 @@ get('update_test.mock_date', NULL)) { + return \DateTime::createFromFormat('Y-m-d', $mock_date)->getTimestamp(); - if ($mock_date = \Drupal::state()->get(TestTime::STATE_KEY, NULL)) { - return \DateTime::createFromFormat(self::TIME_FORMAT, $mock_date)->getTimestamp(); } return parent::getRequestTime(); } diff -u b/core/modules/auto_updates/tests/modules/auto_updates_test/src/ReadinessChecker/TestChecker.php b/core/modules/auto_updates/tests/modules/auto_updates_test/src/ReadinessChecker/TestChecker.php --- b/core/modules/auto_updates/tests/modules/auto_updates_test/src/ReadinessChecker/TestChecker.php +++ b/core/modules/auto_updates/tests/modules/auto_updates_test/src/ReadinessChecker/TestChecker.php @@ -17,7 +17,7 @@ /** * {@inheritDoc} */ - public function run() { + public function run(): array { if ($message = \Drupal::state()->get(static::STATE_KEY, NULL)) { return [$message]; } diff -u b/core/modules/auto_updates/tests/src/Functional/ReadinessCheckerTest.php b/core/modules/auto_updates/tests/src/Functional/ReadinessCheckerTest.php --- b/core/modules/auto_updates/tests/src/Functional/ReadinessCheckerTest.php +++ b/core/modules/auto_updates/tests/src/Functional/ReadinessCheckerTest.php @@ -56,7 +56,7 @@ $this->container->get('module_installer')->uninstall(['automated_cron']); $this->container->get('module_installer')->install(['auto_updates', 'auto_updates_test']); - // If the the site is ready for updates the users will see the same output + // If the site is ready for updates, the users will see the same output // regardless of whether the user has permission to run updates. foreach ([$this->reportViewerUser, $this->checkerRunnerUser] as $user) { $this->drupalLogin($user); @@ -82,11 +82,12 @@ // Run the readiness checks. $this->clickLink('Run readiness checks'); // @todo If coming from the status report page should you be redirected there? - // This is how 'Run cron" works. + // This is how 'Run cron' works. + $assert->statusCodeEquals(200); $assert->addressEquals('/admin/config/auto_updates'); $assert->checkboxChecked('enable_readiness_checks'); - $assert->pageTextNotContains("Access denied"); - $assert->pageTextContains('Your site is currently failing readiness checks for automatic updates. It cannot be automatically updated until further action is performed:'); + $assert->pageTextNotContains('Access denied'); + $assert->pageTextContains('Your site is currently failing readiness checks for automatic updates. It cannot be automatically updated until further action is performed.'); $assert->pageTextContains('OMG 🚒. Your server is on 🔥!'); foreach ([$this->reportViewerUser, $this->checkerRunnerUser] as $user) { @@ -113,6 +114,7 @@ // Confirm that access is denied when manually going to the readiness // checker controller. $this->drupalGet('admin/config/auto_updates/readiness'); + $assert->statusCodeEquals(403); $assert->pageTextContains('Access denied'); $this->drupalGet('admin/reports/status'); diff -u b/core/modules/auto_updates/tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php b/core/modules/auto_updates/tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php --- b/core/modules/auto_updates/tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php +++ b/core/modules/auto_updates/tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php @@ -54,7 +54,7 @@ } /** - * Test checker with the free disk space minimum set to an insanely high number. + * Test checker with the free disk space minimum set to a very high number. */ class TestDiskSpace extends DiskSpace { @@ -73,7 +73,7 @@ /** * {@inheritdoc} */ - protected function areSameLogicalDisk(string $root, string $vendor) { + protected function areSameLogicalDisk(string $root, string $vendor): bool { return FALSE; } only in patch2: unchanged: --- /dev/null +++ b/core/modules/auto_updates/src/ReadinessChecker/FileSystemBase.php @@ -0,0 +1,96 @@ +rootPath = $app_root; + } + + /** + * {@inheritdoc} + */ + public function run(): array { + $messages = []; + if (!file_exists(implode(DIRECTORY_SEPARATOR, [$this->getRootPath(), 'core', 'core.api.php']))) { + $messages[] = $this->t('The web root could not be located.'); + } + if (!is_dir($this->getVendorPath())) { + $messages[] = $this->t('Vendor folder "@vendor" is not a valid directory. Alternate vendor folder locations are not currently supported.', [ + '@vendor' => $this->getVendorPath(), + ]); + } + if ($messages) { + return $messages; + } + return $this->doCheck(); + } + + /** + * Performs checks. + * + * @return array + * An array of translatable strings if any checks fail. + */ + abstract protected function doCheck(): array; + + /** + * Gets the root file path. + * + * @return string + * The root file path. + */ + protected function getRootPath(): string { + return $this->rootPath; + } + + /** + * Get the vendor file path. + * + * @return string + * The vendor file path. + */ + protected function getVendorPath(): string { + // @todo Support finding the 'vendor' directory dynamically in + // https://www.drupal.org/node/3166435. + return $this->getRootPath() . DIRECTORY_SEPARATOR . 'vendor'; + } + + /** + * Determines if the root and vendor directories are on the same logical disk. + * + * @param string $root + * Root file path. + * @param string $vendor + * Vendor file path. + * + * @return bool + * TRUE if they are on the same file system, FALSE otherwise. + */ + protected function areSameLogicalDisk(string $root, string $vendor): bool { + $root_statistics = stat($root); + $vendor_statistics = stat($vendor); + return $root_statistics && $vendor_statistics && $root_statistics['dev'] === $vendor_statistics['dev']; + } + +} only in patch2: unchanged: --- /dev/null +++ b/core/modules/auto_updates/tests/modules/auto_updates_test/auto_updates_test.info.yml @@ -0,0 +1,5 @@ +name: 'Automatic Updates Test' +type: module +description: 'Module for testing Automatic Updates.' +package: Testing +version: VERSION only in patch2: unchanged: --- /dev/null +++ b/core/modules/auto_updates/tests/modules/auto_updates_test/src/Datetime/TestTime.php @@ -0,0 +1,26 @@ +get(TestTime::STATE_KEY, NULL)) { + return \DateTime::createFromFormat(self::TIME_FORMAT, $mock_date)->getTimestamp(); + } + return parent::getRequestTime(); + } + +}