Move expand_acl to ACLFormatter::expand()

- including tests

include/text.php

@@ -3,28 +3,11 @@
  * @file include/text.php
  */
 
-use Friendica\App;
 use Friendica\Content\Text\BBCode;
-use Friendica\Core\Protocol;
 use Friendica\Model\FileTag;
 use Friendica\Model\Group;
 use Friendica\Util\Strings;
 
-/**
- * Turn user/group ACLs stored as angle bracketed text into arrays
- *
- * @param string $s
- * @return array
- */
-function expand_acl($s) {
-	// turn string array of angle-bracketed elements into numeric array
-	// e.g. "<1><2><3>" => array(1,2,3);
-	preg_match_all('/<(' . Group::FOLLOWERS . '|'. Group::MUTUALS . '|[0-9]+)>/', $s, $matches, PREG_PATTERN_ORDER);
-
-	return $matches[1];
-}
-
-
 /**
  * Wrap ACL elements in angle brackets for storage
  * @param string $item

mod/lockview.php

@@ -3,11 +3,13 @@
  * @file mod/lockview.php
  */
 use Friendica\App;
+use Friendica\BaseObject;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;
 use Friendica\Database\DBA;
 use Friendica\Model\Group;
 use Friendica\Model\Item;
+use Friendica\Util\ACLFormatter;
 
 function lockview_content(App $a)
 {
@@ -59,10 +61,13 @@ function lockview_content(App $a)
 		exit();
 	}
 
-	$allowed_users  = expand_acl($item['allow_cid']);
+	/** @var ACLFormatter $aclFormatter */
-	$allowed_groups = expand_acl($item['allow_gid']);
+	$aclFormatter = BaseObject::getClass(ACLFormatter::class);
-	$deny_users     = expand_acl($item['deny_cid']);
+
-	$deny_groups    = expand_acl($item['deny_gid']);
+	$allowed_users = $aclFormatter->expand($item['allow_cid']);
+	$allowed_groups = $aclFormatter->expand($item['allow_gid']);
+	$deny_users = $aclFormatter->expand($item['deny_cid']);
+	$deny_groups = $aclFormatter->expand($item['deny_gid']);
 
 	$o = L10n::t('Visible to:') . '<br />';
 	$l = [];

src/Model/Item.php

@@ -24,6 +24,7 @@ use Friendica\Database\DBA;
 use Friendica\Protocol\ActivityPub;
 use Friendica\Protocol\Diaspora;
 use Friendica\Protocol\OStatus;
+use Friendica\Util\ACLFormatter;
 use Friendica\Util\DateTimeFormat;
 use Friendica\Util\Map;
 use Friendica\Util\Network;
@@ -2892,10 +2893,13 @@ class Item extends BaseObject
 	 */
 	public static function enumeratePermissions(array $obj, bool $check_dead = false)
 	{
-		$allow_people = expand_acl($obj['allow_cid']);
+		/** @var ACLFormatter $aclFormater */
-		$allow_groups = Group::expand($obj['uid'], expand_acl($obj['allow_gid']), $check_dead);
+		$aclFormater = self::getClass(ACLFormatter::class);
-		$deny_people  = expand_acl($obj['deny_cid']);
+
-		$deny_groups  = Group::expand($obj['uid'], expand_acl($obj['deny_gid']), $check_dead);
+		$allow_people = $aclFormater->expand($obj['allow_cid']);
+		$allow_groups = Group::expand($obj['uid'], $aclFormater->expand($obj['allow_gid']), $check_dead);
+		$deny_people  = $aclFormater->expand($obj['deny_cid']);
+		$deny_groups  = Group::expand($obj['uid'], $aclFormater->expand($obj['deny_gid']), $check_dead);
 		$recipients   = array_unique(array_merge($allow_people, $allow_groups));
 		$deny         = array_unique(array_merge($deny_people, $deny_groups));
 		$recipients   = array_diff($recipients, $deny);

src/Module/Item/Compose.php

@@ -16,6 +16,7 @@ use Friendica\Model\Item;
 use Friendica\Model\User;
 use Friendica\Module\Login;
 use Friendica\Network\HTTPException\NotImplementedException;
+use Friendica\Util\ACLFormatter;
 use Friendica\Util\Crypto;
 
 class Compose extends BaseModule
@@ -58,6 +59,9 @@ class Compose extends BaseModule
 
 		$user = User::getById(local_user(), ['allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'hidewall', 'default-location']);
 
+		/** @var ACLFormatter $aclFormatter */
+		$aclFormatter = self::getClass(ACLFormatter::class);
+
 		switch ($posttype) {
 			case Item::PT_PERSONAL_NOTE:
 				$compose_title = L10n::t('Compose new personal note');
@@ -70,8 +74,8 @@ class Compose extends BaseModule
 				$compose_title = L10n::t('Compose new post');
 				$type = 'post';
 				$doesFederate = true;
-				$contact_allow = implode(',', expand_acl($user['allow_cid']));
+				$contact_allow = implode(',', $aclFormatter->expand($user['allow_cid']));
-				$group_allow = implode(',', expand_acl($user['allow_gid'])) ?: Group::FOLLOWERS;
+				$group_allow = implode(',', $aclFormatter->expand($user['allow_gid'])) ?: Group::FOLLOWERS;
 				break;
 		}
 
@@ -82,8 +86,8 @@ class Compose extends BaseModule
 		$wall          = $_REQUEST['wall']          ?? $type == 'post';
 		$contact_allow = $_REQUEST['contact_allow'] ?? $contact_allow;
 		$group_allow   = $_REQUEST['group_allow']   ?? $group_allow;
-		$contact_deny  = $_REQUEST['contact_deny']  ?? implode(',', expand_acl($user['deny_cid']));
+		$contact_deny  = $_REQUEST['contact_deny']  ?? implode(',', $aclFormatter->expand($user['deny_cid']));
-		$group_deny    = $_REQUEST['group_deny']    ?? implode(',', expand_acl($user['deny_gid']));
+		$group_deny    = $_REQUEST['group_deny']    ?? implode(',', $aclFormatter->expand($user['deny_gid']));
 		$visibility    = ($contact_allow . $user['allow_gid'] . $user['deny_cid'] . $user['deny_gid']) ? 'custom' : 'public';
 
 		$acl_contacts = Contact::selectToArray(['id', 'name', 'addr', 'micro'], ['uid' => local_user(), 'pending' => false, 'rel' => [Contact::FOLLOWER, Contact::FRIEND]]);

src/Util/ACLFormatter.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Friendica\Util;
+
+use Friendica\Model\Group;
+
+/**
+ * Util class for ACL formatting
+ */
+final class ACLFormatter
+{
+	/**
+	 * Turn user/group ACLs stored as angle bracketed text into arrays
+	 *
+	 * @param string $ids A angle-bracketed list of IDs
+	 *
+	 * @return array The array based on the IDs
+	 */
+	public function expand(string $ids)
+	{
+		// turn string array of angle-bracketed elements into numeric array
+		// e.g. "<1><2><3>" => array(1,2,3);
+		preg_match_all('/<(' . Group::FOLLOWERS . '|'. Group::MUTUALS . '|[0-9]+)>/', $ids, $matches, PREG_PATTERN_ORDER);
+
+		return $matches[1];
+	}
+}

src/Worker/Notifier.php

@@ -24,6 +24,7 @@ use Friendica\Protocol\ActivityPub;
 use Friendica\Protocol\Diaspora;
 use Friendica\Protocol\OStatus;
 use Friendica\Protocol\Salmon;
+use Friendica\Util\ACLFormatter;
 
 require_once 'include/items.php';
 
@@ -272,10 +273,13 @@ class Notifier
 					$public_message = false; // private recipients, not public
 				}
 
-				$allow_people = expand_acl($parent['allow_cid']);
+				/** @var ACLFormatter $aclFormatter */
-				$allow_groups = Group::expand($uid, expand_acl($parent['allow_gid']),true);
+				$aclFormatter = BaseObject::getClass(ACLFormatter::class);
-				$deny_people  = expand_acl($parent['deny_cid']);
+
-				$deny_groups  = Group::expand($uid, expand_acl($parent['deny_gid']));
+				$allow_people = $aclFormatter->expand($parent['allow_cid']);
+				$allow_groups = Group::expand($uid, $aclFormatter->expand($parent['allow_gid']),true);
+				$deny_people  = $aclFormatter->expand($parent['deny_cid']);
+				$deny_groups  = Group::expand($uid, $aclFormatter->expand($parent['deny_gid']));
 
 				// if our parent is a public forum (forum_mode == 1), uplink to the origional author causing
 				// a delivery fork. private groups (forum_mode == 2) do not uplink

tests/include/TextTest.php

@@ -13,133 +13,6 @@ use PHPUnit\Framework\TestCase;
  */
 class TextTest extends TestCase
 {
-	/**
-	 * test expand_acl, perfect input
-	 */
-	public function testExpandAclNormal()
-	{
-		$text='<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>';
-		$this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), expand_acl($text));
-	}
-
-	/**
-	 * test with a big number
-	 */
-	public function testExpandAclBigNumber()
-	{
-		$text='<1><' . PHP_INT_MAX . '><15>';
-		$this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), expand_acl($text));
-	}
-
-	/**
-	 * test with a string in it.
-	 *
-	 * @todo is this valid input? Otherwise: should there be an exception?
-	 */
-	public function testExpandAclString()
-	{
-		$text="<1><279012><tt>";
-		$this->assertEquals(array('1', '279012'), expand_acl($text));
-	}
-
-	/**
-	 * test with a ' ' in it.
-	 *
-	 * @todo is this valid input? Otherwise: should there be an exception?
-	 */
-	public function testExpandAclSpace()
-	{
-		$text="<1><279 012><32>";
-		$this->assertEquals(array('1', '32'), expand_acl($text));
-	}
-
-	/**
-	 * test empty input
-	 */
-	public function testExpandAclEmpty()
-	{
-		$text="";
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, no < at all
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclNoBrackets()
-	{
-		$text="According to documentation, that's invalid. "; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, just open <
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclJustOneBracket1()
-	{
-		$text="<Another invalid string"; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, just close >
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclJustOneBracket2()
-	{
-		$text="Another invalid> string"; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, just close >
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclCloseOnly()
-	{
-		$text="Another> invalid> string>"; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, just open <
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclOpenOnly()
-	{
-		$text="<Another< invalid string<"; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, open and close do not match
-	 *
-	 * @todo should there be an exception?
-	 */
-	public function testExpandAclNoMatching1()
-	{
-		$text="<Another<> invalid <string>"; //should be invalid
-		$this->assertEquals(array(), expand_acl($text));
-	}
-
-	/**
-	 * test invalid input, empty <>
-	 *
-	 * @todo should there be an exception? Or array(1, 3)
-	 * (This should be array(1,3) - mike)
-	 */
-	public function testExpandAclEmptyMatch()
-	{
-		$text="<1><><3>";
-		$this->assertEquals(array('1', '3'), expand_acl($text));
-	}
-
 	/**
 	 * test hex2bin and reverse
 	 */

tests/src/Util/ACLFormaterTest.php

@@ -0,0 +1,164 @@
+<?php
+
+namespace Friendica\Test\src\Util;
+
+use Friendica\Model\Group;
+use Friendica\Util\ACLFormatter;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @brief ACLFormater utility testing class
+ */
+class ACLFormaterTest extends TestCase
+{
+	/**
+	 * test expand_acl, perfect input
+	 */
+	public function testExpandAclNormal()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text='<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>';
+		$this->assertEquals(array('1', '2', '3', Group::FOLLOWERS, Group::MUTUALS), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test with a big number
+	 */
+	public function testExpandAclBigNumber()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text='<1><' . PHP_INT_MAX . '><15>';
+		$this->assertEquals(array('1', (string)PHP_INT_MAX, '15'), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test with a string in it.
+	 *
+	 * @todo is this valid input? Otherwise: should there be an exception?
+	 */
+	public function testExpandAclString()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<1><279012><tt>";
+		$this->assertEquals(array('1', '279012'), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test with a ' ' in it.
+	 *
+	 * @todo is this valid input? Otherwise: should there be an exception?
+	 */
+	public function testExpandAclSpace()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<1><279 012><32>";
+		$this->assertEquals(array('1', '32'), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test empty input
+	 */
+	public function testExpandAclEmpty()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="";
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, no < at all
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclNoBrackets()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="According to documentation, that's invalid. "; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, just open <
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclJustOneBracket1()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<Another invalid string"; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, just close >
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclJustOneBracket2()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="Another invalid> string"; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, just close >
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclCloseOnly()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="Another> invalid> string>"; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, just open <
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclOpenOnly()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<Another< invalid string<"; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, open and close do not match
+	 *
+	 * @todo should there be an exception?
+	 */
+	public function testExpandAclNoMatching1()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<Another<> invalid <string>"; //should be invalid
+		$this->assertEquals(array(), $aclFormatter->expand($text));
+	}
+
+	/**
+	 * test invalid input, empty <>
+	 *
+	 * @todo should there be an exception? Or array(1, 3)
+	 * (This should be array(1,3) - mike)
+	 */
+	public function testExpandAclEmptyMatch()
+	{
+		$aclFormatter = new ACLFormatter();
+
+		$text="<1><><3>";
+		$this->assertEquals(array('1', '3'), $aclFormatter->expand($text));
+	}
+}