From 9340153c56573862d6ee001eeef60735c13327ed Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 25 Nov 2021 15:59:03 +0100
Subject: [PATCH] Hooks: priority should be respected

The `$priority` parameter in the `register()` method is intended to allow for prioritizing the order in which callbacks are run, however, this priority was effectively not used, so callbacks would be run in the order different priorities were _registered_, not ordered _by_ the priorities.

This commit fixes this + adds a unit test to safeguard the fix.

I've elected to add the sorting to the `dispatch()` method, rather than the `register()` method for performance reasons.
Sorting arrays can be expensive and this way, only the subarrays of those hooks which are actually being run will be sorted, instead of sorting the subarray every time a new callback is registered, whether the hook on which the callback is registered will be called or not.

Fixes 452
---
 src/Hooks.php       |  2 ++
 tests/HooksTest.php | 67 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/src/Hooks.php b/src/Hooks.php
index eb8df6209..74fba0b3e 100644
--- a/src/Hooks.php
+++ b/src/Hooks.php
@@ -86,6 +86,8 @@ public function dispatch($hook, $parameters = []) {
 			$parameters = array_values($parameters);
 		}
 
+		ksort($this->hooks[$hook]);
+
 		foreach ($this->hooks[$hook] as $priority => $hooked) {
 			foreach ($hooked as $callback) {
 				$callback(...$parameters);
diff --git a/tests/HooksTest.php b/tests/HooksTest.php
index 26433fa23..f64b0f19e 100644
--- a/tests/HooksTest.php
+++ b/tests/HooksTest.php
@@ -241,6 +241,73 @@ public function testDispatchDoesntBreakWithKeyedParametersArray() {
 		$this->assertTrue($this->hooks->dispatch('hookname', ['paramA' => 10, 'paramB' => 'text']));
 	}
 
+	/**
+	 * Verify that hooks are executed based on their priority order.
+	 *
+	 * Issue https://github.com/WordPress/Requests/issues/452
+	 *
+	 * @covers ::dispatch
+	 *
+	 * @return void
+	 */
+	public function testDispatchRespectsHookPriority() {
+		// Register multiple callbacks for the same hook with a variation of priorities.
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "no prio 0\n";
+			}
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio 10-1\n";
+			},
+			10
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio -3\n";
+			},
+			-3
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio 5\n";
+			},
+			5
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio 2-1\n";
+			},
+			2
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio 2-2\n";
+			},
+			2
+		);
+		$this->hooks->register(
+			'hook_a',
+			function(&$text) {
+				$text .= "prio 10-2\n";
+			},
+			10
+		);
+
+		$text     = '';
+		$expected = "prio -3\nno prio 0\nprio 2-1\nprio 2-2\nprio 5\nprio 10-1\nprio 10-2\n";
+
+		$this->assertTrue($this->hooks->dispatch('hook_a', [&$text]));
+		$this->assertSame($expected, $text);
+	}
+
 	/**
 	 * Tests receiving an exception when an invalid input type is passed to `register()` as `$hook`.
 	 *