Merge pull request #4675 from nextcloud/fix_4651

Create a photo cache to speedup the contactsmenu

8 files changed, 476 insertions, 245 deletions

diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php
index 04344e83fde..8dea6684742 100644
--- a/apps/dav/appinfo/v1/carddav.php
+++ b/apps/dav/appinfo/v1/carddav.php
@@ -48,7 +48,7 @@ $principalBackend = new Principal(
 	'principals/'
 );
 $db = \OC::$server->getDatabaseConnection();
-$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager());
+$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager(), \OC::$server->getEventDispatcher());
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 
@@ -81,7 +81,7 @@ if ($debugging) {
 
 $server->addPlugin(new \Sabre\DAV\Sync\Plugin());
 $server->addPlugin(new \Sabre\CardDAV\VCFExportPlugin());
-$server->addPlugin(new \OCA\DAV\CardDAV\ImageExportPlugin(\OC::$server->getLogger()));
+$server->addPlugin(new \OCA\DAV\CardDAV\ImageExportPlugin(new \OCA\DAV\CardDAV\PhotoCache(\OC::$server->getAppDataDir('dav-photocache'))));
 $server->addPlugin(new ExceptionLoggerPlugin('carddav', \OC::$server->getLogger()));
 
 // And off we go!
diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php
index b4f16f3cadf..5d89324d4a9 100644
--- a/apps/dav/lib/AppInfo/Application.php
+++ b/apps/dav/lib/AppInfo/Application.php
@@ -24,11 +24,13 @@
  */
 namespace OCA\DAV\AppInfo;
 
+use OC\AppFramework\Utility\SimpleContainer;
 use OCA\DAV\CalDAV\Activity\Backend;
 use OCA\DAV\CalDAV\Activity\Provider\Event;
 use OCA\DAV\CalDAV\BirthdayService;
 use OCA\DAV\Capabilities;
 use OCA\DAV\CardDAV\ContactsManager;
+use OCA\DAV\CardDAV\PhotoCache;
 use OCA\DAV\CardDAV\SyncService;
 use OCA\DAV\HookManager;
 use \OCP\AppFramework\App;
@@ -44,10 +46,19 @@ class Application extends App {
 	public function __construct() {
 		parent::__construct('dav');
 
+		$container = $this->getContainer();
+		$server = $container->getServer();
+
+		$container->registerService(PhotoCache::class, function(SimpleContainer $s) use ($server) {
+			return new PhotoCache(
+				$server->getAppDataDir('dav-photocache')
+			);
+		});
+
 		/*
 		 * Register capabilities
 		 */
-		$this->getContainer()->registerCapability(Capabilities::class);
+		$container->registerCapability(Capabilities::class);
 	}
 
 	/**
@@ -101,6 +112,19 @@ class Application extends App {
 			}
 		});
 
+		$clearPhotoCache = function($event) {
+			if ($event instanceof GenericEvent) {
+				/** @var PhotoCache $p */
+				$p = $this->getContainer()->query(PhotoCache::class);
+				$p->delete(
+					$event->getArgument('addressBookId'),
+					$event->getArgument('cardUri')
+				);
+			}
+		};
+		$dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $clearPhotoCache);
+		$dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::deleteCard', $clearPhotoCache);
+
 		$dispatcher->addListener('OC\AccountManager::userUpdated', function(GenericEvent $event) {
 			$user = $event->getSubject();
 			$syncService = $this->getContainer()->query(SyncService::class);
diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index 5deb648fa7f..983220c6ba0 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -93,7 +93,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
 								IUserManager $userManager,
-								EventDispatcherInterface $dispatcher = null) {
+								EventDispatcherInterface $dispatcher) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
 		$this->userManager = $userManager;
@@ -612,13 +612,11 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		$this->addChange($addressBookId, $cardUri, 1);
 		$this->updateProperties($addressBookId, $cardUri, $cardData);
 
-		if (!is_null($this->dispatcher)) {
-			$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::createCard',
-				new GenericEvent(null, [
-					'addressBookId' => $addressBookId,
-					'cardUri' => $cardUri,
-					'cardData' => $cardData]));
-		}
+		$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::createCard',
+			new GenericEvent(null, [
+				'addressBookId' => $addressBookId,
+				'cardUri' => $cardUri,
+				'cardData' => $cardData]));
 
 		return '"' . $etag . '"';
 	}
@@ -664,13 +662,11 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 		$this->addChange($addressBookId, $cardUri, 2);
 		$this->updateProperties($addressBookId, $cardUri, $cardData);
 
-		if (!is_null($this->dispatcher)) {
-			$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::updateCard',
-				new GenericEvent(null, [
-					'addressBookId' => $addressBookId,
-					'cardUri' => $cardUri,
-					'cardData' => $cardData]));
-		}
+		$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::updateCard',
+			new GenericEvent(null, [
+				'addressBookId' => $addressBookId,
+				'cardUri' => $cardUri,
+				'cardData' => $cardData]));
 
 		return '"' . $etag . '"';
 	}
@@ -696,12 +692,10 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 
 		$this->addChange($addressBookId, $cardUri, 3);
 
-		if (!is_null($this->dispatcher)) {
-			$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::deleteCard',
-				new GenericEvent(null, [
-					'addressBookId' => $addressBookId,
-					'cardUri' => $cardUri]));
-		}
+		$this->dispatcher->dispatch('\OCA\DAV\CardDAV\CardDavBackend::deleteCard',
+			new GenericEvent(null, [
+				'addressBookId' => $addressBookId,
+				'cardUri' => $cardUri]));
 
 		if ($ret === 1) {
 			if ($cardId !== null) {
diff --git a/apps/dav/lib/CardDAV/ImageExportPlugin.php b/apps/dav/lib/CardDAV/ImageExportPlugin.php
index 3ad7983451b..747c879ecd4 100644
--- a/apps/dav/lib/CardDAV/ImageExportPlugin.php
+++ b/apps/dav/lib/CardDAV/ImageExportPlugin.php
@@ -22,25 +22,28 @@
 
 namespace OCA\DAV\CardDAV;
 
+use OCP\Files\NotFoundException;
 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;
+	/** @var PhotoCache */
+	private $cache;
 
-	public function __construct(ILogger $logger) {
-		$this->logger = $logger;
+	/**
+	 * ImageExportPlugin constructor.
+	 *
+	 * @param PhotoCache $cache
+	 */
+	public function __construct(PhotoCache $cache) {
+		$this->cache = $cache;
 	}
 
 	/**
@@ -49,8 +52,7 @@ class ImageExportPlugin extends ServerPlugin {
 	 * @param Server $server
 	 * @return void
 	 */
-	function initialize(Server $server) {
-
+	public function initialize(Server $server) {
 		$this->server = $server;
 		$this->server->on('method:GET', [$this, 'httpGet'], 90);
 	}
@@ -60,9 +62,9 @@ class ImageExportPlugin extends ServerPlugin {
 	 *
 	 * @param RequestInterface $request
 	 * @param ResponseInterface $response
-	 * @return bool|void
+	 * @return bool
 	 */
-	function httpGet(RequestInterface $request, ResponseInterface $response) {
+	public function httpGet(RequestInterface $request, ResponseInterface $response) {
 
 		$queryParams = $request->getQueryParameters();
 		// TODO: in addition to photo we should also add logo some point in time
@@ -70,6 +72,8 @@ class ImageExportPlugin extends ServerPlugin {
 			return true;
 		}
 
+		$size = isset($queryParams['size']) ? (int)$queryParams['size'] : -1;
+
 		$path = $request->getPath();
 		$node = $this->server->tree->getNodeForPath($path);
 
@@ -85,90 +89,28 @@ class ImageExportPlugin extends ServerPlugin {
 			$aclPlugin->checkPrivileges($path, '{DAV:}read');
 		}
 
-		if ($result = $this->getPhoto($node)) {
-			// Allow caching
-			$response->setHeader('Cache-Control', 'private, max-age=3600, must-revalidate');
-			$response->setHeader('Etag', $node->getETag() );
-			$response->setHeader('Pragma', 'public');
+		// Fetch addressbook
+		$addressbookpath = explode('/', $path);
+		array_pop($addressbookpath);
+		$addressbookpath = implode('/', $addressbookpath);
+		/** @var AddressBook $addressbook */
+		$addressbook = $this->server->tree->getNodeForPath($addressbookpath);
 
-			$response->setHeader('Content-Type', $result['Content-Type']);
+		$response->setHeader('Cache-Control', 'private, max-age=3600, must-revalidate');
+		$response->setHeader('Etag', $node->getETag() );
+		$response->setHeader('Pragma', 'public');
+
+		try {
+			$file = $this->cache->get($addressbook->getResourceId(), $node->getName(), $size, $node);
+			$response->setHeader('Content-Type', $file->getMimeType());
 			$response->setHeader('Content-Disposition', 'attachment');
 			$response->setStatus(200);
 
-			$response->setBody($result['body']);
-
-			// Returning false to break the event chain
-			return false;
+			$response->setBody($file->getContent());
+		} catch (NotFoundException $e) {
+			$response->setStatus(404);
 		}
-		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);
-
-			$val = $photo->getValue();
-			if ($photo->getValueType() === 'URI') {
-				$parsed = \Sabre\URI\parse($val);
-				//only allow data://
-				if ($parsed['scheme'] !== 'data') {
-					return false;
-				}
-				if (substr_count($parsed['path'], ';') === 1) {
-					list($type,) = explode(';', $parsed['path']);
-				}
-				$val = file_get_contents($val);
-			}
-
-			$allowedContentTypes = [
-				'image/png',
-				'image/jpeg',
-				'image/gif',
-			];
-
-			if(!in_array($type, $allowedContentTypes, true)) {
-				$type = 'application/octet-stream';
-			}
-
-			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 '';
-	}
 }
diff --git a/apps/dav/lib/CardDAV/PhotoCache.php b/apps/dav/lib/CardDAV/PhotoCache.php
new file mode 100644
index 00000000000..c9625914263
--- /dev/null
+++ b/apps/dav/lib/CardDAV/PhotoCache.php
@@ -0,0 +1,246 @@
+<?php
+
+namespace OCA\DAV\CardDAV;
+
+use OCP\Files\IAppData;
+use OCP\Files\NotFoundException;
+use OCP\Files\NotPermittedException;
+use OCP\Files\SimpleFS\ISimpleFile;
+use OCP\Files\SimpleFS\ISimpleFolder;
+use Sabre\CardDAV\Card;
+use Sabre\VObject\Property\Binary;
+use Sabre\VObject\Reader;
+
+class PhotoCache {
+
+	/** @var IAppData $appData */
+	protected $appData;
+
+	/**
+	 * PhotoCache constructor.
+	 *
+	 * @param IAppData $appData
+	 */
+	public function __construct(IAppData $appData) {
+		$this->appData = $appData;
+	}
+
+	/**
+	 * @param int $addressBookId
+	 * @param string $cardUri
+	 * @param int $size
+	 * @param Card $card
+	 *
+	 * @return ISimpleFile
+	 * @throws NotFoundException
+	 */
+	public function get($addressBookId, $cardUri, $size, Card $card) {
+		$folder = $this->getFolder($addressBookId, $cardUri);
+
+		if ($this->isEmpty($folder)) {
+			$this->init($folder, $card);
+		}
+
+		if (!$this->hasPhoto($folder)) {
+			throw new NotFoundException();
+		}
+
+		if ($size !== -1) {
+			$size = 2 ** ceil(log($size) / log(2));
+		}
+
+		return $this->getFile($folder, $size);
+	}
+
+	/**
+	 * @param ISimpleFolder $folder
+	 * @return bool
+	 */
+	private function isEmpty(ISimpleFolder $folder) {
+		return $folder->getDirectoryListing() === [];
+	}
+
+	/**
+	 * @param ISimpleFolder $folder
+	 * @param Card $card
+	 */
+	private function init(ISimpleFolder $folder, Card $card) {
+		$data = $this->getPhoto($card);
+
+		if ($data === false) {
+			$folder->newFile('nophoto');
+		} else {
+			switch ($data['Content-Type']) {
+				case 'image/png':
+					$ext = 'png';
+					break;
+				case 'image/jpeg':
+					$ext = 'jpg';
+					break;
+				case 'image/gif':
+					$ext = 'gif';
+					break;
+			}
+			$file = $folder->newFile('photo.' . $ext);
+			$file->putContent($data['body']);
+		}
+	}
+
+	private function hasPhoto(ISimpleFolder $folder) {
+		return !$folder->fileExists('nophoto');
+	}
+
+	private function getFile(ISimpleFolder $folder, $size) {
+		$ext = $this->getExtension($folder);
+
+		if ($size === -1) {
+			$path = 'photo.' . $ext;
+		} else {
+			$path = 'photo.' . $size . '.' . $ext;
+		}
+
+		try {
+			$file = $folder->getFile($path);
+		} catch (NotFoundException $e) {
+			if ($size <= 0) {
+				throw new NotFoundException;
+			}
+
+			$photo = new \OC_Image();
+			/** @var ISimpleFile $file */
+			$file = $folder->getFile('photo.' . $ext);
+			$photo->loadFromData($file->getContent());
+
+			$ratio = $photo->width() / $photo->height();
+			if ($ratio < 1) {
+				$ratio = 1/$ratio;
+			}
+			$size = (int)($size * $ratio);
+
+			if ($size !== -1) {
+				$photo->resize($size);
+			}
+			try {
+				$file = $folder->newFile($path);
+				$file->putContent($photo->data());
+			} catch (NotPermittedException $e) {
+
+			}
+		}
+
+		return $file;
+	}
+
+
+	/**
+	 * @param int $addressBookId
+	 * @param string $cardUri
+	 * @return ISimpleFolder
+	 */
+	private function getFolder($addressBookId, $cardUri) {
+		$hash = md5($addressBookId . ' ' . $cardUri);
+		try {
+			return $this->appData->getFolder($hash);
+		} catch (NotFoundException $e) {
+			return $this->appData->newFolder($hash);
+		}
+	}
+
+	/**
+	 * Get the extension of the avatar. If there is no avatar throw Exception
+	 *
+	 * @param ISimpleFolder $folder
+	 * @return string
+	 * @throws NotFoundException
+	 */
+	private function getExtension(ISimpleFolder $folder) {
+		if ($folder->fileExists('photo.jpg')) {
+			return 'jpg';
+		} elseif ($folder->fileExists('photo.png')) {
+			return 'png';
+		} elseif ($folder->fileExists('photo.gif')) {
+			return 'gif';
+		}
+		throw new NotFoundException;
+	}
+
+	private function getPhoto(Card $node) {
+		try {
+			$vObject = $this->readCard($node->get());
+			if (!$vObject->PHOTO) {
+				return false;
+			}
+
+			$photo = $vObject->PHOTO;
+			$type = $this->getType($photo);
+
+			$val = $photo->getValue();
+			if ($photo->getValueType() === 'URI') {
+				$parsed = \Sabre\URI\parse($val);
+				//only allow data://
+				if ($parsed['scheme'] !== 'data') {
+					return false;
+				}
+				if (substr_count($parsed['path'], ';') === 1) {
+					list($type,) = explode(';', $parsed['path']);
+				}
+				$val = file_get_contents($val);
+			}
+
+			$allowedContentTypes = [
+				'image/png',
+				'image/jpeg',
+				'image/gif',
+			];
+
+			if(!in_array($type, $allowedContentTypes, true)) {
+				$type = 'application/octet-stream';
+			}
+
+			return [
+				'Content-Type' => $type,
+				'body' => $val
+			];
+		} catch(\Exception $ex) {
+
+		}
+		return false;
+	}
+
+	/**
+	 * @param string $cardData
+	 * @return \Sabre\VObject\Document
+	 */
+	private function readCard($cardData) {
+		return Reader::read($cardData);
+	}
+
+	/**
+	 * @param Binary $photo
+	 * @return string
+	 */
+	private function getType(Binary $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 '';
+	}
+
+	/**
+	 * @param int $addressBookId
+	 * @param string $cardUri
+	 */
+	public function delete($addressBookId, $cardUri) {
+		$folder = $this->getFolder($addressBookId, $cardUri);
+		$folder->delete();
+	}
+}
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 5b0715b0dad..df5b0ea05b6 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -30,6 +30,7 @@ namespace OCA\DAV;
 
 use OCA\DAV\CalDAV\Schedule\IMipPlugin;
 use OCA\DAV\CardDAV\ImageExportPlugin;
+use OCA\DAV\CardDAV\PhotoCache;
 use OCA\DAV\Comments\CommentsPlugin;
 use OCA\DAV\Connector\Sabre\Auth;
 use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin;
@@ -137,7 +138,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()));
+		$this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache'))));
 
 		// system tags plugins
 		$this->server->addPlugin(new SystemTagPlugin(
diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
index c108432d65b..f3a271a2db2 100644
--- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
+++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -34,9 +34,12 @@ use OCA\DAV\Connector\Sabre\Principal;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IL10N;
+use OCP\IUserManager;
 use Sabre\DAV\PropPatch;
 use Sabre\VObject\Component\VCard;
 use Sabre\VObject\Property\Text;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\EventDispatcher\GenericEvent;
 use Test\TestCase;
 
 /**
@@ -54,9 +57,12 @@ class CardDavBackendTest extends TestCase {
 	/** @var Principal | \PHPUnit_Framework_MockObject_MockObject */
 	private $principal;
 
-	/** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
 
+	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
+	private $dispatcher;
+
 	/** @var  IDBConnection */
 	private $db;
 
@@ -73,9 +79,7 @@ class CardDavBackendTest extends TestCase {
 	public function setUp() {
 		parent::setUp();
 
-		$this->userManager = $this->getMockBuilder('OCP\IUserManager')
-			->disableOriginalConstructor()
-			->getMock();
+		$this->userManager = $this->createMock(IUserManager::class);
 		$this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal')
 			->disableOriginalConstructor()
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
@@ -88,11 +92,11 @@ class CardDavBackendTest extends TestCase {
 		$this->principal->method('getGroupMembership')
 			->withAnyParameters()
 			->willReturn([self::UNIT_TEST_GROUP]);
+		$this->dispatcher = $this->createMock(EventDispatcherInterface::class);
 
 		$this->db = \OC::$server->getDatabaseConnection();
 
-		$this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, null);
-
+		$this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, $this->dispatcher);
 		// start every test with a empty cards_properties and cards table
 		$query = $this->db->getQueryBuilder();
 		$query->delete('cards_properties')->execute();
@@ -172,7 +176,7 @@ class CardDavBackendTest extends TestCase {
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
 		$backend = $this->getMockBuilder(CardDavBackend::class)
-				->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
+				->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
 				->setMethods(['updateProperties', 'purgeProperties'])->getMock();
 
 		// create a new address book
@@ -185,6 +189,16 @@ class CardDavBackendTest extends TestCase {
 		// updateProperties is expected twice, once for createCard and once for updateCard
 		$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, '');
 		$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, '***');
+
+		// Expect event
+		$this->dispatcher->expects($this->at(0))
+			->method('dispatch')
+			->with('\OCA\DAV\CardDAV\CardDavBackend::createCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
+				return $e->getArgument('addressBookId') === $bookId &&
+					$e->getArgument('cardUri') === $uri &&
+					$e->getArgument('cardData') === '';
+			}));
+
 		// create a card
 		$backend->createCard($bookId, $uri, '');
 
@@ -203,11 +217,28 @@ class CardDavBackendTest extends TestCase {
 		$this->assertArrayHasKey('size', $card);
 		$this->assertEquals('', $card['carddata']);
 
+		// Expect event
+		$this->dispatcher->expects($this->at(0))
+			->method('dispatch')
+			->with('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
+				return $e->getArgument('addressBookId') === $bookId &&
+					$e->getArgument('cardUri') === $uri &&
+					$e->getArgument('cardData') === '***';
+			}));
+
 		// update the card
 		$backend->updateCard($bookId, $uri, '***');
 		$card = $backend->getCard($bookId, $uri);
 		$this->assertEquals('***', $card['carddata']);
 
+		// Expect event
+		$this->dispatcher->expects($this->at(0))
+			->method('dispatch')
+			->with('\OCA\DAV\CardDAV\CardDavBackend::deleteCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
+				return $e->getArgument('addressBookId') === $bookId &&
+					$e->getArgument('cardUri') === $uri;
+			}));
+
 		// delete the card
 		$backend->expects($this->once())->method('purgeProperties')->with($bookId, $card['id']);
 		$backend->deleteCard($bookId, $uri);
@@ -218,7 +249,7 @@ class CardDavBackendTest extends TestCase {
 	public function testMultiCard() {
 
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -264,7 +295,7 @@ class CardDavBackendTest extends TestCase {
 
 	public function testDeleteWithoutCard() {
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
 			->setMethods([
 				'getCardId',
 				'addChange',
@@ -304,7 +335,7 @@ class CardDavBackendTest extends TestCase {
 
 	public function testSyncSupport() {
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -363,7 +394,7 @@ class CardDavBackendTest extends TestCase {
 		$cardId = 2;
 
 		$backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, null])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
 			->setMethods(['getCardId'])->getMock();
 
 		$backend->expects($this->any())->method('getCardId')->willReturn($cardId);
diff --git a/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
index ed311e79f4a..e773e41ba5e 100644
--- a/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
+++ b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
@@ -25,9 +25,13 @@
 namespace OCA\DAV\Tests\unit\CardDAV;
 
 
+use OCA\DAV\CardDAV\AddressBook;
 use OCA\DAV\CardDAV\ImageExportPlugin;
-use OCP\ILogger;
+use OCA\DAV\CardDAV\PhotoCache;
+use OCP\Files\NotFoundException;
+use OCP\Files\SimpleFS\ISimpleFile;
 use Sabre\CardDAV\Card;
+use Sabre\DAV\Node;
 use Sabre\DAV\Server;
 use Sabre\DAV\Tree;
 use Sabre\HTTP\RequestInterface;
@@ -36,32 +40,32 @@ use Test\TestCase;
 
 class ImageExportPluginTest extends TestCase {
 
-	/** @var ResponseInterface | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var ResponseInterface|\PHPUnit_Framework_MockObject_MockObject */
 	private $response;
-	/** @var RequestInterface | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
 	private $request;
-	/** @var ImageExportPlugin | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var ImageExportPlugin|\PHPUnit_Framework_MockObject_MockObject */
 	private $plugin;
 	/** @var Server */
 	private $server;
-	/** @var Tree | \PHPUnit_Framework_MockObject_MockObject */
+	/** @var Tree|\PHPUnit_Framework_MockObject_MockObject */
 	private $tree;
-	/** @var ILogger | \PHPUnit_Framework_MockObject_MockObject */
-	private $logger;
+	/** @var PhotoCache|\PHPUnit_Framework_MockObject_MockObject */
+	private $cache;
 
 	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->request = $this->createMock(RequestInterface::class);
+		$this->response = $this->createMock(ResponseInterface::class);
+		$this->server = $this->createMock(Server::class);
+		$this->tree = $this->createMock(Tree::class);
 		$this->server->tree = $this->tree;
-		$this->logger = $this->getMockBuilder('\OCP\ILogger')->getMock();
+		$this->cache = $this->createMock(PhotoCache::class);
 
-		$this->plugin = $this->getMockBuilder('OCA\DAV\CardDAV\ImageExportPlugin')
+		$this->plugin = $this->getMockBuilder(ImageExportPlugin::class)
 			->setMethods(['getPhoto'])
-			->setConstructorArgs([$this->logger])
+			->setConstructorArgs([$this->cache])
 			->getMock();
 		$this->plugin->initialize($this->server);
 	}
@@ -84,126 +88,115 @@ class ImageExportPluginTest extends TestCase {
 		];
 	}
 
-	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);
+	public function testNoCard() {
+		$this->request->method('getQueryParameters')
+			->willReturn([
+				'photo'
+			]);
+		$this->request->method('getPath')
+			->willReturn('user/book/card');
+
+		$node = $this->createMock(Node::class);
+		$this->tree->method('getNodeForPath')
+			->with('user/book/card')
+			->willReturn($node);
+
 		$result = $this->plugin->httpGet($this->request, $this->response);
 		$this->assertTrue($result);
 	}
 
+	public function dataTestCard() {
+		return [
+			[null, false],
+			[null, true],
+			[32, false],
+			[32, true],
+		];
+	}
+
 	/**
-	 * @dataProvider providesCardWithOrWithoutPhoto
-	 * @param bool $expected
-	 * @param array $getPhotoResult
+	 * @dataProvider dataTestCard
+	 *
+	 * @param $size
+	 * @param bool $photo
 	 */
-	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');
+	public function testCard($size, $photo) {
+		$query = ['photo' => null];
+		if ($size !== null) {
+			$query['size'] = $size;
+		}
+
+		$this->request->method('getQueryParameters')
+			->willReturn($query);
+		$this->request->method('getPath')
+			->willReturn('user/book/card');
 
-		$card = $this->getMockBuilder('Sabre\CardDAV\Card')->disableOriginalConstructor()->getMock();
+		$card = $this->createMock(Card::class);
 		$card->method('getETag')
 			->willReturn('"myEtag"');
-		$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->at(0))
-				->method('setHeader')
-				->with('Cache-Control', 'private, max-age=3600, must-revalidate');
-			$this->response
-				->expects($this->at(1))
-				->method('setHeader')
-				->with('Etag', '"myEtag"');
-			$this->response
-				->expects($this->at(2))
+		$card->method('getName')
+			->willReturn('card');
+		$book = $this->createMock(AddressBook::class);
+		$book->method('getResourceId')
+			->willReturn(1);
+
+		$this->tree->method('getNodeForPath')
+			->willReturnCallback(function($path) use ($card, $book) {
+				if ($path === 'user/book/card') {
+					return $card;
+				} else if ($path === 'user/book') {
+					return $book;
+				}
+				$this->fail();
+			});
+
+		$this->response->expects($this->at(0))
+			->method('setHeader')
+			->with('Cache-Control', 'private, max-age=3600, must-revalidate');
+		$this->response->expects($this->at(1))
+			->method('setHeader')
+			->with('Etag', '"myEtag"');
+		$this->response->expects($this->at(2))
+			->method('setHeader')
+			->with('Pragma', 'public');
+
+		$size = $size === null ? -1 : $size;
+
+		if ($photo) {
+			$file = $this->createMock(ISimpleFile::class);
+			$file->method('getMimeType')
+				->willReturn('imgtype');
+			$file->method('getContent')
+				->willReturn('imgdata');
+
+			$this->cache->method('get')
+				->with(1, 'card', $size, $card)
+				->willReturn($file);
+
+			$this->response->expects($this->at(3))
 				->method('setHeader')
-				->with('Pragma', 'public');
-			$this->response
-				->expects($this->at(3))
-				->method('setHeader')
-				->with('Content-Type', $getPhotoResult['Content-Type']);
-			$this->response
-				->expects($this->at(4))
+				->with('Content-Type', 'imgtype');
+			$this->response->expects($this->at(4))
 				->method('setHeader')
 				->with('Content-Disposition', 'attachment');
-			$this->response
-				->expects($this->once())
-				->method('setStatus');
-			$this->response
-				->expects($this->once())
-				->method('setBody');
+
+			$this->response->expects($this->once())
+				->method('setStatus')
+				->with(200);
+			$this->response->expects($this->once())
+				->method('setBody')
+				->with('imgdata');
+
+		} else {
+			$this->cache->method('get')
+				->with(1, 'card', $size, $card)
+				->willThrowException(new NotFoundException());
+			$this->response->expects($this->once())
+				->method('setStatus')
+				->with(404);
 		}
 
 		$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 4.1.1//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 4.1.1//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;ENCODING=b;TYPE=JPEG:MTIzNDU=\r\nEND:VCARD\r\n"
-			],
-			'vcard 3 with PHOTO URL' => [
-				false,
-				"BEGIN:VCARD\r\nVERSION:3.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.1//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO;TYPE=JPEG;VALUE=URI:http://example.com/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 4.1.1//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO:data:image/jpeg;base64,MTIzNDU=\r\nEND:VCARD\r\n"
-			],
-			'vcard 4 with PHOTO URL' => [
-				false,
-				"BEGIN:VCARD\r\nVERSION:4.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.1//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"
-			],
-			'vcard 4 with PHOTO AND INVALID MIMEtYPE' => [
-				[
-					'Content-Type' => 'application/octet-stream',
-					'body' => '12345'
-				],
-				"BEGIN:VCARD\r\nVERSION:4.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.1//EN\r\nUID:12345\r\nFN:12345\r\nN:12345;;;;\r\nPHOTO:data:image/svg;base64,MTIzNDU=\r\nEND:VCARD\r\n"
-			],
-		];
+		$this->assertFalse($result);
 	}
 }