From ccb05dbb170475870e755573f91da6e1914698d5 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 16 Nov 2016 11:40:52 +0100 Subject: [PATCH 1/2] Adds background job to cleanup all previews. * A repair step that inserts a background job for each user * Each background job will delete for 15 seconds if it takes longer we reschedule. This is done so instances that don't use the system cron won't time out. * Added tests Signed-off-by: Roeland Jago Douma --- lib/private/Repair.php | 5 + lib/private/Repair/NC11/CleanPreviews.php | 63 +++++ .../NC11/CleanPreviewsBackgroundJob.php | 121 +++++++++ lib/private/Server.php | 12 + .../NC11/CleanPreviewsBackgroundJobTest.php | 236 ++++++++++++++++++ tests/lib/Repair/NC11/CleanPreviewsTest.php | 93 +++++++ 6 files changed, 530 insertions(+) create mode 100644 lib/private/Repair/NC11/CleanPreviews.php create mode 100644 lib/private/Repair/NC11/CleanPreviewsBackgroundJob.php create mode 100644 tests/lib/Repair/NC11/CleanPreviewsBackgroundJobTest.php create mode 100644 tests/lib/Repair/NC11/CleanPreviewsTest.php diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 7a5ef9fbd9..c9f8dbfff6 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -36,6 +36,7 @@ use OC\Repair\CleanTags; use OC\Repair\Collation; use OC\Repair\DropOldJobs; use OC\Repair\MoveUpdaterStepFile; +use OC\Repair\NC11\CleanPreviews; use OC\Repair\NC11\MoveAvatars; use OC\Repair\OldGroupMembershipShares; use OC\Repair\RemoveGetETagEntries; @@ -155,6 +156,10 @@ class Repair implements IOutput{ \OC::$server->getJobList(), \OC::$server->getSystemConfig() ), + new CleanPreviews( + \OC::$server->getJobList(), + \OC::$server->getUserManager() + ), ]; } diff --git a/lib/private/Repair/NC11/CleanPreviews.php b/lib/private/Repair/NC11/CleanPreviews.php new file mode 100644 index 0000000000..0c6be0165f --- /dev/null +++ b/lib/private/Repair/NC11/CleanPreviews.php @@ -0,0 +1,63 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace OC\Repair\NC11; + +use OCP\BackgroundJob\IJobList; +use OCP\IUser; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class CleanPreviews implements IRepairStep { + + /** @var IJobList */ + private $jobList; + + /** @var IUserManager */ + private $userManager; + + /** + * MoveAvatars constructor. + * + * @param IJobList $jobList + * @param IUserManager $userManager + */ + public function __construct(IJobList $jobList, + IUserManager $userManager) { + $this->jobList = $jobList; + $this->userManager = $userManager; + } + + /** + * @return string + */ + public function getName() { + return 'Add preview cleanup background jobs'; + } + + public function run(IOutput $output) { + $this->userManager->callForSeenUsers(function(IUser $user) { + $this->jobList->add(CleanPreviewsBackgroundJob::class, ['uid' => $user->getUID()]); + }); + } +} diff --git a/lib/private/Repair/NC11/CleanPreviewsBackgroundJob.php b/lib/private/Repair/NC11/CleanPreviewsBackgroundJob.php new file mode 100644 index 0000000000..9efe01508a --- /dev/null +++ b/lib/private/Repair/NC11/CleanPreviewsBackgroundJob.php @@ -0,0 +1,121 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace OC\Repair\NC11; + +use OC\BackgroundJob\QueuedJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; +use OCP\ILogger; + +class CleanPreviewsBackgroundJob extends QueuedJob { + /** @var IRootFolder */ + private $rootFolder; + + /** @var ILogger */ + private $logger; + + /** @var IJobList */ + private $jobList; + + /** @var ITimeFactory */ + private $timeFactory; + + /** + * CleanPreviewsBackgroundJob constructor. + * + * @param IRootFolder $rootFolder + * @param ILogger $logger + * @param IJobList $jobList + * @param ITimeFactory $timeFactory + */ + public function __construct(IRootFolder $rootFolder, + ILogger $logger, + IJobList $jobList, + ITimeFactory $timeFactory) { + $this->rootFolder = $rootFolder; + $this->logger = $logger; + $this->jobList = $jobList; + $this->timeFactory = $timeFactory; + } + + public function run($arguments) { + $uid = $arguments['uid']; + $this->logger->info('Started preview cleanup for ' . $uid); + $empty = $this->cleanupPreviews($uid); + + if (!$empty) { + $this->jobList->add(self::class, ['uid' => $uid]); + $this->logger->info('New preview cleanup scheduled for ' . $uid); + } else { + $this->logger->info('Preview cleanup done for ' . $uid); + } + } + + /** + * @param $uid + * @return bool + */ + private function cleanupPreviews($uid) { + try { + $userFolder = $this->rootFolder->getUserFolder($uid); + } catch (NotFoundException $e) { + return true; + } + + $userRoot = $userFolder->getParent(); + + try { + /** @var Folder $thumbnailFolder */ + $thumbnailFolder = $userRoot->get('thumbnails'); + } catch (NotFoundException $e) { + return true; + } + + $thumbnails = $thumbnailFolder->getDirectoryListing(); + + $start = $this->timeFactory->getTime(); + foreach ($thumbnails as $thumbnail) { + try { + $thumbnail->delete(); + } catch (NotPermittedException $e) { + // Ignore + } + + if (($this->timeFactory->getTime() - $start) > 15) { + return false; + } + } + + try { + $thumbnailFolder->delete(); + } catch (NotPermittedException $e) { + // Ignore + } + + return true; + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 1d3d588e9f..0963f8feb4 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -73,6 +73,7 @@ use OC\Lockdown\LockdownManager; use OC\Mail\Mailer; use OC\Memcache\ArrayCache; use OC\Notification\Manager; +use OC\Repair\NC11\CleanPreviewsBackgroundJob; use OC\RichObjectStrings\Validator; use OC\Security\Bruteforce\Throttler; use OC\Security\CertificateManager; @@ -798,9 +799,20 @@ class Server extends ServerContainer implements IServerContainer { $c->getSystemConfig() ); }); + $this->registerService('LockdownManager', function (Server $c) { return new LockdownManager(); }); + + /* To trick DI since we don't extend the DIContainer here */ + $this->registerService(CleanPreviewsBackgroundJob::class, function (Server $c) { + return new CleanPreviewsBackgroundJob( + $c->getRootFolder(), + $c->getLogger(), + $c->getJobList(), + new TimeFactory() + ); + }); } /** diff --git a/tests/lib/Repair/NC11/CleanPreviewsBackgroundJobTest.php b/tests/lib/Repair/NC11/CleanPreviewsBackgroundJobTest.php new file mode 100644 index 0000000000..e3fb22f082 --- /dev/null +++ b/tests/lib/Repair/NC11/CleanPreviewsBackgroundJobTest.php @@ -0,0 +1,236 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace Test\Repair\NC11; + +use OC\Repair\NC11\CleanPreviewsBackgroundJob; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; +use OCP\ILogger; +use Test\TestCase; + +class CleanPreviewsBackgroundJobTest extends TestCase { + /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ + private $rootFolder; + + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + private $logger; + + /** @var IJobList|\PHPUnit_Framework_MockObject_MockObject */ + private $jobList; + + /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + private $timeFactory; + + /** @var CleanPreviewsBackgroundJob */ + private $job; + + public function setUp() { + parent::setUp(); + + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->logger = $this->createMock(ILogger::class); + $this->jobList = $this->createMock(IJobList::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + + $this->job = new CleanPreviewsBackgroundJob( + $this->rootFolder, + $this->logger, + $this->jobList, + $this->timeFactory); + } + + public function testCleanupPreviewsUnfinished() { + $userFolder = $this->createMock(Folder::class); + $userRoot = $this->createMock(Folder::class); + $thumbnailFolder = $this->createMock(Folder::class); + + $this->rootFolder->method('getUserFolder') + ->with($this->equalTo('myuid')) + ->willReturn($userFolder); + + $userFolder->method('getParent')->willReturn($userRoot); + + $userRoot->method('get') + ->with($this->equalTo('thumbnails')) + ->willReturn($thumbnailFolder); + + $previewFolder1 = $this->createMock(Folder::class); + + $previewFolder1->expects($this->once()) + ->method('delete'); + + $thumbnailFolder->method('getDirectoryListing') + ->willReturn([$previewFolder1]); + $thumbnailFolder->expects($this->never()) + ->method('delete'); + + $this->timeFactory->method('getTime') + ->will($this->onConsecutiveCalls(100, 200)); + + $this->jobList->expects($this->once()) + ->method('add') + ->with( + $this->equalTo(CleanPreviewsBackgroundJob::class), + $this->equalTo(['uid' => 'myuid']) + ); + + $this->logger->expects($this->at(0)) + ->method('info') + ->with($this->equalTo('Started preview cleanup for myuid')); + $this->logger->expects($this->at(1)) + ->method('info') + ->with($this->equalTo('New preview cleanup scheduled for myuid')); + + $this->job->run(['uid' => 'myuid']); + } + + public function testCleanupPreviewsFinished() { + $userFolder = $this->createMock(Folder::class); + $userRoot = $this->createMock(Folder::class); + $thumbnailFolder = $this->createMock(Folder::class); + + $this->rootFolder->method('getUserFolder') + ->with($this->equalTo('myuid')) + ->willReturn($userFolder); + + $userFolder->method('getParent')->willReturn($userRoot); + + $userRoot->method('get') + ->with($this->equalTo('thumbnails')) + ->willReturn($thumbnailFolder); + + $previewFolder1 = $this->createMock(Folder::class); + + $previewFolder1->expects($this->once()) + ->method('delete'); + + $thumbnailFolder->method('getDirectoryListing') + ->willReturn([$previewFolder1]); + + $this->timeFactory->method('getTime') + ->will($this->onConsecutiveCalls(100, 101)); + + $this->jobList->expects($this->never()) + ->method('add'); + + $this->logger->expects($this->at(0)) + ->method('info') + ->with($this->equalTo('Started preview cleanup for myuid')); + $this->logger->expects($this->at(1)) + ->method('info') + ->with($this->equalTo('Preview cleanup done for myuid')); + + $thumbnailFolder->expects($this->once()) + ->method('delete'); + + $this->job->run(['uid' => 'myuid']); + } + + + public function testNoUserFolder() { + $this->rootFolder->method('getUserFolder') + ->with($this->equalTo('myuid')) + ->willThrowException(new NotFoundException()); + + $this->logger->expects($this->at(0)) + ->method('info') + ->with($this->equalTo('Started preview cleanup for myuid')); + $this->logger->expects($this->at(1)) + ->method('info') + ->with($this->equalTo('Preview cleanup done for myuid')); + + $this->job->run(['uid' => 'myuid']); + } + + public function testNoThumbnailFolder() { + $userFolder = $this->createMock(Folder::class); + $userRoot = $this->createMock(Folder::class); + + $this->rootFolder->method('getUserFolder') + ->with($this->equalTo('myuid')) + ->willReturn($userFolder); + + $userFolder->method('getParent')->willReturn($userRoot); + + $userRoot->method('get') + ->with($this->equalTo('thumbnails')) + ->willThrowException(new NotFoundException()); + + $this->logger->expects($this->at(0)) + ->method('info') + ->with($this->equalTo('Started preview cleanup for myuid')); + $this->logger->expects($this->at(1)) + ->method('info') + ->with($this->equalTo('Preview cleanup done for myuid')); + + $this->job->run(['uid' => 'myuid']); + } + + public function testNotPermittedToDelete() { + $userFolder = $this->createMock(Folder::class); + $userRoot = $this->createMock(Folder::class); + $thumbnailFolder = $this->createMock(Folder::class); + + $this->rootFolder->method('getUserFolder') + ->with($this->equalTo('myuid')) + ->willReturn($userFolder); + + $userFolder->method('getParent')->willReturn($userRoot); + + $userRoot->method('get') + ->with($this->equalTo('thumbnails')) + ->willReturn($thumbnailFolder); + + $previewFolder1 = $this->createMock(Folder::class); + + $previewFolder1->expects($this->once()) + ->method('delete') + ->willThrowException(new NotPermittedException()); + + $thumbnailFolder->method('getDirectoryListing') + ->willReturn([$previewFolder1]); + + $this->timeFactory->method('getTime') + ->will($this->onConsecutiveCalls(100, 101)); + + $this->jobList->expects($this->never()) + ->method('add'); + + $this->logger->expects($this->at(0)) + ->method('info') + ->with($this->equalTo('Started preview cleanup for myuid')); + $this->logger->expects($this->at(1)) + ->method('info') + ->with($this->equalTo('Preview cleanup done for myuid')); + + $thumbnailFolder->expects($this->once()) + ->method('delete') + ->willThrowException(new NotPermittedException()); + + $this->job->run(['uid' => 'myuid']); + } +} diff --git a/tests/lib/Repair/NC11/CleanPreviewsTest.php b/tests/lib/Repair/NC11/CleanPreviewsTest.php new file mode 100644 index 0000000000..643f18e89b --- /dev/null +++ b/tests/lib/Repair/NC11/CleanPreviewsTest.php @@ -0,0 +1,93 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ +namespace Test\Repair\NC11; + +use OC\Repair\NC11\CleanPreviews; +use OC\Repair\NC11\CleanPreviewsBackgroundJob; +use OCP\BackgroundJob\IJobList; +use OCP\IUser; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use Test\TestCase; + +class CleanPreviewsTest extends TestCase { + + + /** @var IJobList|\PHPUnit_Framework_MockObject_MockObject */ + private $jobList; + + /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + private $userManager; + + /** @var CleanPreviews */ + private $repair; + + public function setUp() { + parent::setUp(); + + $this->jobList = $this->createMock(IJobList::class); + $this->userManager = $this->createMock(IUserManager::class); + + $this->repair = new CleanPreviews( + $this->jobList, + $this->userManager + ); + } + + public function testGetName() { + $this->assertSame('Add preview cleanup background jobs', $this->repair->getName()); + } + + public function testRun() { + $user1 = $this->createMock(IUser::class); + $user1->method('getUID') + ->willReturn('user1'); + $user2 = $this->createMock(IUser::class); + $user2->method('getUID') + ->willReturn('user2'); + + $this->userManager->expects($this->once()) + ->method('callForSeenUsers') + ->will($this->returnCallback(function (\Closure $function) use ($user1, $user2) { + $function($user1); + $function($user2); + })); + + $this->jobList->expects($this->at(0)) + ->method('add') + ->with( + $this->equalTo(CleanPreviewsBackgroundJob::class), + $this->equalTo(['uid' => 'user1']) + ); + + $this->jobList->expects($this->at(1)) + ->method('add') + ->with( + $this->equalTo(CleanPreviewsBackgroundJob::class), + $this->equalTo(['uid' => 'user2']) + ); + + $this->repair->run($this->createMock(IOutput::class)); + } + +} From 78a318d388bfe82e1a3adebeb75ac195cb1334d7 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sat, 19 Nov 2016 20:26:53 +0100 Subject: [PATCH 2/2] Add test if repair step is already done Signed-off-by: Roeland Jago Douma --- lib/private/Repair.php | 3 +- lib/private/Repair/NC11/CleanPreviews.php | 18 +++++++-- tests/lib/Repair/NC11/CleanPreviewsTest.php | 44 ++++++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/lib/private/Repair.php b/lib/private/Repair.php index c9f8dbfff6..5efbb9f8e2 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -158,7 +158,8 @@ class Repair implements IOutput{ ), new CleanPreviews( \OC::$server->getJobList(), - \OC::$server->getUserManager() + \OC::$server->getUserManager(), + \OC::$server->getConfig() ), ]; } diff --git a/lib/private/Repair/NC11/CleanPreviews.php b/lib/private/Repair/NC11/CleanPreviews.php index 0c6be0165f..94f5d19b79 100644 --- a/lib/private/Repair/NC11/CleanPreviews.php +++ b/lib/private/Repair/NC11/CleanPreviews.php @@ -23,6 +23,7 @@ namespace OC\Repair\NC11; use OCP\BackgroundJob\IJobList; +use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\Migration\IOutput; @@ -36,16 +37,22 @@ class CleanPreviews implements IRepairStep { /** @var IUserManager */ private $userManager; + /** @var IConfig */ + private $config; + /** * MoveAvatars constructor. * * @param IJobList $jobList * @param IUserManager $userManager + * @param IConfig $config */ public function __construct(IJobList $jobList, - IUserManager $userManager) { + IUserManager $userManager, + IConfig $config) { $this->jobList = $jobList; $this->userManager = $userManager; + $this->config = $config; } /** @@ -56,8 +63,11 @@ class CleanPreviews implements IRepairStep { } public function run(IOutput $output) { - $this->userManager->callForSeenUsers(function(IUser $user) { - $this->jobList->add(CleanPreviewsBackgroundJob::class, ['uid' => $user->getUID()]); - }); + if (!$this->config->getAppValue('core', 'previewsCleanedUp', false)) { + $this->userManager->callForSeenUsers(function (IUser $user) { + $this->jobList->add(CleanPreviewsBackgroundJob::class, ['uid' => $user->getUID()]); + }); + $this->config->setAppValue('core', 'previewsCleanedUp', 1); + } } } diff --git a/tests/lib/Repair/NC11/CleanPreviewsTest.php b/tests/lib/Repair/NC11/CleanPreviewsTest.php index 643f18e89b..8abc6b7bab 100644 --- a/tests/lib/Repair/NC11/CleanPreviewsTest.php +++ b/tests/lib/Repair/NC11/CleanPreviewsTest.php @@ -25,6 +25,7 @@ namespace Test\Repair\NC11; use OC\Repair\NC11\CleanPreviews; use OC\Repair\NC11\CleanPreviewsBackgroundJob; use OCP\BackgroundJob\IJobList; +use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\Migration\IOutput; @@ -39,6 +40,9 @@ class CleanPreviewsTest extends TestCase { /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ private $userManager; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; + /** @var CleanPreviews */ private $repair; @@ -47,10 +51,12 @@ class CleanPreviewsTest extends TestCase { $this->jobList = $this->createMock(IJobList::class); $this->userManager = $this->createMock(IUserManager::class); + $this->config = $this->createMock(IConfig::class); $this->repair = new CleanPreviews( $this->jobList, - $this->userManager + $this->userManager, + $this->config ); } @@ -87,6 +93,42 @@ class CleanPreviewsTest extends TestCase { $this->equalTo(['uid' => 'user2']) ); + $this->config->expects($this->once()) + ->method('getAppValue') + ->with( + $this->equalTo('core'), + $this->equalTo('previewsCleanedUp'), + $this->equalTo(false) + )->willReturn(false); + $this->config->expects($this->once()) + ->method('setAppValue') + ->with( + $this->equalTo('core'), + $this->equalTo('previewsCleanedUp'), + $this->equalTo(1) + ); + + $this->repair->run($this->createMock(IOutput::class)); + } + + + public function testRunAlreadyDoone() { + $this->userManager->expects($this->never()) + ->method($this->anything()); + + $this->jobList->expects($this->never()) + ->method($this->anything()); + + $this->config->expects($this->once()) + ->method('getAppValue') + ->with( + $this->equalTo('core'), + $this->equalTo('previewsCleanedUp'), + $this->equalTo(false) + )->willReturn('1'); + $this->config->expects($this->never()) + ->method('setAppValue'); + $this->repair->run($this->createMock(IOutput::class)); }