From 6e89dd48bcbc65b78a37dd8fd02d15b8992ab79b 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 | 21 ++++++--------- lib/Service/CirclesService.php | 44 ++++++++++++++++++++++++++++--- lib/Service/PermissionService.php | 4 +-- tests/psalm-baseline.xml | 37 +++++--------------------- 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 8f21f5dc0..9ea311095 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\IDBConnection; use OCP\IUserManager; @@ -36,10 +37,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { private $stackMapper; private $userManager; private $groupManager; + private $circlesService; private $logger; - private $circlesEnabled; - + /** @var CappedMemoryCache */ private $userBoardCache; public function __construct( @@ -49,6 +50,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { StackMapper $stackMapper, IUserManager $userManager, IGroupManager $groupManager, + CirclesService $circlesService, LoggerInterface $logger ) { parent::__construct($db, 'deck_boards', Board::class); @@ -57,12 +59,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $this->stackMapper = $stackMapper; $this->userManager = $userManager; $this->groupManager = $groupManager; + $this->circlesService = $circlesService; $this->logger = $logger; $this->userBoardCache = new CappedMemoryCache(); - - - $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles'); } @@ -181,12 +181,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } public function findAllByCircles($userId, $limit = null, $offset = null, $since = -1,$includeArchived = true) { - 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 []; } @@ -277,11 +272,11 @@ class BoardMapper extends DeckMapper 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 347863ece..e8206e774 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 e3710d52f..bdaccea2a 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -105,10 +105,6 @@ $boardId - - \OCA\Circles\Api\v1\Circles - \OCA\Circles\Api\v1\Circles - @@ -117,7 +113,6 @@ - $entity->getId() @@ -205,9 +200,11 @@ - \OCA\Circles\Api\v1\Circles + ?Circle - + + $circlesManager + $circlesManager $circlesManager @@ -253,17 +250,16 @@ - + $this->rootFolder $this->rootFolder IRootFolder - ShareNotFound + $circle Member - \OCA\Circles\Api\v1\Circles @@ -271,27 +267,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 f720fdb1a..5c0a08471 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 78cc0cccc..2f6ba5837 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);