From 56b95009a8f73cff98b25efe8dabdcbf4c374ba1 Mon Sep 17 00:00:00 2001 From: lucashedding <lucashedding@1463982.no-reply.drupal.org> Date: Fri, 11 Oct 2019 13:32:05 -0600 Subject: [PATCH] Issue #3087190 by heddn: Move ignored_paths external to ModifiedFiles service --- src/IgnoredPathsIteratorFilter.php | 18 ++++++++++++++++++ src/ReadinessChecker/ModifiedFiles.php | 4 +++- src/Services/ModifiedFiles.php | 15 ++++++--------- src/Services/ModifiedFilesInterface.php | 2 +- .../src/Controller/ModifiedFilesController.php | 7 ++++--- tests/src/Build/InPlaceUpdateTest.php | 4 ++-- tests/src/Build/ModifiedFilesTest.php | 1 - .../ReadinessChecker/ModifiedFilesTest.php | 4 ++-- 8 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 src/IgnoredPathsIteratorFilter.php diff --git a/src/IgnoredPathsIteratorFilter.php b/src/IgnoredPathsIteratorFilter.php new file mode 100644 index 0000000000..e673263d96 --- /dev/null +++ b/src/IgnoredPathsIteratorFilter.php @@ -0,0 +1,18 @@ +<?php + +namespace Drupal\automatic_updates; + +/** + * Provides an iterator filter for file paths which are ignored. + */ +class IgnoredPathsIteratorFilter extends \FilterIterator { + use IgnoredPathsTrait; + + /** + * {@inheritdoc} + */ + public function accept() { + return !$this->isIgnoredPath($this->current()); + } + +} diff --git a/src/ReadinessChecker/ModifiedFiles.php b/src/ReadinessChecker/ModifiedFiles.php index 99c041d1af..c96e25e230 100644 --- a/src/ReadinessChecker/ModifiedFiles.php +++ b/src/ReadinessChecker/ModifiedFiles.php @@ -2,6 +2,7 @@ namespace Drupal\automatic_updates\ReadinessChecker; +use Drupal\automatic_updates\IgnoredPathsIteratorFilter; use Drupal\automatic_updates\ProjectInfoTrait; use Drupal\automatic_updates\Services\ModifiedFilesInterface; use Drupal\Core\Extension\ExtensionList; @@ -92,7 +93,8 @@ class ModifiedFiles implements ReadinessCheckerInterface { } } } - foreach ($this->modifiedFiles->getModifiedFiles($extensions) as $file) { + $filtered_modified_files = new IgnoredPathsIteratorFilter($this->modifiedFiles->getModifiedFiles($extensions)); + foreach ($filtered_modified_files as $file) { $messages[] = $this->t('The hash for @file does not match its original. Updates that include that file will fail and require manual intervention.', ['@file' => $file]); } return $messages; diff --git a/src/Services/ModifiedFiles.php b/src/Services/ModifiedFiles.php index 955d754e21..5a0fe77f83 100644 --- a/src/Services/ModifiedFiles.php +++ b/src/Services/ModifiedFiles.php @@ -74,13 +74,13 @@ class ModifiedFiles implements ModifiedFilesInterface { * {@inheritdoc} */ public function getModifiedFiles(array $extensions = []) { - $modified_files = []; + $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 ($resource) use ($modified_files) { $this->processHashes($resource, $modified_files); }, ]))->promise()->wait(); @@ -92,10 +92,10 @@ class ModifiedFiles implements ModifiedFilesInterface { * * @param array $resource * An array of http response and project info. - * @param array $modified_files + * @param \ArrayIterator $modified_files * The list of modified files. */ - protected function processHashes(array $resource, array &$modified_files) { + protected function processHashes(array $resource, \ArrayIterator $modified_files) { $contents = $resource['contents']; $info = $resource['info']; $directory_root = $info['install path']; @@ -112,16 +112,13 @@ class ModifiedFiles implements ModifiedFilesInterface { $directory_root, $failed_checksum->filename, ])); - if ($this->isIgnoredPath($file_path)) { - continue; - } if (!file_exists($file_path)) { - $modified_files[] = $file_path; + $modified_files->append($file_path); continue; } $actual_hash = @hash_file(strtolower($failed_checksum->algorithm), $file_path); if ($actual_hash === FALSE || empty($actual_hash) || strlen($actual_hash) < 64 || strcmp($actual_hash, $failed_checksum->hex_hash) !== 0) { - $modified_files[] = $file_path; + $modified_files->append($file_path); } } } diff --git a/src/Services/ModifiedFilesInterface.php b/src/Services/ModifiedFilesInterface.php index 982bd3ed42..ba46f254ee 100644 --- a/src/Services/ModifiedFilesInterface.php +++ b/src/Services/ModifiedFilesInterface.php @@ -14,7 +14,7 @@ interface ModifiedFilesInterface { * The list of extensions, keyed by extension name with values an info * array. * - * @return array + * @return \Iterator * The modified files. */ public function getModifiedFiles(array $extensions = []); diff --git a/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php b/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php index ef80d1c243..e1c6622f5c 100644 --- a/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php +++ b/tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php @@ -2,6 +2,7 @@ namespace Drupal\test_automatic_updates\Controller; +use Drupal\automatic_updates\IgnoredPathsIteratorFilter; use Drupal\automatic_updates\ProjectInfoTrait; use Drupal\automatic_updates\Services\ModifiedFilesInterface; use Drupal\Core\Controller\ControllerBase; @@ -68,9 +69,9 @@ class ModifiedFilesController extends ControllerBase { } $response = Response::create('No modified files!'); - $messages = $this->modifiedFiles->getModifiedFiles($extensions); - if (!empty($messages)) { - $response->setContent('Modified files include: ' . implode(', ', $messages)); + $filtered_modified_files = new IgnoredPathsIteratorFilter($this->modifiedFiles->getModifiedFiles($extensions)); + if (iterator_count($filtered_modified_files)) { + $response->setContent('Modified files include: ' . implode(', ', iterator_to_array($filtered_modified_files))); } return $response; } diff --git a/tests/src/Build/InPlaceUpdateTest.php b/tests/src/Build/InPlaceUpdateTest.php index d2040ed7df..c72624bab2 100644 --- a/tests/src/Build/InPlaceUpdateTest.php +++ b/tests/src/Build/InPlaceUpdateTest.php @@ -61,7 +61,7 @@ class InPlaceUpdateTest extends QuickStartTestBase { $fs->chmod($this->getWorkspaceDirectory() . '/sites/default', 0700, 0000); $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer install --no-dev --no-interaction'); $this->assertErrorOutputContains('Generating autoload files'); - $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer require ocramius/package-versions:^1.4 webflo/drupal-finder:^1.1 composer/semver:^1.0 --no-interaction'); + $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer require ocramius/package-versions:^1.4 webflo/drupal-finder:^1.1 composer/semver:^1.0 drupal/php-signify:^1.0@dev --no-interaction'); $this->assertErrorOutputContains('Generating autoload files'); $this->installQuickStart('minimal'); @@ -120,7 +120,7 @@ class InPlaceUpdateTest extends QuickStartTestBase { $fs->chmod($this->getWorkspaceDirectory() . '/sites/default', 0700, 0000); $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer install --no-dev --no-interaction'); $this->assertErrorOutputContains('Generating autoload files'); - $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer require ocramius/package-versions:^1.4 webflo/drupal-finder:^1.1 composer/semver:^1.0 --no-interaction'); + $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer require ocramius/package-versions:^1.4 webflo/drupal-finder:^1.1 composer/semver:^1.0 drupal/php-signify:^1.0@dev --no-interaction'); $this->assertErrorOutputContains('Generating autoload files'); $this->installQuickStart('standard'); diff --git a/tests/src/Build/ModifiedFilesTest.php b/tests/src/Build/ModifiedFilesTest.php index b8d3ae2271..9b973d2329 100644 --- a/tests/src/Build/ModifiedFilesTest.php +++ b/tests/src/Build/ModifiedFilesTest.php @@ -114,7 +114,6 @@ class ModifiedFilesTest extends QuickStartTestBase { * The modified files to assert. */ protected function assertModifications($project_type, $project, array $modifications) { - $this->destroyBuild = FALSE; $this->symfonyFileSystem->chmod($this->getWorkspaceDirectory() . '/sites/default', 0700, 0000); $this->executeCommand('COMPOSER_DISCARD_CHANGES=true composer install --no-dev --no-interaction'); $this->assertErrorOutputContains('Generating autoload files'); diff --git a/tests/src/Kernel/ReadinessChecker/ModifiedFilesTest.php b/tests/src/Kernel/ReadinessChecker/ModifiedFilesTest.php index fa5266ee6e..79e72b9ee4 100644 --- a/tests/src/Kernel/ReadinessChecker/ModifiedFilesTest.php +++ b/tests/src/Kernel/ReadinessChecker/ModifiedFilesTest.php @@ -28,7 +28,7 @@ class ModifiedFilesTest extends KernelTestBase { public function testModifiedFiles() { /** @var \Prophecy\Prophecy\ObjectProphecy|\Drupal\automatic_updates\Services\ModifiedFilesInterface $service */ $service = $this->prophesize(ModifiedFilesInterface::class); - $service->getModifiedFiles(Argument::type('array'))->willReturn([]); + $service->getModifiedFiles(Argument::type('array'))->willReturn(new \ArrayIterator()); $modules = $this->container->get('extension.list.module'); $profiles = $this->container->get('extension.list.profile'); $themes = $this->container->get('extension.list.theme'); @@ -44,7 +44,7 @@ class ModifiedFilesTest extends KernelTestBase { $this->assertEmpty($messages); // Hash doesn't match i.e. modified code. - $service->getModifiedFiles(Argument::type('array'))->willReturn(['core/LICENSE.txt']); + $service->getModifiedFiles(Argument::type('array'))->willReturn(new \ArrayIterator(['core/LICENSE.txt'])); $messages = $modified_files->run(); $this->assertCount(1, $messages); } -- GitLab