From fbfa7a4150a40d6df9347692bb6681f45f804c7d Mon Sep 17 00:00:00 2001 From: Dries Buytaert Date: Tue, 13 Oct 2009 16:38:43 +0000 Subject: [PATCH] - Patch #593522 by sun: a better and faster drupal_alter(). --- includes/common.inc | 83 ++++++++++----------- includes/form.inc | 19 ++--- modules/field/field.api.php | 36 +++++---- modules/field/field.attach.inc | 15 +++- modules/simpletest/tests/common.test | 53 +++++++++++++ modules/simpletest/tests/common_test.module | 32 +++++++- modules/system/system.module | 6 +- modules/upload/upload.module | 3 +- 8 files changed, 166 insertions(+), 81 deletions(-) diff --git a/includes/common.inc b/includes/common.inc index 118953c538..8993047c66 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -4250,52 +4250,51 @@ function drupal_system_listing($mask, $directory, $key = 'name', $min_depth = 1) } /** - * Hands off structured Drupal arrays to type-specific *_alter implementations. + * Hands off alterable variables to type-specific *_alter implementations. * - * This dispatch function hands off structured Drupal arrays to type-specific - * *_alter implementations. It ensures a consistent interface for all altering - * operations. + * This dispatch function hands off the passed in variables to type-specific + * hook_TYPE_alter() implementations in modules. It ensures a consistent + * interface for all altering operations. + * + * A maximum of 2 alterable arguments is supported. In case more arguments need + * to be passed and alterable, modules provide additional variables assigned by + * reference in the last $context argument: + * @code + * $context = array( + * 'alterable' => &$alterable, + * 'unalterable' => $unalterable, + * 'foo' => 'bar', + * ); + * drupal_alter('mymodule_data', $alterable1, $alterable2, $context); + * @endcode + * + * Note that objects are always passed by reference in PHP5. If it is absolutely + * required that no implementation alters a passed object in $context, then an + * object needs to be cloned: + * @code + * $context = array( + * 'unalterable_object' => clone $object, + * ); + * drupal_alter('mymodule_data', $data, $context); + * @endcode * * @param $type - * The data type of the structured array. 'form', 'links', + * A string describing the data type of the alterable $data. 'form', 'links', * 'node_content', and so on are several examples. - * @param $data - * The structured array to be altered. - * @param ... - * Any additional params will be passed on to the called - * hook_$type_alter functions. - */ -function drupal_alter($type, &$data) { - // PHP's func_get_args() always returns copies of params, not references, so - // drupal_alter() can only manipulate data that comes in via the required first - // param. For the edge case functions that must pass in an arbitrary number of - // alterable parameters (hook_form_alter() being the best example), an array of - // those params can be placed in the __drupal_alter_by_ref key of the $data - // array. This is somewhat ugly, but is an unavoidable consequence of a flexible - // drupal_alter() function, and the limitations of func_get_args(). - // @todo: Remove this in Drupal 7. - if (is_array($data) && isset($data['__drupal_alter_by_ref'])) { - $by_ref_parameters = $data['__drupal_alter_by_ref']; - unset($data['__drupal_alter_by_ref']); - } - - // Hang onto a reference to the data array so that it isn't blown away later. - // Also, merge in any parameters that need to be passed by reference. - $args = array(&$data); - if (isset($by_ref_parameters)) { - $args = array_merge($args, $by_ref_parameters); - } - - // Now, use func_get_args() to pull in any additional parameters passed into - // the drupal_alter() call. - $additional_args = func_get_args(); - array_shift($additional_args); - array_shift($additional_args); - $args = array_merge($args, $additional_args); - - foreach (module_implements($type . '_alter') as $module) { - $function = $module . '_' . $type . '_alter'; - call_user_func_array($function, $args); + * @param &$data + * The primary data to be altered. + * @param &$context1 + * (optional) An additional variable that is passed by reference. + * @param &$context2 + * (optional) An additional variable that is passed by reference. If more + * context needs to be provided to implementations, then this should be an + * keyed array as described above. + */ +function drupal_alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { + $hook = $type . '_alter'; + foreach (module_implements($hook) as $module) { + $function = $module . '_' . $hook; + $function($data, $context1, $context2); } } diff --git a/includes/form.inc b/includes/form.inc index 282f3a1b1d..96a182f64c 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -670,20 +670,11 @@ function drupal_prepare_form($form_id, &$form, &$form_state) { } } - // Normally, we would call drupal_alter($form_id, $form, $form_state). - // However, drupal_alter() normally supports just one byref parameter. Using - // the __drupal_alter_by_ref key, we can store any additional parameters - // that need to be altered, and they'll be split out into additional params - // for the hook_form_alter() implementations. - // @todo: Remove this in Drupal 7. - $data = &$form; - $data['__drupal_alter_by_ref'] = array(&$form_state); - drupal_alter('form_' . $form_id, $data); - - // __drupal_alter_by_ref is unset in the drupal_alter() function, we need - // to repopulate it to ensure both calls get the data. - $data['__drupal_alter_by_ref'] = array(&$form_state); - drupal_alter('form', $data, $form_id); + // Invoke hook_form_FORM_ID_alter() implementations. + drupal_alter('form_' . $form_id, $form, $form_state); + + // Invoke hook_form_alter() implementations. + drupal_alter('form', $form, $form_state, $form_id); } diff --git a/modules/field/field.api.php b/modules/field/field.api.php index 52673a39cb..60e54d5d80 100644 --- a/modules/field/field.api.php +++ b/modules/field/field.api.php @@ -932,15 +932,15 @@ function hook_field_attach_presave($obj_type, $object) { * This hook is invoked while preprocessing the field.tpl.php template file. * * @param $variables - * The variables array is passed by reference and will be populated with field values. - * @param $obj_type - * The type of $object; e.g. 'node' or 'user'. - * @param $object - * The object with fields to render. - * @param $element - * The structured array containing the values ready for rendering. + * The variables array is passed by reference and will be populated with field + * values. + * @param $context + * An associative array containing: + * - obj_type: The type of $object; e.g. 'node' or 'user'. + * - object: The object with fields to render. + * - element: The structured array containing the values ready for rendering. */ -function hook_field_attach_preprocess_alter(&$variables, $obj_type, $object, $element) { +function hook_field_attach_preprocess_alter(&$variables, $context) { } /** @@ -1042,18 +1042,16 @@ function hook_field_attach_delete_revision($obj_type, $object) { * * This hook is invoked after the field module has performed the operation. * - * @param $output - * The structured content array tree for all of $object's fields. - * @param $obj_type - * The type of $object; e.g. 'node' or 'user'. - * @param $object - * The object with fields to render. - * @param $build_mode - * Build mode, e.g. 'full', 'teaser'... - * @param $langcode - * The language in which the field values will be displayed. + * @param &$output + * The structured content array tree for all of $object's fields. + * @param $context + * An associative array containing: + * - obj_type: The type of $object; e.g. 'node' or 'user'. + * - object: The object with fields to render. + * - build_mode: Build mode, e.g. 'full', 'teaser'... + * - langcode: The language in which the field values will be displayed. */ -function hook_field_attach_view_alter($output, $obj_type, $object, $build_mode, $langcode) { +function hook_field_attach_view_alter(&$output, $context) { } /** diff --git a/modules/field/field.attach.inc b/modules/field/field.attach.inc index 01abd6f78b..4118a60e15 100644 --- a/modules/field/field.attach.inc +++ b/modules/field/field.attach.inc @@ -1193,7 +1193,13 @@ function field_attach_view($obj_type, $object, $build_mode = 'full', $langcode = $output['#extra_fields'] = field_extra_fields($bundle); // Let other modules make changes after rendering the view. - drupal_alter('field_attach_view', $output, $obj_type, $object, $build_mode, $langcode); + $context = array( + 'obj_type' => $obj_type, + 'object' => $object, + 'build_mode' => $build_mode, + 'langcode' => $langcode, + ); + drupal_alter('field_attach_view', $output, $context); return $output; } @@ -1228,7 +1234,12 @@ function field_attach_preprocess($obj_type, $object, $element, &$variables) { } // Let other modules make changes to the $variables array. - drupal_alter('field_attach_preprocess', $variables, $obj_type, $object, $element); + $context = array( + 'obj_type' => $obj_type, + 'object' => $object, + 'element' => $element, + ); + drupal_alter('field_attach_preprocess', $variables, $context); } /** diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test index 53d4932106..55b4b43326 100644 --- a/modules/simpletest/tests/common.test +++ b/modules/simpletest/tests/common.test @@ -1,6 +1,59 @@ 'drupal_alter() tests', + 'description' => 'Confirm that alteration of arguments passed to drupal_alter() works correctly.', + 'group' => 'System', + ); + } + + function setUp() { + parent::setUp('common_test'); + } + + function testDrupalAlter() { + $array = array('foo' => 'bar'); + $object = new stdClass; + $object->foo = 'bar'; + + // Verify alteration of a single argument. + $array_copy = $array; + $array_expected = array('foo' => 'Drupal'); + drupal_alter('drupal_alter', $array_copy); + $this->assertEqual($array_copy, $array_expected, t('Single array was altered.')); + + $object_copy = clone $object; + $object_expected = clone $object; + $object_expected->foo = 'Drupal'; + drupal_alter('drupal_alter', $object_copy); + $this->assertEqual($object_copy, $object_expected, t('Single object was altered.')); + + // Verify alteration of multiple arguments. + $array_copy = $array; + $array_expected = array('foo' => 'Drupal'); + $object_copy = clone $object; + $object_expected = clone $object; + $object_expected->foo = 'Drupal'; + $array2_copy = $array; + $array2_expected = array('foo' => 'Drupal'); + drupal_alter('drupal_alter', $array_copy, $object_copy, $array2_copy); + $this->assertEqual($array_copy, $array_expected, t('First argument to drupal_alter() was altered.')); + $this->assertEqual($object_copy, $object_expected, t('Second argument to drupal_alter() was altered.')); + $this->assertEqual($array2_copy, $array2_expected, t('Third argument to drupal_alter() was altered.')); + } +} + /** * Tests for URL generation functions. */ diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module index f7abf51b69..74866f098f 100644 --- a/modules/simpletest/tests/common_test.module +++ b/modules/simpletest/tests/common_test.module @@ -10,7 +10,6 @@ * Implement hook_menu(). */ function common_test_menu() { - $items = array(); $items['common-test/drupal_goto'] = array( 'title' => 'Drupal Goto', 'page callback' => 'common_test_drupal_goto_land', @@ -70,6 +69,37 @@ function common_test_drupal_goto_alter(&$args) { } } +/** + * Implement hook_TYPE_alter(). + */ +function common_test_drupal_alter_alter(&$data, &$arg2 = NULL, &$arg3 = NULL) { + // Alter first argument. + if (is_array($data)) { + $data['foo'] = 'Drupal'; + } + elseif (is_object($data)) { + $data->foo = 'Drupal'; + } + // Alter second argument, if present. + if (isset($arg2)) { + if (is_array($arg2)) { + $arg2['foo'] = 'Drupal'; + } + elseif (is_object($arg2)) { + $arg2->foo = 'Drupal'; + } + } + // Try to alter third argument, if present. + if (isset($arg3)) { + if (is_array($arg3)) { + $arg3['foo'] = 'Drupal'; + } + elseif (is_object($arg3)) { + $arg3->foo = 'Drupal'; + } + } +} + /** * Implement hook_theme(). */ diff --git a/modules/system/system.module b/modules/system/system.module index 483ea86437..0e932863d0 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -1916,7 +1916,8 @@ function _system_rebuild_module_data() { // Invoke hook_system_info_alter() to give installed modules a chance to // modify the data in the .info files if necessary. - drupal_alter('system_info', $modules[$key]->info, $modules[$key], 'module'); + $type = 'module'; + drupal_alter('system_info', $modules[$key]->info, $modules[$key], $type); } // The install profile is required. @@ -2009,7 +2010,8 @@ function _system_rebuild_theme_data() { // Invoke hook_system_info_alter() to give installed modules a chance to // modify the data in the .info files if necessary. - drupal_alter('system_info', $themes[$key]->info, $themes[$key], 'theme'); + $type = 'theme'; + drupal_alter('system_info', $themes[$key]->info, $themes[$key], $type); if (!empty($themes[$key]->info['base theme'])) { $sub_themes[] = $key; diff --git a/modules/upload/upload.module b/modules/upload/upload.module index 21f239e24a..51328d97ce 100644 --- a/modules/upload/upload.module +++ b/modules/upload/upload.module @@ -692,8 +692,9 @@ function upload_js() { '#tree' => FALSE, '#parents' => array(), ); - drupal_alter('form', $form, array(), 'upload_js'); $form_state = array('submitted' => FALSE, 'programmed' => FALSE); + $form_id = 'upload_js'; + drupal_alter('form', $form, $form_state, $form_id); $form = form_builder('upload_js', $form, $form_state); $output = theme('status_messages') . drupal_render($form); -- GitLab