Refactor WordPress hook iteration

17817.patch

Index: src/wp-includes/plugin.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/wp-includes/plugin.php  (revision 25823)
+++ src/wp-includes/plugin.php	(revision )
@@ -20,7 +20,7 @@
  */
 
 // Initialize the filter globals.
-global $wp_filter, $wp_actions, $merged_filters, $wp_current_filter;
+global $wp_filter, $wp_actions, $wp_current_filter;
 
 if ( ! isset( $wp_filter ) )
 	$wp_filter = array();
@@ -28,9 +28,6 @@
 if ( ! isset( $wp_actions ) )
 	$wp_actions = array();
 
-if ( ! isset( $merged_filters ) )
-	$merged_filters = array();
-
 if ( ! isset( $wp_current_filter ) )
 	$wp_current_filter = array();
 
@@ -67,7 +64,6 @@
  * @subpackage Plugin
  *
  * @global array $wp_filter      A multidimensional array of all hooks and the callbacks hooked to them.
- * @global array $merged_filters Tracks the tags that need to be merged for later. If the hook is added, it doesn't need to run through that process.
  *
  * @since 0.71
  *
@@ -80,11 +76,10 @@
  * @return boolean true
  */
 function add_filter( $tag, $function_to_add, $priority = 10, $accepted_args = 1 ) {
-	global $wp_filter, $merged_filters;
+	global $wp_filter;
 
 	$idx = _wp_filter_build_unique_id($tag, $function_to_add, $priority);
 	$wp_filter[$tag][$priority][$idx] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
-	unset( $merged_filters[ $tag ] );
 	return true;
 }
 
@@ -150,7 +145,6 @@
  * @subpackage Plugin
  *
  * @global array $wp_filter         Stores all of the filters
- * @global array $merged_filters    Merges the filter hooks using this function.
  * @global array $wp_current_filter stores the list of current filters with the current one last
  *
  * @since 0.71
@@ -161,7 +155,7 @@
  * @return mixed The filtered value after all hooked functions are applied to it.
  */
 function apply_filters( $tag, $value ) {
-	global $wp_filter, $merged_filters, $wp_current_filter;
+	global $wp_filter, $wp_current_filter;
 
 	$args = array();
 
@@ -181,26 +175,17 @@
 	if ( !isset($wp_filter['all']) )
 		$wp_current_filter[] = $tag;
 
-	// Sort
-	if ( !isset( $merged_filters[ $tag ] ) ) {
-		ksort($wp_filter[$tag]);
-		$merged_filters[ $tag ] = true;
-	}
-
-	reset( $wp_filter[ $tag ] );
-
 	if ( empty($args) )
 		$args = func_get_args();
 
-	do {
-		foreach( (array) current($wp_filter[$tag]) as $the_ )
+	$iterator = new WP_Hook_Iterator($tag);
+	foreach ( $iterator as $the_ ) {
-			if ( !is_null($the_['function']) ){
+		if ( !is_null($the_['function']) ) {
-				$args[1] = $value;
-				$value = call_user_func_array($the_['function'], array_slice($args, 1, (int) $the_['accepted_args']));
-			}
+			$args[1] = $value;
+			$value = call_user_func_array($the_['function'], array_slice($args, 1, (int) $the_['accepted_args']));
+		}
+	}
 
-	} while ( next($wp_filter[$tag]) !== false );
-
 	array_pop( $wp_current_filter );
 
 	return $value;
@@ -216,7 +201,6 @@
  * @subpackage Plugin
  * @since 3.0.0
  * @global array $wp_filter Stores all of the filters
- * @global array $merged_filters Merges the filter hooks using this function.
  * @global array $wp_current_filter stores the list of current filters with the current one last
  *
  * @param string $tag The name of the filter hook.
@@ -224,7 +208,7 @@
  * @return mixed The filtered value after all hooked functions are applied to it.
  */
 function apply_filters_ref_array($tag, $args) {
-	global $wp_filter, $merged_filters, $wp_current_filter;
+	global $wp_filter, $wp_current_filter;
 
 	// Do 'all' actions first
 	if ( isset($wp_filter['all']) ) {
@@ -242,21 +226,13 @@
 	if ( !isset($wp_filter['all']) )
 		$wp_current_filter[] = $tag;
 
-	// Sort
-	if ( !isset( $merged_filters[ $tag ] ) ) {
-		ksort($wp_filter[$tag]);
-		$merged_filters[ $tag ] = true;
+	$iterator = new WP_Hook_Iterator($tag);
+	foreach ( $iterator as $the_ ) {
+		if ( !is_null($the_['function']) ) {
+			$args[0] = call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
-	}
+		}
+	}
 
-	reset( $wp_filter[ $tag ] );
-
-	do {
-		foreach( (array) current($wp_filter[$tag]) as $the_ )
-			if ( !is_null($the_['function']) )
-				$args[0] = call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
-
-	} while ( next($wp_filter[$tag]) !== false );
-
 	array_pop( $wp_current_filter );
 
 	return $args[0];
@@ -308,7 +284,7 @@
  * @return bool True when finished.
  */
 function remove_all_filters($tag, $priority = false) {
-	global $wp_filter, $merged_filters;
+	global $wp_filter;
 
 	if( isset($wp_filter[$tag]) ) {
 		if( false !== $priority && isset($wp_filter[$tag][$priority]) )
@@ -317,9 +293,6 @@
 			unset($wp_filter[$tag]);
 	}
 
-	if( isset($merged_filters[$tag]) )
-		unset($merged_filters[$tag]);
-
 	return true;
 }
 
@@ -384,7 +357,7 @@
  * @return null Will return null if $tag does not exist in $wp_filter array
  */
 function do_action($tag, $arg = '') {
-	global $wp_filter, $wp_actions, $merged_filters, $wp_current_filter;
+	global $wp_filter, $wp_actions, $wp_current_filter;
 
 	if ( ! isset($wp_actions[$tag]) )
 		$wp_actions[$tag] = 1;
@@ -415,21 +388,12 @@
 	for ( $a = 2; $a < func_num_args(); $a++ )
 		$args[] = func_get_arg($a);
 
-	// Sort
-	if ( !isset( $merged_filters[ $tag ] ) ) {
-		ksort($wp_filter[$tag]);
-		$merged_filters[ $tag ] = true;
-	}
-
-	reset( $wp_filter[ $tag ] );
-
-	do {
-		foreach ( (array) current($wp_filter[$tag]) as $the_ )
-			if ( !is_null($the_['function']) )
+	$iterator = new WP_Hook_Iterator($tag);
+	foreach ( $iterator as $the_ ) {
+		if ( !is_null($the_['function']) ) {
-				call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
+			call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
-
-	} while ( next($wp_filter[$tag]) !== false );
-
+		}
+	}
 	array_pop($wp_current_filter);
 }
 
@@ -470,7 +434,7 @@
  * @return null Will return null if $tag does not exist in $wp_filter array
  */
 function do_action_ref_array($tag, $args) {
-	global $wp_filter, $wp_actions, $merged_filters, $wp_current_filter;
+	global $wp_filter, $wp_actions, $wp_current_filter;
 
 	if ( ! isset($wp_actions[$tag]) )
 		$wp_actions[$tag] = 1;
@@ -493,21 +457,13 @@
 	if ( !isset($wp_filter['all']) )
 		$wp_current_filter[] = $tag;
 
-	// Sort
-	if ( !isset( $merged_filters[ $tag ] ) ) {
-		ksort($wp_filter[$tag]);
-		$merged_filters[ $tag ] = true;
+	$iterator = new WP_Hook_Iterator($tag);
+	foreach ( $iterator as $the_ ) {
+		if ( !is_null($the_['function']) ) {
+			call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
-	}
+		}
+	}
 
-	reset( $wp_filter[ $tag ] );
-
-	do {
-		foreach( (array) current($wp_filter[$tag]) as $the_ )
-			if ( !is_null($the_['function']) )
-				call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));
-
-	} while ( next($wp_filter[$tag]) !== false );
-
 	array_pop($wp_current_filter);
 }
 
@@ -730,15 +686,12 @@
  * @param array $args The collected parameters from the hook that was called.
  */
 function _wp_call_all_hook($args) {
-	global $wp_filter;
-
-	reset( $wp_filter['all'] );
-	do {
-		foreach( (array) current($wp_filter['all']) as $the_ )
-			if ( !is_null($the_['function']) )
+	$iterator = new WP_Hook_Iterator('all');
+	foreach ( $iterator as $the_ ) {
+		if ( !is_null($the_['function']) ) {
-				call_user_func_array($the_['function'], $args);
+			call_user_func_array($the_['function'], $args);
-
-	} while ( next($wp_filter['all']) !== false );
+		}
+	}
 }
 
 /**
Index: src/wp-settings.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/wp-settings.php	(revision 25823)
+++ src/wp-settings.php	(revision )
@@ -65,6 +65,7 @@
 require( ABSPATH . WPINC . '/functions.php' );
 require( ABSPATH . WPINC . '/class-wp.php' );
 require( ABSPATH . WPINC . '/class-wp-error.php' );
+require( ABSPATH . WPINC . '/class-wp-hook-iterator.php' );
 require( ABSPATH . WPINC . '/plugin.php' );
 require( ABSPATH . WPINC . '/pomo/mo.php' );
 
Index: src/wp-includes/class-wp-hook-iterator.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/wp-includes/class-wp-hook-iterator.php	(revision )
+++ src/wp-includes/class-wp-hook-iterator.php	(revision )
@@ -0,0 +1,124 @@
+<?php
+
+/**
+ * Class WP_Hook_Iterator
+ */
+class WP_Hook_Iterator implements Iterator {
+	private $hook = '';
+	private $current_callback = NULL;
+	private $current_priority = NULL;
+	private $callbacks_for_current_priority = array();
+
+	public function __construct( $hook ) {
+		$this->hook = $hook;
+		$this->rewind();
+	}
+
+	/**
+	 * Return the current element
+	 *
+	 * @link http://php.net/manual/en/iterator.current.php
+	 * @return mixed Can return any type.
+	 */
+	public function current() {
+		return $this->current_callback;
+	}
+
+	/**
+	 * Move forward to next element
+	 * @link http://php.net/manual/en/iterator.next.php
+	 * @return void Any returned value is ignored.
+	 */
+	public function next() {
+		$this->current_callback = NULL;
+		$next = next( $this->callbacks_for_current_priority );
+
+		if ( $next === FALSE ) {
+			do {
+				$this->increment_priority();
+			} while ( empty($this->callbacks_for_current_priority) && isset( $this->current_priority) );
+
+			$next = reset( $this->callbacks_for_current_priority );
+		}
+
+		if ( !empty($next) ) {
+			$this->current_callback = $next;
+		}
+	}
+
+	private function increment_priority() {
+		$this->current_priority = $this->get_next_priority();
+		if ( isset($this->current_priority) ) {
+			$this->callbacks_for_current_priority = $this->get_callbacks($this->current_priority);
+		} else {
+			$this->callbacks_for_current_priority = array();
+		}
+	}
+
+	private function get_next_priority() {
+		global $wp_filter;
+		if ( empty($wp_filter[$this->hook]) ) {
+			return NULL;
+		}
+
+		$priorities = array_keys($wp_filter[$this->hook]);
+
+		if ( !isset($this->current_priority) ) {
+			return min($priorities); // start at the beginning
+		}
+
+		$next = NULL;
+
+		// get the next greater priority
+		// this runs every time so that callbacks can be added at arbitrary times
+		foreach ( $priorities as $p ) {
+			if ( $p > $this->current_priority && ( !isset($next) || $p < $next ) ) {
+				$next = $p;
+			}
+		}
+
+		return $next;
+	}
+
+	private function get_callbacks( $priority ) {
+		global $wp_filter;
+		if ( isset($wp_filter[$this->hook][$priority]) && is_array($wp_filter[$this->hook][$priority]) ) {
+			return $wp_filter[$this->hook][$priority];
+		}
+		return array();
+	}
+
+	/**
+	 * Return the key of the current element
+	 * @link http://php.net/manual/en/iterator.key.php
+	 * @return mixed scalar on success, or null on failure.
+	 */
+	public function key() {
+		if ( empty($this->current_callback) ) {
+			return NULL;
+		}
+		return _wp_filter_build_unique_id($this->hook, $this->current_callback, $this->current_priority);
+	}
+
+	/**
+	 * Checks if current position is valid
+	 * @link http://php.net/manual/en/iterator.valid.php
+	 * @return boolean The return value will be casted to boolean and then evaluated.
+	 * Returns true on success or false on failure.
+	 */
+	public function valid() {
+		return !empty($this->current_callback);
+	}
+
+	/**
+	 * Rewind the Iterator to the first element
+	 * @link http://php.net/manual/en/iterator.rewind.php
+	 * @return void Any returned value is ignored.
+	 */
+	public function rewind() {
+		$this->current_priority = NULL;
+		$this->current_callback = NULL;
+		$this->callbacks_for_current_priority = array();
+		$this->next();
+	}
+}
Index: tests/phpunit/tests/actions.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- tests/phpunit/tests/actions.php	(revision 25823)
+++ tests/phpunit/tests/actions.php	(revision )
@@ -256,4 +256,62 @@
 	function action_self_removal() {
 		remove_action( 'test_action_self_removal', array( $this, 'action_self_removal' ) );
 	}
+
+	/**
+	 * @ticket 17817
+	 */
+	function test_action_recursion() {
+		$tag = rand_str();
+		$a = new MockAction();
+		$b = new MockAction();
+
+		add_action( $tag, array($a, 'action'), 11, 1 );
+		add_action( $tag, array($b, 'action'), 13, 1 );
+		add_action( $tag, array($this, 'action_that_causes_recursion'), 12, 1 );
+		do_action( $tag, $tag );
+
+		$this->assertEquals( 2, $a->get_call_count(), 'recursive actions should call all callbacks with earlier priority' );
+		$this->assertEquals( 2, $b->get_call_count(), 'recursive actions should call callbacks with later priority' );
+	}
+
+	function action_that_causes_recursion( $tag ) {
+		static $recursing = FALSE;
+		if ( !$recursing ) {
+			$recursing = TRUE;
+			do_action( $tag, $tag );
+		}
+		$recursing = FALSE;
+	}
+
+	/**
+	 * @ticket 9968
+	 */
+	function test_action_callback_manipulation_while_running() {
+		$tag = rand_str();
+		$a = new MockAction();
+		$b = new MockAction();
+		$c = new MockAction();
+		$d = new MockAction();
+		$e = new MockAction();
+
+		add_action( $tag, array($a, 'action'), 11, 2 );
+		add_action( $tag, array($this, 'action_that_manipulates_a_running_hook'), 12, 2 );
+		add_action( $tag, array($b, 'action'), 12, 2 );
+
+		do_action( $tag, $tag, array($a,$b,$c,$d,$e) );
+		do_action( $tag, $tag, array($a,$b,$c,$d,$e) );
+
+		$this->assertEquals( 2, $a->get_call_count(), 'callbacks should run unless otherwise instructed' );
+		$this->assertEquals( 1, $b->get_call_count(), 'callback removed by same priority callback should still get called' );
+		$this->assertEquals( 1, $c->get_call_count(), 'callback added by same priority callback should not get called' );
+		$this->assertEquals( 2, $d->get_call_count(), 'callback added by earlier priority callback should get called' );
+		$this->assertEquals( 1, $e->get_call_count(), 'callback added by later priority callback should not get called' );
+	}
+
+	function action_that_manipulates_a_running_hook( $tag, $mocks ) {
+		remove_action( $tag, array($mocks[1], 'action'), 12, 2 );
+		add_action( $tag, array($mocks[2], 'action' ), 12, 2 );
+		add_action( $tag, array($mocks[3], 'action' ), 13, 2 );
+		add_action( $tag, array($mocks[4], 'action' ), 10, 2 );
+	}
 }
Index: tests/phpunit/includes/functions.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- tests/phpunit/includes/functions.php	(revision 25823)
+++ tests/phpunit/includes/functions.php	(revision )
@@ -2,11 +2,10 @@
 
 // For adding hooks before loading WP
 function tests_add_filter($tag, $function_to_add, $priority = 10, $accepted_args = 1) {
-	global $wp_filter, $merged_filters;
+	global $wp_filter;
 
 	$idx = _test_filter_build_unique_id($tag, $function_to_add, $priority);
 	$wp_filter[$tag][$priority][$idx] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
-	unset( $merged_filters[ $tag ] );
 	return true;
 }