From f4ad82bcfb29e3015bf4903311f35e13bd8f73ed Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Fri, 1 Nov 2019 15:43:16 +0100 Subject: [PATCH] make ACLFormatter::expand() nullable and return an empty array - optimize tests --- src/Util/ACLFormatter.php | 6 +- tests/src/Util/ACLFormaterTest.php | 213 +++++++++++------------------ 2 files changed, 83 insertions(+), 136 deletions(-) diff --git a/src/Util/ACLFormatter.php b/src/Util/ACLFormatter.php index d194e2e67e..a7d851508d 100644 --- a/src/Util/ACLFormatter.php +++ b/src/Util/ACLFormatter.php @@ -14,13 +14,13 @@ final class ACLFormatter * * @param string|null $ids A angle-bracketed list of IDs * - * @return array|null The array based on the IDs (null in case there is no list) + * @return array The array based on the IDs (empty in case there is no list) */ public function expand(string $ids = null) { - // In case there is no ID list, return null (=> no ACL set) + // In case there is no ID list, return empty array (=> no ACL set) if (!isset($ids)) { - return null; + return []; } // turn string array of angle-bracketed elements into numeric array diff --git a/tests/src/Util/ACLFormaterTest.php b/tests/src/Util/ACLFormaterTest.php index 630a30e40b..ae875856ac 100644 --- a/tests/src/Util/ACLFormaterTest.php +++ b/tests/src/Util/ACLFormaterTest.php @@ -12,155 +12,96 @@ use PHPUnit\Framework\TestCase; */ 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>"; - $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() + public function assertAcl($text, array $assert = []) { $aclFormatter = new ACLFormatter(); - $text="assertEquals(array(), $aclFormatter->expand($text)); - } + $acl = $aclFormatter->expand($text); - /** - * test invalid input, just close > - * - * @todo should there be an exception? - */ - public function testExpandAclJustOneBracket2() - { - $aclFormatter = new ACLFormatter(); + $this->assertEquals($assert, $acl); - $text="Another invalid> string"; //should be invalid - $this->assertEquals(array(), $aclFormatter->expand($text)); + $this->assertMergable($acl); } - /** - * test invalid input, just close > - * - * @todo should there be an exception? - */ - public function testExpandAclCloseOnly() + public function assertMergable(array $aclOne, array $aclTwo = []) { - $aclFormatter = new ACLFormatter(); + $this->assertTrue(is_array($aclOne)); + $this->assertTrue(is_array($aclTwo)); - $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(); + $aclMerged = array_unique(array_merge($aclOne, $aclTwo)); + $this->assertTrue(is_array($aclMerged)); - $text="assertEquals(array(), $aclFormatter->expand($text)); + return $aclMerged; } - /** - * test invalid input, open and close do not match - * - * @todo should there be an exception? - */ - public function testExpandAclNoMatching1() + public function dataExpand() { - $aclFormatter = new ACLFormatter(); - - $text=" invalid "; //should be invalid - $this->assertEquals(array(), $aclFormatter->expand($text)); + return [ + 'normal' => [ + 'input' => '<1><2><3><' . Group::FOLLOWERS . '><' . Group::MUTUALS . '>', + 'assert' => ['1', '2', '3', Group::FOLLOWERS, Group::MUTUALS], + ], + 'nigNumber' => [ + 'input' => '<1><' . PHP_INT_MAX . '><15>', + 'assert' => ['1', (string)PHP_INT_MAX, '15'], + ], + 'string' => [ + 'input' => '<1><279012>', + 'assert' => ['1', '279012'], + ], + 'space' => [ + 'input' => '<1><279 012><32>', + 'assert' => ['1', '32'], + ], + 'empty' => [ + 'input' => '', + 'assert' => [], + ], + /// @todo should there be an exception? + 'noBrackets' => [ + 'input' => 'According to documentation, that\'s invalid. ', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'justOneBracket' => [ + 'input' => ' [], + ], + /// @todo should there be an exception? + 'justOneBracket2' => [ + 'input' => 'Another invalid> string', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'closeOnly' => [ + 'input' => 'Another> invalid> string>', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? + 'openOnly' => [ + 'input' => ' [], + ], + /// @todo should there be an exception? + 'noMatching1' => [ + 'input' => ' invalid ', //should be invalid + 'assert' => [], + ], + /// @todo should there be an exception? Or array(1, 3) + // (This should be array(1,3) - mike) + 'emptyMatch' => [ + 'input' => '<1><><3>', //should be invalid + 'assert' => ['1', '3'], + ], + ]; } /** - * test invalid input, empty <> - * - * @todo should there be an exception? Or array(1, 3) - * (This should be array(1,3) - mike) + * @dataProvider dataExpand */ - public function testExpandAclEmptyMatch() + public function testExpand($input, array $assert) { - $aclFormatter = new ACLFormatter(); - - $text="<1><><3>"; - $this->assertEquals(array('1', '3'), $aclFormatter->expand($text)); + $this->assertAcl($input, $assert); } /** @@ -170,8 +111,14 @@ class ACLFormaterTest extends TestCase { $aclFormatter = new ACLFormatter(); - $this->assertNull($aclFormatter->expand(null)); - $this->assertNull($aclFormatter->expand()); + $allow_people = $aclFormatter->expand(); + $allow_groups = $aclFormatter->expand(); + + $this->assertEmpty($aclFormatter->expand(null)); + $this->assertEmpty($aclFormatter->expand()); + + $recipients = array_unique(array_merge($allow_people, $allow_groups)); + $this->assertEmpty($recipients); } public function dataAclToString() -- 2.39.5