Skip to content

Commit 517d89d

Browse files
tcitworldMichaIng
tcitworld
authored and
MichaIng
committed
Add repair job to delete calendar subscriptions that were orphaned when
deleteding an user Follow-up to #28419 Signed-off-by: Thomas Citharel <tcit@tcit.fr>
1 parent b8ce28f commit 517d89d

File tree

5 files changed

+317
-1
lines changed

5 files changed

+317
-1
lines changed

‎apps/dav/appinfo/info.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<name>WebDAV</name>
66
<summary>WebDAV endpoint</summary>
77
<description>WebDAV endpoint</description>
8-
<version>1.19.0</version>
8+
<version>1.20.0</version>
99
<licence>agpl</licence>
1010
<author>owncloud.org</author>
1111
<namespace>DAV</namespace>
@@ -38,6 +38,7 @@
3838
<step>OCA\DAV\Migration\RegisterBuildReminderIndexBackgroundJob</step>
3939
<step>OCA\DAV\Migration\RemoveOrphanEventsAndContacts</step>
4040
<step>OCA\DAV\Migration\RemoveClassifiedEventActivity</step>
41+
<step>OCA\DAV\Migration\RemoveDeletedUsersCalendarSubscriptions</step>
4142
</post-migration>
4243
<live-migration>
4344
<step>OCA\DAV\Migration\ChunkCleanup</step>

‎apps/dav/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@
242242
'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => $baseDir . '/../lib/Migration/RegenerateBirthdayCalendars.php',
243243
'OCA\\DAV\\Migration\\RegisterBuildReminderIndexBackgroundJob' => $baseDir . '/../lib/Migration/RegisterBuildReminderIndexBackgroundJob.php',
244244
'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => $baseDir . '/../lib/Migration/RemoveClassifiedEventActivity.php',
245+
'OCA\\DAV\\Migration\\RemoveDeletedUsersCalendarSubscriptions' => $baseDir . '/../lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php',
245246
'OCA\\DAV\\Migration\\RemoveOrphanEventsAndContacts' => $baseDir . '/../lib/Migration/RemoveOrphanEventsAndContacts.php',
246247
'OCA\\DAV\\Migration\\Version1004Date20170825134824' => $baseDir . '/../lib/Migration/Version1004Date20170825134824.php',
247248
'OCA\\DAV\\Migration\\Version1004Date20170919104507' => $baseDir . '/../lib/Migration/Version1004Date20170919104507.php',

‎apps/dav/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ class ComposerStaticInitDAV
257257
'OCA\\DAV\\Migration\\RegenerateBirthdayCalendars' => __DIR__ . '/..' . '/../lib/Migration/RegenerateBirthdayCalendars.php',
258258
'OCA\\DAV\\Migration\\RegisterBuildReminderIndexBackgroundJob' => __DIR__ . '/..' . '/../lib/Migration/RegisterBuildReminderIndexBackgroundJob.php',
259259
'OCA\\DAV\\Migration\\RemoveClassifiedEventActivity' => __DIR__ . '/..' . '/../lib/Migration/RemoveClassifiedEventActivity.php',
260+
'OCA\\DAV\\Migration\\RemoveDeletedUsersCalendarSubscriptions' => __DIR__ . '/..' . '/../lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php',
260261
'OCA\\DAV\\Migration\\RemoveOrphanEventsAndContacts' => __DIR__ . '/..' . '/../lib/Migration/RemoveOrphanEventsAndContacts.php',
261262
'OCA\\DAV\\Migration\\Version1004Date20170825134824' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170825134824.php',
262263
'OCA\\DAV\\Migration\\Version1004Date20170919104507' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170919104507.php',

‎apps/dav/lib/Migration/RemoveDeletedUsersCalendarSubscriptions.php‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2021 Thomas Citharel <nextcloud@tcit.fr>
7+
*
8+
* @author Thomas Citharel <nextcloud@tcit.fr>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
namespace OCA\DAV\Migration;
27+
28+
use OCP\DB\Exception;
29+
use OCP\IDBConnection;
30+
use OCP\IUserManager;
31+
use OCP\Migration\IOutput;
32+
use OCP\Migration\IRepairStep;
33+
34+
class RemoveDeletedUsersCalendarSubscriptions implements IRepairStep {
35+
/** @var IDBConnection */
36+
private $connection;
37+
38+
/** @var IUserManager */
39+
private $userManager;
40+
41+
/** @var int */
42+
private $progress = 0;
43+
44+
private $orphanSubscriptions = [];
45+
46+
private const SUBSCRIPTIONS_CHUNK_SIZE = 1000;
47+
48+
public function __construct(IDBConnection $connection, IUserManager $userManager) {
49+
$this->connection = $connection;
50+
$this->userManager = $userManager;
51+
}
52+
53+
/**
54+
* @inheritdoc
55+
*/
56+
public function getName(): string {
57+
return 'Clean up old calendar subscriptions from deleted users that were not cleaned-up';
58+
}
59+
60+
/**
61+
* @inheritdoc
62+
*/
63+
public function run(IOutput $output) {
64+
$nbSubscriptions = $this->countSubscriptions();
65+
66+
$output->startProgress($nbSubscriptions);
67+
68+
while ($this->progress < $nbSubscriptions) {
69+
$this->checkSubscriptions();
70+
71+
$this->progress += self::SUBSCRIPTIONS_CHUNK_SIZE;
72+
$output->advance(min(self::SUBSCRIPTIONS_CHUNK_SIZE, $nbSubscriptions));
73+
}
74+
$output->finishProgress();
75+
$this->deleteOrphanSubscriptions();
76+
77+
$output->info(sprintf('%d calendar subscriptions without an user have been cleaned up', count($this->orphanSubscriptions)));
78+
}
79+
80+
/**
81+
* @throws Exception
82+
*/
83+
private function countSubscriptions(): int {
84+
$qb = $this->connection->getQueryBuilder();
85+
$query = $qb->select($qb->func()->count('*'))
86+
->from('calendarsubscriptions');
87+
88+
$result = $query->execute();
89+
$count = $result->fetchOne();
90+
$result->closeCursor();
91+
92+
if ($count !== false) {
93+
$count = (int)$count;
94+
} else {
95+
$count = 0;
96+
}
97+
98+
return $count;
99+
}
100+
101+
/**
102+
* @throws Exception
103+
*/
104+
private function checkSubscriptions(): void {
105+
$qb = $this->connection->getQueryBuilder();
106+
$query = $qb->selectDistinct(['id', 'principaluri'])
107+
->from('calendarsubscriptions')
108+
->setMaxResults(self::SUBSCRIPTIONS_CHUNK_SIZE)
109+
->setFirstResult($this->progress);
110+
111+
$result = $query->execute();
112+
while ($row = $result->fetch()) {
113+
$username = $this->getPrincipal($row['principaluri']);
114+
if (!$this->userManager->userExists($username)) {
115+
$this->orphanSubscriptions[] = $row['id'];
116+
}
117+
}
118+
$result->closeCursor();
119+
}
120+
121+
/**
122+
* @throws Exception
123+
*/
124+
private function deleteOrphanSubscriptions(): void {
125+
foreach ($this->orphanSubscriptions as $orphanSubscriptionID) {
126+
$this->deleteOrphanSubscription($orphanSubscriptionID);
127+
}
128+
}
129+
130+
/**
131+
* @throws Exception
132+
*/
133+
private function deleteOrphanSubscription(int $orphanSubscriptionID): void {
134+
$qb = $this->connection->getQueryBuilder();
135+
$qb->delete('calendarsubscriptions')
136+
->where($qb->expr()->eq('id', $qb->createNamedParameter($orphanSubscriptionID)))
137+
->executeStatement();
138+
}
139+
140+
private function getPrincipal(string $principalUri): string {
141+
$uri = explode('/', $principalUri);
142+
return array_pop($uri);
143+
}
144+
}

‎apps/dav/tests/unit/Migration/RemoveDeletedUsersCalendarSubscriptionsTest.php‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2021 Thomas Citharel <nextcloud@tcit.fr>
7+
*
8+
* @author Thomas Citharel <nextcloud@tcit.fr>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
namespace OCA\DAV\Tests\unit\DAV\Migration;
27+
28+
use OCA\DAV\Migration\RemoveDeletedUsersCalendarSubscriptions;
29+
use OCP\DB\IResult;
30+
use OCP\DB\QueryBuilder\IExpressionBuilder;
31+
use OCP\DB\QueryBuilder\IFunctionBuilder;
32+
use OCP\DB\QueryBuilder\IParameter;
33+
use OCP\DB\QueryBuilder\IQueryBuilder;
34+
use OCP\DB\QueryBuilder\IQueryFunction;
35+
use OCP\IDBConnection;
36+
use OCP\IUserManager;
37+
use OCP\Migration\IOutput;
38+
use PHPUnit\Framework\MockObject\MockObject;
39+
use Test\TestCase;
40+
41+
class RemoveDeletedUsersCalendarSubscriptionsTest extends TestCase {
42+
/**
43+
* @var IDBConnection|MockObject
44+
*/
45+
private $dbConnection;
46+
/**
47+
* @var IUserManager|MockObject
48+
*/
49+
private $userManager;
50+
51+
/**
52+
* @var IOutput|MockObject
53+
*/
54+
private $output;
55+
/**
56+
* @var RemoveDeletedUsersCalendarSubscriptions
57+
*/
58+
private $migration;
59+
60+
61+
protected function setUp(): void {
62+
parent::setUp();
63+
64+
$this->dbConnection = $this->createMock(IDBConnection::class);
65+
$this->userManager = $this->createMock(IUserManager::class);
66+
$this->output = $this->createMock(IOutput::class);
67+
68+
$this->migration = new RemoveDeletedUsersCalendarSubscriptions($this->dbConnection, $this->userManager);
69+
}
70+
71+
public function testGetName(): void {
72+
$this->assertEquals(
73+
'Clean up old calendar subscriptions from deleted users that were not cleaned-up',
74+
$this->migration->getName()
75+
);
76+
}
77+
78+
/**
79+
* @dataProvider dataTestRun
80+
* @param array $subscriptions
81+
* @param array $userExists
82+
* @param int $deletions
83+
* @throws \Exception
84+
*/
85+
public function testRun(array $subscriptions, array $userExists, int $deletions): void {
86+
$qb = $this->createMock(IQueryBuilder::class);
87+
88+
$qb->method('select')->willReturn($qb);
89+
90+
$functionBuilder = $this->createMock(IFunctionBuilder::class);
91+
92+
$qb->method('func')->willReturn($functionBuilder);
93+
$functionBuilder->method('count')->willReturn($this->createMock(IQueryFunction::class));
94+
95+
$qb->method('selectDistinct')
96+
->with(['id', 'principaluri'])
97+
->willReturn($qb);
98+
99+
$qb->method('from')
100+
->with('calendarsubscriptions')
101+
->willReturn($qb);
102+
103+
$qb->method('setMaxResults')
104+
->willReturn($qb);
105+
106+
$qb->method('setFirstResult')
107+
->willReturn($qb);
108+
109+
$result = $this->createMock(IResult::class);
110+
111+
$qb->method('execute')
112+
->willReturn($result);
113+
114+
$result->expects($this->at(0))
115+
->method('fetchOne')
116+
->willReturn(count($subscriptions));
117+
118+
$result
119+
->method('fetch')
120+
->willReturnOnConsecutiveCalls(...$subscriptions);
121+
122+
$qb->method('delete')
123+
->with('calendarsubscriptions')
124+
->willReturn($qb);
125+
126+
$expr = $this->createMock(IExpressionBuilder::class);
127+
128+
$qb->method('expr')->willReturn($expr);
129+
$qb->method('createNamedParameter')->willReturn($this->createMock(IParameter::class));
130+
$qb->method('where')->willReturn($qb);
131+
// Only when user exists
132+
$qb->expects($this->exactly($deletions))->method('executeStatement');
133+
134+
$this->dbConnection->method('getQueryBuilder')->willReturn($qb);
135+
136+
137+
$this->output->expects($this->once())->method('startProgress');
138+
139+
$this->output->expects($subscriptions === [] ? $this->never(): $this->once())->method('advance');
140+
if (count($subscriptions)) {
141+
$this->userManager->method('userExists')
142+
->willReturnCallback(function (string $username) use ($userExists) {
143+
return $userExists[$username];
144+
});
145+
}
146+
$this->output->expects($this->once())->method('finishProgress');
147+
$this->output->expects($this->once())->method('info')->with(sprintf('%d calendar subscriptions without an user have been cleaned up', $deletions));
148+
149+
$this->migration->run($this->output);
150+
}
151+
152+
public function dataTestRun(): array {
153+
return [
154+
[[], [], 0],
155+
[[[
156+
'id' => 1,
157+
'principaluri' => 'users/principals/foo1',
158+
],
159+
[
160+
'id' => 2,
161+
'principaluri' => 'users/principals/bar1',
162+
],
163+
[
164+
'id' => 3,
165+
'principaluri' => 'users/principals/bar1',
166+
]], ['foo1' => true, 'bar1' => false], 2]
167+
];
168+
}
169+
}

0 commit comments

Comments
 (0)