Skip to content

Commit 55bbcdc

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 a3dcd62 commit 55bbcdc

5 files changed

Lines changed: 332 additions & 43 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: 104 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCA\Circles\Model\Probes\DataProbe;
1818
use OCP\App\IAppManager;
1919
use OCP\Server;
20+
use Psr\Log\LoggerInterface;
2021
use Throwable;
2122

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

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

@@ -44,14 +45,21 @@ public function getCircle(string $circleId): ?Circle {
4445
}
4546

4647
try {
47-
4848
// Enforce current user condition since we always want the full list of members
49-
$circlesManager = Server::get(CirclesManager::class);
49+
$circlesManager = $this->getCirclesManager();
5050
$circlesManager->startSuperSession();
51-
$dataProbe = new DataProbe();
52-
$dataProbe->add(DataProbe::OWNER);
53-
return $circlesManager->probeCircle($circleId, null, $dataProbe);
51+
try {
52+
$dataProbe = new DataProbe();
53+
$dataProbe->add(DataProbe::OWNER);
54+
return $circlesManager->probeCircle($circleId, null, $dataProbe);
55+
} finally {
56+
$this->stopSessionSafely($circlesManager, 'getCircle');
57+
}
5458
} catch (Throwable $e) {
59+
$this->logger?->debug('CirclesService::getCircle failed', [
60+
'circleId' => $circleId,
61+
'exception' => $e,
62+
]);
5563
}
5664
return null;
5765
}
@@ -66,14 +74,18 @@ public function isUserInCircle(string $circleId, string $userId): bool {
6674
}
6775

6876
try {
69-
$circlesManager = Server::get(CirclesManager::class);
77+
$circlesManager = $this->getCirclesManager();
7078
$federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER);
7179
$circlesManager->startSession($federatedUser);
72-
$dataProbe = new DataProbe();
73-
$dataProbe->add(DataProbe::INITIATOR);
74-
$circle = $circlesManager->probeCircle($circleId, null, $dataProbe);
75-
$member = $circle->getInitiator();
76-
$isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER;
80+
try {
81+
$dataProbe = new DataProbe();
82+
$dataProbe->add(DataProbe::INITIATOR);
83+
$circle = $circlesManager->probeCircle($circleId, null, $dataProbe);
84+
$member = $circle->getInitiator();
85+
$isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER;
86+
} finally {
87+
$this->stopSessionSafely($circlesManager, 'isUserInCircle');
88+
}
7789

7890
if (!isset($this->userCircleCache[$circleId])) {
7991
$this->userCircleCache[$circleId] = [];
@@ -82,6 +94,11 @@ public function isUserInCircle(string $circleId, string $userId): bool {
8294

8395
return $isUserInCircle;
8496
} catch (Throwable $e) {
97+
$this->logger?->debug('CirclesService::isUserInCircle failed', [
98+
'circleId' => $circleId,
99+
'userId' => $userId,
100+
'exception' => $e,
101+
]);
85102
}
86103
return false;
87104
}
@@ -100,18 +117,87 @@ public function getUserCircles(string $userId): array {
100117
}
101118

102119
try {
103-
$circlesManager = Server::get(CirclesManager::class);
120+
$circlesManager = $this->getCirclesManager();
104121
$federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER);
105122
$circlesManager->startSession($federatedUser);
106-
$probe = new CircleProbe();
107-
$probe->mustBeMember();
108-
$circles = array_map(function (Circle $circle) {
109-
return $circle->getSingleId();
110-
}, $circlesManager->probeCircles($probe));
123+
try {
124+
$probe = new CircleProbe();
125+
$probe->mustBeMember();
126+
$circles = array_map(function (Circle $circle) {
127+
return $circle->getSingleId();
128+
}, $circlesManager->probeCircles($probe));
129+
} finally {
130+
$this->stopSessionSafely($circlesManager, 'getUserCircles');
131+
}
111132
$this->userCirclesCache[$userId] = $circles;
112133
return $circles;
113134
} catch (Throwable $e) {
135+
$this->logger?->debug('CirclesService::getUserCircles failed', [
136+
'userId' => $userId,
137+
'exception' => $e,
138+
]);
114139
}
115140
return [];
116141
}
142+
143+
public function clearUserCircleCache(?string $circleId = null, ?string $userId = null): void {
144+
if ($circleId === null && $userId === null) {
145+
$this->userCircleCache = [];
146+
return;
147+
}
148+
149+
if ($circleId !== null && $userId === null) {
150+
unset($this->userCircleCache[$circleId]);
151+
return;
152+
}
153+
154+
if ($circleId !== null && $userId !== null) {
155+
unset($this->userCircleCache[$circleId][$userId]);
156+
if (empty($this->userCircleCache[$circleId])) {
157+
unset($this->userCircleCache[$circleId]);
158+
}
159+
return;
160+
}
161+
162+
foreach ($this->userCircleCache as $cachedCircleId => $users) {
163+
unset($users[$userId]);
164+
if (empty($users)) {
165+
unset($this->userCircleCache[$cachedCircleId]);
166+
continue;
167+
}
168+
$this->userCircleCache[$cachedCircleId] = $users;
169+
}
170+
}
171+
172+
public function clearUserCirclesCache(?string $userId = null): void {
173+
if ($userId === null) {
174+
$this->userCirclesCache = [];
175+
return;
176+
}
177+
178+
unset($this->userCirclesCache[$userId]);
179+
}
180+
181+
public function clearAllCaches(): void {
182+
$this->clearUserCircleCache();
183+
$this->clearUserCirclesCache();
184+
}
185+
186+
protected function getCirclesManager(): CirclesManager {
187+
return Server::get(CirclesManager::class);
188+
}
189+
190+
private function stopSessionSafely(CirclesManager $circlesManager, string $method): void {
191+
if (!method_exists($circlesManager, 'stopSession')) {
192+
return;
193+
}
194+
195+
try {
196+
$circlesManager->stopSession();
197+
} catch (Throwable $e) {
198+
$this->logger?->debug('CirclesService::' . $method . ' stopSession failed', [
199+
'exception' => $e,
200+
]);
201+
}
202+
}
117203
}

‎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)