Merge pull request #44075 from nextcloud/fix/dav/sync-token-retention-time

pull/44400/head
John Molakvoæ 2 months ago committed by GitHub
commit e17b94cf2b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -5,7 +5,7 @@
<name>WebDAV</name>
<summary>WebDAV endpoint</summary>
<description>WebDAV endpoint</description>
<version>1.30.0</version>
<version>1.30.1</version>
<licence>agpl</licence>
<author>owncloud.org</author>
<namespace>DAV</namespace>

@ -315,6 +315,7 @@ return array(
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => $baseDir . '/../lib/Migration/Version1017Date20210216083742.php',
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => $baseDir . '/../lib/Migration/Version1018Date20210312100735.php',
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => $baseDir . '/../lib/Migration/Version1024Date20211221144219.php',
'OCA\\DAV\\Migration\\Version1025Date20240308063933' => $baseDir . '/../lib/Migration/Version1025Date20240308063933.php',
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => $baseDir . '/../lib/Migration/Version1027Date20230504122946.php',
'OCA\\DAV\\Migration\\Version1029Date20221114151721' => $baseDir . '/../lib/Migration/Version1029Date20221114151721.php',
'OCA\\DAV\\Migration\\Version1029Date20231004091403' => $baseDir . '/../lib/Migration/Version1029Date20231004091403.php',

@ -330,6 +330,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => __DIR__ . '/..' . '/../lib/Migration/Version1017Date20210216083742.php',
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => __DIR__ . '/..' . '/../lib/Migration/Version1018Date20210312100735.php',
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => __DIR__ . '/..' . '/../lib/Migration/Version1024Date20211221144219.php',
'OCA\\DAV\\Migration\\Version1025Date20240308063933' => __DIR__ . '/..' . '/../lib/Migration/Version1025Date20240308063933.php',
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => __DIR__ . '/..' . '/../lib/Migration/Version1027Date20230504122946.php',
'OCA\\DAV\\Migration\\Version1029Date20221114151721' => __DIR__ . '/..' . '/../lib/Migration/Version1029Date20221114151721.php',
'OCA\\DAV\\Migration\\Version1029Date20231004091403' => __DIR__ . '/..' . '/../lib/Migration/Version1029Date20231004091403.php',

@ -52,9 +52,10 @@ class PruneOutdatedSyncTokensJob extends TimedJob {
public function run($argument) {
$limit = max(1, (int) $this->config->getAppValue(Application::APP_ID, 'totalNumberOfSyncTokensToKeep', '10000'));
$retention = max(7, (int) $this->config->getAppValue(Application::APP_ID, 'syncTokensRetentionDays', '60')) * 24 * 3600;
$prunedCalendarSyncTokens = $this->calDavBackend->pruneOutdatedSyncTokens($limit);
$prunedAddressBookSyncTokens = $this->cardDavBackend->pruneOutdatedSyncTokens($limit);
$prunedCalendarSyncTokens = $this->calDavBackend->pruneOutdatedSyncTokens($limit, $retention);
$prunedAddressBookSyncTokens = $this->cardDavBackend->pruneOutdatedSyncTokens($limit, $retention);
$this->logger->info('Pruned {calendarSyncTokensNumber} calendar sync tokens and {addressBooksSyncTokensNumber} address book sync tokens', [
'calendarSyncTokensNumber' => $prunedCalendarSyncTokens,

@ -2785,6 +2785,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
'calendarid' => $query->createNamedParameter($calendarId),
'operation' => $query->createNamedParameter($operation),
'calendartype' => $query->createNamedParameter($calendarType),
'created_at' => time(),
]);
foreach ($objectUris as $uri) {
$query->setParameter('uri', $uri);
@ -3259,7 +3260,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
/**
* @throws \InvalidArgumentException
*/
public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
public function pruneOutdatedSyncTokens(int $keep, int $retention): int {
if ($keep < 0) {
throw new \InvalidArgumentException();
}
@ -3277,7 +3278,10 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
$query = $this->db->getQueryBuilder();
$query->delete('calendarchanges')
->where($query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
->where(
$query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT),
$query->expr()->lte('created_at', $query->createNamedParameter($retention)),
);
return $query->executeStatement();
}

@ -983,6 +983,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
'synctoken' => $query->createNamedParameter($syncToken),
'addressbookid' => $query->createNamedParameter($addressBookId),
'operation' => $query->createNamedParameter($operation),
'created_at' => time(),
])
->executeStatement();
@ -1415,7 +1416,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
/**
* @throws \InvalidArgumentException
*/
public function pruneOutdatedSyncTokens(int $keep = 10_000): int {
public function pruneOutdatedSyncTokens(int $keep, int $retention): int {
if ($keep < 0) {
throw new \InvalidArgumentException();
}
@ -1433,7 +1434,10 @@ class CardDavBackend implements BackendInterface, SyncSupport {
$query = $this->db->getQueryBuilder();
$query->delete('addressbookchanges')
->where($query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
->where(
$query->expr()->lte('id', $query->createNamedParameter($maxId - $keep, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT),
$query->expr()->lte('created_at', $query->createNamedParameter($retention)),
);
return $query->executeStatement();
}

@ -0,0 +1,84 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2024 Christoph Wurst <christoph@winzerhof-wurst.at>
*
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\DAV\Migration;
use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\DB\Types;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
class Version1025Date20240308063933 extends SimpleMigrationStep {
private IDBConnection $db;
public function __construct(IDBConnection $db) {
$this->db = $db;
}
/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
foreach (['addressbookchanges', 'calendarchanges'] as $tableName) {
$table = $schema->getTable($tableName);
if (!$table->hasColumn('created_at')) {
$table->addColumn('created_at', Types::INTEGER, [
'notnull' => true,
'length' => 4,
'default' => 0,
]);
}
}
return $schema;
}
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options): void {
foreach (['addressbookchanges', 'calendarchanges'] as $tableName) {
$qb = $this->db->getQueryBuilder();
$update = $qb->update($tableName)
->set('created_at', $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT))
->where(
$qb->expr()->eq('created_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)),
);
$updated = $update->executeStatement();
$output->debug('Added a default creation timestamp to ' . $updated . ' rows in ' . $tableName);
}
}
}

@ -29,6 +29,7 @@ declare(strict_types=1);
*/
namespace OCA\DAV\Tests\unit\BackgroundJob;
use InvalidArgumentException;
use OCA\DAV\AppInfo\Application;
use OCA\DAV\BackgroundJob\PruneOutdatedSyncTokensJob;
use OCA\DAV\CalDAV\CalDavBackend;
@ -72,18 +73,27 @@ class PruneOutdatedSyncTokensJobTest extends TestCase {
/**
* @dataProvider dataForTestRun
*/
public function testRun(string $configValue, int $actualLimit, int $deletedCalendarSyncTokens, int $deletedAddressBookSyncTokens): void {
$this->config->expects($this->once())
public function testRun(string $configToKeep, string $configRetentionDays, int $actualLimit, int $retentionDays, int $deletedCalendarSyncTokens, int $deletedAddressBookSyncTokens): void {
$this->config->expects($this->exactly(2))
->method('getAppValue')
->with(Application::APP_ID, 'totalNumberOfSyncTokensToKeep', '10000')
->willReturn($configValue);
->with(Application::APP_ID, self::anything(), self::anything())
->willReturnCallback(function ($app, $key) use ($configToKeep, $configRetentionDays) {
switch ($key) {
case 'totalNumberOfSyncTokensToKeep':
return $configToKeep;
case 'syncTokensRetentionDays':
return $configRetentionDays;
default:
throw new InvalidArgumentException();
}
});
$this->calDavBackend->expects($this->once())
->method('pruneOutdatedSyncTokens')
->with($actualLimit)
->willReturn($deletedCalendarSyncTokens);
$this->cardDavBackend->expects($this->once())
->method('pruneOutdatedSyncTokens')
->with($actualLimit)
->with($actualLimit, $retentionDays)
->willReturn($deletedAddressBookSyncTokens);
$this->logger->expects($this->once())
->method('info')
@ -97,8 +107,9 @@ class PruneOutdatedSyncTokensJobTest extends TestCase {
public function dataForTestRun(): array {
return [
['100', 100, 2, 3],
['0', 1, 0, 0]
['100', '2', 100, 7 * 24 * 3600, 2, 3],
['100', '14', 100, 14 * 24 * 3600, 2, 3],
['0', '60', 1, 60 * 24 * 3600, 0, 0]
];
}
}

@ -46,6 +46,7 @@ use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\PropPatch;
use Sabre\DAV\Xml\Property\Href;
use Sabre\DAVACL\IACL;
use function time;
/**
* Class CalDavBackendTest
@ -1357,7 +1358,12 @@ END:VEVENT
END:VCALENDAR
EOD;
$this->backend->updateCalendarObject($calendarId, $uri, $calData);
$deleted = $this->backend->pruneOutdatedSyncTokens(0);
// Keep everything
$deleted = $this->backend->pruneOutdatedSyncTokens(0, 0);
self::assertSame(0, $deleted);
$deleted = $this->backend->pruneOutdatedSyncTokens(0, time());
// At least one from the object creation and one from the object update
$this->assertGreaterThanOrEqual(2, $deleted);
$changes = $this->backend->getChangesForCalendar($calendarId, $syncToken, 1);
@ -1423,7 +1429,7 @@ EOD;
$this->assertEmpty($changes['deleted']);
// Delete all but last change
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
$this->assertEquals(1, $deleted); // We had two changes before, now one
// Only update should remain
@ -1433,7 +1439,8 @@ EOD;
$this->assertEmpty($changes['deleted']);
// Check that no crash occurs when prune is called without current changes
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
self::assertSame(0, $deleted);
}
public function testSearchAndExpandRecurrences() {

@ -60,6 +60,7 @@ use Sabre\DAV\PropPatch;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;
use Test\TestCase;
use function time;
/**
* Class CardDavBackendTest
@ -880,7 +881,12 @@ class CardDavBackendTest extends TestCase {
$uri = $this->getUniqueID('card');
$this->backend->createCard($addressBookId, $uri, $this->vcardTest0);
$this->backend->updateCard($addressBookId, $uri, $this->vcardTest1);
$deleted = $this->backend->pruneOutdatedSyncTokens(0);
// Do not delete anything if week data as old as ts=0
$deleted = $this->backend->pruneOutdatedSyncTokens(0, 0);
self::assertSame(0, $deleted);
$deleted = $this->backend->pruneOutdatedSyncTokens(0, time());
// At least one from the object creation and one from the object update
$this->assertGreaterThanOrEqual(2, $deleted);
$changes = $this->backend->getChangesForAddressBook($addressBookId, $syncToken, 1);
@ -912,7 +918,7 @@ class CardDavBackendTest extends TestCase {
$this->assertEmpty($changes['deleted']);
// Delete all but last change
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
$this->assertEquals(1, $deleted); // We had two changes before, now one
// Only update should remain
@ -920,8 +926,8 @@ class CardDavBackendTest extends TestCase {
$this->assertEmpty($changes['added']);
$this->assertEquals(1, count($changes['modified']));
$this->assertEmpty($changes['deleted']);
// Check that no crash occurs when prune is called without current changes
$deleted = $this->backend->pruneOutdatedSyncTokens(1);
$deleted = $this->backend->pruneOutdatedSyncTokens(1, time());
}
}

Loading…
Cancel
Save