Always dispatch Carddav events

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

3 files changed, 58 insertions, 33 deletions

diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php
index 04344e83fde..d1aeeefb03f 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);
 
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/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);