From d0b45498cdc1676ced6b8017f617d145ea9f099b Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Thu, 23 May 2019 11:10:48 +0200 Subject: [PATCH] Don't run repair step for every individual user, outsource that to background job Signed-off-by: Georg Ehrke --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../RegisterRegenerateBirthdayCalendars.php | 66 ++++++++++++ .../Migration/RegenerateBirthdayCalendars.php | 19 +--- ...erateBirthdayCalendarBackgroundJobTest.php | 3 - ...egisterRegenerateBirthdayCalendarsTest.php | 101 ++++++++++++++++++ .../RegenerateBirthdayCalendarsTest.php | 51 ++------- 7 files changed, 179 insertions(+), 63 deletions(-) create mode 100644 apps/dav/lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php create mode 100644 apps/dav/tests/unit/BackgroundJob/RegisterRegenerateBirthdayCalendarsTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 61fd5c2591..6452cd298d 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -15,6 +15,7 @@ return array( 'OCA\\DAV\\BackgroundJob\\CleanupInvitationTokenJob' => $baseDir . '/../lib/BackgroundJob/CleanupInvitationTokenJob.php', 'OCA\\DAV\\BackgroundJob\\GenerateBirthdayCalendarBackgroundJob' => $baseDir . '/../lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\RefreshWebcalJob' => $baseDir . '/../lib/BackgroundJob/RefreshWebcalJob.php', + 'OCA\\DAV\\BackgroundJob\\RegisterRegenerateBirthdayCalendars' => $baseDir . '/../lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php', 'OCA\\DAV\\BackgroundJob\\UpdateCalendarResourcesRoomsBackgroundJob' => $baseDir . '/../lib/BackgroundJob/UpdateCalendarResourcesRoomsBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\UploadCleanup' => $baseDir . '/../lib/BackgroundJob/UploadCleanup.php', 'OCA\\DAV\\CalDAV\\Activity\\Backend' => $baseDir . '/../lib/CalDAV/Activity/Backend.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 4898c78021..19c74d3be0 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -30,6 +30,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\BackgroundJob\\CleanupInvitationTokenJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupInvitationTokenJob.php', 'OCA\\DAV\\BackgroundJob\\GenerateBirthdayCalendarBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\RefreshWebcalJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/RefreshWebcalJob.php', + 'OCA\\DAV\\BackgroundJob\\RegisterRegenerateBirthdayCalendars' => __DIR__ . '/..' . '/../lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php', 'OCA\\DAV\\BackgroundJob\\UpdateCalendarResourcesRoomsBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/UpdateCalendarResourcesRoomsBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\UploadCleanup' => __DIR__ . '/..' . '/../lib/BackgroundJob/UploadCleanup.php', 'OCA\\DAV\\CalDAV\\Activity\\Backend' => __DIR__ . '/..' . '/../lib/CalDAV/Activity/Backend.php', diff --git a/apps/dav/lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php b/apps/dav/lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php new file mode 100644 index 0000000000..e20547053c --- /dev/null +++ b/apps/dav/lib/BackgroundJob/RegisterRegenerateBirthdayCalendars.php @@ -0,0 +1,66 @@ + + * + * @author Georg Ehrke + * + * @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 OCA\DAV\BackgroundJob; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\BackgroundJob\QueuedJob; +use OCP\IUser; +use OCP\IUserManager; + +class RegisterRegenerateBirthdayCalendars extends QueuedJob { + + /** @var IUserManager */ + private $userManager; + + /** @var IJobList */ + private $jobList; + + /** + * RegisterRegenerateBirthdayCalendars constructor. + * + * @param ITimeFactory $time + * @param IUserManager $userManager + * @param IJobList $jobList + */ + public function __construct(ITimeFactory $time, + IUserManager $userManager, + IJobList $jobList) { + parent::__construct($time); + $this->userManager = $userManager; + $this->jobList = $jobList; + } + + /** + * @inheritDoc + */ + public function run($argument) { + $this->userManager->callForSeenUsers(function(IUser $user) { + $this->jobList->add(GenerateBirthdayCalendarBackgroundJob::class, [ + 'userId' => $user->getUID(), + 'purgeBeforeGenerating' => true + ]); + }); + } + +} \ No newline at end of file diff --git a/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php index 263e6d00db..492ae876e8 100644 --- a/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php +++ b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php @@ -22,19 +22,14 @@ */ namespace OCA\DAV\Migration; -use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; +use OCA\DAV\BackgroundJob\RegisterRegenerateBirthdayCalendars; use OCP\BackgroundJob\IJobList; use OCP\IConfig; -use OCP\IUser; -use OCP\IUserManager; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; class RegenerateBirthdayCalendars implements IRepairStep { - /** @var IUserManager */ - private $userManager; - /** @var IJobList */ private $jobList; @@ -42,14 +37,11 @@ class RegenerateBirthdayCalendars implements IRepairStep { private $config; /** - * @param IUserManager $userManager, * @param IJobList $jobList * @param IConfig $config */ - public function __construct(IUserManager $userManager, - IJobList $jobList, + public function __construct(IJobList $jobList, IConfig $config) { - $this->userManager = $userManager; $this->jobList = $jobList; $this->config = $config; } @@ -72,12 +64,7 @@ class RegenerateBirthdayCalendars implements IRepairStep { } $output->info('Adding background jobs to regenerate birthday calendar'); - $this->userManager->callForSeenUsers(function(IUser $user) { - $this->jobList->add(GenerateBirthdayCalendarBackgroundJob::class, [ - 'userId' => $user->getUID(), - 'purgeBeforeGenerating' => true - ]); - }); + $this->jobList->add(RegisterRegenerateBirthdayCalendars::class); // if all were done, no need to redo the repair during next upgrade $this->config->setAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'); diff --git a/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php b/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php index 1d672ff22a..acf1306a63 100644 --- a/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php +++ b/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php @@ -24,10 +24,7 @@ namespace OCA\DAV\Tests\unit\BackgroundJob; use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; use OCA\DAV\CalDAV\BirthdayService; -use OCA\DAV\CalDAV\CalDavBackend; -use OCA\DAV\CalDAV\CalendarHome; use OCP\IConfig; -use Sabre\DAV\MkCol; use Test\TestCase; class GenerateBirthdayCalendarBackgroundJobTest extends TestCase { diff --git a/apps/dav/tests/unit/BackgroundJob/RegisterRegenerateBirthdayCalendarsTest.php b/apps/dav/tests/unit/BackgroundJob/RegisterRegenerateBirthdayCalendarsTest.php new file mode 100644 index 0000000000..c118e7497f --- /dev/null +++ b/apps/dav/tests/unit/BackgroundJob/RegisterRegenerateBirthdayCalendarsTest.php @@ -0,0 +1,101 @@ + + * + * @author Georg Ehrke + * + * @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 OCA\DAV\Tests\unit\BackgroundJob; + +use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; +use OCA\DAV\BackgroundJob\RegisterRegenerateBirthdayCalendars; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\IConfig; +use OCP\IUser; +use OCP\IUserManager; +use Test\TestCase; + +class RegisterRegenerateBirthdayCalendarsTest extends TestCase { + + /** @var ITimeFactory | \PHPUnit_Framework_MockObject_MockObject */ + private $time; + + /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */ + private $userManager; + + /** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */ + private $jobList; + + /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ + private $config; + + /** @var RegisterRegenerateBirthdayCalendars */ + private $backgroundJob; + + protected function setUp() { + parent::setUp(); + + $this->time = $this->createMock(ITimeFactory::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->jobList = $this->createMock(IJobList::class); + $this->config = $this->createMock(IConfig::class); + + $this->backgroundJob = new RegisterRegenerateBirthdayCalendars($this->time, + $this->userManager, $this->jobList); + } + + public function testRun() { + $this->userManager->expects($this->once()) + ->method('callForSeenUsers') + ->will($this->returnCallback(function($closure) { + $user1 = $this->createMock(IUser::class); + $user1->method('getUID')->will($this->returnValue('uid1')); + $user2 = $this->createMock(IUser::class); + $user2->method('getUID')->will($this->returnValue('uid2')); + $user3 = $this->createMock(IUser::class); + $user3->method('getUID')->will($this->returnValue('uid3')); + + $closure($user1); + $closure($user2); + $closure($user3); + })); + + $this->jobList->expects($this->at(0)) + ->method('add') + ->with(GenerateBirthdayCalendarBackgroundJob::class, [ + 'userId' => 'uid1', + 'purgeBeforeGenerating' => true + ]); + $this->jobList->expects($this->at(1)) + ->method('add') + ->with(GenerateBirthdayCalendarBackgroundJob::class, [ + 'userId' => 'uid2', + 'purgeBeforeGenerating' => true + ]); + $this->jobList->expects($this->at(2)) + ->method('add') + ->with(GenerateBirthdayCalendarBackgroundJob::class, [ + 'userId' => 'uid3', + 'purgeBeforeGenerating' => true + ]); + + $this->backgroundJob->run([]); + } +} \ No newline at end of file diff --git a/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php index 74af6af88d..657d4dcebb 100644 --- a/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php +++ b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php @@ -22,39 +22,32 @@ namespace OCA\DAV\Tests\unit\DAV\Migration; -use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; -use OCA\DAV\Migration\RefreshWebcalJobRegistrar; +use OCA\DAV\BackgroundJob\RegisterRegenerateBirthdayCalendars; use OCA\DAV\Migration\RegenerateBirthdayCalendars; use OCP\BackgroundJob\IJobList; use OCP\IConfig; -use OCP\IUser; -use OCP\IUserManager; use OCP\Migration\IOutput; use Test\TestCase; class RegenerateBirthdayCalendarsTest extends TestCase { - /** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */ - private $userManager; - /** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */ private $jobList; /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ private $config; - /** @var RefreshWebcalJobRegistrar */ + /** @var RegenerateBirthdayCalendars */ private $migration; protected function setUp() { parent::setUp(); - $this->userManager = $this->createMock(IUserManager::class); $this->jobList = $this->createMock(IJobList::class); $this->config = $this->createMock(IConfig::class); - $this->migration = new RegenerateBirthdayCalendars($this->userManager, - $this->jobList, $this->config); + $this->migration = new RegenerateBirthdayCalendars($this->jobList, + $this->config); } public function testGetName() { @@ -75,39 +68,9 @@ class RegenerateBirthdayCalendarsTest extends TestCase { ->method('info') ->with('Adding background jobs to regenerate birthday calendar'); - $this->userManager->expects($this->once()) - ->method('callForSeenUsers') - ->will($this->returnCallback(function($closure) { - $user1 = $this->createMock(IUser::class); - $user1->method('getUID')->will($this->returnValue('uid1')); - $user2 = $this->createMock(IUser::class); - $user2->method('getUID')->will($this->returnValue('uid2')); - $user3 = $this->createMock(IUser::class); - $user3->method('getUID')->will($this->returnValue('uid3')); - - $closure($user1); - $closure($user2); - $closure($user3); - })); - $this->jobList->expects($this->at(0)) ->method('add') - ->with(GenerateBirthdayCalendarBackgroundJob::class, [ - 'userId' => 'uid1', - 'purgeBeforeGenerating' => true - ]); - $this->jobList->expects($this->at(1)) - ->method('add') - ->with(GenerateBirthdayCalendarBackgroundJob::class, [ - 'userId' => 'uid2', - 'purgeBeforeGenerating' => true - ]); - $this->jobList->expects($this->at(2)) - ->method('add') - ->with(GenerateBirthdayCalendarBackgroundJob::class, [ - 'userId' => 'uid3', - 'purgeBeforeGenerating' => true - ]); + ->with(RegisterRegenerateBirthdayCalendars::class); $this->config->expects($this->at(1)) ->method('setAppValue') @@ -127,8 +90,8 @@ class RegenerateBirthdayCalendarsTest extends TestCase { ->method('info') ->with('Repair step already executed'); - $this->userManager->expects($this->never()) - ->method('callForSeenUsers'); + $this->jobList->expects($this->never()) + ->method('add'); $this->migration->run($output); }