aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMario <mario@mariovavti.com>2023-07-20 09:05:42 +0000
committerMario <mario@mariovavti.com>2023-07-20 09:05:42 +0000
commitca994735bee5f479d840da13674b38c40930ba98 (patch)
treeb4ba1ef14653e4a9e8ccb4eee8ae63f42a9765f3
parent7200c716736d879501a665c9797ccf9e0131b24c (diff)
parent718c303086e6ab6061b2a920bd8293b2c0d11348 (diff)
downloadvolse-hubzilla-ca994735bee5f479d840da13674b38c40930ba98.tar.gz
volse-hubzilla-ca994735bee5f479d840da13674b38c40930ba98.tar.bz2
volse-hubzilla-ca994735bee5f479d840da13674b38c40930ba98.zip
Merge branch 'zotlabs/improve-access-list-type-safety' into 'dev'
Zotlabs: Improve type safety for AccessList class. See merge request hubzilla/core!2052
-rw-r--r--Zotlabs/Access/AccessList.php56
-rw-r--r--tests/unit/Access/AccessListTest.php36
2 files changed, 71 insertions, 21 deletions
diff --git a/Zotlabs/Access/AccessList.php b/Zotlabs/Access/AccessList.php
index 790ef4745..3f5271e87 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.
*
@@ -17,29 +18,48 @@ class AccessList {
* @brief Allow contacts
* @var string
*/
- private $allow_cid;
+ private ?string $allow_cid;
/**
* @brief Allow groups
* @var string
*/
- private $allow_gid;
+ private ?string $allow_gid;
/**
* @brief Deny contacts
* @var string
*/
- private $deny_cid;
+ private ?string $deny_cid;
/**
* @brief Deny groups
* @var string
*/
- private $deny_gid;
+ private ?string $deny_gid;
/**
* @brief Indicates if we are using the default constructor values or
* values that have been set explicitly.
* @var boolean
*/
- private $explicit;
+ private bool $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,13 +91,24 @@ class AccessList {
$this->explicit = false;
}
+ private function validate_input_array(array $arr, array $required_keys) : void {
+ $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.
*
* @return boolean
*/
- function get_explicit() {
+ function get_explicit() : bool {
return $this->explicit;
}
@@ -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) : void {
+ $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'];
@@ -112,7 +146,7 @@ class AccessList {
* * \e string \b deny_cid => string of denied cids
* * \e string \b deny_gid => string of denied gids
*/
- function get() {
+ function get() : array {
return [
'allow_cid' => $this->allow_cid,
'allow_gid' => $this->allow_gid,
@@ -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) : void {
$arr['contact_allow'] = $arr['contact_allow'] ?? [];
$arr['group_allow'] = $arr['group_allow'] ?? [];
$arr['contact_deny'] = $arr['contact_deny'] ?? [];
@@ -161,7 +195,7 @@ class AccessList {
*
* @return boolean Return true if any of allow_* deny_* values is set.
*/
- function is_private() {
+ function is_private() : bool {
return (($this->allow_cid || $this->allow_gid || $this->deny_cid || $this->deny_gid) ? true : false);
}
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