Merge pull request #12071 from nextcloud/addressbook-uid-check-migration

Addressbook uid check migration
This commit is contained in:
Roeland Jago Douma 2018-11-02 19:37:48 +01:00 committed by GitHub
commit 9d89f8bbac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 371 additions and 23 deletions

View File

@ -156,6 +156,7 @@ return array(
'OCA\\DAV\\Migration\\Version1005Date20180530124431' => $baseDir . '/../lib/Migration/Version1005Date20180530124431.php',
'OCA\\DAV\\Migration\\Version1006Date20180619154313' => $baseDir . '/../lib/Migration/Version1006Date20180619154313.php',
'OCA\\DAV\\Migration\\Version1007Date20181007225117' => $baseDir . '/../lib/Migration/Version1007Date20181007225117.php',
'OCA\\DAV\\Migration\\Version1008Date20181030113700' => $baseDir . '/../lib/Migration/Version1008Date20181030113700.php',
'OCA\\DAV\\RootCollection' => $baseDir . '/../lib/RootCollection.php',
'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php',
'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php',

View File

@ -171,6 +171,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Migration\\Version1005Date20180530124431' => __DIR__ . '/..' . '/../lib/Migration/Version1005Date20180530124431.php',
'OCA\\DAV\\Migration\\Version1006Date20180619154313' => __DIR__ . '/..' . '/../lib/Migration/Version1006Date20180619154313.php',
'OCA\\DAV\\Migration\\Version1007Date20181007225117' => __DIR__ . '/..' . '/../lib/Migration/Version1007Date20181007225117.php',
'OCA\\DAV\\Migration\\Version1008Date20181030113700' => __DIR__ . '/..' . '/../lib/Migration/Version1008Date20181030113700.php',
'OCA\\DAV\\RootCollection' => __DIR__ . '/..' . '/../lib/RootCollection.php',
'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php',
'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php',

View File

@ -494,7 +494,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
*/
function getCards($addressBookId) {
$query = $this->db->getQueryBuilder();
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata'])
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
->from('cards')
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)));
@ -525,7 +525,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
*/
function getCard($addressBookId, $cardUri) {
$query = $this->db->getQueryBuilder();
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata'])
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
->from('cards')
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
->andWhere($query->expr()->eq('uri', $query->createNamedParameter($cardUri)))
@ -563,7 +563,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
$cards = [];
$query = $this->db->getQueryBuilder();
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata'])
$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
->from('cards')
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
->andWhere($query->expr()->in('uri', $query->createParameter('uri')));
@ -609,6 +609,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
*/
function createCard($addressBookId, $cardUri, $cardData) {
$etag = md5($cardData);
$uid = $this->getUID($cardData);
$query = $this->db->getQueryBuilder();
$query->insert('cards')
@ -619,6 +620,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
'addressbookid' => $query->createNamedParameter($addressBookId),
'size' => $query->createNamedParameter(strlen($cardData)),
'etag' => $query->createNamedParameter($etag),
'uid' => $query->createNamedParameter($uid),
])
->execute();
@ -661,6 +663,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
*/
function updateCard($addressBookId, $cardUri, $cardData) {
$uid = $this->getUID($cardData);
$etag = md5($cardData);
$query = $this->db->getQueryBuilder();
$query->update('cards')
@ -668,6 +671,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
->set('lastmodified', $query->createNamedParameter(time()))
->set('size', $query->createNamedParameter(strlen($cardData)))
->set('etag', $query->createNamedParameter($etag))
->set('uid', $query->createNamedParameter($uid))
->where($query->expr()->eq('uri', $query->createNamedParameter($cardUri)))
->andWhere($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
->execute();
@ -1125,4 +1129,25 @@ class CardDavBackend implements BackendInterface, SyncSupport {
$addressbookInfo[$displaynameKey] = $principalInformation['{DAV:}displayname'];
}
}
/**
* Extract UID from vcard
*
* @param string $cardData the vcard raw data
* @return string the uid
* @throws BadRequest if no UID is available
*/
private function getUID($cardData) {
if ($cardData != '') {
$vCard = Reader::read($cardData);
if ($vCard->UID) {
$uid = $vCard->UID->getValue();
return $uid;
}
// should already be handled, but just in case
throw new BadRequest('vCards on CardDAV servers MUST have a UID property');
}
// should already be handled, but just in case
throw new BadRequest('vCard can not be empty');
}
}

View File

@ -0,0 +1,52 @@
<?php
/**
* @copyright Copyright (c) 2018, John Molakvoæ (skjnldsv@protonmail.com)
*
* @author John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
*
* @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 Doctrine\DBAL\Types\Type;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\SimpleMigrationStep;
use OCP\Migration\IOutput;
/**
* add column for share notes
*
* Class Version15000Date20180927120000
*/
class Version1008Date20181030113700 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
$table = $schema->getTable('cards');
$table->addColumn('uid', Type::STRING, [
'notnull' => false,
'length' => 255
]);
return $schema;
}
}

View File

@ -87,6 +87,14 @@ class CardDavBackendTest extends TestCase {
const UNIT_TEST_USER1 = 'principals/users/carddav-unit-test1';
const UNIT_TEST_GROUP = 'principals/groups/carddav-unit-test-group';
private $vcardTest = 'BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:Test'.PHP_EOL.
'FN:Test'.PHP_EOL.
'N:Test;;;;'.PHP_EOL.
'END:VCARD';
public function setUp() {
parent::setUp();
@ -121,7 +129,6 @@ class CardDavBackendTest extends TestCase {
$query = $this->db->getQueryBuilder();
$query->delete('cards')->execute();
$this->tearDown();
}
@ -217,8 +224,8 @@ class CardDavBackendTest extends TestCase {
$uri = $this->getUniqueID('card');
// updateProperties is expected twice, once for createCard and once for updateCard
$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, '');
$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, '***');
$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, $this->vcardTest);
$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, $this->vcardTest);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -226,16 +233,16 @@ class CardDavBackendTest extends TestCase {
->with('\OCA\DAV\CardDAV\CardDavBackend::createCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
return $e->getArgument('addressBookId') === $bookId &&
$e->getArgument('cardUri') === $uri &&
$e->getArgument('cardData') === '';
$e->getArgument('cardData') === $this->vcardTest;
}));
// create a card
$backend->createCard($bookId, $uri, '');
$backend->createCard($bookId, $uri, $this->vcardTest);
// get all the cards
$cards = $backend->getCards($bookId);
$this->assertEquals(1, count($cards));
$this->assertEquals('', $cards[0]['carddata']);
$this->assertEquals($this->vcardTest, $cards[0]['carddata']);
// get the cards
$card = $backend->getCard($bookId, $uri);
@ -245,7 +252,7 @@ class CardDavBackendTest extends TestCase {
$this->assertArrayHasKey('lastmodified', $card);
$this->assertArrayHasKey('etag', $card);
$this->assertArrayHasKey('size', $card);
$this->assertEquals('', $card['carddata']);
$this->assertEquals($this->vcardTest, $card['carddata']);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -253,13 +260,13 @@ class CardDavBackendTest extends TestCase {
->with('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
return $e->getArgument('addressBookId') === $bookId &&
$e->getArgument('cardUri') === $uri &&
$e->getArgument('cardData') === '***';
$e->getArgument('cardData') === $this->vcardTest;
}));
// update the card
$backend->updateCard($bookId, $uri, '***');
$backend->updateCard($bookId, $uri, $this->vcardTest);
$card = $backend->getCard($bookId, $uri);
$this->assertEquals('***', $card['carddata']);
$this->assertEquals($this->vcardTest, $card['carddata']);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -290,18 +297,18 @@ class CardDavBackendTest extends TestCase {
// create a card
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, '');
$this->backend->createCard($bookId, $uri0, $this->vcardTest);
$uri1 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri1, '');
$this->backend->createCard($bookId, $uri1, $this->vcardTest);
$uri2 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri2, '');
$this->backend->createCard($bookId, $uri2, $this->vcardTest);
// get all the cards
$cards = $this->backend->getCards($bookId);
$this->assertEquals(3, count($cards));
$this->assertEquals('', $cards[0]['carddata']);
$this->assertEquals('', $cards[1]['carddata']);
$this->assertEquals('', $cards[2]['carddata']);
$this->assertEquals($this->vcardTest, $cards[0]['carddata']);
$this->assertEquals($this->vcardTest, $cards[1]['carddata']);
$this->assertEquals($this->vcardTest, $cards[2]['carddata']);
// get the cards
$cards = $this->backend->getMultipleCards($bookId, [$uri1, $uri2]);
@ -312,7 +319,7 @@ class CardDavBackendTest extends TestCase {
$this->assertArrayHasKey('lastmodified', $card);
$this->assertArrayHasKey('etag', $card);
$this->assertArrayHasKey('size', $card);
$this->assertEquals('', $card['carddata']);
$this->assertEquals($this->vcardTest, $card['carddata']);
}
// delete the card
@ -357,7 +364,7 @@ class CardDavBackendTest extends TestCase {
->method('purgeProperties');
// create a card
$this->backend->createCard($bookId, $uri, '');
$this->backend->createCard($bookId, $uri, $this->vcardTest);
// delete the card
$this->assertTrue($this->backend->deleteCard($bookId, $uri));
@ -380,7 +387,7 @@ class CardDavBackendTest extends TestCase {
// add a change
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, '');
$this->backend->createCard($bookId, $uri0, $this->vcardTest);
// look for changes
$changes = $this->backend->getChangesForAddressBook($bookId, $syncToken, 1);
@ -683,7 +690,7 @@ class CardDavBackendTest extends TestCase {
}
$result = $this->backend->getContact(0, 'uri0');
$this->assertSame(7, count($result));
$this->assertSame(8, count($result));
$this->assertSame(0, (int)$result['addressbookid']);
$this->assertSame('uri0', $result['uri']);
$this->assertSame(5489543, (int)$result['lastmodified']);

View File

@ -966,6 +966,7 @@ return array(
'OC\\Repair\\NC13\\RepairInvalidPaths' => $baseDir . '/lib/private/Repair/NC13/RepairInvalidPaths.php',
'OC\\Repair\\NC14\\AddPreviewBackgroundCleanupJob' => $baseDir . '/lib/private/Repair/NC14/AddPreviewBackgroundCleanupJob.php',
'OC\\Repair\\NC14\\RepairPendingCronJobs' => $baseDir . '/lib/private/Repair/NC14/RepairPendingCronJobs.php',
'OC\\Repair\\NC15\\SetVcardDatabaseUID' => $baseDir . '/lib/private/Repair/NC15/SetVcardDatabaseUID.php',
'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',

View File

@ -996,6 +996,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Repair\\NC13\\RepairInvalidPaths' => __DIR__ . '/../../..' . '/lib/private/Repair/NC13/RepairInvalidPaths.php',
'OC\\Repair\\NC14\\AddPreviewBackgroundCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC14/AddPreviewBackgroundCleanupJob.php',
'OC\\Repair\\NC14\\RepairPendingCronJobs' => __DIR__ . '/../../..' . '/lib/private/Repair/NC14/RepairPendingCronJobs.php',
'OC\\Repair\\NC15\\SetVcardDatabaseUID' => __DIR__ . '/../../..' . '/lib/private/Repair/NC15/SetVcardDatabaseUID.php',
'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',

View File

@ -39,6 +39,7 @@ use OC\Repair\NC11\FixMountStorages;
use OC\Repair\NC13\AddLogRotateJob;
use OC\Repair\NC14\AddPreviewBackgroundCleanupJob;
use OC\Repair\NC14\RepairPendingCronJobs;
use OC\Repair\NC15\SetVcardDatabaseUID;
use OC\Repair\OldGroupMembershipShares;
use OC\Repair\Owncloud\DropAccountTermsTable;
use OC\Repair\Owncloud\SaveAccountsTableData;
@ -139,6 +140,7 @@ class Repair implements IOutput{
new AddPreviewBackgroundCleanupJob(\OC::$server->getJobList()),
new AddCleanupUpdaterBackupsJob(\OC::$server->getJobList()),
new RepairPendingCronJobs(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()),
new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()),
];
}

View File

@ -0,0 +1,137 @@
<?php
/**
* @copyright Copyright (c) 2018 John Molakvoæ <skjnldsv@protonmail.com>
*
* @author John Molakvoæ <skjnldsv@protonmail.com>
*
* @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 OC\Repair\NC15;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
use Sabre\VObject\Reader;
class SetVcardDatabaseUID implements IRepairStep {
const MAX_ROWS = 1000;
/** @var IDBConnection */
private $connection;
/** @var IConfig */
private $config;
private $updateQuery;
public function __construct(IDBConnection $connection, IConfig $config) {
$this->connection = $connection;
$this->config = $config;
}
public function getName() {
return 'Extract the vcard uid and store it in the db';
}
/**
* @return \Generator
* @suppress SqlInjectionChecker
*/
private function getInvalidEntries() {
$builder = $this->connection->getQueryBuilder();
$builder->select('id', 'carddata')
->from('cards')
->where($builder->expr()->isNull('uid'))
->setMaxResults(self::MAX_ROWS);
do {
$result = $builder->execute();
$rows = $result->fetchAll();
foreach ($rows as $row) {
yield $row;
}
$result->closeCursor();
} while (count($rows) > 0);
}
/**
* Extract UID from vcard
*
* @param string $cardData the vcard raw data
* @return string the uid or empty if none
*/
private function getUID(string $cardData): string {
$vCard = Reader::read($cardData);
if ($vCard->UID) {
$uid = $vCard->UID->getValue();
return $uid;
}
return '';
}
/**
* @param int $id
* @param string $uid
*/
private function update(int $id, string $uid) {
if (!$this->updateQuery) {
$builder = $this->connection->getQueryBuilder();
$this->updateQuery = $builder->update('cards')
->set('uid', $builder->createParameter('uid'))
->where($builder->expr()->eq('id', $builder->createParameter('id')));
}
$this->updateQuery->setParameter('id', $id);
$this->updateQuery->setParameter('uid', $uid);
$this->updateQuery->execute();
}
private function repair(): int {
$this->connection->beginTransaction();
$entries = $this->getInvalidEntries();
$count = 0;
foreach ($entries as $entry) {
$count++;
$uid = $this->getUID($entry['carddata']);
$this->update($entry['id'], $uid);
}
$this->connection->commit();
return $count;
}
private function shouldRun() {
$versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0.0');
// was added to 15.0.0.2
return version_compare($versionFromBeforeUpdate, '15.0.0.2', '<=');
}
public function run(IOutput $output) {
if ($this->shouldRun()) {
$count = $this->repair();
$output->info('Fixed ' . $count . ' vcards');
}
}
}

View File

@ -0,0 +1,121 @@
<?php
/**
* @copyright Copyright (c) 2018 John Molakvoæ <skjnldsv@protonmail.com>
*
* @author John Molakvoæ <skjnldsv@protonmail.com>
*
* @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 Test\Repair;
use OCP\IConfig;
use OC\Repair\NC15\SetVcardDatabaseUID;
use Test\TestCase;
/**
* @group DB
*/
class SetVcardDatabaseUIDTest extends TestCase {
/** @var SetVcardDatabaseUID */
private $repair;
/** @var IConfig */
private $config;
protected function setUp() {
parent::setUp();
$this->config = $this->createMock(IConfig::class);
$this->repair = new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), $this->config);
}
protected function tearDown() {
return parent::tearDown();
}
public function dataTestVcards() {
return [
// classic vcard
['BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:Test'.PHP_EOL.
'FN:Test'.PHP_EOL.
'N:Test;;;;'.PHP_EOL.
'END:VCARD', 'Test'],
// UID as url
['BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:https://User@old.domain.com/remote.php/carddav/addressbooks/User/contacts/2EAF6525-17ADC861-38D6BB1D.vcf'.PHP_EOL.
'FN:Test'.PHP_EOL.
'N:Test;;;;'.PHP_EOL.
'END:VCARD', 'https://User@old.domain.com/remote.php/carddav/addressbooks/User/contacts/2EAF6525-17ADC861-38D6BB1D.vcf'],
// No uid
['BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'FN:Test'.PHP_EOL.
'N:Test;;;;'.PHP_EOL.
'END:VCARD', false]
];
}
/**
* @dataProvider dataTestVcards
*
* @param string $from
* @param string|boolean $expected
*/
public function testExtractUIDFromVcard($from, $expected) {
$uid = $this->invokePrivate($this->repair, 'getUid', ['carddata' => $from]);
$this->assertEquals($expected, $uid);
}
public function shouldRunDataProvider() {
return [
['11.0.0.0', true],
['15.0.0.3', false],
['13.0.5.2', true],
['12.0.0.0', true],
['16.0.0.1', false],
['15.0.0.2', true],
['13.0.0.0', true],
['13.0.0.1', true]
];
}
/**
* @dataProvider shouldRunDataProvider
*
* @param string $from
* @param boolean $expected
*/
public function testShouldRun($from, $expected) {
$this->config->expects($this->any())
->method('getSystemValue')
->with('version', '0.0.0.0')
->willReturn($from);
$this->assertEquals($expected, $this->invokePrivate($this->repair, 'shouldRun'));
}
}