Fix update of avatar image

diff --git a/apps/dav/lib/carddav/converter.php b/apps/dav/lib/carddav/converter.php
index b113f1894ddb928d25b42fa98a38a0858cdffe0a..67492378ade3bcdff774ffb11a0448d0fb3fc699 100644 (file)
--- 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 b4a0ee2a664d02e0c708560685a952b34fa36001..f4e2ea3f0023d635bf8eb75e60dd58a910ea98c9 100644 (file)
--- 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;
+       }
 }