From 3fb420115d8754208498c374170ad6839029392d Mon Sep 17 00:00:00 2001 From: Christopher Ng Date: Wed, 6 Apr 2022 18:46:32 +0000 Subject: Improve strictness of account file regex Signed-off-by: Christopher Ng --- apps/settings/tests/UserMigration/AccountMigratorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php index c668bb9e6af..1ad68dbd341 100644 --- a/apps/settings/tests/UserMigration/AccountMigratorTest.php +++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php @@ -58,7 +58,7 @@ class AccountMigratorTest extends TestCase { private const ASSETS_DIR = __DIR__ . '/assets/'; - private const REGEX_ACCOUNT_FILE = '/' . Application::APP_ID . '\/' . '[a-z]+\.json' . '/'; + private const REGEX_ACCOUNT_FILE = '/^' . Application::APP_ID . '\/' . '[a-z]+\.json' . '$/'; protected function setUp(): void { $app = new App(Application::APP_ID); -- cgit v1.2.3 From 32fc848fcff1ec3a31a198735930fde5b4c194f2 Mon Sep 17 00:00:00 2001 From: Christopher Ng Date: Wed, 6 Apr 2022 20:13:47 +0000 Subject: Test avatar migration Signed-off-by: Christopher Ng --- .../tests/UserMigration/AccountMigratorTest.php | 67 ++++++++++++++++++--- .../tests/UserMigration/assets/account-complex.jpg | Bin 0 -> 1063519 bytes .../tests/UserMigration/assets/account.png | Bin 0 -> 913877 bytes 3 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 apps/settings/tests/UserMigration/assets/account-complex.jpg create mode 100644 apps/settings/tests/UserMigration/assets/account.png diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php index 1ad68dbd341..74455bc91e1 100644 --- a/apps/settings/tests/UserMigration/AccountMigratorTest.php +++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php @@ -30,6 +30,7 @@ use OCA\Settings\AppInfo\Application; use OCA\Settings\UserMigration\AccountMigrator; use OCP\Accounts\IAccountManager; use OCP\AppFramework\App; +use OCP\IAvatarManager; use OCP\IUserManager; use OCP\UserMigration\IExportDestination; use OCP\UserMigration\IImportSource; @@ -45,6 +46,8 @@ class AccountMigratorTest extends TestCase { private IUserManager $userManager; + private IAvatarManager $avatarManager; + private AccountMigrator $migrator; /** @var IImportSource|MockObject */ @@ -60,11 +63,14 @@ class AccountMigratorTest extends TestCase { private const REGEX_ACCOUNT_FILE = '/^' . Application::APP_ID . '\/' . '[a-z]+\.json' . '$/'; + private const REGEX_AVATAR_FILE = '/^' . Application::APP_ID . '\/' . 'avatar\.(jpg|png)' . '$/'; + protected function setUp(): void { $app = new App(Application::APP_ID); $container = $app->getContainer(); $this->userManager = $container->get(IUserManager::class); + $this->avatarManager = $container->get(IAvatarManager::class); $this->migrator = $container->get(AccountMigrator::class); $this->importSource = $this->createMock(IImportSource::class); @@ -74,14 +80,23 @@ class AccountMigratorTest extends TestCase { public function dataImportExportAccount(): array { return array_map( - fn (string $filename) => [ - UUIDUtil::getUUID(), - json_decode(file_get_contents(self::ASSETS_DIR . $filename), true, 512, JSON_THROW_ON_ERROR), - ], - array_diff( - scandir(self::ASSETS_DIR), - // Exclude current and parent directories - ['.', '..'], + function (string $filename) { + $dataPath = self::ASSETS_DIR . $filename; + $avatarBasename = pathinfo($filename, PATHINFO_FILENAME); + $avatarPath = self::ASSETS_DIR . (file_exists(self::ASSETS_DIR . "$avatarBasename.jpg") ? "$avatarBasename.jpg" : "$avatarBasename.png"); + return [ + UUIDUtil::getUUID(), + json_decode(file_get_contents($dataPath), true, 512, JSON_THROW_ON_ERROR), + $avatarPath, + ]; + }, + array_filter( + array_diff( + scandir(self::ASSETS_DIR), + // Exclude current and parent directories + ['.', '..'], + ), + fn (string $filename) => pathinfo($filename, PATHINFO_EXTENSION) === 'json', ), ); } @@ -89,8 +104,9 @@ class AccountMigratorTest extends TestCase { /** * @dataProvider dataImportExportAccount */ - public function testImportExportAccount(string $userId, array $importData): void { + public function testImportExportAccount(string $userId, array $importData, string $avatarPath): void { $user = $this->userManager->createUser($userId, 'topsecretpassword'); + $avatarExt = pathinfo($avatarPath, PATHINFO_EXTENSION); $exportData = $importData; // Verification status of email will be set to in progress on import so we set the export data to reflect that $exportData[IAccountManager::PROPERTY_EMAIL]['verified'] = IAccountManager::VERIFICATION_IN_PROGRESS; @@ -107,14 +123,47 @@ class AccountMigratorTest extends TestCase { ->with($this->matchesRegularExpression(self::REGEX_ACCOUNT_FILE)) ->willReturn(json_encode($importData)); + $this->importSource + ->expects($this->once()) + ->method('getFolderListing') + ->with(Application::APP_ID . '/') + ->willReturn(["avatar.$avatarExt"]); + + $this->importSource + ->expects($this->once()) + ->method('getFileAsStream') + ->with($this->matchesRegularExpression(self::REGEX_AVATAR_FILE)) + ->willReturn(fopen($avatarPath, 'r')); + $this->migrator->import($user, $this->importSource, $this->output); + $importedAvatar = $this->avatarManager->getAvatar($user->getUID()); + $this->assertTrue($importedAvatar->isCustomAvatar()); + + /** + * Avatar images are re-encoded on import therefore JPEG images which use lossy compression cannot be checked for equality + * @see https://github.com/nextcloud/server/blob/9644b7e505dc90a1e683f77ad38dc6dc4e90fa2f/lib/private/legacy/OC_Image.php#L383-L390 + */ + + if ($avatarExt !== 'jpg') { + $this->assertStringEqualsFile( + $avatarPath, + $importedAvatar->getFile(-1)->getContent(), + ); + } + $this->exportDestination ->expects($this->once()) ->method('addFileContents') ->with($this->matchesRegularExpression(self::REGEX_ACCOUNT_FILE), json_encode($exportData)) ->willReturn(true); + $this->exportDestination + ->expects($this->once()) + ->method('addFileAsStream') + ->with($this->matchesRegularExpression(self::REGEX_AVATAR_FILE), $this->isType('resource')) + ->willReturn(true); + $this->migrator->export($user, $this->exportDestination, $this->output); } } diff --git a/apps/settings/tests/UserMigration/assets/account-complex.jpg b/apps/settings/tests/UserMigration/assets/account-complex.jpg new file mode 100644 index 00000000000..bece3675c11 Binary files /dev/null and b/apps/settings/tests/UserMigration/assets/account-complex.jpg differ diff --git a/apps/settings/tests/UserMigration/assets/account.png b/apps/settings/tests/UserMigration/assets/account.png new file mode 100644 index 00000000000..12226f14334 Binary files /dev/null and b/apps/settings/tests/UserMigration/assets/account.png differ -- cgit v1.2.3 From 6e62c3ddc57b3b84f6dad9d9e90302724565f102 Mon Sep 17 00:00:00 2001 From: Christopher Ng Date: Fri, 8 Apr 2022 18:02:49 +0000 Subject: Remove unnecessary array_diff Signed-off-by: Christopher Ng --- apps/settings/tests/UserMigration/AccountMigratorTest.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php index 74455bc91e1..051af68017c 100644 --- a/apps/settings/tests/UserMigration/AccountMigratorTest.php +++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php @@ -91,11 +91,7 @@ class AccountMigratorTest extends TestCase { ]; }, array_filter( - array_diff( - scandir(self::ASSETS_DIR), - // Exclude current and parent directories - ['.', '..'], - ), + scandir(self::ASSETS_DIR), fn (string $filename) => pathinfo($filename, PATHINFO_EXTENSION) === 'json', ), ); -- cgit v1.2.3 From 8e7372df441c3fde67b92f318fd14799fc5be97f Mon Sep 17 00:00:00 2001 From: Christopher Ng Date: Fri, 8 Apr 2022 18:03:33 +0000 Subject: Add comment explaining avatar basename Signed-off-by: Christopher Ng --- apps/settings/tests/UserMigration/AccountMigratorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php index 051af68017c..045a0cf3ded 100644 --- a/apps/settings/tests/UserMigration/AccountMigratorTest.php +++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php @@ -82,6 +82,7 @@ class AccountMigratorTest extends TestCase { return array_map( function (string $filename) { $dataPath = self::ASSETS_DIR . $filename; + // For each json file there is an avatar image with the same basename $avatarBasename = pathinfo($filename, PATHINFO_FILENAME); $avatarPath = self::ASSETS_DIR . (file_exists(self::ASSETS_DIR . "$avatarBasename.jpg") ? "$avatarBasename.jpg" : "$avatarBasename.png"); return [ -- cgit v1.2.3