Skip to content

Commit 8633a10

Browse files
ChristophWurstbackportbot[bot]
ChristophWurst
authored and
backportbot[bot]
committed
fix(dav): Add retention time to sync token cleanup
Signed-off-by: Christoph Wurst <[email protected]> [skip ci]
1 parent 82bcbce commit 8633a10

9 files changed

Lines changed: 138 additions & 20 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@
307307
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => $baseDir . '/../lib/Migration/Version1017Date20210216083742.php',
308308
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => $baseDir . '/../lib/Migration/Version1018Date20210312100735.php',
309309
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => $baseDir . '/../lib/Migration/Version1024Date20211221144219.php',
310+
'OCA\\DAV\\Migration\\Version1025Date20240308063933' => $baseDir . '/../lib/Migration/Version1025Date20240308063933.php',
310311
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => $baseDir . '/../lib/Migration/Version1027Date20230504122946.php',
311312
'OCA\\DAV\\Migration\\Version1029Date20221114151721' => $baseDir . '/../lib/Migration/Version1029Date20221114151721.php',
312313
'OCA\\DAV\\Migration\\Version1029Date20231004091403' => $baseDir . '/../lib/Migration/Version1029Date20231004091403.php',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ class ComposerStaticInitDAV
322322
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => __DIR__ . '/..' . '/../lib/Migration/Version1017Date20210216083742.php',
323323
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => __DIR__ . '/..' . '/../lib/Migration/Version1018Date20210312100735.php',
324324
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => __DIR__ . '/..' . '/../lib/Migration/Version1024Date20211221144219.php',
325+
'OCA\\DAV\\Migration\\Version1025Date20240308063933' => __DIR__ . '/..' . '/../lib/Migration/Version1025Date20240308063933.php',
325326
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => __DIR__ . '/..' . '/../lib/Migration/Version1027Date20230504122946.php',
326327
'OCA\\DAV\\Migration\\Version1029Date20221114151721' => __DIR__ . '/..' . '/../lib/Migration/Version1029Date20221114151721.php',
327328
'OCA\\DAV\\Migration\\Version1029Date20231004091403' => __DIR__ . '/..' . '/../lib/Migration/Version1029Date20231004091403.php',

‎apps/dav/lib/BackgroundJob/PruneOutdatedSyncTokensJob.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,10 @@ public function __construct(ITimeFactory $timeFactory, CalDavBackend $calDavBack
5252

5353
public function run($argument) {
5454
$limit = max(1, (int) $this->config->getAppValue(Application::APP_ID, 'totalNumberOfSyncTokensToKeep', '10000'));
55+
$retention = max(7, (int) $this->config->getAppValue(Application::APP_ID, 'syncTokensRetentionDays', '60')) * 24 * 3600;
5556

56-
$prunedCalendarSyncTokens = $this->calDavBackend->pruneOutdatedSyncTokens($limit);
57-
$prunedAddressBookSyncTokens = $this->cardDavBackend->pruneOutdatedSyncTokens($limit);
57+
$prunedCalendarSyncTokens = $this->calDavBackend->pruneOutdatedSyncTokens($limit, $retention);
58+
$prunedAddressBookSyncTokens = $this->cardDavBackend->pruneOutdatedSyncTokens($limit, $retention);
5859

5960
$this->logger->info('Pruned {calendarSyncTokensNumber} calendar sync tokens and {addressBooksSyncTokensNumber} address book sync tokens', [
6061
'calendarSyncTokensNumber' => $prunedCalendarSyncTokens,

‎apps/dav/lib/CalDAV/CalDavBackend.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3206,7 +3206,7 @@ protected function getCalendarObjectId($calendarId, $uri, $calendarType):int {
32063206
/**
32073207
* @throws \InvalidArgumentException
32083208
*/
3209-
public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
3209+
public function pruneOutdatedSyncTokens(int $keep, int $retention): int {
32103210
if ($keep < 0) {
32113211
throw new \InvalidArgumentException();
32123212
}
@@ -3224,7 +3224,10 @@ public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
32243224

32253225
$query = $this->db->getQueryBuilder();
32263226
$query->delete('calendarchanges')
3227-
->where($query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
3227+
->where(
3228+
$query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT),
3229+
$query->expr()->lte('created_at', $query->createNamedParameter($retention)),
3230+
);
32283231
return $query->executeStatement();
32293232
}
32303233

‎apps/dav/lib/CardDAV/CardDavBackend.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,7 @@ protected function addChange(int $addressBookId, string $objectUri, int $operati
993993
'synctoken' => $query->createNamedParameter($syncToken),
994994
'addressbookid' => $query->createNamedParameter($addressBookId),
995995
'operation' => $query->createNamedParameter($operation),
996+
'created_at' => time(),
996997
])
997998
->executeStatement();
998999

@@ -1424,7 +1425,7 @@ public function applyShareAcl(int $addressBookId, array $acl): array {
14241425
/**
14251426
* @throws \InvalidArgumentException
14261427
*/
1427-
public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
1428+
public function pruneOutdatedSyncTokens(int $keep, int $retention): int {
14281429
if ($keep < 0) {
14291430
throw new \InvalidArgumentException();
14301431
}
@@ -1442,7 +1443,10 @@ public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
14421443

14431444
$query = $this->db->getQueryBuilder();
14441445
$query->delete('addressbookchanges')
1445-
->where($query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
1446+
->where(
1447+
$query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT),
1448+
$query->expr()->lte('created_at', $query->createNamedParameter($retention)),
1449+
);
14461450
return $query->executeStatement();
14471451
}
14481452

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2024 Christoph Wurst <[email protected]>
7+
*
8+
* @author Christoph Wurst <[email protected]>
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+
27+
namespace OCA\DAV\Migration;
28+
29+
use Closure;
30+
use OCP\DB\ISchemaWrapper;
31+
use OCP\DB\QueryBuilder\IQueryBuilder;
32+
use OCP\DB\Types;
33+
use OCP\IDBConnection;
34+
use OCP\Migration\IOutput;
35+
use OCP\Migration\SimpleMigrationStep;
36+
37+
class Version1025Date20240308063933 extends SimpleMigrationStep {
38+
39+
private IDBConnection $db;
40+
41+
public function __construct(IDBConnection $db) {
42+
$this->db = $db;
43+
}
44+
45+
/**
46+
* @param IOutput $output
47+
* @param Closure(): ISchemaWrapper $schemaClosure
48+
* @param array $options
49+
* @return null|ISchemaWrapper
50+
*/
51+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
52+
/** @var ISchemaWrapper $schema */
53+
$schema = $schemaClosure();
54+
55+
foreach (['addressbookchanges', 'calendarchanges'] as $tableName) {
56+
$table = $schema->getTable($tableName);
57+
if (!$table->hasColumn('created_at')) {
58+
$table->addColumn('created_at', Types::INTEGER, [
59+
'notnull' => true,
60+
'length' => 4,
61+
'default' => 0,
62+
]);
63+
}
64+
}
65+
66+
return $schema;
67+
}
68+
69+
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options): void {
70+
foreach (['addressbookchanges', 'calendarchanges'] as $tableName) {
71+
$qb = $this->db->getQueryBuilder();
72+
73+
$update = $qb->update($tableName)
74+
->set('created_at', $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT))
75+
->where(
76+
$qb->expr()->eq('created_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)),
77+
);
78+
79+
$updated = $update->executeStatement();
80+
$output->debug('Added a default creation timestamp to ' . $updated . ' rows in ' . $tableName);
81+
}
82+
}
83+
84+
}

‎apps/dav/tests/unit/BackgroundJob/PruneOutdatedSyncTokensJobTest.php‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
*/
3030
namespace OCA\DAV\Tests\unit\BackgroundJob;
3131

32+
use InvalidArgumentException;
3233
use OCA\DAV\AppInfo\Application;
3334
use OCA\DAV\BackgroundJob\PruneOutdatedSyncTokensJob;
3435
use OCA\DAV\CalDAV\CalDavBackend;
@@ -72,18 +73,27 @@ protected function setUp(): void {
7273
/**
7374
* @dataProvider dataForTestRun
7475
*/
75-
public function testRun(string $configValue, int $actualLimit, int $deletedCalendarSyncTokens, int $deletedAddressBookSyncTokens): void {
76-
$this->config->expects($this->once())
76+
public function testRun(string $configToKeep, string $configRetentionDays, int $actualLimit, int $retentionDays, int $deletedCalendarSyncTokens, int $deletedAddressBookSyncTokens): void {
77+
$this->config->expects($this->exactly(2))
7778
->method('getAppValue')
78-
->with(Application::APP_ID, 'totalNumberOfSyncTokensToKeep', '10000')
79-
->willReturn($configValue);
79+
->with(Application::APP_ID, self::anything(), self::anything())
80+
->willReturnCallback(function ($app, $key) use ($configToKeep, $configRetentionDays) {
81+
switch ($key) {
82+
case 'totalNumberOfSyncTokensToKeep':
83+
return $configToKeep;
84+
case 'syncTokensRetentionDays':
85+
return $configRetentionDays;
86+
default:
87+
throw new InvalidArgumentException();
88+
}
89+
});
8090
$this->calDavBackend->expects($this->once())
8191
->method('pruneOutdatedSyncTokens')
8292
->with($actualLimit)
8393
->willReturn($deletedCalendarSyncTokens);
8494
$this->cardDavBackend->expects($this->once())
8595
->method('pruneOutdatedSyncTokens')
86-
->with($actualLimit)
96+
->with($actualLimit, $retentionDays)
8797
->willReturn($deletedAddressBookSyncTokens);
8898
$this->logger->expects($this->once())
8999
->method('info')
@@ -97,8 +107,9 @@ public function testRun(string $configValue, int $actualLimit, int $deletedCalen
97107

98108
public function dataForTestRun(): array {
99109
return [
100-
['100', 100, 2, 3],
101-
['0', 1, 0, 0]
110+
['100', '2', 100, 7 * 24 * 3600, 2, 3],
111+
['100', '14', 100, 14 * 24 * 3600, 2, 3],
112+
['0', '60', 1, 60 * 24 * 3600, 0, 0]
102113
];
103114
}
104115
}

‎apps/dav/tests/unit/CalDAV/CalDavBackendTest.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use Sabre\DAV\PropPatch;
4747
use Sabre\DAV\Xml\Property\Href;
4848
use Sabre\DAVACL\IACL;
49+
use function time;
4950

5051
/**
5152
* Class CalDavBackendTest
@@ -1344,7 +1345,12 @@ public function testPruneOutdatedSyncTokens(): void {
13441345
END:VCALENDAR
13451346
EOD;
13461347
$this->backend->updateCalendarObject($calendarId, $uri, $calData);
1347-
$deleted = $this->backend->pruneOutdatedSyncTokens(0);
1348+
1349+
// Keep everything
1350+
$deleted = $this->backend->pruneOutdatedSyncTokens(0, 0);
1351+
self::assertSame(0, $deleted);
1352+
1353+
$deleted = $this->backend->pruneOutdatedSyncTokens(0, time());
13481354
// At least one from the object creation and one from the object update
13491355
$this->assertGreaterThanOrEqual(2, $deleted);
13501356
$changes = $this->backend->getChangesForCalendar($calendarId, $syncToken, 1);
@@ -1410,7 +1416,7 @@ public function testPruneOutdatedSyncTokens(): void {
14101416
$this->assertEmpty($changes['deleted']);
14111417

14121418
// Delete all but last change
1413-
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
1419+
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
14141420
$this->assertEquals(1, $deleted); // We had two changes before, now one
14151421

14161422
// Only update should remain
@@ -1420,7 +1426,8 @@ public function testPruneOutdatedSyncTokens(): void {
14201426
$this->assertEmpty($changes['deleted']);
14211427

14221428
// Check that no crash occurs when prune is called without current changes
1423-
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
1429+
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
1430+
self::assertSame(0, $deleted);
14241431
}
14251432

14261433
public function testSearchAndExpandRecurrences() {

‎apps/dav/tests/unit/CardDAV/CardDavBackendTest.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
use Sabre\VObject\Component\VCard;
5656
use Sabre\VObject\Property\Text;
5757
use Test\TestCase;
58+
use function time;
5859

5960
/**
6061
* Class CardDavBackendTest
@@ -859,7 +860,12 @@ public function testPruneOutdatedSyncTokens(): void {
859860
$uri = $this->getUniqueID('card');
860861
$this->backend->createCard($addressBookId, $uri, $this->vcardTest0);
861862
$this->backend->updateCard($addressBookId, $uri, $this->vcardTest1);
862-
$deleted = $this->backend->pruneOutdatedSyncTokens(0);
863+
864+
// Do not delete anything if week data as old as ts=0
865+
$deleted = $this->backend->pruneOutdatedSyncTokens(0, 0);
866+
self::assertSame(0, $deleted);
867+
868+
$deleted = $this->backend->pruneOutdatedSyncTokens(0, time());
863869
// At least one from the object creation and one from the object update
864870
$this->assertGreaterThanOrEqual(2, $deleted);
865871
$changes = $this->backend->getChangesForAddressBook($addressBookId, $syncToken, 1);
@@ -891,16 +897,16 @@ public function testPruneOutdatedSyncTokens(): void {
891897
$this->assertEmpty($changes['deleted']);
892898

893899
// Delete all but last change
894-
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
900+
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
895901
$this->assertEquals(1, $deleted); // We had two changes before, now one
896902

897903
// Only update should remain
898904
$changes = $this->backend->getChangesForAddressBook($addressBookId, $syncToken, 100);
899905
$this->assertEmpty($changes['added']);
900906
$this->assertEquals(1, count($changes['modified']));
901907
$this->assertEmpty($changes['deleted']);
902-
908+
903909
// Check that no crash occurs when prune is called without current changes
904-
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
910+
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
905911
}
906912
}

0 commit comments

Comments
 (0)