From 517d89d70bff5313157c221e73f5e4b48af2c40b Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sat, 4 Sep 2021 20:52:14 +0200 Subject: [PATCH] Add repair job to delete calendar subscriptions that were orphaned when deleteding an user Follow-up to https://github.com/nextcloud/server/pull/28419 Signed-off-by: Thomas Citharel --- apps/dav/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + ...emoveDeletedUsersCalendarSubscriptions.php | 144 +++++++++++++++ ...eDeletedUsersCalendarSubscriptionsTest.php | 169 ++++++++++++++++++ 5 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 apps/dav/lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php create mode 100644 apps/dav/tests/unit/Migration/RemoveDeletedUsersCalendarSubscriptionsTest.php diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 0f1696f3db9e1..8bbfbbdeba109 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -5,7 +5,7 @@ WebDAV WebDAV endpoint WebDAV endpoint - 1.19.0 + 1.20.0 agpl owncloud.org DAV @@ -38,6 +38,7 @@ OCA\DAV\Migration\RegisterBuildReminderIndexBackgroundJob OCA\DAV\Migration\RemoveOrphanEventsAndContacts OCA\DAV\Migration\RemoveClassifiedEventActivity + OCA\DAV\Migration\RemoveDeletedUsersCalendarSubscriptions OCA\DAV\Migration\ChunkCleanup diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 13e78cee71645..0572fe3cff8bc 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -242,6 +242,7 @@ 'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => $baseDir . '/../lib/Migration/RegenerateBirthdayCalendars.php', 'OCA\\DAV\\Migration\\RegisterBuildReminderIndexBackgroundJob' => $baseDir . '/../lib/Migration/RegisterBuildReminderIndexBackgroundJob.php', 'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => $baseDir . '/../lib/Migration/RemoveClassifiedEventActivity.php', + 'OCA\\DAV\\Migration\\RemoveDeletedUsersCalendarSubscriptions' => $baseDir . '/../lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php', 'OCA\\DAV\\Migration\\RemoveOrphanEventsAndContacts' => $baseDir . '/../lib/Migration/RemoveOrphanEventsAndContacts.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 42ba59e0582e2..01a5df8c1b8b0 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -257,6 +257,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => __DIR__ . '/..' . '/../lib/Migration/RegenerateBirthdayCalendars.php', 'OCA\\DAV\\Migration\\RegisterBuildReminderIndexBackgroundJob' => __DIR__ . '/..' . '/../lib/Migration/RegisterBuildReminderIndexBackgroundJob.php', 'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => __DIR__ . '/..' . '/../lib/Migration/RemoveClassifiedEventActivity.php', + 'OCA\\DAV\\Migration\\RemoveDeletedUsersCalendarSubscriptions' => __DIR__ . '/..' . '/../lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php', 'OCA\\DAV\\Migration\\RemoveOrphanEventsAndContacts' => __DIR__ . '/..' . '/../lib/Migration/RemoveOrphanEventsAndContacts.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/Migration/RemoveDeletedUsersCalendarSubscriptions.php b/apps/dav/lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php new file mode 100644 index 0000000000000..50becf81e78f1 --- /dev/null +++ b/apps/dav/lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php @@ -0,0 +1,144 @@ + + * + * @author Thomas Citharel + * + * @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 OCP\DB\Exception; +use OCP\IDBConnection; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class RemoveDeletedUsersCalendarSubscriptions implements IRepairStep { + /** @var IDBConnection */ + private $connection; + + /** @var IUserManager */ + private $userManager; + + /** @var int */ + private $progress = 0; + + private $orphanSubscriptions = []; + + private const SUBSCRIPTIONS_CHUNK_SIZE = 1000; + + public function __construct(IDBConnection $connection, IUserManager $userManager) { + $this->connection = $connection; + $this->userManager = $userManager; + } + + /** + * @inheritdoc + */ + public function getName(): string { + return 'Clean up old calendar subscriptions from deleted users that were not cleaned-up'; + } + + /** + * @inheritdoc + */ + public function run(IOutput $output) { + $nbSubscriptions = $this->countSubscriptions(); + + $output->startProgress($nbSubscriptions); + + while ($this->progress < $nbSubscriptions) { + $this->checkSubscriptions(); + + $this->progress += self::SUBSCRIPTIONS_CHUNK_SIZE; + $output->advance(min(self::SUBSCRIPTIONS_CHUNK_SIZE, $nbSubscriptions)); + } + $output->finishProgress(); + $this->deleteOrphanSubscriptions(); + + $output->info(sprintf('%d calendar subscriptions without an user have been cleaned up', count($this->orphanSubscriptions))); + } + + /** + * @throws Exception + */ + private function countSubscriptions(): int { + $qb = $this->connection->getQueryBuilder(); + $query = $qb->select($qb->func()->count('*')) + ->from('calendarsubscriptions'); + + $result = $query->execute(); + $count = $result->fetchOne(); + $result->closeCursor(); + + if ($count !== false) { + $count = (int)$count; + } else { + $count = 0; + } + + return $count; + } + + /** + * @throws Exception + */ + private function checkSubscriptions(): void { + $qb = $this->connection->getQueryBuilder(); + $query = $qb->selectDistinct(['id', 'principaluri']) + ->from('calendarsubscriptions') + ->setMaxResults(self::SUBSCRIPTIONS_CHUNK_SIZE) + ->setFirstResult($this->progress); + + $result = $query->execute(); + while ($row = $result->fetch()) { + $username = $this->getPrincipal($row['principaluri']); + if (!$this->userManager->userExists($username)) { + $this->orphanSubscriptions[] = $row['id']; + } + } + $result->closeCursor(); + } + + /** + * @throws Exception + */ + private function deleteOrphanSubscriptions(): void { + foreach ($this->orphanSubscriptions as $orphanSubscriptionID) { + $this->deleteOrphanSubscription($orphanSubscriptionID); + } + } + + /** + * @throws Exception + */ + private function deleteOrphanSubscription(int $orphanSubscriptionID): void { + $qb = $this->connection->getQueryBuilder(); + $qb->delete('calendarsubscriptions') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($orphanSubscriptionID))) + ->executeStatement(); + } + + private function getPrincipal(string $principalUri): string { + $uri = explode('/', $principalUri); + return array_pop($uri); + } +} diff --git a/apps/dav/tests/unit/Migration/RemoveDeletedUsersCalendarSubscriptionsTest.php b/apps/dav/tests/unit/Migration/RemoveDeletedUsersCalendarSubscriptionsTest.php new file mode 100644 index 0000000000000..c0d6518d2051e --- /dev/null +++ b/apps/dav/tests/unit/Migration/RemoveDeletedUsersCalendarSubscriptionsTest.php @@ -0,0 +1,169 @@ + + * + * @author Thomas Citharel + * + * @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\DAV\Migration; + +use OCA\DAV\Migration\RemoveDeletedUsersCalendarSubscriptions; +use OCP\DB\IResult; +use OCP\DB\QueryBuilder\IExpressionBuilder; +use OCP\DB\QueryBuilder\IFunctionBuilder; +use OCP\DB\QueryBuilder\IParameter; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\IQueryFunction; +use OCP\IDBConnection; +use OCP\IUserManager; +use OCP\Migration\IOutput; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +class RemoveDeletedUsersCalendarSubscriptionsTest extends TestCase { + /** + * @var IDBConnection|MockObject + */ + private $dbConnection; + /** + * @var IUserManager|MockObject + */ + private $userManager; + + /** + * @var IOutput|MockObject + */ + private $output; + /** + * @var RemoveDeletedUsersCalendarSubscriptions + */ + private $migration; + + + protected function setUp(): void { + parent::setUp(); + + $this->dbConnection = $this->createMock(IDBConnection::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->output = $this->createMock(IOutput::class); + + $this->migration = new RemoveDeletedUsersCalendarSubscriptions($this->dbConnection, $this->userManager); + } + + public function testGetName(): void { + $this->assertEquals( + 'Clean up old calendar subscriptions from deleted users that were not cleaned-up', + $this->migration->getName() + ); + } + + /** + * @dataProvider dataTestRun + * @param array $subscriptions + * @param array $userExists + * @param int $deletions + * @throws \Exception + */ + public function testRun(array $subscriptions, array $userExists, int $deletions): void { + $qb = $this->createMock(IQueryBuilder::class); + + $qb->method('select')->willReturn($qb); + + $functionBuilder = $this->createMock(IFunctionBuilder::class); + + $qb->method('func')->willReturn($functionBuilder); + $functionBuilder->method('count')->willReturn($this->createMock(IQueryFunction::class)); + + $qb->method('selectDistinct') + ->with(['id', 'principaluri']) + ->willReturn($qb); + + $qb->method('from') + ->with('calendarsubscriptions') + ->willReturn($qb); + + $qb->method('setMaxResults') + ->willReturn($qb); + + $qb->method('setFirstResult') + ->willReturn($qb); + + $result = $this->createMock(IResult::class); + + $qb->method('execute') + ->willReturn($result); + + $result->expects($this->at(0)) + ->method('fetchOne') + ->willReturn(count($subscriptions)); + + $result + ->method('fetch') + ->willReturnOnConsecutiveCalls(...$subscriptions); + + $qb->method('delete') + ->with('calendarsubscriptions') + ->willReturn($qb); + + $expr = $this->createMock(IExpressionBuilder::class); + + $qb->method('expr')->willReturn($expr); + $qb->method('createNamedParameter')->willReturn($this->createMock(IParameter::class)); + $qb->method('where')->willReturn($qb); + // Only when user exists + $qb->expects($this->exactly($deletions))->method('executeStatement'); + + $this->dbConnection->method('getQueryBuilder')->willReturn($qb); + + + $this->output->expects($this->once())->method('startProgress'); + + $this->output->expects($subscriptions === [] ? $this->never(): $this->once())->method('advance'); + if (count($subscriptions)) { + $this->userManager->method('userExists') + ->willReturnCallback(function (string $username) use ($userExists) { + return $userExists[$username]; + }); + } + $this->output->expects($this->once())->method('finishProgress'); + $this->output->expects($this->once())->method('info')->with(sprintf('%d calendar subscriptions without an user have been cleaned up', $deletions)); + + $this->migration->run($this->output); + } + + public function dataTestRun(): array { + return [ + [[], [], 0], + [[[ + 'id' => 1, + 'principaluri' => 'users/principals/foo1', + ], + [ + 'id' => 2, + 'principaluri' => 'users/principals/bar1', + ], + [ + 'id' => 3, + 'principaluri' => 'users/principals/bar1', + ]], ['foo1' => true, 'bar1' => false], 2] + ]; + } +}