Contacts API: replace raw image data with url (#25081)

* add uri to AddressBookImpl array

* Introduce ImageExportPlugin for CardDav

* add plugin to v1 routes

* replace binary contact photo with link

* update tests

* Adding unit tests

4 files changed, 185 insertions, 20 deletions

diff --git a/apps/dav/tests/unit/CardDAV/AddressBookImplTest.php b/apps/dav/tests/unit/CardDAV/AddressBookImplTest.php
index ddfec3dc3ac..4ccc841157f 100644
--- a/apps/dav/tests/unit/CardDAV/AddressBookImplTest.php
+++ b/apps/dav/tests/unit/CardDAV/AddressBookImplTest.php
@@ -43,6 +43,9 @@ class AddressBookImplTest extends TestCase {
 	/** @var  AddressBook | \PHPUnit_Framework_MockObject_MockObject */
 	private $addressBook;
 
+	/** @var \OCP\IURLGenerator | \PHPUnit_Framework_MockObject_MockObject */
+	private $urlGenerator;
+
 	/** @var  CardDavBackend | \PHPUnit_Framework_MockObject_MockObject */
 	private $backend;
 
@@ -61,11 +64,13 @@ class AddressBookImplTest extends TestCase {
 		$this->backend = $this->getMockBuilder('\OCA\DAV\CardDAV\CardDavBackend')
 			->disableOriginalConstructor()->getMock();
 		$this->vCard = $this->getMock('Sabre\VObject\Component\VCard');
+		$this->urlGenerator = $this->getMock('OCP\IURLGenerator');
 
 		$this->addressBookImpl = new AddressBookImpl(
 			$this->addressBook,
 			$this->addressBookInfo,
-			$this->backend
+			$this->backend,
+			$this->urlGenerator
 		);
 	}
 
@@ -87,7 +92,8 @@ class AddressBookImplTest extends TestCase {
 				[
 					$this->addressBook,
 					$this->addressBookInfo,
-					$this->backend
+					$this->backend,
+					$this->urlGenerator,
 				]
 			)
 			->setMethods(['vCard2Array', 'readCard'])
@@ -100,15 +106,18 @@ class AddressBookImplTest extends TestCase {
 			->with($this->addressBookInfo['id'], $pattern, $searchProperties)
 			->willReturn(
 				[
-					'cardData1',
-					'cardData2'
+					['uri' => 'foo.vcf', 'carddata' => 'cardData1'],
+					['uri' => 'bar.vcf', 'carddata' => 'cardData2']
 				]
 			);
 
 		$addressBookImpl->expects($this->exactly(2))->method('readCard')
 			->willReturn($this->vCard);
 		$addressBookImpl->expects($this->exactly(2))->method('vCard2Array')
-			->with($this->vCard)->willReturn('vCard');
+			->withConsecutive(
+				['foo.vcf', $this->vCard],
+				['bar.vcf', $this->vCard]
+			)->willReturn('vCard');
 
 		$result = $addressBookImpl->search($pattern, $searchProperties, []);
 		$this->assertTrue((is_array($result)));
@@ -130,7 +139,8 @@ class AddressBookImplTest extends TestCase {
 				[
 					$this->addressBook,
 					$this->addressBookInfo,
-					$this->backend
+					$this->backend,
+					$this->urlGenerator,
 				]
 			)
 			->setMethods(['vCard2Array', 'createUid', 'createEmptyVCard'])
@@ -146,7 +156,7 @@ class AddressBookImplTest extends TestCase {
 		$this->backend->expects($this->never())->method('updateCard');
 		$this->backend->expects($this->never())->method('getCard');
 		$addressBookImpl->expects($this->once())->method('vCard2Array')
-			->with($this->vCard)->willReturn(true);
+			->with('uid.vcf', $this->vCard)->willReturn(true);
 
 		$this->assertTrue($addressBookImpl->createOrUpdate($properties));
 	}
@@ -161,7 +171,8 @@ class AddressBookImplTest extends TestCase {
 	public function testUpdate() {
 
 		$uid = 'uid';
-		$properties = ['UID' => $uid, 'FN' => 'John Doe'];
+		$uri = 'bla.vcf';
+		$properties = ['URI' => $uri, 'UID' => $uid, 'FN' => 'John Doe'];
 
 		/** @var \PHPUnit_Framework_MockObject_MockObject | AddressBookImpl $addressBookImpl */
 		$addressBookImpl = $this->getMockBuilder('OCA\DAV\CardDAV\AddressBookImpl')
@@ -169,7 +180,8 @@ class AddressBookImplTest extends TestCase {
 				[
 					$this->addressBook,
 					$this->addressBookInfo,
-					$this->backend
+					$this->backend,
+					$this->urlGenerator,
 				]
 			)
 			->setMethods(['vCard2Array', 'createUid', 'createEmptyVCard', 'readCard'])
@@ -178,7 +190,7 @@ class AddressBookImplTest extends TestCase {
 		$addressBookImpl->expects($this->never())->method('createUid');
 		$addressBookImpl->expects($this->never())->method('createEmptyVCard');
 		$this->backend->expects($this->once())->method('getCard')
-			->with($this->addressBookInfo['id'], $uid . '.vcf')
+			->with($this->addressBookInfo['id'], $uri)
 			->willReturn(['carddata' => 'data']);
 		$addressBookImpl->expects($this->once())->method('readCard')
 			->with('data')->willReturn($this->vCard);
@@ -187,7 +199,7 @@ class AddressBookImplTest extends TestCase {
 		$this->backend->expects($this->never())->method('createCard');
 		$this->backend->expects($this->once())->method('updateCard');
 		$addressBookImpl->expects($this->once())->method('vCard2Array')
-			->with($this->vCard)->willReturn(true);
+			->with($uri, $this->vCard)->willReturn(true);
 
 		$this->assertTrue($addressBookImpl->createOrUpdate($properties));
 	}
@@ -251,7 +263,8 @@ class AddressBookImplTest extends TestCase {
 				[
 					$this->addressBook,
 					$this->addressBookInfo,
-					$this->backend
+					$this->backend,
+					$this->urlGenerator,
 				]
 			)
 			->setMethods(['getUid'])
diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
index c7cd4a30052..58a93befe68 100644
--- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
+++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -535,8 +535,8 @@ class CardDavBackendTest extends TestCase {
 		$found = [];
 		foreach ($result as $r) {
 			foreach ($expected as $exp) {
-				if (strpos($r, $exp) > 0) {
-					$found[$exp] = true;
+				if ($r['uri'] === $exp[0] && strpos($r['carddata'], $exp[1]) > 0) {
+					$found[$exp[1]] = true;
 					break;
 				}
 			}
@@ -547,11 +547,11 @@ class CardDavBackendTest extends TestCase {
 
 	public function dataTestSearch() {
 		return [
-				['John', ['FN'], ['John Doe', 'John M. Doe']],
-				['M. Doe', ['FN'], ['John M. Doe']],
-				['Do', ['FN'], ['John Doe', 'John M. Doe']],
-				'check if duplicates are handled correctly' => ['John', ['FN', 'CLOUD'], ['John Doe', 'John M. Doe']],
-				'case insensitive' => ['john', ['FN'], ['John Doe', 'John M. Doe']]
+				['John', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
+				['M. Doe', ['FN'], [['uri1', 'John M. Doe']]],
+				['Do', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
+				'check if duplicates are handled correctly' => ['John', ['FN', 'CLOUD'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]],
+				'case insensitive' => ['john', ['FN'], [['uri0', 'John Doe'], ['uri1', 'John M. Doe']]]
 		];
 	}
 
diff --git a/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php b/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php
index 2abd9da66e7..23a49a1962f 100644
--- a/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php
+++ b/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php
@@ -32,6 +32,7 @@ class ContactsManagerTest extends TestCase {
 		/** @var IManager | \PHPUnit_Framework_MockObject_MockObject $cm */
 		$cm = $this->getMockBuilder('OCP\Contacts\IManager')->disableOriginalConstructor()->getMock();
 		$cm->expects($this->exactly(2))->method('registerAddressBook');
+		$urlGenerator = $this->getMockBuilder('OCP\IUrlGenerator')->disableOriginalConstructor()->getMock();
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backEnd */
 		$backEnd = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
 		$backEnd->method('getAddressBooksForUser')->willReturn([
@@ -39,6 +40,6 @@ class ContactsManagerTest extends TestCase {
 			]);
 
 		$app = new ContactsManager($backEnd);
-		$app->setupContactsProvider($cm, 'user01');
+		$app->setupContactsProvider($cm, 'user01', $urlGenerator);
 	}
 }
diff --git a/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
new file mode 100644
index 00000000000..8583df0b6f9
--- /dev/null
+++ b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
@@ -0,0 +1,151 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+
+namespace OCA\DAV\Tests\unit\CardDAV;
+
+
+use OCA\DAV\CardDAV\ImageExportPlugin;
+use OCP\ILogger;
+use Sabre\CardDAV\Card;
+use Sabre\DAV\Server;
+use Sabre\DAV\Tree;
+use Sabre\HTTP\RequestInterface;
+use Sabre\HTTP\ResponseInterface;
+use Test\TestCase;
+
+class ImageExportPluginTest extends TestCase {
+
+	/** @var ResponseInterface | \PHPUnit_Framework_MockObject_MockObject */
+	private $response;
+	/** @var RequestInterface | \PHPUnit_Framework_MockObject_MockObject */
+	private $request;
+	/** @var ImageExportPlugin | \PHPUnit_Framework_MockObject_MockObject */
+	private $plugin;
+	/** @var Server */
+	private $server;
+	/** @var Tree | \PHPUnit_Framework_MockObject_MockObject */
+	private $tree;
+	/** @var ILogger | \PHPUnit_Framework_MockObject_MockObject */
+	private $logger;
+
+	function setUp() {
+		parent::setUp();
+
+		$this->request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')->getMock();
+		$this->response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')->getMock();
+		$this->server = $this->getMockBuilder('Sabre\DAV\Server')->getMock();
+		$this->tree = $this->getMockBuilder('Sabre\DAV\Tree')->disableOriginalConstructor()->getMock();
+		$this->server->tree = $this->tree;
+		$this->logger = $this->getMockBuilder('\OCP\ILogger')->getMock();
+
+		$this->plugin = $this->getMock('OCA\DAV\CardDAV\ImageExportPlugin', ['getPhoto'], [$this->logger]);
+		$this->plugin->initialize($this->server);
+	}
+
+	/**
+	 * @dataProvider providesQueryParams
+	 * @param $param
+	 */
+	public function testQueryParams($param) {
+		$this->request->expects($this->once())->method('getQueryParameters')->willReturn($param);
+		$result = $this->plugin->httpGet($this->request, $this->response);
+		$this->assertTrue($result);
+	}
+
+	public function providesQueryParams() {
+		return [
+			[[]],
+			[['1']],
+			[['foo' => 'bar']],
+		];
+	}
+
+	public function testNotACard() {
+		$this->request->expects($this->once())->method('getQueryParameters')->willReturn(['photo' => true]);
+		$this->request->expects($this->once())->method('getPath')->willReturn('/files/welcome.txt');
+		$this->tree->expects($this->once())->method('getNodeForPath')->with('/files/welcome.txt')->willReturn(null);
+		$result = $this->plugin->httpGet($this->request, $this->response);
+		$this->assertTrue($result);
+	}
+
+	/**
+	 * @dataProvider providesCardWithOrWithoutPhoto
+	 * @param bool $expected
+	 * @param array $getPhotoResult
+	 */
+	public function testCardWithOrWithoutPhoto($expected, $getPhotoResult) {
+		$this->request->expects($this->once())->method('getQueryParameters')->willReturn(['photo' => true]);
+		$this->request->expects($this->once())->method('getPath')->willReturn('/files/welcome.txt');
+
+		$card = $this->getMockBuilder('Sabre\CardDAV\Card')->disableOriginalConstructor()->getMock();
+		$this->tree->expects($this->once())->method('getNodeForPath')->with('/files/welcome.txt')->willReturn($card);
+
+		$this->plugin->expects($this->once())->method('getPhoto')->willReturn($getPhotoResult);
+
+		if (!$expected) {
+			$this->response->expects($this->once())->method('setHeader');
+			$this->response->expects($this->once())->method('setStatus');
+			$this->response->expects($this->once())->method('setBody');
+		}
+
+		$result = $this->plugin->httpGet($this->request, $this->response);
+		$this->assertEquals($expected, $result);
+	}
+
+	public function providesCardWithOrWithoutPhoto() {
+		return [
+			[true, null],
+			[false, ['Content-Type' => 'image/jpeg', 'body' => '1234']],
+		];
+	}
+
+	/**
+	 * @dataProvider providesPhotoData
+	 * @param $expected
+	 * @param $cardData
+	 */
+	public function testGetPhoto($expected, $cardData) {
+		/** @var Card | \PHPUnit_Framework_MockObject_MockObject $card */
+		$card = $this->getMockBuilder('Sabre\CardDAV\Card')->disableOriginalConstructor()->getMock();
+		$card->expects($this->once())->method('get')->willReturn($cardData);
+
+		$this->plugin = new ImageExportPlugin($this->logger);
+		$this->plugin->initialize($this->server);
+
+		$result = $this->plugin->getPhoto($card);
+		$this->assertEquals($expected, $result);
+	}
+
+	public function providesPhotoData() {
+		return [
+			'empty vcard' => [false, ''],
+			'vcard without PHOTO' => [false, "BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.5.0//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nEND:VCARD\r\n"],
+			'vcard 3 with PHOTO' => [['Content-Type' => 'image/jpeg', 'body' => '12345'], "BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.5.0//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU=\r\nEND:VCARD\r\n"],
+			//
+			// TODO: these three below are not working - needs debugging
+			//
+			//'vcard 3 with PHOTO URL' => [['Content-Type' => 'image/jpeg', 'body' => '12345'], "BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 3.5.0//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;TYPE=JPEG:http://example.org/photo.jpg\r\nEND:VCARD\r\n"],
+			//'vcard 4 with PHOTO' => [['Content-Type' => 'image/jpeg', 'body' => '12345'], "BEGIN:VCARD\r\nVERSION:4.0\r\nPRODID:-//Sabre//Sabre VObject 3.5.0//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO:data:image/jpeg;MTIzNDU=\r\nEND:VCARD\r\n"],
+			'vcard 4 with PHOTO URL' => [['Content-Type' => 'image/jpeg', 'body' => 'http://example.org/photo.jpg'], "BEGIN:VCARD\r\nVERSION:4.0\r\nPRODID:-//Sabre//Sabre VObject 3.5.0//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;MEDIATYPE=image/jpeg:http://example.org/photo.jpg\r\nEND:VCARD\r\n"],
+		];
+	}
+}