From b4ce77320e997c22d53593eaa341645d4d41d398 Mon Sep 17 00:00:00 2001 From: Hugo Sales Date: Tue, 16 Nov 2021 14:10:19 +0000 Subject: [PATCH] [TESTS] Fix remaining tests, back to 100% passed. Some minor semantic changes --- src/Core/GNUsocial.php | 2 +- src/DataFixtures/CoreFixtures.php | 1 + src/Util/Common.php | 14 +++++---- src/Util/Nickname.php | 47 ++++++++++++++++--------------- tests/Core/DB/DBTest.php | 2 +- tests/Core/RouterTest.php | 2 +- tests/Entity/ActorTest.php | 2 +- tests/Entity/LocalUserTest.php | 3 +- tests/Util/NicknameTest.php | 23 ++++----------- tests/Util/TemporaryFileTest.php | 2 +- 10 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/Core/GNUsocial.php b/src/Core/GNUsocial.php index cf7608604a..e73b084af8 100644 --- a/src/Core/GNUsocial.php +++ b/src/Core/GNUsocial.php @@ -194,7 +194,7 @@ class GNUsocial implements EventSubscriberInterface $this->request = $event->getRequest(); // Save the target path, so we can redirect back after logging in - if (!(!$event->isMasterRequest() || $this->request->isXmlHttpRequest() || Common::isRoute(['login', 'register', 'logout']))) { + if (!(!$event->isMainRequest() || $this->request->isXmlHttpRequest() || Common::isRoute(['login', 'register', 'logout']))) { $this->saveTargetPath($this->session, 'main', $this->request->getBaseUrl()); } diff --git a/src/DataFixtures/CoreFixtures.php b/src/DataFixtures/CoreFixtures.php index 0ba09192a4..02b1bab0a7 100644 --- a/src/DataFixtures/CoreFixtures.php +++ b/src/DataFixtures/CoreFixtures.php @@ -24,6 +24,7 @@ class CoreFixtures extends Fixture foreach ([ 'taken_user' => [LocalUser::class, 'setId', ['password' => LocalUser::hashPassword('foobar'), 'outgoing_email' => 'email@provider']], 'some_user' => [LocalUser::class, 'setId', []], + 'local_user_test_user' => [LocalUser::class, 'setId', ['password' => LocalUser::hashPassword('foobar')]], 'form_personal_info_test_user' => [LocalUser::class, 'setId', []], 'form_account_test_user' => [LocalUser::class, 'setId', ['password' => LocalUser::hashPassword('some password')]], 'taken_group' => [LocalGroup::class, 'setGroupId', []], diff --git a/src/Util/Common.php b/src/Util/Common.php index 1f1bd41afd..bb0976be94 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -98,14 +98,18 @@ abstract class Common /** * Set sysadmin's configuration preferences for GNU social + * + * @param $transient keep this setting in memory only */ - public static function setConfig(string $section, string $setting, $value): void + public static function setConfig(string $section, string $setting, $value, bool $transient = false): void { self::$config[$section][$setting] = $value; - $diff = self::arrayDiffRecursive(self::$config, self::$defaults); - $yaml = (new Yaml\Dumper(indentation: 2))->dump(['parameters' => ['locals' => ['gnusocial' => $diff]]], Yaml\Yaml::DUMP_OBJECT_AS_MAP); - rename(INSTALLDIR . '/social.local.yaml', INSTALLDIR . '/social.local.yaml.back'); - file_put_contents(INSTALLDIR . '/social.local.yaml', $yaml); + if (!$transient) { + $diff = self::arrayDiffRecursive(self::$config, self::$defaults); + $yaml = (new Yaml\Dumper(indentation: 2))->dump(['parameters' => ['locals' => ['gnusocial' => $diff]]], Yaml\Yaml::DUMP_OBJECT_AS_MAP); + rename(INSTALLDIR . '/social.local.yaml', INSTALLDIR . '/social.local.yaml.back'); + file_put_contents(INSTALLDIR . '/social.local.yaml', $yaml); + } } public static function getConfigDefaults() diff --git a/src/Util/Nickname.php b/src/Util/Nickname.php index 70748b67dd..2501017357 100644 --- a/src/Util/Nickname.php +++ b/src/Util/Nickname.php @@ -23,14 +23,15 @@ declare(strict_types = 1); namespace App\Util; -use App\Entity\LocalUser; +use App\Core\DB\DB; +use App\Util\Exception\DuplicateFoundException; use App\Util\Exception\NicknameEmptyException; use App\Util\Exception\NicknameException; use App\Util\Exception\NicknameInvalidException; use App\Util\Exception\NicknameNotAllowedException; use App\Util\Exception\NicknameTakenException; use App\Util\Exception\NicknameTooLongException; -use App\Util\Exception\NicknameTooShortException; +use App\Util\Exception\NotFoundException; use App\Util\Exception\NotImplementedException; use Functional as F; use InvalidArgumentException; @@ -58,7 +59,7 @@ class Nickname /** * Maximum number of characters in a canonical-form nickname. Changes must validate regexs */ - const MAX_LEN = 64; + public const MAX_LEN = 64; /** * Regex fragment for pulling a formated nickname *OR* ID number. @@ -69,8 +70,6 @@ class Nickname * if you just need to check if it's properly formatted. * * This, DISPLAY_FMT, and CANONICAL_FMT should not be enclosed in []s. - * - * @fixme would prefer to define in reference to the other constants */ public const INPUT_FMT = '(?:[0-9]+|[0-9a-zA-Z_]{1,' . self::MAX_LEN . '})'; @@ -110,7 +109,8 @@ class Nickname * * This, INPUT_FMT and DISPLAY_FMT should not be enclosed in []s. */ - const CANONICAL_FMT = '[0-9a-z]{1,' . self::MAX_LEN . '}'; + // const CANONICAL_FMT = '[0-9A-Za-z_]{1,' . self::MAX_LEN . '}'; + public const CANONICAL_FMT = self::DISPLAY_FMT; /** * Regex with non-capturing group that matches whitespace and some @@ -128,11 +128,7 @@ class Nickname /** * Check if a nickname is valid or throw exceptions if it's not. * Can optionally check if the nickname is currently in use - * @param string $nickname - * @param bool $check_already_used - * @param int $which - * @param bool $check_is_allowed - * @return bool + * * @throws NicknameEmptyException * @throws NicknameNotAllowedException * @throws NicknameTakenException @@ -152,9 +148,13 @@ class Nickname } elseif ($check_already_used) { switch ($which) { case self::CHECK_LOCAL_USER: - $lu = LocalUser::getWithPK(['nickname' => $nickname]); - if ($lu !== null) { + try { + $lu = DB::findOneBy('local_user', ['nickname' => $nickname]); throw new NicknameTakenException($lu->getActor()); + } catch (NotFoundException) { + // continue + } catch (DuplicateFoundException) { + Log::critial("Duplicate entry in `local_user` for nickname={$nickname}"); } break; // @codeCoverageIgnoreStart @@ -176,7 +176,7 @@ class Nickname * The canonical form will be returned, or an exception thrown if invalid. * * @throws NicknameEmptyException - * @throws NicknameException (base class) + * @throws NicknameException (base class) * @throws NicknameInvalidException * @throws NicknameNotAllowedException * @throws NicknameTakenException @@ -185,7 +185,6 @@ class Nickname public static function normalize(string $nickname, bool $check_already_used = false, int $which = self::CHECK_LOCAL_USER, bool $check_is_allowed = true): string { $nickname = trim($nickname); - $nickname = str_replace('_', '', $nickname); $nickname = mb_strtolower($nickname); // We could do UTF-8 normalization (å to a, etc.) with something like Normalizer::normalize($nickname, Normalizer::FORM_C) // We won't as it could confuse tremendously the user, he must know what is valid and should fix his own input @@ -218,8 +217,6 @@ class Nickname /** * Is the given string a valid canonical nickname form? - * @param string $nickname - * @return bool */ public static function isCanonical(string $nickname): bool { @@ -228,8 +225,7 @@ class Nickname /** * Is the given string in our nickname blacklist? - * @param string $nickname - * @return bool + * * @throws NicknameEmptyException * @throws NicknameInvalidException * @throws NicknameNotAllowedException @@ -242,8 +238,15 @@ class Nickname if (empty($reserved)) { return false; } - return in_array($nickname, array_merge($reserved, F\map($reserved, function ($n) { - return self::normalize($n, check_already_used: false, check_is_allowed: true); - }))); + return \in_array( + $nickname, + array_merge( + $reserved, + F\map( + $reserved, + fn ($n) => self::normalize($n, check_already_used: false, check_is_allowed: false), + ), + ), + ); } } diff --git a/tests/Core/DB/DBTest.php b/tests/Core/DB/DBTest.php index 92ee2d03f7..f439268aa5 100644 --- a/tests/Core/DB/DBTest.php +++ b/tests/Core/DB/DBTest.php @@ -80,7 +80,7 @@ class DBTest extends GNUsocialTestCase public function testPersistWithSameId() { - $actor = Actor::create(['nickname' => 'test', 'normalized_nickname' => 'test']); + $actor = Actor::create(['nickname' => 'test', 'bio' => 'some very interesting bio']); $user = LocalUser::create(['nickname' => 'test']); $id = DB::persistWithSameId($actor, $user, fn ($id) => $id); static::assertTrue($id != 0); diff --git a/tests/Core/RouterTest.php b/tests/Core/RouterTest.php index 511533b5e4..89cc4f93b7 100644 --- a/tests/Core/RouterTest.php +++ b/tests/Core/RouterTest.php @@ -51,6 +51,6 @@ class RouterTest extends GNUsocialTestCase public function testURLGen() { parent::bootKernel(); - static::assertSame('/thumbnail/1', Router::url('thumbnail', ['id' => 1])); + static::assertSame('/attachment/1/thumbnail/big', Router::url('attachment_thumbnail', ['id' => 1, 'size' => 'big'])); } } diff --git a/tests/Entity/ActorTest.php b/tests/Entity/ActorTest.php index 3670a0f955..1780951ccf 100644 --- a/tests/Entity/ActorTest.php +++ b/tests/Entity/ActorTest.php @@ -34,7 +34,7 @@ class ActorTest extends GNUsocialTestCase public function testGetAvatarUrl() { $actor = DB::findOneBy('actor', ['nickname' => 'taken_user']); - static::assertSame("/{$actor->getId()}/avatar", $actor->getAvatarUrl()); + static::assertSame('/avatar/default', $actor->getAvatarUrl()); } public function testSelfTags() diff --git a/tests/Entity/LocalUserTest.php b/tests/Entity/LocalUserTest.php index 560f3b90d3..59afbfb7d6 100644 --- a/tests/Entity/LocalUserTest.php +++ b/tests/Entity/LocalUserTest.php @@ -50,7 +50,8 @@ class LocalUserTest extends GNUsocialTestCase public function testChangePassword() { parent::bootKernel(); - $user = LocalUser::getByNickname('form_personal_info_test_user'); + $user = LocalUser::getByNickname('local_user_test_user'); + static::assertFalse($user->changePassword(old_password_plain_text: '', new_password_plain_text: 'password', override: false)); static::assertTrue($user->changePassword(old_password_plain_text: '', new_password_plain_text: 'password', override: true)); static::assertTrue($user->changePassword(old_password_plain_text: 'password', new_password_plain_text: 'new_password', override: false)); static::assertFalse($user->changePassword(old_password_plain_text: 'wrong_password', new_password_plain_text: 'new_password', override: false)); diff --git a/tests/Util/NicknameTest.php b/tests/Util/NicknameTest.php index a20348722e..44bfdf9d2d 100644 --- a/tests/Util/NicknameTest.php +++ b/tests/Util/NicknameTest.php @@ -30,7 +30,6 @@ use App\Util\Exception\NicknameTooLongException; use App\Util\GNUsocialTestCase; use App\Util\Nickname; use Jchook\AssertThrows\AssertThrows; -use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface; class NicknameTest extends GNUsocialTestCase { @@ -38,12 +37,8 @@ class NicknameTest extends GNUsocialTestCase public function testNormalize() { - $conf = ['nickname' => ['min_length' => 4, 'reserved' => ['this_nickname_is_reserved']]]; - $cb = $this->createMock(ContainerBagInterface::class); - static::assertTrue($cb instanceof ContainerBagInterface); - $cb->method('get') - ->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]); - Common::setupConfig($cb); + static::bootKernel(); + Common::setConfig('nickname', 'blacklist', ['this_nickname_is_reserved'], transient: true); static::assertThrows(NicknameTooLongException::class, fn () => Nickname::normalize(str_repeat('longstring-', 128), check_already_used: false)); static::assertThrows(NicknameInvalidException::class, fn () => Nickname::normalize('null\0', check_already_used: false)); @@ -55,7 +50,6 @@ class NicknameTest extends GNUsocialTestCase // static::assertThrows(NicknameInvalidException::class, fn () => Nickname::normalize('FóóBár', check_already_used: false)); static::assertThrows(NicknameNotAllowedException::class, fn () => Nickname::normalize('this_nickname_is_reserved', check_already_used: false)); - static::bootKernel(); static::assertSame('foobar', Nickname::normalize('foobar', check_already_used: true)); static::assertThrows(NicknameTakenException::class, fn () => Nickname::normalize('taken_user', check_already_used: true)); } @@ -74,18 +68,13 @@ class NicknameTest extends GNUsocialTestCase public function testIsReserved() { - $conf = ['nickname' => ['min_length' => 4, 'reserved' => ['this_nickname_is_reserved']]]; - $cb = $this->createMock(ContainerBagInterface::class); - static::assertTrue($cb instanceof ContainerBagInterface); - $cb->method('get')->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]); - Common::setupConfig($cb); + static::bootKernel(); + Common::setConfig('nickname', 'blacklist', ['this_nickname_is_reserved'], transient: true); + static::assertTrue(Nickname::isBlacklisted('this_nickname_is_reserved')); static::assertFalse(Nickname::isBlacklisted('this_nickname_is_not_reserved')); - $conf = ['nickname' => ['min_length' => 4, 'reserved' => []]]; - $cb = $this->createMock(ContainerBagInterface::class); - $cb->method('get')->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]); - Common::setupConfig($cb); + Common::setConfig('nickname', 'blacklist', [], transient: true); static::assertFalse(Nickname::isBlacklisted('this_nickname_is_reserved')); } } diff --git a/tests/Util/TemporaryFileTest.php b/tests/Util/TemporaryFileTest.php index 1a91bdbdf7..5d20bdca17 100644 --- a/tests/Util/TemporaryFileTest.php +++ b/tests/Util/TemporaryFileTest.php @@ -45,7 +45,7 @@ class TemporaryFileTest extends WebTestCase $temp = new TemporaryFile(); $filepath = $temp->getRealPath(); static::assertThrows(TemporaryFileException::class, fn () => $temp->commit($filepath)); - static::assertThrows(TemporaryFileException::class, fn () => $temp->commit('/root/cannot_write_here')); + static::assertThrows(TemporaryFileException::class, fn () => $temp->commit('/root/not-a-folder/cannot_write_here')); } public function testCreateDirectory()