[TESTS] Fix remaining tests, back to 100% passed. Some minor semantic changes

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());
         }
 

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', []],

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);
+        if (!$transient) {
-        $yaml                             = (new Yaml\Dumper(indentation: 2))->dump(['parameters' => ['locals' => ['gnusocial' => $diff]]], Yaml\Yaml::DUMP_OBJECT_AS_MAP);
+            $diff = self::arrayDiffRecursive(self::$config, self::$defaults);
-        rename(INSTALLDIR . '/social.local.yaml', INSTALLDIR . '/social.local.yaml.back');
+            $yaml = (new Yaml\Dumper(indentation: 2))->dump(['parameters' => ['locals' => ['gnusocial' => $diff]]], Yaml\Yaml::DUMP_OBJECT_AS_MAP);
-        file_put_contents(INSTALLDIR . '/social.local.yaml', $yaml);
+            rename(INSTALLDIR . '/social.local.yaml', INSTALLDIR . '/social.local.yaml.back');
+            file_put_contents(INSTALLDIR . '/social.local.yaml', $yaml);
+        }
     }
 
     public static function getConfigDefaults()

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]);
+                        try {
-                        if ($lu !== null) {
+                            $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 \in_array(
-            return self::normalize($n, check_already_used: false, check_is_allowed: true);
+            $nickname,
-        })));
+            array_merge(
+                $reserved,
+                F\map(
+                    $reserved,
+                    fn ($n) => self::normalize($n, check_already_used: false, check_is_allowed: false),
+                ),
+            ),
+        );
     }
 }

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);

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']));
     }
 }

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()

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));

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']]];
+        static::bootKernel();
-        $cb   = $this->createMock(ContainerBagInterface::class);
+        Common::setConfig('nickname', 'blacklist', ['this_nickname_is_reserved'], transient: true);
-        static::assertTrue($cb instanceof ContainerBagInterface);
-        $cb->method('get')
-            ->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]);
-        Common::setupConfig($cb);
 
         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']]];
+        static::bootKernel();
-        $cb   = $this->createMock(ContainerBagInterface::class);
+        Common::setConfig('nickname', 'blacklist', ['this_nickname_is_reserved'], transient: true);
-        static::assertTrue($cb instanceof ContainerBagInterface);
+
-        $cb->method('get')->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]);
-        Common::setupConfig($cb);
         static::assertTrue(Nickname::isBlacklisted('this_nickname_is_reserved'));
         static::assertFalse(Nickname::isBlacklisted('this_nickname_is_not_reserved'));
 
-        $conf = ['nickname' => ['min_length' => 4, 'reserved' => []]];
+        Common::setConfig('nickname', 'blacklist', [], transient: true);
-        $cb   = $this->createMock(ContainerBagInterface::class);
-        $cb->method('get')->willReturnMap([['gnusocial', $conf], ['gnusocial_defaults', $conf]]);
-        Common::setupConfig($cb);
         static::assertFalse(Nickname::isBlacklisted('this_nickname_is_reserved'));
     }
 }

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()