From 3acde071f306a2b0092eb5c81116c8ded971eaeb Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Sat, 16 Feb 2019 16:18:58 +0100 Subject: [PATCH] Add Repair step to regenerate birthday calendar Signed-off-by: Georg Ehrke --- apps/dav/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../GenerateBirthdayCalendarBackgroundJob.php | 5 + apps/dav/lib/CalDAV/BirthdayService.php | 19 ++- .../Migration/RegenerateBirthdayCalendars.php | 85 +++++++++++ ...erateBirthdayCalendarBackgroundJobTest.php | 26 ++++ .../unit/CardDAV/BirthdayServiceTest.php | 26 ++++ .../RegenerateBirthdayCalendarsTest.php | 137 ++++++++++++++++++ 9 files changed, 299 insertions(+), 4 deletions(-) create mode 100644 apps/dav/lib/Migration/RegenerateBirthdayCalendars.php create mode 100644 apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 0ef960b017..37e0d2c12f 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -5,7 +5,7 @@ WebDAV WebDAV endpoint WebDAV endpoint - 1.9.1 + 1.9.2 agpl owncloud.org DAV @@ -28,6 +28,7 @@ OCA\DAV\Migration\FixBirthdayCalendarComponent + OCA\DAV\Migration\RegenerateBirthdayCalendars OCA\DAV\Migration\CalDAVRemoveEmptyValue OCA\DAV\Migration\BuildCalendarSearchIndex OCA\DAV\Migration\RefreshWebcalJobRegistrar diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index e9aaf35f64..3c7f98e36c 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -157,6 +157,7 @@ return array( 'OCA\\DAV\\Migration\\ChunkCleanup' => $baseDir . '/../lib/Migration/ChunkCleanup.php', 'OCA\\DAV\\Migration\\FixBirthdayCalendarComponent' => $baseDir . '/../lib/Migration/FixBirthdayCalendarComponent.php', 'OCA\\DAV\\Migration\\RefreshWebcalJobRegistrar' => $baseDir . '/../lib/Migration/RefreshWebcalJobRegistrar.php', + 'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => $baseDir . '/../lib/Migration/RegenerateBirthdayCalendars.php', 'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => $baseDir . '/../lib/Migration/RemoveClassifiedEventActivity.php', 'OCA\\DAV\\Migration\\Version1004Date20170825134824' => $baseDir . '/../lib/Migration/Version1004Date20170825134824.php', 'OCA\\DAV\\Migration\\Version1004Date20170919104507' => $baseDir . '/../lib/Migration/Version1004Date20170919104507.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 40fb07033f..01bbb917e0 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -172,6 +172,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\ChunkCleanup' => __DIR__ . '/..' . '/../lib/Migration/ChunkCleanup.php', 'OCA\\DAV\\Migration\\FixBirthdayCalendarComponent' => __DIR__ . '/..' . '/../lib/Migration/FixBirthdayCalendarComponent.php', 'OCA\\DAV\\Migration\\RefreshWebcalJobRegistrar' => __DIR__ . '/..' . '/../lib/Migration/RefreshWebcalJobRegistrar.php', + 'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => __DIR__ . '/..' . '/../lib/Migration/RegenerateBirthdayCalendars.php', 'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => __DIR__ . '/..' . '/../lib/Migration/RemoveClassifiedEventActivity.php', 'OCA\\DAV\\Migration\\Version1004Date20170825134824' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170825134824.php', 'OCA\\DAV\\Migration\\Version1004Date20170919104507' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170919104507.php', diff --git a/apps/dav/lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php b/apps/dav/lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php index c4279c5108..dd6fca73c8 100644 --- a/apps/dav/lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php +++ b/apps/dav/lib/BackgroundJob/GenerateBirthdayCalendarBackgroundJob.php @@ -51,6 +51,7 @@ class GenerateBirthdayCalendarBackgroundJob extends QueuedJob { */ public function run($arguments) { $userId = $arguments['userId']; + $purgeBeforeGenerating = $arguments['purgeBeforeGenerating'] ?? false; // make sure admin didn't change his mind $isGloballyEnabled = $this->config->getAppValue('dav', 'generateBirthdayCalendar', 'yes'); @@ -64,6 +65,10 @@ class GenerateBirthdayCalendarBackgroundJob extends QueuedJob { return; } + if ($purgeBeforeGenerating) { + $this->birthdayService->resetForUser($userId); + } + $this->birthdayService->syncUser($userId); } } diff --git a/apps/dav/lib/CalDAV/BirthdayService.php b/apps/dav/lib/CalDAV/BirthdayService.php index 25cf19b460..0e8926e775 100644 --- a/apps/dav/lib/CalDAV/BirthdayService.php +++ b/apps/dav/lib/CalDAV/BirthdayService.php @@ -137,9 +137,9 @@ class BirthdayService { * @throws \Sabre\DAV\Exception\BadRequest */ public function ensureCalendarExists($principal) { - $book = $this->calDavBackEnd->getCalendarByUri($principal, self::BIRTHDAY_CALENDAR_URI); - if (!is_null($book)) { - return $book; + $calendar = $this->calDavBackEnd->getCalendarByUri($principal, self::BIRTHDAY_CALENDAR_URI); + if (!is_null($calendar)) { + return $calendar; } $this->calDavBackEnd->createCalendar($principal, self::BIRTHDAY_CALENDAR_URI, [ '{DAV:}displayname' => 'Contact birthdays', @@ -270,6 +270,19 @@ class BirthdayService { return $vCal; } + /** + * @param string $user + */ + public function resetForUser($user) { + $principal = 'principals/users/'.$user; + $calendar = $this->calDavBackEnd->getCalendarByUri($principal, self::BIRTHDAY_CALENDAR_URI); + $calendarObjects = $this->calDavBackEnd->getCalendarObjects($calendar['id'], CalDavBackend::CALENDAR_TYPE_CALENDAR); + + foreach($calendarObjects as $calendarObject) { + $this->calDavBackEnd->deleteCalendarObject($calendar['id'], $calendarObject['uri'], CalDavBackend::CALENDAR_TYPE_CALENDAR); + } + } + /** * @param string $user */ diff --git a/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php new file mode 100644 index 0000000000..2c2b4eb0e9 --- /dev/null +++ b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php @@ -0,0 +1,85 @@ + + * + * @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\Migration; + +use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; +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; + + /** @var IConfig */ + private $config; + + /** + * @param IUserManager $userManager, + * @param IJobList $jobList + * @param IConfig $config + */ + public function __construct(IUserManager $userManager, + IJobList $jobList, + IConfig $config) { + $this->userManager = $userManager; + $this->jobList = $jobList; + $this->config = $config; + } + + /** + * @return string + */ + public function getName() { + return 'Regenerating birthday calendars to use new icons and fix old birthday events without year'; + } + + /** + * @param IOutput $output + */ + public function run(IOutput $output) { + // only run once + if ($this->config->getAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix') === 'yes') { + $output->info('Repair step already executed'); + return; + } + + $output->info('Adding background jobs to regenerate birthday calendar'); + $this->userManager->callForAllUsers(function(IUser $user) { + $this->jobList->add(GenerateBirthdayCalendarBackgroundJob::class, [ + 'userId' => $user->getUID(), + 'purgeBeforeGenerating' => true + ]); + }); + + // if all were done, no need to redo the repair during next upgrade + $this->config->setAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'); + } +} \ No newline at end of file diff --git a/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php b/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php index 010289a745..1d672ff22a 100644 --- a/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php +++ b/apps/dav/tests/unit/BackgroundJob/GenerateBirthdayCalendarBackgroundJobTest.php @@ -62,6 +62,10 @@ class GenerateBirthdayCalendarBackgroundJobTest extends TestCase { ->with('user123', 'dav', 'generateBirthdayCalendar', 'yes') ->will($this->returnValue('yes')); + $this->birthdayService->expects($this->never()) + ->method('resetForUser') + ->with('user123'); + $this->birthdayService->expects($this->once()) ->method('syncUser') ->with('user123'); @@ -69,6 +73,28 @@ class GenerateBirthdayCalendarBackgroundJobTest extends TestCase { $this->backgroundJob->run(['userId' => 'user123']); } + public function testRunAndReset() { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('dav', 'generateBirthdayCalendar', 'yes') + ->will($this->returnValue('yes')); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('user123', 'dav', 'generateBirthdayCalendar', 'yes') + ->will($this->returnValue('yes')); + + $this->birthdayService->expects($this->once()) + ->method('resetForUser') + ->with('user123'); + + $this->birthdayService->expects($this->once()) + ->method('syncUser') + ->with('user123'); + + $this->backgroundJob->run(['userId' => 'user123', 'purgeBeforeGenerating' => true]); + } + public function testRunGloballyDisabled() { $this->config->expects($this->once()) ->method('getAppValue') diff --git a/apps/dav/tests/unit/CardDAV/BirthdayServiceTest.php b/apps/dav/tests/unit/CardDAV/BirthdayServiceTest.php index 9587640386..46aa06e853 100644 --- a/apps/dav/tests/unit/CardDAV/BirthdayServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/BirthdayServiceTest.php @@ -329,6 +329,32 @@ class BirthdayServiceTest extends TestCase { $this->service->ensureCalendarExists('principal001'); } + public function testResetForUser() { + $this->calDav->expects($this->at(0)) + ->method('getCalendarByUri') + ->with('principals/users/user123', 'contact_birthdays') + ->willReturn(['id' => 42]); + + $this->calDav->expects($this->at(1)) + ->method('getCalendarObjects') + ->with(42, 0) + ->willReturn([['uri' => '1.ics'], ['uri' => '2.ics'], ['uri' => '3.ics']]); + + $this->calDav->expects($this->at(2)) + ->method('deleteCalendarObject') + ->with(42, '1.ics', 0); + + $this->calDav->expects($this->at(3)) + ->method('deleteCalendarObject') + ->with(42, '2.ics', 0); + + $this->calDav->expects($this->at(4)) + ->method('deleteCalendarObject') + ->with(42, '3.ics', 0); + + $this->service->resetForUser('user123'); + } + public function providesBirthday() { return [ [true, diff --git a/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php new file mode 100644 index 0000000000..05e12bbda6 --- /dev/null +++ b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php @@ -0,0 +1,137 @@ + + * + * @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 OCA\DAV\Tests\unit\DAV\Migration; + +use OCA\DAV\BackgroundJob\GenerateBirthdayCalendarBackgroundJob; +use OCA\DAV\Migration\RefreshWebcalJobRegistrar; +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 */ + 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); + } + + public function testGetName() { + $this->assertEquals( + 'Regenerating birthday calendars to use new icons and fix old birthday events without year', + $this->migration->getName() + ); + } + + public function testRun() { + $this->config->expects($this->at(0)) + ->method('getAppValue') + ->with('dav', 'regeneratedBirthdayCalendarsForYearFix') + ->willReturn(null); + + $output = $this->createMock(IOutput::class); + $output->expects($this->once()) + ->method('info') + ->with('Adding background jobs to regenerate birthday calendar'); + + $this->userManager->expects($this->once()) + ->method('callForAllUsers') + ->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->config->expects($this->at(1)) + ->method('setAppValue') + ->with('dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'); + + $this->migration->run($output); + } + + public function testRunSecondTime() { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('dav', 'regeneratedBirthdayCalendarsForYearFix') + ->willReturn('yes'); + + $output = $this->createMock(IOutput::class); + $output->expects($this->once()) + ->method('info') + ->with('Repair step already executed'); + + $this->userManager->expects($this->never()) + ->method('callForAllUsers'); + + $this->migration->run($output); + } + + +} \ No newline at end of file