- Patch #593522 by sun: a better and faster drupal_alter().

8 files changed, 166 insertions, 81 deletions

diff --git a/includes/common.inc b/includes/common.inc
index 118953c53..8993047c6 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 282f3a1b1..96a182f64 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 52673a39c..60e54d5d8 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 01abd6f78..4118a60e1 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 53d493210..55b4b4332 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -2,6 +2,59 @@
 // $Id$
 
 /**
+ * @file
+ * Tests for common.inc functionality.
+ */
+
+/**
+ * Tests for URL generation functions.
+ */
+class DrupalAlterTestCase extends DrupalWebTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => '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.
  */
 class CommonURLUnitTest extends DrupalUnitTestCase {
diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module
index f7abf51b6..74866f098 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',
@@ -71,6 +70,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().
  */
 function common_test_theme() {
diff --git a/modules/system/system.module b/modules/system/system.module
index 483ea8643..0e932863d 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 21f239e24..51328d97c 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);