From 2c19aac9e1452872400e09fadd5a0629c1681596 Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=B4me=20Chilliet?= Date: Thu, 20 Jul 2023 16:43:50 +0200 Subject: [PATCH] Move UpdateGroups methods to a service MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/user_ldap/lib/Jobs/UpdateGroups.php | 180 +----------------- .../lib/Service/UpdateGroupsService.php | 179 +++++++++++++++++ .../UpdateGroupsServiceTest.php} | 94 ++++----- 5 files changed, 222 insertions(+), 233 deletions(-) create mode 100644 apps/user_ldap/lib/Service/UpdateGroupsService.php rename apps/user_ldap/tests/{Jobs/UpdateGroupsTest.php => Service/UpdateGroupsServiceTest.php} (66%) diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 1e75c485300..74ffce2caff 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -78,6 +78,7 @@ return array( 'OCA\\User_LDAP\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => $baseDir . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => $baseDir . '/../lib/Proxy.php', + 'OCA\\User_LDAP\\Service\\UpdateGroupsService' => $baseDir . '/../lib/Service/UpdateGroupsService.php', 'OCA\\User_LDAP\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', 'OCA\\User_LDAP\\Settings\\Section' => $baseDir . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => $baseDir . '/../lib/UserPluginManager.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index c488b2b0a60..cead1740b88 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -93,6 +93,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', 'OCA\\User_LDAP\\PagedResults\\TLinkId' => __DIR__ . '/..' . '/../lib/PagedResults/TLinkId.php', 'OCA\\User_LDAP\\Proxy' => __DIR__ . '/..' . '/../lib/Proxy.php', + 'OCA\\User_LDAP\\Service\\UpdateGroupsService' => __DIR__ . '/..' . '/../lib/Service/UpdateGroupsService.php', 'OCA\\User_LDAP\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', 'OCA\\User_LDAP\\Settings\\Section' => __DIR__ . '/..' . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => __DIR__ . '/..' . '/../lib/UserPluginManager.php', diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 1ae8e5fcd6b..487f36f4f30 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -1,10 +1,14 @@ * @author Bart Visscher * @author Christoph Wurst + * @author Côme Chilliet * @author Joas Schilling * @author Lukas Reschke * @author Morris Jobke @@ -30,32 +34,15 @@ namespace OCA\User_LDAP\Jobs; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; -use OCP\User_LDAP\Db\GroupMembership; -use OCP\User_LDAP\Db\GroupMembershipMapper; -use OCA\User_LDAP\Group_Proxy; use OCP\DB\Exception; -use OCP\EventDispatcher\IEventDispatcher; -use OCP\Group\Events\UserAddedEvent; -use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; -use OCP\IDBConnection; -use OCP\IGroupManager; -use OCP\IUser; -use OCP\IUserManager; +use OCA\User_LDAP\Service\UpdateGroupsService; use Psr\Log\LoggerInterface; class UpdateGroups extends TimedJob { - /** @var ?array */ - private ?array $groupsFromDB = null; - public function __construct( - private Group_Proxy $groupBackend, - private IEventDispatcher $dispatcher, - private IGroupManager $groupManager, - private IUserManager $userManager, + private UpdateGroupsService $service, private LoggerInterface $logger, - private IDBConnection $dbc, - private GroupMembershipMapper $groupMembershipMapper, IConfig $config, ITimeFactory $timeFactory, ) { @@ -68,158 +55,7 @@ class UpdateGroups extends TimedJob { * @throws Exception */ public function run($argument): void { - $this->updateGroups(); - } - - /** - * @throws Exception - */ - public function updateGroups(): void { - $this->logger->debug( - 'Run background job "updateGroups"', - ['app' => 'user_ldap'] - ); - - $knownGroups = $this->groupMembershipMapper->getKnownGroups(); - $actualGroups = $this->groupBackend->getGroups(); - - if (empty($actualGroups) && empty($knownGroups)) { - $this->logger->info( - 'bgJ "updateGroups" – groups do not seem to be configured properly, aborting.', - ['app' => 'user_ldap'] - ); - return; - } - - $this->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); - $this->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); - $this->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); - - $this->logger->debug( - 'bgJ "updateGroups" – Finished.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $groups - * @throws Exception - */ - private function handleKnownGroups(array $groups): void { - $this->logger->debug( - 'bgJ "updateGroups" – Dealing with known Groups.', - ['app' => 'user_ldap'] - ); - - foreach ($groups as $group) { - $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); - $knownUsers = array_map( - fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), - $groupMemberships - ); - $groupMemberships = array_combine($knownUsers, $groupMemberships); - $actualUsers = $this->groupBackend->usersInGroup($group); - - $groupObject = $this->groupManager->get($group); - if ($groupObject === null) { - /* We are not expecting the group to not be found since it was returned by $this->groupBackend->getGroups() */ - $this->logger->error( - 'bgJ "updateGroups" – Failed to get group {group} for update', - [ - 'app' => 'user_ldap', - 'group' => $group - ] - ); - continue; - } - foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { - $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); - $userObject = $this->userManager->get($removedUser); - if ($userObject instanceof IUser) { - $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); - } - $this->logger->info( - 'bgJ "updateGroups" – {user} removed from {group}', - [ - 'app' => 'user_ldap', - 'user' => $removedUser, - 'group' => $group - ] - ); - } - foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); - $userObject = $this->userManager->get($addedUser); - if ($userObject instanceof IUser) { - $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); - } - $this->logger->info( - 'bgJ "updateGroups" – {user} added to {group}', - [ - 'app' => 'user_ldap', - 'user' => $addedUser, - 'group' => $group - ] - ); - } - } - $this->logger->debug( - 'bgJ "updateGroups" – FINISHED dealing with known Groups.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $createdGroups - * @throws Exception - */ - private function handleCreatedGroups(array $createdGroups): void { - $this->logger->debug( - 'bgJ "updateGroups" – dealing with created Groups.', - ['app' => 'user_ldap'] - ); - - foreach ($createdGroups as $createdGroup) { - $this->logger->info( - 'bgJ "updateGroups" – new group "' . $createdGroup . '" found.', - ['app' => 'user_ldap'] - ); - - $users = $this->groupBackend->usersInGroup($createdGroup); - foreach ($users as $user) { - $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); - } - $groupObject = $this->groupManager->get($group); - if ($groupObject instanceof IGroup) { - $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); - } - } - $this->logger->debug( - 'bgJ "updateGroups" – FINISHED dealing with created Groups.', - ['app' => 'user_ldap'] - ); - } - - /** - * @param string[] $removedGroups - * @throws Exception - */ - private function handleRemovedGroups(array $removedGroups): void { - $this->logger->debug( - 'bgJ "updateGroups" – dealing with removed groups.', - ['app' => 'user_ldap'] - ); - - $this->groupMembershipMapper->deleteGroups($removedGroups); - - //TODO find a way to dispatch GroupDeletedEvent - - $this->logger->info( - 'bgJ "updateGroups" – groups {removedGroups} were removed.', - [ - 'app' => 'user_ldap', - 'removedGroups' => $removedGroups - ] - ); + $this->logger->debug('Run background job "updateGroups"'); + $this->service->updateGroups(); } } diff --git a/apps/user_ldap/lib/Service/UpdateGroupsService.php b/apps/user_ldap/lib/Service/UpdateGroupsService.php new file mode 100644 index 00000000000..aaa86435695 --- /dev/null +++ b/apps/user_ldap/lib/Service/UpdateGroupsService.php @@ -0,0 +1,179 @@ + + * @author Bart Visscher + * @author Christoph Wurst + * @author Côme Chilliet + * @author Joas Schilling + * @author Lukas Reschke + * @author Morris Jobke + * @author Robin Appelman + * @author Robin McCorkell + * + * @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 + * + */ + +namespace OCA\User_LDAP\Service; + +use OCA\User_LDAP\Db\GroupMembership; +use OCA\User_LDAP\Db\GroupMembershipMapper; +use OCA\User_LDAP\Group_Proxy; +use OCP\DB\Exception; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; +use OCP\IGroupManager; +use OCP\IUser; +use OCP\IUserManager; +use Psr\Log\LoggerInterface; + +class UpdateGroupsService { + public function __construct( + private Group_Proxy $groupBackend, + private IEventDispatcher $dispatcher, + private IGroupManager $groupManager, + private IUserManager $userManager, + private LoggerInterface $logger, + private GroupMembershipMapper $groupMembershipMapper, + ) { + } + + /** + * @throws Exception + */ + public function updateGroups(): void { + $knownGroups = $this->groupMembershipMapper->getKnownGroups(); + $actualGroups = $this->groupBackend->getGroups(); + + if (empty($actualGroups) && empty($knownGroups)) { + $this->logger->info( + 'service "updateGroups" – groups do not seem to be configured properly, aborting.', + ); + return; + } + + $this->service->handleKnownGroups(array_intersect($actualGroups, $knownGroups)); + $this->service->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); + $this->service->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); + + $this->logger->debug('service "updateGroups" – Finished.'); + } + + /** + * @param string[] $groups + * @throws Exception + */ + public function handleKnownGroups(array $groups): void { + $this->logger->debug('service "updateGroups" – Dealing with known Groups.'); + + foreach ($groups as $group) { + $groupMemberships = $this->groupMembershipMapper->findGroupMemberships($group); + $knownUsers = array_map( + fn (GroupMembership $groupMembership): string => $groupMembership->getUserid(), + $groupMemberships + ); + $groupMemberships = array_combine($knownUsers, $groupMemberships); + $actualUsers = $this->groupBackend->usersInGroup($group); + + $groupObject = $this->groupManager->get($group); + if ($groupObject === null) { + /* We are not expecting the group to not be found since it was returned by $this->groupBackend->getGroups() */ + $this->logger->error( + 'service "updateGroups" – Failed to get group {group} for update', + [ + 'group' => $group + ] + ); + continue; + } + foreach (array_diff($knownUsers, $actualUsers) as $removedUser) { + $this->groupMembershipMapper->delete($groupMemberships[$removedUser]); + $userObject = $this->userManager->get($removedUser); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserRemovedEvent($groupObject, $userObject)); + } + $this->logger->info( + 'service "updateGroups" – {user} removed from {group}', + [ + 'user' => $removedUser, + 'group' => $group + ] + ); + } + foreach (array_diff($actualUsers, $knownUsers) as $addedUser) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $group,'userid' => $addedUser])); + $userObject = $this->userManager->get($addedUser); + if ($userObject instanceof IUser) { + $this->dispatcher->dispatchTyped(new UserAddedEvent($groupObject, $userObject)); + } + $this->logger->info( + 'service "updateGroups" – {user} added to {group}', + [ + 'user' => $addedUser, + 'group' => $group + ] + ); + } + } + $this->logger->debug('service "updateGroups" – FINISHED dealing with known Groups.'); + } + + /** + * @param string[] $createdGroups + * @throws Exception + */ + public function handleCreatedGroups(array $createdGroups): void { + $this->logger->debug('service "updateGroups" – dealing with created Groups.'); + + foreach ($createdGroups as $createdGroup) { + $this->logger->info('service "updateGroups" – new group "' . $createdGroup . '" found.'); + + $users = $this->groupBackend->usersInGroup($createdGroup); + foreach ($users as $user) { + $this->groupMembershipMapper->insert(GroupMembership::fromParams(['groupid' => $createdGroup,'userid' => $user])); + } + $groupObject = $this->groupManager->get($group); + if ($groupObject instanceof IGroup) { + $this->dispatcher->dispatchTyped(new GroupCreatedEvent($groupObject)); + } + } + $this->logger->debug('service "updateGroups" – FINISHED dealing with created Groups.'); + } + + /** + * @param string[] $removedGroups + * @throws Exception + */ + public function handleRemovedGroups(array $removedGroups): void { + $this->logger->debug('service "updateGroups" – dealing with removed groups.'); + + $this->groupMembershipMapper->deleteGroups($removedGroups); + + //TODO find a way to dispatch GroupDeletedEvent + + $this->logger->info( + 'service "updateGroups" – groups {removedGroups} were removed.', + [ + 'removedGroups' => $removedGroups + ] + ); + } +} diff --git a/apps/user_ldap/tests/Jobs/UpdateGroupsTest.php b/apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php similarity index 66% rename from apps/user_ldap/tests/Jobs/UpdateGroupsTest.php rename to apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php index 22a02451e27..abf9c57c9c1 100644 --- a/apps/user_ldap/tests/Jobs/UpdateGroupsTest.php +++ b/apps/user_ldap/tests/Service/UpdateGroupsServiceTest.php @@ -6,6 +6,7 @@ declare(strict_types=1); * @copyright Copyright (c) 2020 Arthur Schiwon * * @author Arthur Schiwon + * @author Côme Chilliet * @author Joas Schilling * * @license GNU AGPL version 3 or any later version @@ -24,19 +25,17 @@ declare(strict_types=1); * along with this program. If not, see . * */ -namespace OCA\user_ldap\tests\Jobs; +namespace OCA\user_ldap\tests\Service; +use OCA\User_LDAP\Db\GroupMembership; +use OCA\User_LDAP\Db\GroupMembershipMapper; use OCA\User_LDAP\Group_Proxy; -use OCA\User_LDAP\Jobs\UpdateGroups; +use OCA\User_LDAP\Service\UpdateGroupsService; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\DB\IResult; -use OCP\DB\QueryBuilder\IExpressionBuilder; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; -use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; @@ -44,10 +43,8 @@ use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; -use function serialize; - -class UpdateGroupsTest extends TestCase { +class UpdateGroupsServiceTest extends TestCase { /** @var Group_Proxy|MockObject */ protected $groupBackend; /** @var IEventDispatcher|MockObject */ @@ -58,14 +55,14 @@ class UpdateGroupsTest extends TestCase { protected $userManager; /** @var LoggerInterface|MockObject */ protected $logger; - /** @var IDBConnection|MockObject */ - protected $dbc; + /** @var GroupMembershipMapper|MockObject */ + protected $groupMembershipMapper; /** @var IConfig|MockObject */ protected $config; /** @var ITimeFactory|MockObject */ protected $timeFactory; - protected UpdateGroups $updateGroupsJob; + protected UpdateGroupsService $updateGroupsService; public function setUp(): void { $this->groupBackend = $this->createMock(Group_Proxy::class); @@ -73,17 +70,17 @@ class UpdateGroupsTest extends TestCase { $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->dbc = $this->createMock(IDBConnection::class); + $this->groupMembershipMapper = $this->createMock(GroupMembershipMapper::class); $this->config = $this->createMock(IConfig::class); $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->updateGroupsJob = new UpdateGroups( + $this->updateGroupsService = new UpdateGroupsService( $this->groupBackend, $this->dispatcher, $this->groupManager, $this->userManager, $this->logger, - $this->dbc, + $this->groupMembershipMapper, $this->config, $this->timeFactory ); @@ -91,12 +88,12 @@ class UpdateGroupsTest extends TestCase { public function testHandleKnownGroups(): void { $knownGroups = [ - 'emptyGroup' => serialize([]), - 'stableGroup' => serialize(['userA', 'userC', 'userE']), - 'groupWithAdditions' => serialize(['userA', 'userC', 'userE']), - 'groupWithRemovals' => serialize(['userA', 'userC', 'userDeleted', 'userE']), - 'groupWithAdditionsAndRemovals' => serialize(['userA', 'userC', 'userE']), - 'vanishedGroup' => serialize(['userB', 'userDeleted']) + 'emptyGroup' => [], + 'stableGroup' => ['userA', 'userC', 'userE'], + 'groupWithAdditions' => ['userA', 'userC', 'userE'], + 'groupWithRemovals' => ['userA', 'userC', 'userDeleted', 'userE'], + 'groupWithAdditionsAndRemovals' => ['userA', 'userC', 'userE'], + 'vanishedGroup' => ['userB', 'userDeleted'], ]; $knownGroupsDB = []; foreach ($knownGroups as $gid => $members) { @@ -115,45 +112,20 @@ class UpdateGroupsTest extends TestCase { ]; $groups = array_intersect(array_keys($knownGroups), array_keys($actualGroups)); - /** @var IQueryBuilder|MockObject $updateQb */ - $updateQb = $this->createMock(IQueryBuilder::class); - $updateQb->expects($this->once()) - ->method('update') - ->willReturn($updateQb); - $updateQb->expects($this->once()) - ->method('set') - ->willReturn($updateQb); - $updateQb->expects($this->once()) - ->method('where') - ->willReturn($updateQb); - // three groups need to be updated - $updateQb->expects($this->exactly(3)) - ->method('setParameters'); - $updateQb->expects($this->exactly(3)) - ->method('executeStatement'); - $updateQb->expects($this->any()) - ->method('expr') - ->willReturn($this->createMock(IExpressionBuilder::class)); - - $stmt = $this->createMock(IResult::class); - $stmt->expects($this->once()) - ->method('fetchAll') - ->willReturn($knownGroupsDB); - - $selectQb = $this->createMock(IQueryBuilder::class); - $selectQb->expects($this->once()) - ->method('select') - ->willReturn($selectQb); - $selectQb->expects($this->once()) - ->method('from') - ->willReturn($selectQb); - $selectQb->expects($this->once()) - ->method('executeQuery') - ->willReturn($stmt); - - $this->dbc->expects($this->any()) - ->method('getQueryBuilder') - ->willReturnOnConsecutiveCalls($updateQb, $selectQb); + $this->groupMembershipMapper->expects($this->never()) + ->method('getKnownGroups'); + $this->groupMembershipMapper->expects($this->exactly(5)) + ->method('findGroupMemberships') + ->willReturnCallback( + fn ($group) => array_map( + fn ($userid) => GroupMembership::fromParams(['groupid' => $group,'userid' => $userid]), + $knownGroups[$group] + ) + ); + $this->groupMembershipMapper->expects($this->exactly(3)) + ->method('delete'); + $this->groupMembershipMapper->expects($this->exactly(2)) + ->method('insert'); $this->groupBackend->expects($this->any()) ->method('usersInGroup') @@ -192,7 +164,7 @@ class UpdateGroupsTest extends TestCase { } }); - $this->invokePrivate($this->updateGroupsJob, 'handleKnownGroups', [$groups]); + $this->updateGroupsService->handleKnownGroups($groups); $this->assertSame(2, $removedEvents); $this->assertSame(2, $addedEvents); -- 2.39.5