Skip to content

Commit 702b669

Browse files
fix: generate UID for contacts with a missing UID
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
1 parent d352924 commit 702b669

2 files changed

Lines changed: 49 additions & 30 deletions

File tree

‎apps/dav/lib/CardDAV/Import/ImportService.php‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,8 @@ public function importProcess($source, AddressBookImpl $addressBook, ContactsImp
100100
continue;
101101
}
102102
$vObject = $value;
103-
// determine if the object has a uid
104103
if (!isset($vObject->UID)) {
105-
$errorMessage = 'One or more objects discovered without a UID';
106-
if ($options->getErrors() === $options::ERROR_FAIL) {
107-
throw new InvalidArgumentException('Error importing calendar data: ' . $errorMessage);
108-
}
109-
yield new ImportObjectEvent(
110-
disposition: ImportDisposition::Error,
111-
identifier: null,
112-
errors: [$errorMessage]
113-
);
114-
continue;
104+
$vObject->UID = UUIDUtil::getUUID();
115105
}
116106
$uid = $vObject->UID->getValue();
117107
// validate object

‎apps/dav/tests/unit/CardDAV/Import/ImportServiceTest.php‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
use OCA\DAV\CardDAV\Import\ImportObjectEvent;
1515
use OCA\DAV\CardDAV\Import\ImportService;
1616
use OCP\Contacts\ContactsImportOptions;
17+
use PHPUnit\Framework\Attributes\DataProvider;
1718
use PHPUnit\Framework\MockObject\MockObject;
19+
use Sabre\VObject\Reader;
1820

1921
class ImportServiceTest extends \Test\TestCase {
2022

@@ -30,40 +32,67 @@ protected function setUp(): void {
3032
$this->addressBook = $this->createMock(AddressBookImpl::class);
3133
}
3234

33-
public function testImportWithoutUID(): void {
34-
// Arrange
35-
// construct stream with two cards that have no UID
35+
public static function validationProvider(): array {
36+
return [
37+
'default validation' => [ContactsImportOptions::VALIDATE_SKIP],
38+
'no validation' => [ContactsImportOptions::VALIDATE_NONE],
39+
'strict validation' => [ContactsImportOptions::VALIDATE_FAIL],
40+
];
41+
}
42+
43+
#[DataProvider('validationProvider')]
44+
public function testImportWithoutUID(int $validation): void {
3645
$stream = fopen('php://memory', 'r+');
3746
fwrite($stream, "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Erika Mustermann\r\nN:Mustermann;Erika;;;\r\nEND:VCARD\r\n"
47+
. "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:existing-uid\r\nFN:Existing Contact\r\nN:Contact;Existing;;;\r\nEND:VCARD\r\n"
3848
. "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Max Mustermann\r\nN:Mustermann;Max;;;\r\nEND:VCARD\r\n");
3949
rewind($stream);
40-
// construct import options
4150
$options = new ContactsImportOptions();
42-
$options->setFormat('vcf');
4351
$options->setCounts(true);
44-
$options->setErrors(ContactsImportOptions::ERROR_CONTINUE);
52+
$options->setValidate($validation);
4553

46-
// Mock address book methods
4754
$this->addressBook->expects($this->once())
4855
->method('getKey')
4956
->willReturn('1');
5057

51-
// Mock backend methods
52-
$this->backend->expects($this->never())
53-
->method('createCard');
58+
$lookedUpUIDs = [];
59+
$this->backend->expects($this->exactly(3))
60+
->method('getCardByUID')
61+
->willReturnCallback(function (int $addressBookId, string $uid) use (&$lookedUpUIDs): ?array {
62+
$this->assertSame(1, $addressBookId);
63+
$this->assertNotSame('', $uid);
64+
$lookedUpUIDs[] = $uid;
65+
return null;
66+
});
5467

55-
// Act
56-
$result = iterator_to_array($this->service->import($stream, $this->addressBook, $options), false);
68+
$cards = [];
69+
$this->backend->expects($this->exactly(3))
70+
->method('createCard')
71+
->willReturnCallback(function (int $addressBookId, string $uri, string $data) use (&$cards): string {
72+
$this->assertSame(1, $addressBookId);
73+
$cards[] = Reader::read($data);
74+
return 'etag';
75+
});
76+
$this->backend->expects($this->never())->method('updateCard');
77+
78+
try {
79+
$result = iterator_to_array($this->service->import($stream, $this->addressBook, $options), false);
80+
} finally {
81+
fclose($stream);
82+
}
5783

58-
// Assert
59-
$this->assertCount(3, $result, 'Import result should contain counts and two items');
84+
$this->assertCount(4, $result);
6085
$this->assertInstanceOf(ImportCountEvent::class, $result[0]);
61-
$this->assertSame(2, $result[0]->vcard);
62-
foreach ([$result[1], $result[2]] as $event) {
86+
$this->assertSame(3, $result[0]->vcard);
87+
$this->assertCount(3, array_unique($lookedUpUIDs));
88+
$this->assertSame('existing-uid', $lookedUpUIDs[1]);
89+
$this->assertSame(['Erika Mustermann', 'Existing Contact', 'Max Mustermann'], array_map(fn ($card) => (string)$card->FN, $cards));
90+
foreach ($cards as $index => $card) {
91+
$this->assertSame($lookedUpUIDs[$index], (string)$card->UID);
92+
$event = $result[$index + 1];
6393
$this->assertInstanceOf(ImportObjectEvent::class, $event);
64-
$this->assertNull($event->identifier);
65-
$this->assertSame(ImportDisposition::Error, $event->disposition);
66-
$this->assertSame(['One or more objects discovered without a UID'], $event->errors);
94+
$this->assertSame($lookedUpUIDs[$index], $event->identifier);
95+
$this->assertSame(ImportDisposition::Created, $event->disposition);
6796
}
6897
}
6998
}

0 commit comments

Comments
 (0)