From e979b9c73561cb28f2be4c783477d8bdc34489b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 23 Mar 2016 12:28:54 +0100 Subject: [PATCH 1/2] Propagate birthdays of shared addressbooks to the sharee's birthday calendar as well --- apps/dav/appinfo/application.php | 2 + apps/dav/lib/caldav/birthdayservice.php | 58 +++++++++++++++++-------- apps/dav/lib/dav/sharing/backend.php | 3 +- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/apps/dav/appinfo/application.php b/apps/dav/appinfo/application.php index d06daf97f5..4ddd23d86b 100644 --- a/apps/dav/appinfo/application.php +++ b/apps/dav/appinfo/application.php @@ -147,6 +147,7 @@ class Application extends App { $listener = function($event) { if ($event instanceof GenericEvent) { + /** @var BirthdayService $b */ $b = $this->getContainer()->query('BirthdayService'); $b->onCardChanged( $event->getArgument('addressBookId'), @@ -161,6 +162,7 @@ class Application extends App { $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $listener); $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::deleteCard', function($event) { if ($event instanceof GenericEvent) { + /** @var BirthdayService $b */ $b = $this->getContainer()->query('BirthdayService'); $b->onCardDeleted( $event->getArgument('addressBookId'), diff --git a/apps/dav/lib/caldav/birthdayservice.php b/apps/dav/lib/caldav/birthdayservice.php index 274341949b..7fca4dc8c6 100644 --- a/apps/dav/lib/caldav/birthdayservice.php +++ b/apps/dav/lib/caldav/birthdayservice.php @@ -48,22 +48,33 @@ class BirthdayService { */ public function onCardChanged($addressBookId, $cardUri, $cardData) { + $shares = $this->cardDavBackEnd->getShares($addressBookId); + // TODO: resolve group shares + $shares = array_filter($shares, function($share) { + return !$share['{http://owncloud.org/ns}group-share']; + }); + $targetPrincipals = array_map(function($share) { + return $share['{http://owncloud.org/ns}principal']; + }, $shares); + $book = $this->cardDavBackEnd->getAddressBookById($addressBookId); - $principalUri = $book['principaluri']; - $calendar = $this->ensureCalendarExists($principalUri); - $objectUri = $book['uri'] . '-' . $cardUri. '.ics'; - $calendarData = $this->buildBirthdayFromContact($cardData); - $existing = $this->calDavBackEnd->getCalendarObject($calendar['id'], $objectUri); - if (is_null($calendarData)) { - if (!is_null($existing)) { - $this->calDavBackEnd->deleteCalendarObject($calendar['id'], $objectUri); - } - } else { - if (is_null($existing)) { - $this->calDavBackEnd->createCalendarObject($calendar['id'], $objectUri, $calendarData->serialize()); + $targetPrincipals[] = $book['principaluri']; + foreach ($targetPrincipals as $principalUri) { + $calendar = $this->ensureCalendarExists($principalUri); + $objectUri = $book['uri'] . '-' . $cardUri. '.ics'; + $calendarData = $this->buildBirthdayFromContact($cardData); + $existing = $this->calDavBackEnd->getCalendarObject($calendar['id'], $objectUri); + if (is_null($calendarData)) { + if (!is_null($existing)) { + $this->calDavBackEnd->deleteCalendarObject($calendar['id'], $objectUri); + } } else { - if ($this->birthdayEvenChanged($existing['calendardata'], $calendarData)) { - $this->calDavBackEnd->updateCalendarObject($calendar['id'], $objectUri, $calendarData->serialize()); + if (is_null($existing)) { + $this->calDavBackEnd->createCalendarObject($calendar['id'], $objectUri, $calendarData->serialize()); + } else { + if ($this->birthdayEvenChanged($existing['calendardata'], $calendarData)) { + $this->calDavBackEnd->updateCalendarObject($calendar['id'], $objectUri, $calendarData->serialize()); + } } } } @@ -74,11 +85,22 @@ class BirthdayService { * @param string $cardUri */ public function onCardDeleted($addressBookId, $cardUri) { + $shares = $this->cardDavBackEnd->getShares($addressBookId); + // TODO: resolve group shares + $shares = array_filter($shares, function($share) { + return !$share['{http://owncloud.org/ns}group-share']; + }); + $targetPrincipals = array_map(function($share) { + return $share['href']; + }, $shares); + $book = $this->cardDavBackEnd->getAddressBookById($addressBookId); - $principalUri = $book['principaluri']; - $calendar = $this->ensureCalendarExists($principalUri); - $objectUri = $book['uri'] . '-' . $cardUri. '.ics'; - $this->calDavBackEnd->deleteCalendarObject($calendar['id'], $objectUri); + $targetPrincipals[] = $book['principaluri']; + foreach ($targetPrincipals as $principalUri) { + $calendar = $this->ensureCalendarExists($principalUri); + $objectUri = $book['uri'] . '-' . $cardUri . '.ics'; + $this->calDavBackEnd->deleteCalendarObject($calendar['id'], $objectUri); + } } /** diff --git a/apps/dav/lib/dav/sharing/backend.php b/apps/dav/lib/dav/sharing/backend.php index ffc4193e34..225b773713 100644 --- a/apps/dav/lib/dav/sharing/backend.php +++ b/apps/dav/lib/dav/sharing/backend.php @@ -161,7 +161,8 @@ class Backend { 'commonName' => isset($p['{DAV:}displayname']) ? $p['{DAV:}displayname'] : '', 'status' => 1, 'readOnly' => ($row['access'] == self::ACCESS_READ), - '{'.\OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD.'}principal' => $row['principaluri'] + '{http://owncloud.org/ns}principal' => $row['principaluri'], + '{http://owncloud.org/ns}group-share' => is_null($p) ]; } From c8d6a9594ab7c0525cc9485b394547ae960d8f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 23 Mar 2016 14:12:50 +0100 Subject: [PATCH 2/2] Propagate birthday to group shares as well --- apps/dav/appinfo/application.php | 13 +++-- apps/dav/lib/caldav/birthdayservice.php | 47 +++++++++++------- apps/dav/lib/dav/groupprincipalbackend.php | 43 +++++++++++++--- .../unit/carddav/birthdayservicetest.php | 49 ++++++++++++++++++- 4 files changed, 121 insertions(+), 31 deletions(-) diff --git a/apps/dav/appinfo/application.php b/apps/dav/appinfo/application.php index 4ddd23d86b..593cd770be 100644 --- a/apps/dav/appinfo/application.php +++ b/apps/dav/appinfo/application.php @@ -27,6 +27,8 @@ use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\CardDAV\ContactsManager; use OCA\DAV\CardDAV\SyncJob; use OCA\DAV\CardDAV\SyncService; +use OCA\DAV\Connector\Sabre\Principal; +use OCA\DAV\DAV\GroupPrincipalBackend; use OCA\DAV\HookManager; use OCA\Dav\Migration\AddressBookAdapter; use OCA\Dav\Migration\CalendarAdapter; @@ -79,7 +81,7 @@ class Application extends App { /** @var IAppContainer $c */ $db = $c->getServer()->getDatabaseConnection(); $dispatcher = $c->getServer()->getEventDispatcher(); - $principal = new \OCA\DAV\Connector\Sabre\Principal( + $principal = new Principal( $c->getServer()->getUserManager(), $c->getServer()->getGroupManager() ); @@ -89,7 +91,7 @@ class Application extends App { $container->registerService('CalDavBackend', function($c) { /** @var IAppContainer $c */ $db = $c->getServer()->getDatabaseConnection(); - $principal = new \OCA\DAV\Connector\Sabre\Principal( + $principal = new Principal( $c->getServer()->getUserManager(), $c->getServer()->getGroupManager() ); @@ -122,11 +124,14 @@ class Application extends App { $container->registerService('BirthdayService', function($c) { /** @var IAppContainer $c */ + $g = new GroupPrincipalBackend( + $c->getServer()->getGroupManager() + ); return new BirthdayService( $c->query('CalDavBackend'), - $c->query('CardDavBackend') + $c->query('CardDavBackend'), + $g ); - }); } diff --git a/apps/dav/lib/caldav/birthdayservice.php b/apps/dav/lib/caldav/birthdayservice.php index 7fca4dc8c6..31897130ca 100644 --- a/apps/dav/lib/caldav/birthdayservice.php +++ b/apps/dav/lib/caldav/birthdayservice.php @@ -23,6 +23,7 @@ namespace OCA\DAV\CalDAV; use Exception; use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\DAV\GroupPrincipalBackend; use Sabre\VObject\Component\VCalendar; use Sabre\VObject\Reader; @@ -30,15 +31,20 @@ class BirthdayService { const BIRTHDAY_CALENDAR_URI = 'contact_birthdays'; + /** @var GroupPrincipalBackend */ + private $principalBackend; + /** * BirthdayService constructor. * * @param CalDavBackend $calDavBackEnd * @param CardDavBackend $cardDavBackEnd + * @param GroupPrincipalBackend $principalBackend */ - public function __construct($calDavBackEnd, $cardDavBackEnd) { + public function __construct($calDavBackEnd, $cardDavBackEnd, $principalBackend) { $this->calDavBackEnd = $calDavBackEnd; $this->cardDavBackEnd = $cardDavBackEnd; + $this->principalBackend = $principalBackend; } /** @@ -48,14 +54,7 @@ class BirthdayService { */ public function onCardChanged($addressBookId, $cardUri, $cardData) { - $shares = $this->cardDavBackEnd->getShares($addressBookId); - // TODO: resolve group shares - $shares = array_filter($shares, function($share) { - return !$share['{http://owncloud.org/ns}group-share']; - }); - $targetPrincipals = array_map(function($share) { - return $share['{http://owncloud.org/ns}principal']; - }, $shares); + $targetPrincipals = $this->getAllAffectedPrincipals($addressBookId); $book = $this->cardDavBackEnd->getAddressBookById($addressBookId); $targetPrincipals[] = $book['principaluri']; @@ -85,15 +84,7 @@ class BirthdayService { * @param string $cardUri */ public function onCardDeleted($addressBookId, $cardUri) { - $shares = $this->cardDavBackEnd->getShares($addressBookId); - // TODO: resolve group shares - $shares = array_filter($shares, function($share) { - return !$share['{http://owncloud.org/ns}group-share']; - }); - $targetPrincipals = array_map(function($share) { - return $share['href']; - }, $shares); - + $targetPrincipals = $this->getAllAffectedPrincipals($addressBookId); $book = $this->cardDavBackEnd->getAddressBookById($addressBookId); $targetPrincipals[] = $book['principaluri']; foreach ($targetPrincipals as $principalUri) { @@ -207,4 +198,24 @@ class BirthdayService { return false; } + /** + * @param $addressBookId + * @return mixed + */ + protected function getAllAffectedPrincipals($addressBookId) { + $targetPrincipals = []; + $shares = $this->cardDavBackEnd->getShares($addressBookId); + foreach ($shares as $share) { + if ($share['{http://owncloud.org/ns}group-share']) { + $users = $this->principalBackend->getGroupMemberSet($share['{http://owncloud.org/ns}principal']); + foreach ($users as $user) { + $targetPrincipals[] = $user['uri']; + } + } else { + $targetPrincipals[] = $share['{http://owncloud.org/ns}principal']; + } + } + return array_values(array_unique($targetPrincipals, SORT_STRING)); + } + } diff --git a/apps/dav/lib/dav/groupprincipalbackend.php b/apps/dav/lib/dav/groupprincipalbackend.php index 34c00b7927..e056863978 100644 --- a/apps/dav/lib/dav/groupprincipalbackend.php +++ b/apps/dav/lib/dav/groupprincipalbackend.php @@ -22,6 +22,7 @@ namespace OCA\DAV\DAV; use OCP\IGroup; use OCP\IGroupManager; +use OCP\IUser; use Sabre\DAV\Exception; use \Sabre\DAV\PropPatch; use Sabre\DAVACL\PrincipalBackend\BackendInterface; @@ -82,10 +83,10 @@ class GroupPrincipalBackend implements BackendInterface { return null; } $name = $elements[2]; - $user = $this->groupManager->get($name); + $group = $this->groupManager->get($name); - if (!is_null($user)) { - return $this->groupToPrincipal($user); + if (!is_null($group)) { + return $this->groupToPrincipal($group); } return null; @@ -99,8 +100,23 @@ class GroupPrincipalBackend implements BackendInterface { * @throws Exception */ public function getGroupMemberSet($principal) { - // TODO: implement if we want that - return []; + $elements = explode('/', $principal); + if ($elements[0] !== 'principals') { + return []; + } + if ($elements[1] !== 'groups') { + return []; + } + $name = $elements[2]; + $group = $this->groupManager->get($name); + + if (is_null($group)) { + return []; + } + + return array_map(function($user) { + return $this->userToPrincipal($user); + }, $group->getUsers()); } /** @@ -162,8 +178,21 @@ class GroupPrincipalBackend implements BackendInterface { protected function groupToPrincipal($group) { $groupId = $group->getGID(); $principal = [ - 'uri' => "principals/groups/$groupId", - '{DAV:}displayname' => $groupId, + 'uri' => "principals/groups/$groupId", + '{DAV:}displayname' => $groupId, + ]; + + return $principal; + } + + /** + * @param IUser $user + * @return array + */ + protected function userToPrincipal($user) { + $principal = [ + 'uri' => 'principals/users/' . $user->getUID(), + '{DAV:}displayname' => $user->getDisplayName(), ]; return $principal; diff --git a/apps/dav/tests/unit/carddav/birthdayservicetest.php b/apps/dav/tests/unit/carddav/birthdayservicetest.php index 2efb3c09ae..e15edd16c6 100644 --- a/apps/dav/tests/unit/carddav/birthdayservicetest.php +++ b/apps/dav/tests/unit/carddav/birthdayservicetest.php @@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\Unit\CardDAV; use OCA\DAV\CalDAV\BirthdayService; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\DAV\GroupPrincipalBackend; use Sabre\VObject\Component\VCalendar; use Sabre\VObject\Reader; use Test\TestCase; @@ -36,14 +37,17 @@ class BirthdayServiceTest extends TestCase { private $calDav; /** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject */ private $cardDav; + /** @var GroupPrincipalBackend | \PHPUnit_Framework_MockObject_MockObject */ + private $groupPrincialBackend; public function setUp() { parent::setUp(); $this->calDav = $this->getMockBuilder('OCA\DAV\CalDAV\CalDavBackend')->disableOriginalConstructor()->getMock(); $this->cardDav = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock(); + $this->groupPrincialBackend = $this->getMockBuilder('OCA\DAV\DAV\GroupPrincipalBackend')->disableOriginalConstructor()->getMock(); - $this->service = new BirthdayService($this->calDav, $this->cardDav); + $this->service = new BirthdayService($this->calDav, $this->cardDav, $this->groupPrincialBackend); } /** @@ -77,6 +81,7 @@ class BirthdayServiceTest extends TestCase { 'id' => 1234 ]); $this->calDav->expects($this->once())->method('deleteCalendarObject')->with(1234, 'default-gump.vcf.ics'); + $this->cardDav->expects($this->once())->method('getShares')->willReturn([]); $this->service->onCardDeleted(666, 'gump.vcf'); } @@ -96,10 +101,11 @@ class BirthdayServiceTest extends TestCase { ->willReturn([ 'id' => 1234 ]); + $this->cardDav->expects($this->once())->method('getShares')->willReturn([]); /** @var BirthdayService | \PHPUnit_Framework_MockObject_MockObject $service */ $service = $this->getMock('\OCA\DAV\CalDAV\BirthdayService', - ['buildBirthdayFromContact', 'birthdayEvenChanged'], [$this->calDav, $this->cardDav]); + ['buildBirthdayFromContact', 'birthdayEvenChanged'], [$this->calDav, $this->cardDav, $this->groupPrincialBackend]); if ($expectedOp === 'delete') { $this->calDav->expects($this->once())->method('getCalendarObject')->willReturn(''); @@ -132,6 +138,45 @@ class BirthdayServiceTest extends TestCase { $this->assertEquals($expected, $this->service->birthdayEvenChanged($old, $new)); } + public function testGetAllAffectedPrincipals() { + $this->cardDav->expects($this->once())->method('getShares')->willReturn([ + [ + '{http://owncloud.org/ns}group-share' => false, + '{http://owncloud.org/ns}principal' => 'principals/users/user01' + ], + [ + '{http://owncloud.org/ns}group-share' => false, + '{http://owncloud.org/ns}principal' => 'principals/users/user01' + ], + [ + '{http://owncloud.org/ns}group-share' => false, + '{http://owncloud.org/ns}principal' => 'principals/users/user02' + ], + [ + '{http://owncloud.org/ns}group-share' => true, + '{http://owncloud.org/ns}principal' => 'principals/groups/users' + ], + ]); + $this->groupPrincialBackend->expects($this->once())->method('getGroupMemberSet') + ->willReturn([ + [ + 'uri' => 'principals/users/user01', + ], + [ + 'uri' => 'principals/users/user02', + ], + [ + 'uri' => 'principals/users/user03', + ], + ]); + $users = $this->invokePrivate($this->service, 'getAllAffectedPrincipals', [6666]); + $this->assertEquals([ + 'principals/users/user01', + 'principals/users/user02', + 'principals/users/user03' + ], $users); + } + public function providesBirthday() { return [ [true,