From f558ac7dd583c8135e40933e9e35d3505b9b6cb6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20M=C3=BCller?= Date: Wed, 2 Dec 2015 11:09:15 +0100 Subject: [PATCH] Fix update of avatar image --- apps/dav/lib/carddav/converter.php | 25 ++++++-- apps/dav/tests/unit/carddav/convertertest.php | 57 +++++++++---------- 2 files changed, 49 insertions(+), 33 deletions(-) diff --git a/apps/dav/lib/carddav/converter.php b/apps/dav/lib/carddav/converter.php index b113f1894dd..67492378ade 100644 --- a/apps/dav/lib/carddav/converter.php +++ b/apps/dav/lib/carddav/converter.php @@ -21,6 +21,7 @@ namespace OCA\DAV\CardDAV; +use OCP\IImage; use OCP\IUser; use Sabre\VObject\Component\VCard; use Sabre\VObject\Property\Text; @@ -68,21 +69,22 @@ class Converter { $image = $user->getAvatarImage(-1); $updated = false; - if((is_null($vCard->FN) && !empty($displayName)) || (!is_null($vCard->FN) && $vCard->FN->getValue() !== $displayName)) { + if($this->propertyNeedsUpdate($vCard, 'FN', $displayName)) { $vCard->FN = new Text($vCard, 'FN', $displayName); unset($vCard->N); $vCard->add(new Text($vCard, 'N', $this->splitFullName($displayName))); $updated = true; } - if((is_null($vCard->EMail) && !empty($emailAddress)) || (!is_null($vCard->EMail) && $vCard->EMail->getValue() !== $emailAddress)) { + if($this->propertyNeedsUpdate($vCard, 'EMAIL', $emailAddress)) { $vCard->EMAIL = new Text($vCard, 'EMAIL', $emailAddress); $updated = true; } - if((is_null($vCard->CLOUD) && !empty($cloudId)) || (!is_null($vCard->CLOUD) && $vCard->CLOUD->getValue() !== $cloudId)) { + if($this->propertyNeedsUpdate($vCard, 'CLOUD', $cloudId)) { $vCard->CLOUD = new Text($vCard, 'CLOUD', $cloudId); $updated = true; } - if((is_null($vCard->PHOTO) && !empty($image)) || (!is_null($vCard->PHOTO) && $vCard->PHOTO->getValue() !== $image)) { + + if($this->propertyNeedsUpdate($vCard, 'PHOTO', $image)) { $vCard->add('PHOTO', $image->data(), ['ENCODING' => 'b', 'TYPE' => $image->mimeType()]); $updated = true; } @@ -103,6 +105,20 @@ class Converter { return $updated; } + private function propertyNeedsUpdate(VCard $vCard, $name, $newValue) { + if (is_null($newValue)) { + return false; + } + $value = $vCard->__get($name); + if (!is_null($value)) { + $value = $value->getValue(); + $newValue = $newValue instanceof IImage ? $newValue->data() : $newValue; + + return $value !== $newValue; + } + return true; + } + /** * @param string $fullName * @return string[] @@ -126,4 +142,5 @@ class Converter { return $result; } + } diff --git a/apps/dav/tests/unit/carddav/convertertest.php b/apps/dav/tests/unit/carddav/convertertest.php index b4a0ee2a664..f4e2ea3f002 100644 --- a/apps/dav/tests/unit/carddav/convertertest.php +++ b/apps/dav/tests/unit/carddav/convertertest.php @@ -30,11 +30,7 @@ class ConverterTests extends TestCase { * @dataProvider providesNewUsers */ public function testCreation($expectedVCard, $displayName = null, $eMailAddress = null, $cloudId = null) { - $user = $this->getMockBuilder('OCP\IUser')->disableOriginalConstructor()->getMock(); - $user->method('getUID')->willReturn('12345'); - $user->method('getDisplayName')->willReturn($displayName); - $user->method('getEMailAddress')->willReturn($eMailAddress); - $user->method('getCloudId')->willReturn($cloudId); + $user = $this->getUserMock($displayName, $eMailAddress, $cloudId); $converter = new Converter(); $vCard = $converter->createCardFromUser($user); @@ -45,22 +41,18 @@ class ConverterTests extends TestCase { public function providesNewUsers() { return [ - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nEND:VCARD\r\n"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nEND:VCARD\r\n", "Dr. Foo Bar"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nEMAIL;TYPE=OTHER:foo@bar.net\r\nEND:VCARD\r\n", "Dr. Foo Bar", "foo@bar.net"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nCLOUD:foo@bar.net\r\nEND:VCARD\r\n", "Dr. Foo Bar", null, "foo@bar.net"], + ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU2Nzg5\r\nEND:VCARD\r\n"], + ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU2Nzg5\r\nEND:VCARD\r\n", "Dr. Foo Bar"], + ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nEMAIL;TYPE=OTHER:foo@bar.net\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU2Nzg5\r\nEND:VCARD\r\n", "Dr. Foo Bar", "foo@bar.net"], + ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nCLOUD:foo@bar.net\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU2Nzg5\r\nEND:VCARD\r\n", "Dr. Foo Bar", null, "foo@bar.net"], ]; } /** - * @dataProvider providesUsersForUpdate + * @dataProvider providesNewUsers */ public function testUpdateOfUnchangedUser($expectedVCard, $displayName = null, $eMailAddress = null, $cloudId = null) { - $user = $this->getMockBuilder('OCP\IUser')->disableOriginalConstructor()->getMock(); - $user->method('getUID')->willReturn('12345'); - $user->method('getDisplayName')->willReturn($displayName); - $user->method('getEMailAddress')->willReturn($eMailAddress); - $user->method('getCloudId')->willReturn($cloudId); + $user = $this->getUserMock($displayName, $eMailAddress, $cloudId); $converter = new Converter(); $vCard = $converter->createCardFromUser($user); @@ -71,24 +63,11 @@ class ConverterTests extends TestCase { $this->assertEquals($expectedVCard, $cardData); } - public function providesUsersForUpdate() { - return [ - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nEND:VCARD\r\n"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nEND:VCARD\r\n", "Dr. Foo Bar"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nEMAIL;TYPE=OTHER:foo@bar.net\r\nEND:VCARD\r\n", "Dr. Foo Bar", "foo@bar.net"], - ["BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.4.7//EN\r\nUID:12345\r\nFN:Dr. Foo Bar\r\nN:Bar;Dr.;Foo;;\r\nCLOUD:foo@bar.net\r\nEND:VCARD\r\n", "Dr. Foo Bar", null, "foo@bar.net"], - ]; - } - /** * @dataProvider providesUsersForUpdateOfRemovedElement */ public function testUpdateOfRemovedElement($expectedVCard, $displayName = null, $eMailAddress = null, $cloudId = null) { - $user = $this->getMockBuilder('OCP\IUser')->disableOriginalConstructor()->getMock(); - $user->method('getUID')->willReturn('12345'); - $user->method('getDisplayName')->willReturn($displayName); - $user->method('getEMailAddress')->willReturn($eMailAddress); - $user->method('getCloudId')->willReturn($cloudId); + $user = $this->getUserMock($displayName, $eMailAddress, $cloudId); $converter = new Converter(); $vCard = $converter->createCardFromUser($user); @@ -98,6 +77,7 @@ class ConverterTests extends TestCase { $user1->method('getDisplayName')->willReturn(null); $user1->method('getEMailAddress')->willReturn(null); $user1->method('getCloudId')->willReturn(null); + $user1->method('getAvatarImage')->willReturn(null); $updated = $converter->updateCard($vCard, $user1); $this->assertTrue($updated); @@ -134,4 +114,23 @@ class ConverterTests extends TestCase { ['Tolkien;John;Ronald Reuel;;', 'John Ronald Reuel Tolkien'], ]; } + + /** + * @param $displayName + * @param $eMailAddress + * @param $cloudId + * @return \PHPUnit_Framework_MockObject_MockObject + */ + protected function getUserMock($displayName, $eMailAddress, $cloudId) { + $image0 = $this->getMockBuilder('OCP\IImage')->disableOriginalConstructor()->getMock(); + $image0->method('mimeType')->willReturn('JPEG'); + $image0->method('data')->willReturn('123456789'); + $user = $this->getMockBuilder('OCP\IUser')->disableOriginalConstructor()->getMock(); + $user->method('getUID')->willReturn('12345'); + $user->method('getDisplayName')->willReturn($displayName); + $user->method('getEMailAddress')->willReturn($eMailAddress); + $user->method('getCloudId')->willReturn($cloudId); + $user->method('getAvatarImage')->willReturn($image0); + return $user; + } } -- 2.39.5