fix: #1894286 System updates are executed without priority (fe330dd2) · Commits · project / drupal

core/includes/update.inc

+62 −0

Original line number	Diff line number	Diff line
		@@ -554,6 +554,11 @@ function update_build_dependency_graph($update_functions): array {
		// dependencies between update functions.
		$graph = [];

		// Helper variables used to to manipulate the graph so that system updates run
		// as early as possible.
		$max_system_update = 0;
		$min_non_system_updates = [];

		// Go through each update function and build an initial list of dependencies.
		foreach ($update_functions as $module => $functions) {
		$previous_function = NULL;
		@@ -569,13 +574,25 @@ function update_build_dependency_graph($update_functions): array {
		// Define the module and update number associated with this function.
		$graph[$function]['module'] = $module;
		$graph[$function]['number'] = $number;
		if ($module == 'system') {
		// Save the maximum system update number so independent update functions
		// can be run after all system updates.
		$max_system_update = max($max_system_update, $number);
		}
		else {
		$min_non_system_updates[$module] = min($min_non_system_updates[$module] ?? \PHP_INT_MAX, $number);
		}
		}
		}

		$has_non_system_updates_that_must_run_before_system_updates = FALSE;
		// Now add any explicit update dependencies declared by modules.
		$update_dependencies = update_retrieve_dependencies();
		foreach ($graph as $function => $data) {
		if (!empty($update_dependencies[$data['module']][$data['number']])) {
		if ($data['module'] === 'system') {
		$has_non_system_updates_that_must_run_before_system_updates = TRUE;
		}
		foreach ($update_dependencies[$data['module']][$data['number']] as $module => $number) {
		$dependency = $module . '_update_' . $number;
		$graph[$dependency]['edges'][$function] = TRUE;
		@@ -585,6 +602,51 @@ function update_build_dependency_graph($update_functions): array {
		}
		}

		if ($max_system_update === 0) {
		// There are no system updates, so there is nothing else to do.
		return $graph;
		}

		if (!$has_non_system_updates_that_must_run_before_system_updates) {
		// This is the simple case where all non-system updates run after all
		// system updates.
		foreach ($min_non_system_updates as $module => $min_number) {
		$function = $module . '_update_' . $min_number;
		$graph['system_update_' . $max_system_update]['edges'][$function] = TRUE;
		}
		return $graph;
		}

		// In this case, there are non-system updates that must run before a system
		// update. These updates must run after any system updates that are not
		// dependent on them. Build the current graph to work this out.
		$processed_graph = (new Graph($graph))->searchAndSort();
		foreach ($processed_graph as $function => $data) {
		if ($data['module'] !== 'system') {
		$min_system_update = NULL;
		// The paths key contains the full list of update functions that must run
		// after.
		foreach (array_keys($data['paths']) as $dependent) {
		if (str_starts_with($dependent, 'system_update_')) {
		$system_update_number = (int) substr($dependent, 14);
		$min_system_update = $min_system_update !== NULL ? min($min_system_update, $system_update_number) : $system_update_number;
		}
		}
		if ($min_system_update === NULL) {
		// If there are no system updates in the paths key, then ensure this
		// update function runs after all system updates.
		$graph['system_update_' . $max_system_update]['edges'][$function] = TRUE;
		}
		else {
		// If there are system updates in the paths key, then ensure this update
		// runs after the previous system update.
		$previous_system_update = 'system_update_' . ($min_system_update - 1);
		if (isset($graph[$previous_system_update])) {
		$graph[$previous_system_update]['edges'][$function] = TRUE;
		}
		}
		}
		}
		return $graph;
		}

core/tests/Drupal/Tests/Core/Update/UpdateOrderingTest.php

0 → 100644

+211 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\Tests\Core\Update;

		use Drupal\Component\DependencyInjection\ContainerInterface;
		use Drupal\Core\Extension\ModuleExtensionList;
		use Drupal\Core\Extension\ModuleHandlerInterface;
		use Drupal\Core\Update\UpdateHookRegistry;
		use Drupal\Tests\UnitTestCase;
		use PHPUnit\Framework\Attributes\Group;
		use PHPUnit\Framework\Attributes\PreserveGlobalState;
		use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses;
		use Prophecy\Argument;

		/**
		* Tests update ordering.
		*
		* Note code is loaded and mock the container, so isolate the tests.
		*/
		#[Group('Update')]
		#[PreserveGlobalState(FALSE)]
		#[RunTestsInSeparateProcesses]
		class UpdateOrderingTest extends UnitTestCase {

		/**
		* The return value of hook_update_dependencies().
		*
		* @see hook_update_dependencies()
		*/
		public static array $updateDependenciesHookReturn = [];

		/**
		* {@inheritdoc}
		*/
		protected function setUp(): void {
		parent::setUp();
		require_once $this->root . '/core/includes/update.inc';
		// Load a hook_update_dependencies() implementation that allows this test
		// to control the update ordering.
		require_once $this->root . '/core/tests/fixtures/test_update_ordering/test_update_ordering.php';

		$registry = $this->prophesize(UpdateHookRegistry::class);
		$registry->getAllInstalledVersions()->willReturn(['a_module' => 8000, 'system' => 8000, 'z_module' => 8000]);
		$registry->getAvailableUpdates('system')->willReturn([9000, 9001]);
		$registry->getInstalledVersion('system')->willReturn(8000);
		$registry->getAvailableUpdates('a_module')->willReturn([9000]);
		$registry->getInstalledVersion('a_module')->willReturn(8000);
		$registry->getAvailableUpdates('z_module')->willReturn([9000, 9001]);
		$registry->getInstalledVersion('z_module')->willReturn(8000);
		$extension_list = $this->prophesize(ModuleExtensionList::class);
		$extension_list->exists(Argument::any())->willReturn(TRUE);
		$module_handler = $this->prophesize(ModuleHandlerInterface::class);
		$container = $this->prophesize(ContainerInterface::class);
		$container->get('extension.list.module')->willReturn($extension_list->reveal());
		$container->get('update.update_hook_registry')->willReturn($registry->reveal());
		$container->get('module_handler')->willReturn($module_handler->reveal());
		\Drupal::setContainer($container->reveal());
		}

		/**
		* Tests updates to ensure without dependencies system updates come first.
		*/
		public function testUpdateOrdering(): void {
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'system_update_9000',
		'system_update_9001',
		'z_module_update_9000',
		'z_module_update_9001',
		'a_module_update_9000',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with a dependency.
		*/
		public function testUpdateOrderingWithDependency(): void {
		// Indicate that the a_module_update_9000() function must run before the
		// system_update_9000() function.
		static::$updateDependenciesHookReturn['system'][9000] = [
		'a_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'a_module_update_9000',
		'system_update_9000',
		'system_update_9001',
		'z_module_update_9000',
		'z_module_update_9001',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with a dependency chain.
		*/
		public function testUpdateOrderingWithDependencyChain(): void {
		// Indicate that the z_module_update_9000() function must run before the
		// a_module_update_9000() function.
		static::$updateDependenciesHookReturn['a_module'][9000] = [
		'z_module' => 9000,
		];
		// Indicate that the a_module_update_9000() function must run before the
		// system_update_9000() function.
		static::$updateDependenciesHookReturn['system'][9000] = [
		'a_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'z_module_update_9000',
		'a_module_update_9000',
		'system_update_9000',
		'system_update_9001',
		'z_module_update_9001',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with dependencies not on system updates.
		*/
		public function testUpdateOrderingWithNonSystemDependency(): void {
		// Indicate that the a_module_update_9000() function must run before the
		// z_module_update_9000() function.
		static::$updateDependenciesHookReturn['z_module'][9000] = [
		'a_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'system_update_9000',
		'system_update_9001',
		'a_module_update_9000',
		'z_module_update_9000',
		'z_module_update_9001',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with a dependency in between system updates.
		*/
		public function testUpdateOrderingWithInBetweenDependency(): void {
		// Indicate that the z_module_update_9000() function must run before the
		// system_update_9001() function.
		static::$updateDependenciesHookReturn['system'][9001] = [
		'z_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'system_update_9000',
		'z_module_update_9000',
		'system_update_9001',
		'z_module_update_9001',
		'a_module_update_9000',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with an impossible dependency.
		*/
		public function testUpdateOrderingAlreadyRunUpdate(): void {
		// Indicate that the a_module_update_9000() function must run before the
		// system_update_8999() function. Note, this is not impossible as the update
		// has already run.
		static::$updateDependenciesHookReturn['system'][8999] = [
		'a_module' => 9000,
		];
		// Indicate that the a_module_update_9000() function must run before the
		// z_module_update_9000() function.
		static::$updateDependenciesHookReturn['z_module'][9000] = [
		'a_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'system_update_9000',
		'system_update_9001',
		'a_module_update_9000',
		'z_module_update_9000',
		'z_module_update_9001',
		], array_keys($updates));
		}

		/**
		* Tests update ordering with multiple dependencies to system updates.
		*/
		public function testUpdateOrderingComplexSystemDependencies(): void {
		// Indicate that the z_module_update_9001() function must run before the
		// a_module_update_9000() function.
		static::$updateDependenciesHookReturn['a_module'][9000] = [
		'z_module' => 9001,
		];
		// Indicate that the z_module_update_9000() function must run before the
		// system_update_9000() function.
		static::$updateDependenciesHookReturn['system'][9000] = [
		'z_module' => 9000,
		];
		// Indicate that the a_module_update_9000() function must run before the
		// system_update_9001() function.
		static::$updateDependenciesHookReturn['system'][9001] = [
		'a_module' => 9000,
		];
		$updates = update_resolve_dependencies(['a_module' => 9000, 'system' => '9000', 'z_module' => 9000]);
		$this->assertSame([
		'z_module_update_9000',
		'system_update_9000',
		'z_module_update_9001',
		'a_module_update_9000',
		'system_update_9001',
		], array_keys($updates));
		}

		}

core/tests/fixtures/test_update_ordering/test_update_ordering.php

0 → 100644

+17 −0

Original line number	Diff line number	Diff line
		<?php

		/**
		* @file
		* Test fixture.
		*/

		use Drupal\Tests\Core\Update\UpdateOrderingTest;

		/**
		* Implements hook_update_dependencies().
		*
		* @see hook_update_dependencies()
		*/
		function a_module_update_dependencies(): array {
		return UpdateOrderingTest::$updateDependenciesHookReturn;
		}