From dd854541aa0f6615545101a56b5f5b31aebfe953 Mon Sep 17 00:00:00 2001
From: Heinz Wiesinger <heinz.wiesinger@moveagency.com>
Date: Wed, 29 May 2024 16:40:05 +0200
Subject: [PATCH] Hooks: Centrally prevent duplicate hook registration

---
 src/Hooks.php       |  4 ++-
 tests/HooksTest.php | 64 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/Hooks.php b/src/Hooks.php
index 74fba0b3e..3748b40d2 100644
--- a/src/Hooks.php
+++ b/src/Hooks.php
@@ -55,7 +55,9 @@ public function register($hook, $callback, $priority = 0) {
 			$this->hooks[$hook][$priority] = [];
 		}
 
-		$this->hooks[$hook][$priority][] = $callback;
+		if (!in_array($callback, $this->hooks[$hook][$priority], true)) {
+			$this->hooks[$hook][$priority][] = $callback;
+		}
 	}
 
 	/**
diff --git a/tests/HooksTest.php b/tests/HooksTest.php
index f64b0f19e..398610d00 100644
--- a/tests/HooksTest.php
+++ b/tests/HooksTest.php
@@ -97,6 +97,70 @@ public function testRegister() {
 		);
 	}
 
+	/**
+	 * Technical test to verify the functionality of the Hooks::register() method.
+	 *
+	 * @covers ::register
+	 *
+	 * @return void
+	 */
+	public function testRegisterDuplicateHook() {
+		// Verify initial state or the hooks property.
+		$this->assertSame(
+			[],
+			$this->getPropertyValue($this->hooks, 'hooks'),
+			'Initial state of $hooks is not an empty array'
+		);
+
+		// Verify that the subkeys are created correctly when they don't exist yet.
+		$this->hooks->register('hookname', [$this, 'dummyCallback1']);
+		$this->assertSame(
+			[
+				'hookname' => [
+					0 => [
+						[$this, 'dummyCallback1'],
+					],
+				],
+			],
+			$this->getPropertyValue($this->hooks, 'hooks'),
+			'Initial hook registration failed'
+		);
+
+		// Verify that the subkeys are re-used when they already exist.
+		$this->hooks->register('hookname', [$this, 'dummyCallback1']);
+		$this->assertSame(
+			[
+				'hookname' => [
+					0 => [
+						[$this, 'dummyCallback1'],
+					],
+				],
+			],
+			$this->getPropertyValue($this->hooks, 'hooks'),
+			'Registering the same callback on the same hook with the same priority does not register twice'
+		);
+
+		/*
+		 * Verify that new subkeys are created when needed.
+		 * Also verifies that the input validation isn't too strict for the priority.
+		 */
+		$this->hooks->register('hookname', [$this, 'dummyCallback1'], '10');
+		$this->assertSame(
+			[
+				'hookname' => [
+					0  => [
+						[$this, 'dummyCallback1'],
+					],
+					10 => [
+						[$this, 'dummyCallback1'],
+					],
+				],
+			],
+			$this->getPropertyValue($this->hooks, 'hooks'),
+			'Registering the same callback on a different priority for an existing hook succeeds'
+		);
+	}
+
 	/**
 	 * Technical test to verify and safeguard Hooks::register() accepts closure callbacks.
 	 *