diff --git a/includes/update.inc b/includes/update.inc index eeac6ea02febab7d45222ff8ee8f3d758d795f5e..bfc026ec9a537e7b30950636d2727d05c7f58a77 100644 --- a/includes/update.inc +++ b/includes/update.inc @@ -1100,16 +1100,10 @@ function update_build_dependency_graph($update_functions) { } // Now add any explicit update dependencies declared by modules. - $update_dependencies = update_invoke_all('update_dependencies'); + $update_dependencies = update_retrieve_dependencies(); foreach ($graph as $function => $data) { if (!empty($update_dependencies[$data['module']][$data['number']])) { foreach ($update_dependencies[$data['module']][$data['number']] as $module => $number) { - // If we have an explicit dependency on more than one update from a - // particular module, choose the highest one, since that contains the - // actual direct dependency. - if (is_array($number)) { - $number = max($number); - } $dependency = $module . '_update_' . $number; $graph[$dependency]['edges'][$function] = TRUE; $graph[$dependency]['module'] = $module; @@ -1158,39 +1152,66 @@ function update_already_performed($module, $number) { } /** - * Invoke an update system hook in all installed modules. - * - * This function is similar to module_invoke_all(), except it does not require - * that a module be enabled to invoke its hook, only that it be installed. This - * allows the update system to properly perform updates even on modules that - * are currently disabled. + * Invoke hook_update_dependencies() in all installed modules. * - * @param $hook - * The name of the hook to invoke. - * @param ... - * Arguments to pass to the hook. + * This function is similar to module_invoke_all(), with the main difference + * that it does not require that a module be enabled to invoke its hook, only + * that it be installed. This allows the update system to properly perform + * updates even on modules that are currently disabled. * * @return - * An array of return values of the hook implementations. If modules return - * arrays from their implementations, those are merged into one array. + * An array of return values obtained by merging the results of the + * hook_update_dependencies() implementations in all installed modules. * * @see module_invoke_all() + * @see hook_update_dependencies() */ -function update_invoke_all() { - $args = func_get_args(); - $hook = $args[0]; - unset($args[0]); +function update_retrieve_dependencies() { $return = array(); - $modules = db_query("SELECT name FROM {system} WHERE type = 'module' AND schema_version != :schema", array(':schema' => SCHEMA_UNINSTALLED))->fetchCol(); + // Get a list of installed modules, arranged so that we invoke their hooks in + // the same order that module_invoke_all() does. + $modules = db_query("SELECT name FROM {system} WHERE type = 'module' AND schema_version != :schema ORDER BY weight ASC, name ASC", array(':schema' => SCHEMA_UNINSTALLED))->fetchCol(); foreach ($modules as $module) { - $function = $module . '_' . $hook; + $function = $module . '_update_dependencies'; if (function_exists($function)) { - $result = call_user_func_array($function, $args); + $result = $function(); + // Each implementation of hook_update_dependencies() returns a + // multidimensional, associative array containing some keys that + // represent module names (which are strings) and other keys that + // represent update function numbers (which are integers). We cannot use + // array_merge_recursive() to properly merge these results, since it + // treats strings and integers differently. Therefore, we have to + // explicitly loop through the expected array structure here and perform + // the merge manually. if (isset($result) && is_array($result)) { - $return = array_merge_recursive($return, $result); - } - elseif (isset($result)) { - $return[] = $result; + foreach ($result as $module => $module_data) { + foreach ($module_data as $update => $update_data) { + foreach ($update_data as $module_dependency => $update_dependency) { + // If there are redundant dependencies declared for the same + // update function (so that it is declared to depend on more than + // one update from a particular module), record the dependency on + // the highest numbered update here, since that automatically + // implies the previous ones. For example, if one module's + // implementation of hook_update_dependencies() required this + // ordering: + // + // system_update_7001 ---> user_update_7000 + // + // but another module's implementation of the hook required this + // one: + // + // system_update_7002 ---> user_update_7000 + // + // we record the second one, since system_update_7001() is always + // guaranteed to run before system_update_7002() anyway (within + // an individual module, updates are always run in numerical + // order). + if (!isset($return[$module][$update][$module_dependency]) || $update_dependency > $return[$module][$update][$module_dependency]) { + $return[$module][$update][$module_dependency] = $update_dependency; + } + } + } + } } } } diff --git a/modules/simpletest/tests/update.test b/modules/simpletest/tests/update.test index 0809690e63c99dc1c07099db786704d9f703a70f..c4d3828473355033dd62be045c8ed3a138ac4ef5 100644 --- a/modules/simpletest/tests/update.test +++ b/modules/simpletest/tests/update.test @@ -86,3 +86,31 @@ class UpdateDependencyMissingTestCase extends DrupalWebTestCase { } } +/** + * Tests for the invocation of hook_update_dependencies(). + */ +class UpdateDependencyHookInvocationTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Update dependency hook invocation', + 'description' => 'Test that the hook invocation for determining update dependencies works correctly.', + 'group' => 'Update API', + ); + } + + function setUp() { + parent::setUp('update_test_1', 'update_test_2'); + require_once DRUPAL_ROOT . '/includes/update.inc'; + } + + /** + * Test the structure of the array returned by hook_update_dependencies(). + */ + function testHookUpdateDependencies() { + $update_dependencies = update_retrieve_dependencies(); + $this->assertTrue($update_dependencies['system'][7000]['update_test_1'] == 7000, t('An update function that has a dependency on two separate modules has the first dependency recorded correctly.')); + $this->assertTrue($update_dependencies['system'][7000]['update_test_2'] == 7001, t('An update function that has a dependency on two separate modules has the second dependency recorded correctly.')); + $this->assertTrue($update_dependencies['system'][7001]['update_test_1'] == 7002, t('An update function that depends on more than one update from the same module only has the dependency on the higher-numbered update function recorded.')); + } +} + diff --git a/modules/simpletest/tests/update_test_1.install b/modules/simpletest/tests/update_test_1.install index 96e8787b3bd63a8bd55a3fb6ed3225b5925882af..ac69a358e0c12fce625fb5bcb99ae50a09135d2d 100644 --- a/modules/simpletest/tests/update_test_1.install +++ b/modules/simpletest/tests/update_test_1.install @@ -6,6 +6,35 @@ * Install, update and uninstall functions for the update_test_1 module. */ +/** + * Implements hook_update_dependencies(). + * + * @see update_test_2_update_dependencies() + */ +function update_test_1_update_dependencies() { + // These dependencies are used in combination with those declared in + // update_test_2_update_dependencies() for the sole purpose of testing that + // the results of hook_update_dependencies() are collected correctly and have + // the correct array structure. Therefore, we use updates from System module + // (which have already run), so that they will not get in the way of other + // tests. + $dependencies['system'][7000] = array( + // Compare to update_test_2_update_dependencies(), where the same System + // module update function is forced to depend on an update function from a + // different module. This allows us to test that both dependencies are + // correctly recorded. + 'update_test_1' => 7000, + ); + $dependencies['system'][7001] = array( + // Compare to update_test_2_update_dependencies(), where the same System + // module update function is forced to depend on a different update + // function within the same module. This allows us to test that only the + // dependency on the higher-numbered update function is recorded. + 'update_test_1' => 7002, + ); + return $dependencies; +} + /** * Dummy update_test_1 update 7000. */ diff --git a/modules/simpletest/tests/update_test_2.install b/modules/simpletest/tests/update_test_2.install index e2b4fb25fb38974812977d3a3fb22f52436b79fc..e12b77b930b6620bf54065c58ebe97c889f3d3b3 100644 --- a/modules/simpletest/tests/update_test_2.install +++ b/modules/simpletest/tests/update_test_2.install @@ -8,11 +8,30 @@ /** * Implements hook_update_dependencies(). + * + * @see update_test_1_update_dependencies() + * @see update_test_3_update_dependencies() */ function update_test_2_update_dependencies() { + // Combined with update_test_3_update_dependencies(), we are declaring here + // that these two modules run updates in the following order: + // 1. update_test_2_update_7000() + // 2. update_test_3_update_7000() + // 3. update_test_2_update_7001() + // 4. update_test_2_update_7002() $dependencies['update_test_2'][7001] = array( 'update_test_3' => 7000, ); + + // These are coordinated with the corresponding dependencies declared in + // update_test_1_update_dependencies(). + $dependencies['system'][7000] = array( + 'update_test_2' => 7001, + ); + $dependencies['system'][7001] = array( + 'update_test_1' => 7001, + ); + return $dependencies; } diff --git a/modules/simpletest/tests/update_test_3.install b/modules/simpletest/tests/update_test_3.install index 952ebf0b67a51dae2a5681e2c1a99eccae6bfe4e..1954123275c09d20882b1cba53e0b821daa4eb05 100644 --- a/modules/simpletest/tests/update_test_3.install +++ b/modules/simpletest/tests/update_test_3.install @@ -8,6 +8,8 @@ /** * Implements hook_update_dependencies(). + * + * @see update_test_2_update_dependencies() */ function update_test_3_update_dependencies() { $dependencies['update_test_3'][7000] = array(