diff --git a/lib/private/Avatar.php b/lib/private/Avatar.php index b8e4dcd0748c..293499b3464c 100644 --- a/lib/private/Avatar.php +++ b/lib/private/Avatar.php @@ -73,7 +73,7 @@ public function __construct(IStorage $storage, IL10N $l, User $user, ILogger $lo } private function buildAvatarPath() { - return \substr_replace(\substr_replace(\md5($this->user->getUID()), '/', 4, 0), '/', 2, 0); + return 'avatars/' . \substr_replace(\substr_replace(\md5($this->user->getUID()), '/', 4, 0), '/', 2, 0); } /** diff --git a/lib/private/Repair.php b/lib/private/Repair.php index f9cb6dacdefd..c96ac3cb6bd3 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -33,6 +33,7 @@ use OC\Repair\Collation; use OC\Repair\DisableExtraThemes; use OC\Repair\DropOldJobs; +use OC\Repair\MoveAvatarIntoSubFolder; use OC\Repair\OldGroupMembershipShares; use OC\Repair\RemoveGetETagEntries; use OC\Repair\RemoveRootShares; @@ -151,6 +152,15 @@ public static function getRepairSteps() { \OC::$server->getL10N('core'), \OC::$server->getLogger() ), + new MoveAvatarIntoSubFolder( + \OC::$server->getConfig(), + \OC::$server->getDatabaseConnection(), + \OC::$server->getUserManager(), + \OC::$server->getAvatarManager(), + \OC::$server->getLazyRootFolder(), + \OC::$server->getL10N('core'), + \OC::$server->getLogger() + ), new RemoveRootShares(\OC::$server->getDatabaseConnection(), \OC::$server->getUserManager(), \OC::$server->getLazyRootFolder()), new RepairUnmergedShares( \OC::$server->getConfig(), diff --git a/lib/private/Repair/MoveAvatarIntoSubFolder.php b/lib/private/Repair/MoveAvatarIntoSubFolder.php new file mode 100644 index 000000000000..df27e288812a --- /dev/null +++ b/lib/private/Repair/MoveAvatarIntoSubFolder.php @@ -0,0 +1,200 @@ + + * + * @copyright Copyright (c) 2019, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +namespace OC\Repair; + +use OC\Files\Node\File; +use OC\NotSquareException; +use OC\User\NoUserException; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\Storage\IStorage; +use OCP\Files\StorageNotAvailableException; +use OCP\IDBConnection; +use OCP\IL10N; +use OCP\ILogger; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use OCP\IUser; +use OC\Avatar; +use OCP\IConfig; +use OCP\IAvatarManager; +use OCP\Files\NotFoundException; + +/** + * OC 10.2.0 spreads avatars into multiple subdirectories of data dir + * This repair step puts them under data/avatars/ + * + * @package OC\Repair + */ +class MoveAvatarIntoSubFolder implements IRepairStep { + /** @var \OCP\IConfig */ + protected $config; + + /** @var IDBConnection */ + private $connection; + + /** @var IUserManager */ + private $userManager; + + /** @var IAvatarManager */ + private $avatarManager; + + /** @var IRootFolder */ + private $rootFolder; + + /** @var \OCP\ILogger */ + private $logger; + + /** @var \OCP\IL10N */ + private $l; + + /** + * @param IConfig $config config + * @param IDBConnection $connection database connection + * @param IUserManager $userManager user manager + * @param IAvatarManager $avatarManager + * @param IRootFolder $rootFolder + * @param IL10N $l10n + * @param ILogger $logger + */ + public function __construct( + IConfig $config, + IDBConnection $connection, + IUserManager $userManager, + IAvatarManager $avatarManager, + IRootFolder $rootFolder, + IL10N $l10n, + ILogger $logger + ) { + $this->config = $config; + $this->connection = $connection; + $this->userManager = $userManager; + $this->avatarManager = $avatarManager; + $this->rootFolder = $rootFolder; + $this->l = $l10n; + $this->logger = $logger; + } + + /** + * @return string + */ + public function getName() { + return 'Fix user avatars location'; + } + + /** + * Move avatars outside of their homes + * + * @param IOutput $out + * @param IUser $user + */ + private function moveAvatars(IOutput $out, IUser $user) { + $userId = $user->getUID(); + + try { + \OC\Files\Filesystem::initMountPoints($userId); + + $brokenPath = \substr_replace( + \substr_replace(\md5($userId), '/', 4, 0), + '/', + 2, + 0 + ); + if ($this->brokenAvatarExists($brokenPath)) { + $newAvatarStorage = $this->rootFolder->get('/avatars/')->getStorage(); + $avatar = new Avatar($newAvatarStorage, $this->l, $user, $this->logger); + // Overwrite pre 10.2 avatar if any as they were reuploaded + try { + $oldAvatarFile = $this->rootFolder->get("{$brokenPath}/avatar.jpg"); + } catch (NotFoundException $e) { + $oldAvatarFile = $this->rootFolder->get("{$brokenPath}/avatar.png"); + } + $avatar->set($oldAvatarFile->getContent()); + + // Delete old avatars path only if it does not contain any other items + // as it might collide with a real user directory + // e.g. data/de/ad/3b7a931377ad4ab5ad6a9cd718aa could be used by a real user de + // so we need to keep de if it has any other child except ad + $oldAvatarDir = $this->rootFolder->get($brokenPath); + $oldAvatarDir->delete(); + $brokenParent = $oldAvatarDir->getParent(); + $brokenRoot = $brokenParent->getParent(); + foreach ([$brokenParent, $brokenRoot] as $dir) { + /** @var \OCP\Files\Folder $dir */ + if ($dir->isDeletable() === false + || \count($dir->getDirectoryListing()) > 0 + ) { + break; + } + $dir->delete(); + } + } + } catch (NoUserException $e) { + $this->logger->warning("Skipping avatar move for $userId: User does not exist.", ['app' => __METHOD__]); + } catch (NotFoundException $e) { + // not all users have a home, ignore + $this->logger->debug("Skipping avatar move for $userId: User has no home or avatar folder.", ['app' => __METHOD__]); + } catch (NotSquareException $e) { + $this->logger->warning("Skipping avatar move for $userId: avatar image {$oldAvatarFile->getPath()} for user $userId is not square.", ['app' => __METHOD__]); + } catch (\Exception $e) { + $this->logger->logException($e, ['app' => __METHOD__, 'message' => "Skipping avatar move for $userId"]); + } + + \OC_Util::tearDownFS(); + } + + /** + * @param string $basePath + * @return bool + * @throws NotFoundException + */ + private function brokenAvatarExists($basePath) { + try { + $storage = $this->rootFolder->get('/')->getStorage(); + return $storage->file_exists("{$basePath}/avatar.jpg") + || $storage->file_exists("{$basePath}/avatar.png"); + } catch (StorageNotAvailableException $e) { + return false; + } + } + + /** + * @param IOutput $output + */ + public function run(IOutput $output) { + $ocVersionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0'); + if (\version_compare($ocVersionFromBeforeUpdate, '10.2.0.5', '=')) { + $function = function (IUser $user) use ($output) { + $this->moveAvatars($output, $user); + $output->advance(); + }; + + $output->startProgress($this->userManager->countSeenUsers()); + + $this->userManager->callForSeenUsers($function); + + $output->finishProgress(); + } else { + $output->info("No action required"); + } + } +} diff --git a/tests/acceptance/features/cliMain/maintenance.feature b/tests/acceptance/features/cliMain/maintenance.feature index 447540cb1ba1..2159965b3d05 100644 --- a/tests/acceptance/features/cliMain/maintenance.feature +++ b/tests/acceptance/features/cliMain/maintenance.feature @@ -8,7 +8,7 @@ Feature: Maintenance command Scenario: Repair steps should be listed correctly When the administrator list the repair steps using the occ command Then the command should have been successful - And the command output should contain the text "Found 16 repair steps" + And the command output should contain the text "Found 17 repair steps" And the command output table should contain the following text: | table_column | | OC\Repair\RepairMimeTypes | @@ -22,6 +22,7 @@ Feature: Maintenance command | OC\Repair\RepairSubShares | | OC\Repair\SharePropagation | | OC\Repair\MoveAvatarOutsideHome | + | OC\Repair\MoveAvatarIntoSubFolder | | OC\Repair\RemoveRootShares | | OC\Repair\RepairUnmergedShares | | OC\Repair\DisableExtraThemes | @@ -31,4 +32,4 @@ Feature: Maintenance command Scenario: Running single repair step without providing value should fail When the administrator invokes occ command "maintenance:repair --single" Then the command should have failed with exit code 1 - And the command error output should contain the text 'The "--single" option requires a value' \ No newline at end of file + And the command error output should contain the text 'The "--single" option requires a value' diff --git a/tests/lib/AvatarTest.php b/tests/lib/AvatarTest.php index 7614d4a6ffba..447512b243cf 100644 --- a/tests/lib/AvatarTest.php +++ b/tests/lib/AvatarTest.php @@ -54,10 +54,10 @@ public function testPathBuilding($expectedPath, $userId) { public function providesUserIds() { return [ - ['21/23/2f297a57a5a743894a0e4a801fc3', 'admin'], - ['c4/ca/4238a0b923820dcc509a6f75849b', '1'], - ['f9/5b/70fdc3088560732a5ac135644506', '{'], - ['d4/1d/8cd98f00b204e9800998ecf8427e', ''], + ['avatars/21/23/2f297a57a5a743894a0e4a801fc3', 'admin'], + ['avatars/c4/ca/4238a0b923820dcc509a6f75849b', '1'], + ['avatars/f9/5b/70fdc3088560732a5ac135644506', '{'], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e', ''], ]; } @@ -68,13 +68,13 @@ public function testGetNoAvatar() { public function testGetAvatarSizeMatch() { $this->storage->method('file_exists') ->will($this->returnValueMap([ - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.128.jpg', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.128.jpg', true], ])); $expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png'); - $this->storage->method('file_get_contents')->with('d4/1d/8cd98f00b204e9800998ecf8427e/avatar.128.jpg')->willReturn($expected->data()); + $this->storage->method('file_get_contents')->with('avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.128.jpg')->willReturn($expected->data()); $this->assertEquals($expected->data(), $this->avatar->get(128)->data()); } @@ -82,12 +82,12 @@ public function testGetAvatarSizeMatch() { public function testGetAvatarSizeMinusOne() { $this->storage->method('file_exists') ->will($this->returnValueMap([ - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], ])); $expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png'); - $this->storage->method('file_get_contents')->with('d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg')->willReturn($expected->data()); + $this->storage->method('file_get_contents')->with('avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg')->willReturn($expected->data()); $this->assertEquals($expected->data(), $this->avatar->get(-1)->data()); } @@ -95,8 +95,8 @@ public function testGetAvatarSizeMinusOne() { public function testGetAvatarNoSizeMatch() { $this->storage->method('file_exists') ->will($this->returnValueMap([ - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', true], - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png', false], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png', false], ])); $expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png'); @@ -106,10 +106,10 @@ public function testGetAvatarNoSizeMatch() { $this->storage->method('file_get_contents') ->will($this->returnCallback( function ($path) use ($expected, $expected2) { - if ($path === 'd4/1d/8cd98f00b204e9800998ecf8427e/avatar.png') { + if ($path === 'avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png') { return $expected->data(); } - if ($path === 'd4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png') { + if ($path === 'avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png') { return $expected2->data(); } throw new \OCP\Files\NotFoundException(); @@ -118,7 +118,7 @@ function ($path) use ($expected, $expected2) { $this->storage->expects($this->once()) ->method('file_put_contents') - ->with('d4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png', $expected2->data()); + ->with('avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.32.png', $expected2->data()); $this->assertEquals($expected2->data(), $this->avatar->get(32)->data()); } @@ -130,8 +130,8 @@ public function testExistsNo() { public function testExiststJPG() { $this->storage->method('file_exists') ->will($this->returnValueMap([ - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', false], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', false], ])); $this->assertTrue($this->avatar->exists()); } @@ -139,8 +139,8 @@ public function testExiststJPG() { public function testExistsPNG() { $this->storage->method('file_exists') ->will($this->returnValueMap([ - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', false], - ['d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', true], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.jpg', false], + ['avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', true], ])); $this->assertTrue($this->avatar->exists()); } @@ -153,11 +153,11 @@ public function testExistsStorageNotAvailable() { public function testSetAvatar() { $this->storage->expects($this->once())->method('rmdir') - ->with('d4/1d/8cd98f00b204e9800998ecf8427e'); + ->with('avatars/d4/1d/8cd98f00b204e9800998ecf8427e'); $image = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png'); $this->storage->method('file_put_contents') - ->with('d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', $image->data()); + ->with('avatars/d4/1d/8cd98f00b204e9800998ecf8427e/avatar.png', $image->data()); // One on remove and once on setting the new avatar $this->user->expects($this->exactly(2))->method('triggerChange');