Zotlabs: Improve type safety for AccessList class.

Add type annotations for constructor and set* methods, and throw an
exception if the passed in arrays are missing required keys. This means
that both invalid input types and missing keys will throw and exception
rather than just die with a runtime error.

There's not checks to verify that the contents of the required array
keys are valid or make sense, though. They are just assigned, and
returned as is by the get method when requested.

Also, the set_from_array method is not well tested at the moment.

2 files changed, 63 insertions, 13 deletions

diff --git a/Zotlabs/Access/AccessList.php b/Zotlabs/Access/AccessList.php
index 790ef4745..a7da1274f 100644
--- a/Zotlabs/Access/AccessList.php
+++ b/Zotlabs/Access/AccessList.php
@@ -2,6 +2,7 @@
 
 namespace Zotlabs\Access;
 
+
 /**
  * @brief AccessList class which represents individual content ACLs.
  *
@@ -40,6 +41,25 @@ class AccessList {
 	 */
 	private $explicit;
 
+	/**
+	 * @brief Keys required by the constructor if the channel array is given.
+	 */
+	private const REQUIRED_KEYS_CONSTRUCTOR = [
+		'channel_allow_cid',
+		'channel_allow_gid',
+		'channel_deny_cid',
+		'channel_deny_gid'
+	];
+
+	/**
+	 * @brief Keys required by the set method.
+	 */
+	private const REQUIRED_KEYS_SET = [
+		'allow_cid',
+		'allow_gid',
+		'deny_cid',
+		'deny_gid'
+	];
 
 	/**
 	 * @brief Constructor for AccessList class.
@@ -53,8 +73,9 @@ class AccessList {
 	 *   * \e string \b channel_deny_cid => string of denied cids
 	 *   * \e string \b channel_deny_gid => string of denied gids
 	 */
-	function __construct($channel) {
+	function __construct(array $channel) {
 		if ($channel) {
+			$this->validate_input_array($channel, self::REQUIRED_KEYS_CONSTRUCTOR);
 			$this->allow_cid = $channel['channel_allow_cid'];
 			$this->allow_gid = $channel['channel_allow_gid'];
 			$this->deny_cid  = $channel['channel_deny_cid'];
@@ -70,6 +91,17 @@ class AccessList {
 		$this->explicit = false;
 	}
 
+	private function validate_input_array(array $arr, array $required_keys) {
+		$missing_keys = array_diff($required_keys, array_keys($arr));
+
+		if (!empty($missing_keys)) {
+			throw new \Exception(
+				'Invalid AccessList object: Expected array with keys: '
+				. implode(', ', $missing_keys)
+			);
+		}
+	}
+
 	/**
 	 * @brief Get if we are using the default constructor values
 	 * or values that have been set explicitly.
@@ -94,7 +126,9 @@ class AccessList {
 	 *   * \e string \b deny_gid  => string of denied gids
 	 * @param boolean $explicit (optional) default true
 	 */
-	function set($arr, $explicit = true) {
+	function set(array $arr, bool $explicit = true) {
+		$this->validate_input_array($arr, self::REQUIRED_KEYS_SET);
+
 		$this->allow_cid = $arr['allow_cid'];
 		$this->allow_gid = $arr['allow_gid'];
 		$this->deny_cid  = $arr['deny_cid'];
@@ -138,7 +172,7 @@ class AccessList {
 	 *   * \e array|string \b group_deny    => array with gids or comma-seperated string
 	 * @param boolean $explicit (optional) default true
 	 */
-	function set_from_array($arr, $explicit = true) {
+	function set_from_array(array $arr, bool $explicit = true) {
 		$arr['contact_allow'] = $arr['contact_allow'] ?? [];
 		$arr['group_allow'] = $arr['group_allow'] ?? [];
 		$arr['contact_deny'] = $arr['contact_deny'] ?? [];
diff --git a/tests/unit/Access/AccessListTest.php b/tests/unit/Access/AccessListTest.php
index 2f185db17..635f09930 100644
--- a/tests/unit/Access/AccessListTest.php
+++ b/tests/unit/Access/AccessListTest.php
@@ -60,14 +60,24 @@ class AccessListTest extends UnitTestCase {
 	}
 
 	/**
-	 * @expectedException PHPUnit\Framework\Error\Error
+	 * AccessList constructor should throw an exception if input is not
+	 * an array.
 	 */
-/*
-	public function testPHPErrorOnInvalidConstructor() {
+	public function testConstructorThrowsOnInvalidInputType() {
+		$this->expectException("TypeError");
 		$accessList = new AccessList('invalid');
-		// Causes: "Illegal string offset 'channel_allow_cid'"
 	}
-*/
+
+	/**
+	 * AccessList constructor should throw an exception on
+	 * invalid input.
+	 */
+	public function testConstructorThrowsOnMissingKeysInArray() {
+		$this->expectException("Exception");
+		$this->expectExceptionMessage("Invalid AccessList object");
+		$accessList = new AccessList(['something_else' => 'should_this_fail?']);
+	}
+
 	/**
 	 * Test that the defaults are as expected when constructed with
 	 * an empty array.
@@ -111,16 +121,22 @@ class AccessListTest extends UnitTestCase {
 	}
 
 	/**
-	 * @expectedException PHPUnit\Framework\Error\Error
+	 * The set method should throw an exception if input is not an array.
 	 */
-/*
-	public function testPHPErrorOnInvalidSet() {
+	public function testSetThrowsOnInvalidInputType() {
+		$this->expectException('TypeError');
 		$accessList = new AccessList([]);
 
 		$accessList->set('invalid');
-		// Causes: "Illegal string offset 'allow_cid'"
 	}
-*/
+
+	public function testSetThrowsOnMissingKeysInArray() {
+		$this->expectException('Exception');
+		$this->expectExceptionMessage('Invalid AccessList object');
+
+		$accessList = new AccessList([]);
+		$accessList->set(['something_else' => 'should_this_fail?']);
+	}
 
 	/**
 	 * The set_from_array() function calls some other functions, too which are