From 85b969d98677df49e36c459a7173e3b081b64a38 Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Wed, 9 Mar 2022 22:00:29 +0000 Subject: [PATCH] Issue #3268363 by phenaproxima, tedbow: Load new services after staged changes are applied --- package_manager/src/Stage.php | 9 ++ .../tests/fixtures/alpha/1.0.0/composer.json | 5 + .../tests/fixtures/alpha/1.1.0/composer.json | 5 + .../updated_module/1.0.0/composer.json | 5 + .../1.0.0/updated_module.info.yml | 4 + .../updated_module/1.1.0/composer.json | 5 + .../1.1.0/src/PostApplySubscriber.php | 52 +++++++++++ .../1.1.0/updated_module.info.yml | 4 + .../1.1.0/updated_module.services.yml | 7 ++ .../package_manager_test_api.routing.yml | 6 +- .../src/ApiController.php | 43 ++++++--- .../tests/src/Build/PackageUpdateTest.php | 91 +++++++++++++++++++ .../tests/src/Build/StagedUpdateTest.php | 87 ------------------ .../tests/src/Kernel/StageEventsTest.php | 13 +++ tests/src/Kernel/CronUpdaterTest.php | 14 +++ 15 files changed, 247 insertions(+), 103 deletions(-) create mode 100644 package_manager/tests/fixtures/alpha/1.0.0/composer.json create mode 100644 package_manager/tests/fixtures/alpha/1.1.0/composer.json create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/composer.json create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/updated_module.info.yml create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/composer.json create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/src/PostApplySubscriber.php create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/updated_module.info.yml create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml create mode 100644 package_manager/tests/src/Build/PackageUpdateTest.php delete mode 100644 package_manager/tests/src/Build/StagedUpdateTest.php diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php index 0a906eda06..9ae7ebfe4b 100644 --- a/package_manager/src/Stage.php +++ b/package_manager/src/Stage.php @@ -331,6 +331,15 @@ class Stage { $this->committer->commit($stage_dir, $active_dir, $event->getExcludedPaths()); $this->tempStore->delete(self::TEMPSTORE_APPLY_TIME_KEY); + + // Rebuild the container and clear all caches, to ensure that new services + // are picked up. + drupal_flush_all_caches(); + // Refresh the event dispatcher so that new or changed event subscribers + // will be called. The other services we depend on are either stateless or + // unlikely to call newly added code during the current request. + $this->eventDispatcher = \Drupal::service('event_dispatcher'); + $this->dispatch(new PostApplyEvent($this)); } diff --git a/package_manager/tests/fixtures/alpha/1.0.0/composer.json b/package_manager/tests/fixtures/alpha/1.0.0/composer.json new file mode 100644 index 0000000000..35db7d858c --- /dev/null +++ b/package_manager/tests/fixtures/alpha/1.0.0/composer.json @@ -0,0 +1,5 @@ +{ + "name": "drupal/alpha", + "type": "drupal-module", + "version": "1.0.0" +} diff --git a/package_manager/tests/fixtures/alpha/1.1.0/composer.json b/package_manager/tests/fixtures/alpha/1.1.0/composer.json new file mode 100644 index 0000000000..f21a204a76 --- /dev/null +++ b/package_manager/tests/fixtures/alpha/1.1.0/composer.json @@ -0,0 +1,5 @@ +{ + "name": "drupal/alpha", + "type": "drupal-module", + "version": "1.1.0" +} diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/composer.json b/package_manager/tests/fixtures/updated_module/1.0.0/composer.json new file mode 100644 index 0000000000..777cd741d2 --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.0.0/composer.json @@ -0,0 +1,5 @@ +{ + "name": "drupal/updated_module", + "type": "drupal-module", + "version": "1.0.0" +} diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.info.yml b/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.info.yml new file mode 100644 index 0000000000..ebf1452ec9 --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.info.yml @@ -0,0 +1,4 @@ +name: 'Updated module' +description: 'A module which will change during an update, to ensure that the changes are picked up.' +type: module +package: Testing diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/composer.json b/package_manager/tests/fixtures/updated_module/1.1.0/composer.json new file mode 100644 index 0000000000..6f997dad4c --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.1.0/composer.json @@ -0,0 +1,5 @@ +{ + "name": "drupal/updated_module", + "type": "drupal-module", + "version": "1.1.0" +} diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/src/PostApplySubscriber.php b/package_manager/tests/fixtures/updated_module/1.1.0/src/PostApplySubscriber.php new file mode 100644 index 0000000000..2e7e988a3b --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.1.0/src/PostApplySubscriber.php @@ -0,0 +1,52 @@ +<?php + +namespace Drupal\updated_module; + +use Drupal\package_manager\Event\PostApplyEvent; +use Drupal\package_manager\PathLocator; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * Writes a file after staged changes are applied to the active directory. + * + * This event subscriber doesn't exist in version 1.0.0 of this module, so we + * use it to test that new event subscribers are picked up after staged changes + * have been applied. + */ +class PostApplySubscriber implements EventSubscriberInterface { + + /** + * The path locator service. + * + * @var \Drupal\package_manager\PathLocator + */ + private $pathLocator; + + /** + * Constructs a PostApplySubscriber. + * + * @param \Drupal\package_manager\PathLocator $path_locator + * The path locator service. + */ + public function __construct(PathLocator $path_locator) { + $this->pathLocator = $path_locator; + } + + /** + * Writes a file when staged changes are applied to the active directory. + */ + public function postApply(): void { + $dir = $this->pathLocator->getProjectRoot(); + file_put_contents("$dir/bravo.txt", 'Bravo!'); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + return [ + PostApplyEvent::class => 'postApply', + ]; + } + +} diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.info.yml b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.info.yml new file mode 100644 index 0000000000..ebf1452ec9 --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.info.yml @@ -0,0 +1,4 @@ +name: 'Updated module' +description: 'A module which will change during an update, to ensure that the changes are picked up.' +type: module +package: Testing diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml new file mode 100644 index 0000000000..1179d2fb06 --- /dev/null +++ b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml @@ -0,0 +1,7 @@ +services: + updated_module.post_apply_subscriber: + class: Drupal\updated_module\PostApplySubscriber + arguments: + - '@package_manager.path_locator' + tags: + - { name: event_subscriber } diff --git a/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.routing.yml b/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.routing.yml index 4b5cf58ca1..f400c0f4fc 100644 --- a/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.routing.yml +++ b/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.routing.yml @@ -1,6 +1,6 @@ -package_manager_test_api.require: - path: '/package-manager-test-api/require' +package_manager_test_api: + path: '/package-manager-test-api' defaults: - _controller: 'Drupal\package_manager_test_api\ApiController::require' + _controller: 'Drupal\package_manager_test_api\ApiController::run' requirements: _access: 'TRUE' diff --git a/package_manager/tests/modules/package_manager_test_api/src/ApiController.php b/package_manager/tests/modules/package_manager_test_api/src/ApiController.php index cf6c28efa6..45eb27f0f8 100644 --- a/package_manager/tests/modules/package_manager_test_api/src/ApiController.php +++ b/package_manager/tests/modules/package_manager_test_api/src/ApiController.php @@ -3,6 +3,7 @@ namespace Drupal\package_manager_test_api; use Drupal\Core\Controller\ControllerBase; +use Drupal\package_manager\PathLocator; use Drupal\package_manager\Stage; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\JsonResponse; @@ -20,14 +21,24 @@ class ApiController extends ControllerBase { */ private $stage; + /** + * The path locator service. + * + * @var \Drupal\package_manager\PathLocator + */ + private $pathLocator; + /** * Constructs an ApiController object. * * @param \Drupal\package_manager\Stage $stage * The stage. + * @param \Drupal\package_manager\PathLocator $path_locator + * The path locator service. */ - public function __construct(Stage $stage) { + public function __construct(Stage $stage, PathLocator $path_locator) { $this->stage = $stage; + $this->pathLocator = $path_locator; } /** @@ -45,39 +56,45 @@ class ApiController extends ControllerBase { $container->get('tempstore.shared'), $container->get('datetime.time') ); - return new static($stage); + return new static( + $stage, + $container->get('package_manager.path_locator') + ); } /** - * Creates a staging area and requires packages into it. + * Runs a complete stage life cycle. + * + * Creates a staging area, requires packages into it, applies changes to the + * active directory, and destroys the stage. * * @param \Symfony\Component\HttpFoundation\Request $request * The request. The runtime and dev dependencies are expected to be in * either the query string or request body, under the 'runtime' and 'dev' * keys, respectively. There may also be a 'files_to_return' key, which - * contains an array of file paths, relative to the stage directory, whose + * contains an array of file paths, relative to the project root, whose * contents should be returned in the response. * * @return \Symfony\Component\HttpFoundation\JsonResponse * A JSON response containing an associative array of the contents of the - * staged files listed in the 'files_to_return' request key. The array will - * be keyed by path, relative to the stage directory. + * files listed in the 'files_to_return' request key. The array will be + * keyed by path, relative to the project root. */ - public function require(Request $request): JsonResponse { + public function run(Request $request): JsonResponse { $this->stage->create(); $this->stage->require( $request->get('runtime', []), $request->get('dev', []) ); + $this->stage->apply(); + $this->stage->destroy(); - $stage_dir = $this->stage->getStageDirectory(); - $staged_file_contents = []; + $dir = $this->pathLocator->getProjectRoot(); + $file_contents = []; foreach ($request->get('files_to_return', []) as $path) { - $staged_file_contents[$path] = file_get_contents($stage_dir . '/' . $path); + $file_contents[$path] = file_get_contents($dir . '/' . $path); } - $this->stage->destroy(); - - return new JsonResponse($staged_file_contents); + return new JsonResponse($file_contents); } } diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php new file mode 100644 index 0000000000..a546ca0a33 --- /dev/null +++ b/package_manager/tests/src/Build/PackageUpdateTest.php @@ -0,0 +1,91 @@ +<?php + +namespace Drupal\Tests\package_manager\Build; + +/** + * Tests updating packages in a staging area. + * + * @group package_manager + */ +class PackageUpdateTest extends TemplateProjectTestBase { + + /** + * Tests updating packages in a staging area. + */ + public function testPackageUpdate(): void { + $this->createTestProject('RecommendedProject'); + + $this->addRepository('alpha', __DIR__ . '/../../fixtures/alpha/1.0.0'); + $this->addRepository('updated_module', __DIR__ . '/../../fixtures/updated_module/1.0.0'); + $this->runComposer('COMPOSER_MIRROR_PATH_REPOS=1 composer require drupal/alpha drupal/updated_module --update-with-all-dependencies', 'project'); + + $this->installQuickStart('minimal'); + $this->formLogin($this->adminUsername, $this->adminPassword); + // The updated_module provides actual Drupal-facing functionality that we're + // testing as well, so we need to install it. + $this->installModules(['package_manager_test_api', 'updated_module']); + + // Change both modules' upstream version. + $this->addRepository('alpha', __DIR__ . '/../../fixtures/alpha/1.1.0'); + $this->addRepository('updated_module', __DIR__ . '/../../fixtures/updated_module/1.1.0'); + + // Use the API endpoint to create a stage and update updated_module to + // 1.1.0. Even though both modules have version 1.1.0 available, only + // updated_module should be updated. We ask the API to return the contents + // of both modules' composer.json files, so we can assert that they were + // updated to the versions we expect. + // @see \Drupal\package_manager_test_api\ApiController::run() + $query = http_build_query([ + 'runtime' => [ + 'drupal/updated_module:1.1.0', + ], + 'files_to_return' => [ + 'web/modules/contrib/alpha/composer.json', + 'web/modules/contrib/updated_module/composer.json', + 'bravo.txt', + ], + ]); + $this->visit("/package-manager-test-api?$query"); + $mink = $this->getMink(); + $mink->assertSession()->statusCodeEquals(200); + + $file_contents = $mink->getSession()->getPage()->getContent(); + $file_contents = json_decode($file_contents, TRUE); + + $expected_versions = [ + 'alpha' => '1.0.0', + 'updated_module' => '1.1.0', + ]; + foreach ($expected_versions as $module_name => $expected_version) { + $path = "web/modules/contrib/$module_name/composer.json"; + $module_composer_json = json_decode($file_contents[$path]); + $this->assertSame($expected_version, $module_composer_json->version); + } + // The post-apply event subscriber in updated_module 1.1.0 should have + // created this file. + // @see \Drupal\updated_module\PostApplySubscriber::postApply() + $this->assertSame('Bravo!', $file_contents['bravo.txt']); + } + + /** + * Adds a path repository to the test site. + * + * @param string $name + * An arbitrary name for the repository. + * @param string $path + * The path of the repository. Must exist in the file system. + */ + private function addRepository(string $name, string $path): void { + $this->assertDirectoryExists($path); + + $repository = json_encode([ + 'type' => 'path', + 'url' => $path, + 'options' => [ + 'symlink' => FALSE, + ], + ]); + $this->runComposer("composer config repo.$name '$repository'", 'project'); + } + +} diff --git a/package_manager/tests/src/Build/StagedUpdateTest.php b/package_manager/tests/src/Build/StagedUpdateTest.php deleted file mode 100644 index 8dbf403633..0000000000 --- a/package_manager/tests/src/Build/StagedUpdateTest.php +++ /dev/null @@ -1,87 +0,0 @@ -<?php - -namespace Drupal\Tests\package_manager\Build; - -/** - * Tests updating packages in a staging area. - * - * @group package_manager - */ -class StagedUpdateTest extends TemplateProjectTestBase { - - /** - * Tests that a stage only updates packages with changed constraints. - */ - public function testStagedUpdate(): void { - $this->createTestProject('RecommendedProject'); - - $this->createModule('alpha'); - $this->createModule('bravo'); - $this->runComposer('COMPOSER_MIRROR_PATH_REPOS=1 composer require drupal/alpha drupal/bravo --update-with-all-dependencies', 'project'); - - $this->installQuickStart('minimal'); - $this->formLogin($this->adminUsername, $this->adminPassword); - $this->installModules(['package_manager_test_api']); - - // Change both modules' upstream version. - $this->runComposer('composer config version 1.1.0', 'alpha'); - $this->runComposer('composer config version 1.1.0', 'bravo'); - - // Use the API endpoint to create a stage and update bravo to 1.1.0. Even - // though both modules are at version 1.1.0, only bravo should be updated. - // We ask the API to return the contents of both modules' staged - // composer.json files, so we can assert that the staged versions are what - // we expect. - // @see \Drupal\package_manager_test_api\ApiController::require() - $query = http_build_query([ - 'runtime' => [ - 'drupal/bravo:1.1.0', - ], - 'files_to_return' => [ - 'web/modules/contrib/alpha/composer.json', - 'web/modules/contrib/bravo/composer.json', - ], - ]); - $this->visit("/package-manager-test-api/require?$query"); - $mink = $this->getMink(); - $mink->assertSession()->statusCodeEquals(200); - - $staged_file_contents = $mink->getSession()->getPage()->getContent(); - $staged_file_contents = json_decode($staged_file_contents, TRUE); - - $expected_versions = [ - 'alpha' => '1.0.0', - 'bravo' => '1.1.0', - ]; - foreach ($expected_versions as $module_name => $expected_version) { - $path = "web/modules/contrib/$module_name/composer.json"; - $staged_composer_json = json_decode($staged_file_contents[$path]); - $this->assertSame($expected_version, $staged_composer_json->version); - } - } - - /** - * Creates an empty module for testing purposes. - * - * @param string $name - * The machine name of the module, which can be added to the test site as - * 'drupal/$name'. - */ - private function createModule(string $name): void { - $dir = $this->getWorkspaceDirectory() . '/' . $name; - mkdir($dir); - $this->assertDirectoryExists($dir); - $this->runComposer("composer init --name drupal/$name --type drupal-module", $name); - $this->runComposer('composer config version 1.0.0', $name); - - $repository = json_encode([ - 'type' => 'path', - 'url' => $dir, - 'options' => [ - 'symlink' => FALSE, - ], - ]); - $this->runComposer("composer config repo.$name '$repository'", 'project'); - } - -} diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index b352b58889..e154e95d97 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\package_manager\Kernel; +use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; use Drupal\package_manager\Event\PostDestroyEvent; @@ -46,6 +47,18 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc $this->stage = $this->createStage(); } + /** + * {@inheritdoc} + */ + public function register(ContainerBuilder $container) { + parent::register($container); + + // Since this test adds arbitrary event listeners that aren't services, we + // need to ensure they will persist even if the container is rebuilt when + // staged changes are applied. + $container->getDefinition('event_dispatcher')->addTag('persist'); + } + /** * {@inheritdoc} */ diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php index 792e1638f2..4c07670396 100644 --- a/tests/src/Kernel/CronUpdaterTest.php +++ b/tests/src/Kernel/CronUpdaterTest.php @@ -4,6 +4,7 @@ namespace Drupal\Tests\automatic_updates\Kernel; use Drupal\automatic_updates\CronUpdater; use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1; +use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Form\FormState; use Drupal\Core\Logger\RfcLogLevel; use Drupal\package_manager\Event\PostApplyEvent; @@ -63,6 +64,19 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { ->addLogger($this->logger); } + /** + * {@inheritdoc} + */ + public function register(ContainerBuilder $container) { + parent::register($container); + + // Since this test dynamically adds additional loggers to certain channels, + // we need to ensure they will persist even if the container is rebuilt when + // staged changes are applied. + // @see ::testStageDestroyedOnError() + $container->getDefinition('logger.factory')->addTag('persist'); + } + /** * Data provider for ::testUpdaterCalled(). * -- GitLab