Commit 1452b74d authored by Georg Ehrke's avatar Georg Ehrke Committed by Thomas Müller
Browse files

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
parent f2f1eab7
......@@ -75,6 +75,7 @@ if ($debugging) {
}
$server->addPlugin(new \Sabre\CardDAV\VCFExportPlugin());
$server->addPlugin(new \OCA\DAV\CardDAV\ImageExportPlugin(\OC::$server->getLogger()));
$server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->getLogger()));
// And off we go!
......
......@@ -136,7 +136,8 @@ class Application extends App {
public function setupContactsProvider(IManager $contactsManager, $userID) {
/** @var ContactsManager $cm */
$cm = $this->getContainer()->query('ContactsManager');
$cm->setupContactsProvider($contactsManager, $userID);
$urlGenerator = $this->getContainer()->getServer()->getURLGenerator();
$cm->setupContactsProvider($contactsManager, $userID, $urlGenerator);
}
public function registerHooks() {
......
......@@ -24,6 +24,7 @@ namespace OCA\DAV\CardDAV;
use OCP\Constants;
use OCP\IAddressBook;
use OCP\IURLGenerator;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;
use Sabre\VObject\Reader;
......@@ -40,21 +41,27 @@ class AddressBookImpl implements IAddressBook {
/** @var AddressBook */
private $addressBook;
/** @var IURLGenerator */
private $urlGenerator;
/**
* AddressBookImpl constructor.
*
* @param AddressBook $addressBook
* @param array $addressBookInfo
* @param CardDavBackend $backend
* @param IUrlGenerator $urlGenerator
*/
public function __construct(
AddressBook $addressBook,
array $addressBookInfo,
CardDavBackend $backend) {
CardDavBackend $backend,
IURLGenerator $urlGenerator) {
$this->addressBook = $addressBook;
$this->addressBookInfo = $addressBookInfo;
$this->backend = $backend;
$this->urlGenerator = $urlGenerator;
}
/**
......@@ -83,11 +90,11 @@ class AddressBookImpl implements IAddressBook {
* @since 5.0.0
*/
public function search($pattern, $searchProperties, $options) {
$result = $this->backend->search($this->getKey(), $pattern, $searchProperties);
$results = $this->backend->search($this->getKey(), $pattern, $searchProperties);
$vCards = [];
foreach ($result as $cardData) {
$vCards[] = $this->vCard2Array($this->readCard($cardData));
foreach ($results as $result) {
$vCards[] = $this->vCard2Array($result['uri'], $this->readCard($result['carddata']));
}
return $vCards;
......@@ -100,13 +107,12 @@ class AddressBookImpl implements IAddressBook {
*/
public function createOrUpdate($properties) {
$update = false;
if (!isset($properties['UID'])) { // create a new contact
if (!isset($properties['URI'])) { // create a new contact
$uid = $this->createUid();
$uri = $uid . '.vcf';
$vCard = $this->createEmptyVCard($uid);
} else { // update existing contact
$uid = $properties['UID'];
$uri = $uid . '.vcf';
$uri = $properties['URI'];
$vCardData = $this->backend->getCard($this->getKey(), $uri);
$vCard = $this->readCard($vCardData['carddata']);
$update = true;
......@@ -122,7 +128,7 @@ class AddressBookImpl implements IAddressBook {
$this->backend->createCard($this->getKey(), $uri, $vCard->serialize());
}
return $this->vCard2Array($vCard);
return $this->vCard2Array($uri, $vCard);
}
......@@ -207,13 +213,31 @@ class AddressBookImpl implements IAddressBook {
/**
* create array with all vCard properties
*
* @param string $uri
* @param VCard $vCard
* @return array
*/
protected function vCard2Array(VCard $vCard) {
$result = [];
protected function vCard2Array($uri, VCard $vCard) {
$result = [
'URI' => $uri,
];
foreach ($vCard->children as $property) {
$result[$property->name] = $property->getValue();
if ($property->name === 'PHOTO' && $property->getValueType() === 'BINARY') {
$url = $this->urlGenerator->getAbsoluteURL(
$this->urlGenerator->linkTo('', 'remote.php') . '/dav/');
$url .= implode('/', [
'addressbooks',
substr($this->addressBookInfo['principaluri'], 11), //cut off 'principals/'
$this->addressBookInfo['uri'],
$uri
]) . '?photo';
$result['PHOTO'] = 'VALUE=uri:' . $url;
} else {
$result[$property->name] = $property->getValue();
}
}
if ($this->addressBookInfo['principaluri'] === 'principals/system/system' &&
$this->addressBookInfo['uri'] === 'system') {
......
......@@ -780,7 +780,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
}
$query2->andWhere($query2->expr()->eq('cp.addressbookid', $query->createNamedParameter($addressBookId)));
$query->select('c.carddata')->from($this->dbCardsTable, 'c')
$query->select('c.carddata', 'c.uri')->from($this->dbCardsTable, 'c')
->where($query->expr()->in('c.id', $query->createFunction($query2->getSQL())));
$result = $query->execute();
......@@ -788,8 +788,10 @@ class CardDavBackend implements BackendInterface, SyncSupport {
$result->closeCursor();
return array_map(function($array) {return $this->readBlob($array['carddata']);}, $cards);
return array_map(function($array) {
$array['carddata'] = $this->readBlob($array['carddata']);
return $array;
}, $cards);
}
/**
......
......@@ -22,6 +22,7 @@
namespace OCA\DAV\CardDAV;
use OCP\Contacts\IManager;
use OCP\IURLGenerator;
class ContactsManager {
......@@ -37,26 +38,29 @@ class ContactsManager {
/**
* @param IManager $cm
* @param string $userId
* @param IURLGenerator $urlGenerator
*/
public function setupContactsProvider(IManager $cm, $userId) {
public function setupContactsProvider(IManager $cm, $userId, IURLGenerator $urlGenerator) {
$addressBooks = $this->backend->getAddressBooksForUser("principals/users/$userId");
$this->register($cm, $addressBooks);
$this->register($cm, $addressBooks, $urlGenerator);
$addressBooks = $this->backend->getAddressBooksForUser("principals/system/system");
$this->register($cm, $addressBooks);
$this->register($cm, $addressBooks, $urlGenerator);
}
/**
* @param IManager $cm
* @param $addressBooks
* @param IURLGenerator $urlGenerator
*/
private function register(IManager $cm, $addressBooks) {
private function register(IManager $cm, $addressBooks, $urlGenerator) {
foreach ($addressBooks as $addressBookInfo) {
$addressBook = new \OCA\DAV\CardDAV\AddressBook($this->backend, $addressBookInfo);
$cm->registerAddressBook(
new AddressBookImpl(
$addressBook,
$addressBookInfo,
$this->backend
$this->backend,
$urlGenerator
)
);
}
......
<?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\CardDAV;
use OCP\ILogger;
use Sabre\CardDAV\Card;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Sabre\VObject\Parameter;
use Sabre\VObject\Property\Binary;
use Sabre\VObject\Reader;
class ImageExportPlugin extends ServerPlugin {
/** @var Server */
protected $server;
/** @var ILogger */
private $logger;
public function __construct(ILogger $logger) {
$this->logger = $logger;
}
/**
* Initializes the plugin and registers event handlers
*
* @param Server $server
* @return void
*/
function initialize(Server $server) {
$this->server = $server;
$this->server->on('method:GET', [$this, 'httpGet'], 90);
}
/**
* Intercepts GET requests on addressbook urls ending with ?photo.
*
* @param RequestInterface $request
* @param ResponseInterface $response
* @return bool|void
*/
function httpGet(RequestInterface $request, ResponseInterface $response) {
$queryParams = $request->getQueryParameters();
// TODO: in addition to photo we should also add logo some point in time
if (!array_key_exists('photo', $queryParams)) {
return true;
}
$path = $request->getPath();
$node = $this->server->tree->getNodeForPath($path);
if (!($node instanceof Card)) {
return true;
}
$this->server->transactionType = 'carddav-image-export';
// Checking ACL, if available.
if ($aclPlugin = $this->server->getPlugin('acl')) {
/** @var \Sabre\DAVACL\Plugin $aclPlugin */
$aclPlugin->checkPrivileges($path, '{DAV:}read');
}
if ($result = $this->getPhoto($node)) {
$response->setHeader('Content-Type', $result['Content-Type']);
$response->setStatus(200);
$response->setBody($result['body']);
// Returning false to break the event chain
return false;
}
return true;
}
function getPhoto(Card $node) {
// TODO: this is kind of expensive - load carddav data from database and parse it
// we might want to build up a cache one day
try {
$vObject = $this->readCard($node->get());
if (!$vObject->PHOTO) {
return false;
}
$photo = $vObject->PHOTO;
$type = $this->getType($photo);
$valType = $photo->getValueType();
$val = ($valType === 'URI' ? $photo->getRawMimeDirValue() : $photo->getValue());
return [
'Content-Type' => $type,
'body' => $val
];
} catch(\Exception $ex) {
$this->logger->logException($ex);
}
return false;
}
private function readCard($cardData) {
return Reader::read($cardData);
}
/**
* @param Binary $photo
* @return Parameter
*/
private function getType($photo) {
$params = $photo->parameters();
if (isset($params['TYPE']) || isset($params['MEDIATYPE'])) {
/** @var Parameter $typeParam */
$typeParam = isset($params['TYPE']) ? $params['TYPE'] : $params['MEDIATYPE'];
$type = $typeParam->getValue();
if (strpos($type, 'image/') === 0) {
return $type;
} else {
return 'image/' . strtolower($type);
}
}
return '';
}
}
......@@ -25,6 +25,7 @@
namespace OCA\DAV;
use OCA\DAV\CalDAV\Schedule\IMipPlugin;
use OCA\DAV\CardDAV\ImageExportPlugin;
use OCA\DAV\Connector\Sabre\Auth;
use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin;
use OCA\DAV\Connector\Sabre\DavAclPlugin;
......@@ -103,6 +104,7 @@ class Server {
// addressbook plugins
$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
$this->server->addPlugin(new VCFExportPlugin());
$this->server->addPlugin(new ImageExportPlugin(\OC::$server->getLogger()));
// system tags plugins
$this->server->addPlugin(new \OCA\DAV\SystemTag\SystemTagPlugin(
......
......@@ -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'])
......
......@@ -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']]]
];
}
......
......@@ -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);
}
}
<?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']],
];