don't add empty values to the vcard

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

3 files changed, 56 insertions, 40 deletions

diff --git a/apps/dav/lib/CardDAV/Converter.php b/apps/dav/lib/CardDAV/Converter.php
index 0f8ea799f1e..eb43fe7d671 100644
--- a/apps/dav/lib/CardDAV/Converter.php
+++ b/apps/dav/lib/CardDAV/Converter.php
@@ -61,9 +61,15 @@ class Converter {
 		$publish = false;
 
 		foreach ($userData as $property => $value) {
-			if ($value['scope'] === AccountManager::VISIBILITY_CONTACTS_ONLY ||
-				$value['scope'] === AccountManager::VISIBILITY_PUBLIC
-			) {
+
+			$shareWithTrustedServers =
+				$value['scope'] === AccountManager::VISIBILITY_CONTACTS_ONLY ||
+				$value['scope'] === AccountManager::VISIBILITY_PUBLIC;
+
+			$emptyValue = !isset($value['value']) || $value['value'] === '';
+			$noImage = $image === null;
+
+			if ($shareWithTrustedServers && (!$emptyValue || !$noImage)) {
 				$publish = true;
 				switch ($property) {
 					case AccountManager::PROPERTY_DISPLAYNAME:
diff --git a/apps/dav/tests/unit/CardDAV/ConverterTest.php b/apps/dav/tests/unit/CardDAV/ConverterTest.php
index 528b3aa9ef4..ff218bfc78c 100644
--- a/apps/dav/tests/unit/CardDAV/ConverterTest.php
+++ b/apps/dav/tests/unit/CardDAV/ConverterTest.php
@@ -173,6 +173,16 @@ class ConverterTest extends  TestCase {
 				null,
 				"foo@cloud.net"
 			],
+			[
+				[
+					'cloud' => 'foo@cloud.net',
+					'fn' => 'Dr. Foo Bar',
+					'photo' => 'data:image/jpeg;base64,MTIzNDU2Nzg5',
+				],
+				'Dr. Foo Bar',
+				'',
+				'foo@cloud.net'
+			],
 		];
 	}
 
diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
index 68345def66b..de4e8ead4c0 100644
--- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
+++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php
@@ -103,50 +103,50 @@ class SyncServiceTest extends TestCase {
 		$user->method('getBackendClassName')->willReturn('unittest');
 		$user->method('getUID')->willReturn('test-user');
 		$user->method('getCloudId')->willReturn('cloudId');
+		$user->method('getDisplayName')->willReturn('test-user');
 		$accountManager = $this->getMockBuilder('OC\Accounts\AccountManager')->disableOriginalConstructor()->getMock();
 		$accountManager->expects($this->any())->method('getUser')
 			->willReturn([
-				AccountManager::PROPERTY_DISPLAYNAME =>
-					[
-						'value' => $user->getDisplayName(),
-						'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
-					],
-				AccountManager::PROPERTY_ADDRESS =>
-					[
-						'value' => '',
-						'scope' => AccountManager::VISIBILITY_PRIVATE,
-					],
-				AccountManager::PROPERTY_WEBSITE =>
-					[
-						'value' => '',
-						'scope' => AccountManager::VISIBILITY_PRIVATE,
-					],
-				AccountManager::PROPERTY_EMAIL =>
-					[
-						'value' => $user->getEMailAddress(),
-						'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
-					],
-				AccountManager::PROPERTY_AVATAR =>
-					[
-						'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY
-					],
-				AccountManager::PROPERTY_PHONE =>
-					[
-						'value' => '',
-						'scope' => AccountManager::VISIBILITY_PRIVATE,
-					],
-				AccountManager::PROPERTY_TWITTER =>
-					[
-						'value' => '',
-						'scope' => AccountManager::VISIBILITY_PRIVATE,
-					],
-			]);
+					AccountManager::PROPERTY_DISPLAYNAME =>
+						[
+							'value' => $user->getDisplayName(),
+							'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
+						],
+					AccountManager::PROPERTY_ADDRESS =>
+						[
+							'value' => '',
+							'scope' => AccountManager::VISIBILITY_PRIVATE,
+						],
+					AccountManager::PROPERTY_WEBSITE =>
+						[
+							'value' => '',
+							'scope' => AccountManager::VISIBILITY_PRIVATE,
+						],
+					AccountManager::PROPERTY_EMAIL =>
+						[
+							'value' => $user->getEMailAddress(),
+							'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
+						],
+					AccountManager::PROPERTY_AVATAR =>
+						[
+							'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY
+						],
+					AccountManager::PROPERTY_PHONE =>
+						[
+							'value' => '',
+							'scope' => AccountManager::VISIBILITY_PRIVATE,
+						],
+					AccountManager::PROPERTY_TWITTER =>
+						[
+							'value' => '',
+							'scope' => AccountManager::VISIBILITY_PRIVATE,
+						],
+				]
+			);
 
 		$ss = new SyncService($backend, $userManager, $logger, $accountManager);
 		$ss->updateUser($user);
 
-		$user->method('getDisplayName')->willReturn('A test user for unit testing');
-
 		$ss->updateUser($user);
 
 		$ss->deleteUser($user);