From eaf6defe59ce1f3ab25e79767b3fd16f4915e245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 29 Dec 2021 16:09:24 +0100 Subject: [PATCH] Move any circles API usage to internal service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/BoardMapper.php | 19 +++++-------- lib/Service/CirclesService.php | 44 ++++++++++++++++++++++++++++--- lib/Service/PermissionService.php | 4 +-- tests/psalm-baseline.xml | 39 +++++---------------------- tests/unit/Db/AclMapperTest.php | 2 ++ tests/unit/Db/BoardMapperTest.php | 2 ++ 6 files changed, 60 insertions(+), 50 deletions(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 2a204c50f..6b08000ba 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -24,6 +24,7 @@ namespace OCA\Deck\Db; use OC\Cache\CappedMemoryCache; +use OCA\Deck\Service\CirclesService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -38,10 +39,9 @@ class BoardMapper extends QBMapper implements IPermissionMapper { private $stackMapper; private $userManager; private $groupManager; + private $circlesService; private $logger; - private $circlesEnabled; - /** @var CappedMemoryCache */ private $userBoardCache; /** @var CappedMemoryCache */ @@ -54,6 +54,7 @@ class BoardMapper extends QBMapper implements IPermissionMapper { StackMapper $stackMapper, IUserManager $userManager, IGroupManager $groupManager, + CirclesService $circlesService, LoggerInterface $logger ) { parent::__construct($db, 'deck_boards', Board::class); @@ -62,12 +63,11 @@ class BoardMapper extends QBMapper implements IPermissionMapper { $this->stackMapper = $stackMapper; $this->userManager = $userManager; $this->groupManager = $groupManager; + $this->circlesService = $circlesService; $this->logger = $logger; $this->userBoardCache = new CappedMemoryCache(); $this->boardCache = new CappedMemoryCache(); - - $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles'); } @@ -315,12 +315,7 @@ class BoardMapper extends QBMapper implements IPermissionMapper { public function findAllByCircles(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, bool $includeArchived = true, ?int $before = null, ?string $term = null) { - if (!$this->circlesEnabled) { - return []; - } - $circles = array_map(function ($circle) { - return $circle->getUniqueId(); - }, \OCA\Circles\Api\v1\Circles::joinedCircles($userId, true)); + $circles = $this->circlesService->getUserCircles($userId); if (count($circles) === 0) { return []; } @@ -449,11 +444,11 @@ class BoardMapper extends QBMapper implements IPermissionMapper { return null; } if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { - if (!$this->circlesEnabled) { + if (!$this->circlesService->isCirclesEnabled()) { return null; } try { - $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); + $circle = $this->circlesService->getCircle($acl->getParticipant()); if ($circle) { return new Circle($circle); } diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index 7f5157a15..6a471b41d 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -27,8 +27,11 @@ declare(strict_types=1); namespace OCA\Deck\Service; use OCA\Circles\CirclesManager; +use OCA\Circles\Model\Circle; use OCA\Circles\Model\Member; +use OCA\Circles\Model\Probes\CircleProbe; use OCP\App\IAppManager; +use Throwable; /** * Wrapper around circles app API since it is not in a public namespace so we need to make sure that @@ -45,15 +48,24 @@ class CirclesService { return $this->circlesEnabled; } - public function getCircle($circleId) { + public function getCircle(string $circleId): ?Circle { if (!$this->circlesEnabled) { return null; } - return \OCA\Circles\Api\v1\Circles::detailsCircle($circleId, true); + try { + + // Enforce current user condition since we always want the full list of members + /** @var CirclesManager $circlesManager */ + $circlesManager = \OC::$server->get(CirclesManager::class); + $circlesManager->startSuperSession(); + return $circlesManager->getCircle($circleId); + } catch (Throwable $e) { + } + return null; } - public function isUserInCircle($circleId, $userId): bool { + public function isUserInCircle(string $circleId, string $userId): bool { if (!$this->circlesEnabled) { return false; } @@ -66,8 +78,32 @@ class CirclesService { $circle = $circlesManager->getCircle($circleId); $member = $circle->getInitiator(); return $member !== null && $member->getLevel() >= Member::LEVEL_MEMBER; - } catch (\Exception $e) { + } catch (Throwable $e) { } return false; } + + /** + * @param string $userId + * @return string[] circle single ids + */ + public function getUserCircles(string $userId): array { + if (!$this->circlesEnabled) { + return []; + } + + try { + /** @var CirclesManager $circlesManager */ + $circlesManager = \OC::$server->get(CirclesManager::class); + $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); + $circlesManager->startSession($federatedUser); + $probe = new CircleProbe(); + $probe->mustBeMember(); + return array_map(function (Circle $circle) { + return $circle->getSingleId(); + }, $circlesManager->getCircles($probe)); + } catch (Throwable $e) { + } + return []; + } } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 8691b930b..498e8904c 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -280,14 +280,14 @@ class PermissionService { if ($this->circlesService->isCirclesEnabled() && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { try { - $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); + $circle = $this->circlesService->getCircle($acl->getParticipant()); if ($circle === null) { $this->logger->info('No circle found for acl rule ' . $acl->getId()); continue; } foreach ($circle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1 || $member->getLevel() >= Member::LEVEL_MEMBER) { + if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { // deck currently only supports user members in circles continue; } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index fa465d41a..a0f44facc 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + $message !== null @@ -124,10 +124,6 @@ $offset !== null $offset !== null - - \OCA\Circles\Api\v1\Circles - \OCA\Circles\Api\v1\Circles - @@ -136,7 +132,6 @@ - $entity->getId() @@ -224,9 +219,11 @@ - \OCA\Circles\Api\v1\Circles + ?Circle - + + $circlesManager + $circlesManager $circlesManager @@ -272,17 +269,16 @@ - + $this->rootFolder $this->rootFolder IRootFolder - ShareNotFound + $circle Member - \OCA\Circles\Api\v1\Circles @@ -290,27 +286,6 @@ BadRquestException - - - $shares - - - getSharesInFolder - - - throw new GenericShareException('Already shared', $this->l->t('Path is already shared with this card'), 403); - - - GenericShareException - GenericShareException - ShareNotFound - ShareNotFound - ShareNotFound - ShareNotFound - ShareNotFound - ShareNotFound - - [self::class, 'listenPreShare'] diff --git a/tests/unit/Db/AclMapperTest.php b/tests/unit/Db/AclMapperTest.php index 7272e5626..b985669f7 100644 --- a/tests/unit/Db/AclMapperTest.php +++ b/tests/unit/Db/AclMapperTest.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Db; +use OCA\Deck\Service\CirclesService; use OCP\IGroupManager; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -56,6 +57,7 @@ class AclMapperTest extends MapperTestUtility { \OC::$server->query(StackMapper::class), $this->userManager, $this->groupManager, + $this->createMock(CirclesService::class), $this->createMock(LoggerInterface::class) ); diff --git a/tests/unit/Db/BoardMapperTest.php b/tests/unit/Db/BoardMapperTest.php index e0dd8a8c0..43420333d 100644 --- a/tests/unit/Db/BoardMapperTest.php +++ b/tests/unit/Db/BoardMapperTest.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Db; +use OCA\Deck\Service\CirclesService; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; @@ -63,6 +64,7 @@ class BoardMapperTest extends MapperTestUtility { \OC::$server->query(StackMapper::class), $this->userManager, $this->groupManager, + $this->createMock(CirclesService::class), $this->createMock(LoggerInterface::class) ); $this->aclMapper = \OC::$server->query(AclMapper::class);