From b691b7fd246bfd347b02206e4704bd916c5a1980 Mon Sep 17 00:00:00 2001 From: Piotr Mrowczynski Date: Sun, 11 Mar 2018 23:16:05 +0100 Subject: [PATCH] Dont use isSyncMaintained --- lib/private/Group/Backend.php | 4 -- lib/private/Group/Database.php | 7 --- lib/private/Group/SyncService.php | 47 +++++++++++++++++--- lib/private/User/Session.php | 4 +- lib/public/GroupInterface.php | 8 ---- tests/Core/Command/Group/SyncBackendTest.php | 1 - tests/lib/Group/ManagerTest.php | 3 +- tests/lib/Group/SyncServiceTest.php | 1 - tests/lib/User/SessionTest.php | 11 ++--- 9 files changed, 46 insertions(+), 40 deletions(-) diff --git a/lib/private/Group/Backend.php b/lib/private/Group/Backend.php index c581677c1bd0..a4deb0eb5a16 100644 --- a/lib/private/Group/Backend.php +++ b/lib/private/Group/Backend.php @@ -133,8 +133,4 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) { public function isVisibleForScope($scope) { return true; } - - public function isSyncMaintained() { - return false; - } } diff --git a/lib/private/Group/Database.php b/lib/private/Group/Database.php index 4929de0c009f..aa1eb6c1e295 100644 --- a/lib/private/Group/Database.php +++ b/lib/private/Group/Database.php @@ -149,11 +149,4 @@ public function groupExists($groupName) { $stmt->closeCursor(); return isset($data['exists']); } - - /** - * Groups and memberships of this backend are maintained by the users - */ - public function isSyncMaintained() { - return false; - } } \ No newline at end of file diff --git a/lib/private/Group/SyncService.php b/lib/private/Group/SyncService.php index 04e4bc34ab44..a046643f5b63 100644 --- a/lib/private/Group/SyncService.php +++ b/lib/private/Group/SyncService.php @@ -51,6 +51,10 @@ class SyncService { private $logger; /** @var string[] backendclass -> gid array map */ private $prefetchedGroupIds; + /** @var string[] backendclass -> BackendGroup array map */ + private $membershipsByBackend; + /** @var string uid - uid of last user for which sync was run */ + private $lastUser; /** * SyncService constructor. @@ -69,6 +73,8 @@ public function __construct(GroupMapper $groupMapper, $this->membershipManager = $membershipManager; $this->logger = $logger; $this->prefetchedGroupIds = []; + $this->membershipsByBackend = []; + $this->lastUser = null; } /** @@ -221,13 +227,7 @@ public function syncUserMemberships(GroupInterface $backend, $uid) { $remoteGids = $backend->getUserGroups($uid); /* @var BackendGroup[] $localSyncGidGroupMap */ - $localSyncGidGroupMap = []; - $localSyncGroups = $this->membershipManager->getMemberBackendGroupsByType($uid, - MembershipManager::MEMBERSHIP_TYPE_GROUP_USER, - MembershipManager::MAINTENANCE_TYPE_SYNC); - foreach($localSyncGroups as $localSyncGroup) { - $localSyncGidGroupMap[$localSyncGroup->getGroupId()] = $localSyncGroup; - } + $localSyncGidGroupMap = $this->getLocalBackendMemberships($backend, $uid); // Check which memberships need to removed or added $membershipsToRemove = array_diff(array_keys($localSyncGidGroupMap), $remoteGids); @@ -278,6 +278,39 @@ public function syncUserMemberships(GroupInterface $backend, $uid) { ); } + /** + * Return gid->BackendGroup map of user groups for this backend + * + * @param GroupInterface $backend + * @param $uid + */ + private function getLocalBackendMemberships(GroupInterface $backend, $uid) { + if ($this->lastUser !== $uid) { + // Fetch all memberships for this user and segregate by backend class + $this->lastUser = $uid; + $this->membershipsByBackend = []; + + /* @var BackendGroup[] $localSyncGroups */ + $localSyncGroups = $this->membershipManager->getMemberBackendGroupsByType($uid, + MembershipManager::MEMBERSHIP_TYPE_GROUP_USER, + MembershipManager::MAINTENANCE_TYPE_SYNC); + foreach($localSyncGroups as $localSyncGroup) { + if (!isset($this->membershipsByBackend[$localSyncGroup->getBackend()])) { + $this->membershipsByBackend[$localSyncGroup->getBackend()] = []; + } + $this->membershipsByBackend[$localSyncGroup->getBackend()][$localSyncGroup->getGroupId()] = $localSyncGroup; + } + } + + // Return and invalidate the cache for selected backend + if (isset($this->membershipsByBackend[get_class($backend)])) { + $memberships = $this->membershipsByBackend[get_class($backend)]; + unset($this->membershipsByBackend[get_class($backend)]); + return $memberships; + } + return []; + } + /** * Fetch backend service groups and * use callback function for each of the groups, if callback has been specified. diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 908469e0342e..c95e29669b37 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -495,9 +495,7 @@ public function tryBasicAuthLogin(IRequest $request) { private function updateUserMemberships(IUser $user) { // Update only the backends which are sync maintained foreach($this->groupManager->getBackends() as $groupBackend) { - if ($groupBackend->isSyncMaintained()) { - $this->groupSyncService->syncUserMemberships($groupBackend, $user->getUID()); - } + $this->groupSyncService->syncUserMemberships($groupBackend, $user->getUID()); }; } diff --git a/lib/public/GroupInterface.php b/lib/public/GroupInterface.php index 6a10caf318b4..0fe6b088d173 100644 --- a/lib/public/GroupInterface.php +++ b/lib/public/GroupInterface.php @@ -126,12 +126,4 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0); */ public function isVisibleForScope($scope); - /** - * Returns whether the groups and memberships are to be maintained by core - * and sync mechanism to be used. - * - * @since 10.0.0 - */ - public function isSyncMaintained(); - } diff --git a/tests/Core/Command/Group/SyncBackendTest.php b/tests/Core/Command/Group/SyncBackendTest.php index c87cbd36606c..9089302d0030 100644 --- a/tests/Core/Command/Group/SyncBackendTest.php +++ b/tests/Core/Command/Group/SyncBackendTest.php @@ -101,7 +101,6 @@ protected function setUp() { 'addToGroup', 'removeFromGroup', 'isVisibleForScope', - 'isSyncMaintained', ]) ->getMock(); $this->userBackend1 = $this->getMockBuilder(UserInterface::class) diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 9d2dbcdc3a41..515691593aad 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -90,8 +90,7 @@ private function getTestBackend($implementedActions = null, $visibleForScopes = 'deleteGroup', 'addToGroup', 'removeFromGroup', - 'isVisibleForScope', - 'isSyncMaintained' + 'isVisibleForScope' ]) ->getMock(); $backend->expects($this->any()) diff --git a/tests/lib/Group/SyncServiceTest.php b/tests/lib/Group/SyncServiceTest.php index 1b1e6c314c18..8fecc2574cfb 100644 --- a/tests/lib/Group/SyncServiceTest.php +++ b/tests/lib/Group/SyncServiceTest.php @@ -80,7 +80,6 @@ public function setUp() { 'addToGroup', 'removeFromGroup', 'isVisibleForScope', - 'isSyncMaintained', ]) ->getMock(); diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index fa087d41dfe2..e02123e67330 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -76,9 +76,6 @@ protected function setUp() { $this->groupSyncService = $this->createMock(\OC\Group\SyncService::class); $groupBackend = $this->createMock(GroupInterface::class); - $groupBackend->expects($this->any()) - ->method('isSyncMaintained') - ->willReturn(true); $this->groupManager->expects($this->any()) ->method('getBackends') ->willReturn([$groupBackend]); @@ -1052,8 +1049,8 @@ public function testLogout() { ->method('set') ->with('user_id', 'foo'); - /** @var Manager $manager */ - $manager = $this->createMock(Manager::class); + /** @var Manager $userManager */ + $userManager = $this->createMock(Manager::class); /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */ $user = $this->createMock(IUser::class); @@ -1061,8 +1058,8 @@ public function testLogout() { ->method('getUID') ->will($this->returnValue('foo')); - $userSession = new Session($manager, $session, $this->timeFactory, - $this->tokenProvider, $this->config, $this->serviceLoader, $this->userSyncService); + $userSession = new Session($userManager, $this->groupManager, $session, $this->timeFactory, + $this->tokenProvider, $this->config, $this->serviceLoader, $this->userSyncService, $this->groupSyncService); $userSession->setUser($user); $this->assertTrue($userSession->logout());