Skip to content

Commit 768599f

Browse files
jospoortvlietjospoortvliet
committed
fix some boards not showing up for users when added to a circle after initial share and improve performance.
Signed-off-by: Jos Poortvliet <jospoortvliet@gmail.com>
1 parent 03bf7db commit 768599f

5 files changed

Lines changed: 328 additions & 39 deletions

File tree

‎lib/Service/BoardService.php‎

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\Deck\Service;
99

1010
use OC\User\LazyUser;
11+
use OCA\Circles\Model\Member;
1112
use OCA\Deck\Activity\ActivityManager;
1213
use OCA\Deck\Activity\ChangeSet;
1314
use OCA\Deck\AppInfo\Application;
@@ -412,6 +413,7 @@ public function addAcl(int $boardId, int $type, $participant, bool $edit, bool $
412413

413414
$board = $this->boardMapper->find($boardId);
414415
$this->clearBoardFromCache($board);
416+
$this->invalidateAclCaches($boardId, $type, (string)$participant);
415417

416418
// TODO: use the dispatched event for this
417419
try {
@@ -474,6 +476,7 @@ public function updateAcl(int $id, bool $edit, bool $share, bool $manage): Acl {
474476
}
475477

476478
$this->eventDispatcher->dispatchTyped(new AclUpdatedEvent($acl));
479+
$this->invalidateAclCaches($acl->getBoardId(), $acl->getType(), (string)$acl->getParticipant());
477480

478481
return $acl;
479482
}
@@ -510,6 +513,7 @@ public function deleteAcl(int $id): ?Acl {
510513

511514
$deletedAcl = $this->aclMapper->delete($acl);
512515
$this->eventDispatcher->dispatchTyped(new AclDeletedEvent($acl));
516+
$this->invalidateAclCaches($acl->getBoardId(), $acl->getType(), (string)$acl->getParticipant());
513517

514518
return $deletedAcl;
515519
}
@@ -759,23 +763,26 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array {
759763

760764
private function annotateAclRetainedAccess(Board $board): void {
761765
$acls = $board->getAcl() ?? [];
762-
foreach ($acls as $acl) {
763-
if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
764-
$acl->setRetainsAccessViaMembership(true);
765-
continue;
766-
}
767766

768-
if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) {
769-
if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE
770-
&& (string)$acl->getParticipant() === $board->getOwner()) {
771-
$acl->setRetainsAccessViaMembership(true);
772-
continue;
767+
// For circle-owned boards fetch members once instead of N isUserInCircle() calls
768+
$circleMemberIds = null;
769+
if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE
770+
&& $this->circlesService->isCirclesEnabled()
771+
) {
772+
$circle = $this->circlesService->getCircle($board->getOwner());
773+
if ($circle !== null) {
774+
$circleMemberIds = [];
775+
foreach ($circle->getInheritedMembers() as $member) {
776+
if ($member->getUserType() === Member::TYPE_USER
777+
&& $member->getLevel() >= Member::LEVEL_MEMBER
778+
) {
779+
$circleMemberIds[$member->getUserId()] = true;
780+
}
773781
}
774-
775-
$acl->setRetainsAccessViaMembership(true);
776-
continue;
777782
}
783+
}
778784

785+
foreach ($acls as $acl) {
779786
if ($acl->getType() !== Acl::PERMISSION_TYPE_USER) {
780787
$acl->setRetainsAccessViaMembership(false);
781788
continue;
@@ -792,14 +799,10 @@ private function annotateAclRetainedAccess(Board $board): void {
792799
continue;
793800
}
794801

795-
if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) {
796-
try {
797-
if ($this->circlesService->isUserInCircle($board->getOwner(), $participant)) {
798-
$acl->setRetainsAccessViaMembership(true);
799-
continue;
800-
}
801-
} catch (\Throwable) {
802-
}
802+
// Circle-owned board: check prebuilt membership set (1 Circles call total)
803+
if ($circleMemberIds !== null) {
804+
$acl->setRetainsAccessViaMembership(isset($circleMemberIds[$participant]));
805+
continue;
803806
}
804807

805808
$otherAcls = array_filter($acls, static fn (Acl $candidate): bool => $candidate->getId() !== $acl->getId());
@@ -925,6 +928,14 @@ private function clearBoardFromCache(Board $board): void {
925928
unset($this->boardsCachePartial[$boardId]);
926929
}
927930

931+
private function invalidateAclCaches(int $boardId, int $aclType, string $participant): void {
932+
$this->permissionService->clearUsersCache($boardId);
933+
934+
if ($aclType === Acl::PERMISSION_TYPE_CIRCLE && $participant !== '') {
935+
$this->circlesService->clearUserCircleCache($participant);
936+
}
937+
}
938+
928939
private function enrichWithCards(Board $board): void {
929940
$stacks = $this->stackMapper->findAll($board->getId());
930941
if (\count($stacks) === 0) {

‎lib/Service/CirclesService.php‎

Lines changed: 100 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCA\Circles\Model\Probes\CircleProbe;
1717
use OCP\App\IAppManager;
1818
use OCP\Server;
19+
use Psr\Log\LoggerInterface;
1920
use Throwable;
2021

2122
/**
@@ -29,7 +30,7 @@ class CirclesService {
2930
/** @var array<string, string[]> */
3031
private array $userCirclesCache = [];
3132

32-
public function __construct(IAppManager $appManager) {
33+
public function __construct(IAppManager $appManager, private ?LoggerInterface $logger = null) {
3334
$this->circlesEnabled = $appManager->isEnabledForUser('circles');
3435
}
3536

@@ -43,12 +44,19 @@ public function getCircle(string $circleId): ?Circle {
4344
}
4445

4546
try {
46-
4747
// Enforce current user condition since we always want the full list of members
48-
$circlesManager = Server::get(CirclesManager::class);
48+
$circlesManager = $this->getCirclesManager();
4949
$circlesManager->startSuperSession();
50-
return $circlesManager->getCircle($circleId);
50+
try {
51+
return $circlesManager->getCircle($circleId);
52+
} finally {
53+
$this->stopSessionSafely($circlesManager, 'getCircle');
54+
}
5155
} catch (Throwable $e) {
56+
$this->logger?->debug('CirclesService::getCircle failed', [
57+
'circleId' => $circleId,
58+
'exception' => $e,
59+
]);
5260
}
5361
return null;
5462
}
@@ -63,12 +71,16 @@ public function isUserInCircle(string $circleId, string $userId): bool {
6371
}
6472

6573
try {
66-
$circlesManager = Server::get(CirclesManager::class);
74+
$circlesManager = $this->getCirclesManager();
6775
$federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER);
6876
$circlesManager->startSession($federatedUser);
69-
$circle = $circlesManager->getCircle($circleId);
70-
$member = $circle->getInitiator();
71-
$isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER;
77+
try {
78+
$circle = $circlesManager->getCircle($circleId);
79+
$member = $circle->getInitiator();
80+
$isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER;
81+
} finally {
82+
$this->stopSessionSafely($circlesManager, 'isUserInCircle');
83+
}
7284

7385
if (!isset($this->userCircleCache[$circleId])) {
7486
$this->userCircleCache[$circleId] = [];
@@ -77,6 +89,11 @@ public function isUserInCircle(string $circleId, string $userId): bool {
7789

7890
return $isUserInCircle;
7991
} catch (Throwable $e) {
92+
$this->logger?->debug('CirclesService::isUserInCircle failed', [
93+
'circleId' => $circleId,
94+
'userId' => $userId,
95+
'exception' => $e,
96+
]);
8097
}
8198
return false;
8299
}
@@ -95,18 +112,87 @@ public function getUserCircles(string $userId): array {
95112
}
96113

97114
try {
98-
$circlesManager = Server::get(CirclesManager::class);
115+
$circlesManager = $this->getCirclesManager();
99116
$federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER);
100117
$circlesManager->startSession($federatedUser);
101-
$probe = new CircleProbe();
102-
$probe->mustBeMember();
103-
$circles = array_map(function (Circle $circle) {
104-
return $circle->getSingleId();
105-
}, $circlesManager->getCircles($probe));
118+
try {
119+
$probe = new CircleProbe();
120+
$probe->mustBeMember();
121+
$circles = array_map(function (Circle $circle) {
122+
return $circle->getSingleId();
123+
}, $circlesManager->getCircles($probe));
124+
} finally {
125+
$this->stopSessionSafely($circlesManager, 'getUserCircles');
126+
}
106127
$this->userCirclesCache[$userId] = $circles;
107128
return $circles;
108129
} catch (Throwable $e) {
130+
$this->logger?->debug('CirclesService::getUserCircles failed', [
131+
'userId' => $userId,
132+
'exception' => $e,
133+
]);
109134
}
110135
return [];
111136
}
137+
138+
public function clearUserCircleCache(?string $circleId = null, ?string $userId = null): void {
139+
if ($circleId === null && $userId === null) {
140+
$this->userCircleCache = [];
141+
return;
142+
}
143+
144+
if ($circleId !== null && $userId === null) {
145+
unset($this->userCircleCache[$circleId]);
146+
return;
147+
}
148+
149+
if ($circleId !== null && $userId !== null) {
150+
unset($this->userCircleCache[$circleId][$userId]);
151+
if (empty($this->userCircleCache[$circleId])) {
152+
unset($this->userCircleCache[$circleId]);
153+
}
154+
return;
155+
}
156+
157+
foreach ($this->userCircleCache as $cachedCircleId => $users) {
158+
unset($users[$userId]);
159+
if (empty($users)) {
160+
unset($this->userCircleCache[$cachedCircleId]);
161+
continue;
162+
}
163+
$this->userCircleCache[$cachedCircleId] = $users;
164+
}
165+
}
166+
167+
public function clearUserCirclesCache(?string $userId = null): void {
168+
if ($userId === null) {
169+
$this->userCirclesCache = [];
170+
return;
171+
}
172+
173+
unset($this->userCirclesCache[$userId]);
174+
}
175+
176+
public function clearAllCaches(): void {
177+
$this->clearUserCircleCache();
178+
$this->clearUserCirclesCache();
179+
}
180+
181+
protected function getCirclesManager(): CirclesManager {
182+
return Server::get(CirclesManager::class);
183+
}
184+
185+
private function stopSessionSafely(CirclesManager $circlesManager, string $method): void {
186+
if (!method_exists($circlesManager, 'stopSession')) {
187+
return;
188+
}
189+
190+
try {
191+
$circlesManager->stopSession();
192+
} catch (Throwable $e) {
193+
$this->logger?->debug('CirclesService::' . $method . ' stopSession failed', [
194+
'exception' => $e,
195+
]);
196+
}
197+
}
112198
}

‎lib/Service/PermissionService.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,4 +405,13 @@ public function setUserId(string $userId): void {
405405
$this->userId = $userId;
406406
$this->permissionCache->clear();
407407
}
408+
409+
public function clearUsersCache(?int $boardId = null): void {
410+
if ($boardId === null) {
411+
$this->users = [];
412+
return;
413+
}
414+
415+
unset($this->users[(string)$boardId]);
416+
}
408417
}

0 commit comments

Comments
 (0)