diff --git a/lib/Db/MemberRequest.php b/lib/Db/MemberRequest.php index df29cd0db..518882111 100644 --- a/lib/Db/MemberRequest.php +++ b/lib/Db/MemberRequest.php @@ -413,6 +413,25 @@ public function getMemberInstances(string $circleId): array { ); } + /** + * @param list $singleIds + * + * @return list + * @throws RequestBuilderException + */ + public function getMembersBySingleIds(array $singleIds): array { + $members = []; + foreach (array_chunk(array_values(array_unique($singleIds)), 1000) as $chunk) { + $qb = $this->getMemberSelectSql(); + $qb->leftJoinCircle(CoreQueryBuilder::MEMBER); + $qb->limitInArray('single_id', $chunk); + + array_push($members, ...$this->getItemsFromRequest($qb)); + } + + return $members; + } + /** * @param string $singleId * diff --git a/lib/Service/MembershipService.php b/lib/Service/MembershipService.php index 79f5dcf7f..d63acbae5 100644 --- a/lib/Service/MembershipService.php +++ b/lib/Service/MembershipService.php @@ -128,49 +128,49 @@ public function deleteFederatedUser(FederatedUser $federatedUser) { /** * @param string $singleId - * @param string $circleId - * @param array $memberships - * @param array $knownIds - * @param array $path * - * @return array + * @return Membership[] * @throws RequestBuilderException */ - private function generateMemberships( - string $singleId, - string $circleId = '', - array &$memberships = [], - array $knownIds = [], - array $path = [], - ): array { - $circleId = ($circleId === '') ? $singleId : $circleId; - $path[] = $circleId; - $knownIds[] = $circleId; - - $members = $this->memberRequest->getMembersBySingleId($circleId); - foreach ($members as $member) { - if (!$member->hasCircle() || $member->getLevel() < Member::LEVEL_MEMBER) { - continue; - } + private function generateMemberships(string $singleId): array { + $memberships = []; + $pathMap = [$singleId => [$singleId]]; + $knownIds = [$singleId => true]; + $currentLevelIds = [$singleId]; + + while (!empty($currentLevelIds)) { + $nextLevelIds = []; + $newPaths = []; + foreach ($this->memberRequest->getMembersBySingleIds($currentLevelIds) as $member) { + if (!$member->hasCircle() || $member->getLevel() < Member::LEVEL_MEMBER) { + continue; + } - if ($member->getCircle()->isConfig(Circle::CFG_NO_OWNER) - && $member->getLevel() === Member::LEVEL_OWNER) { - continue; + if ($member->getCircle()->isConfig(Circle::CFG_NO_OWNER) + && $member->getLevel() === Member::LEVEL_OWNER) { + continue; + } + + $path = $pathMap[$member->getSingleId()]; + $circleId = $member->getCircleId(); + $membershipPath = array_merge($path, [$circleId]); + + $membership = new Membership($singleId, count($path) > 1 ? $path[1] : '', $member); + $membership->setInheritancePath(array_reverse($path)) + ->setInheritanceDepth(count($path)); + $this->fillMemberships($membership, $memberships); + + if (!isset($knownIds[$circleId])) { + $newPaths[$circleId] ??= $membershipPath; + $nextLevelIds[$circleId] = $circleId; + } } - $membership = new Membership($singleId, count($path) > 1 ? $path[1] : '', $member); - $membership->setInheritancePath(array_reverse($path)) - ->setInheritanceDepth(sizeof($path)); - $this->fillMemberships($membership, $memberships); - if (!in_array($member->getCircleId(), $knownIds)) { - $this->generateMemberships( - $singleId, - $member->getCircleId(), - $memberships, - $knownIds, - $path - ); + foreach ($newPaths as $circleId => $path) { + $pathMap[$circleId] = $path; + $knownIds[$circleId] = true; } + $currentLevelIds = array_values($nextLevelIds); } return $memberships; diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index c7c9412d8..d00389944 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -276,11 +276,6 @@ - - - getCircleId(), $knownIds)) {]]> - - getUserId())]]> diff --git a/tests/unit/lib/Service/MembershipServiceTest.php b/tests/unit/lib/Service/MembershipServiceTest.php new file mode 100644 index 000000000..a38cbbad1 --- /dev/null +++ b/tests/unit/lib/Service/MembershipServiceTest.php @@ -0,0 +1,161 @@ +memberRequest = $this->createMock(MemberRequest::class); + $this->service = new MembershipService( + $this->createMock(MembershipRequest::class), + $this->createMock(CircleRequest::class), + $this->memberRequest, + $this->createMock(EventService::class), + $this->createMock(ShareWrapperService::class), + $this->createMock(OutputService::class), + ); + } + + public function testUserWithoutMembershipsGetsNothing(): void { + $this->mockGraph([]); + + self::assertSame([], $this->generate('user')); + } + + public function testDirectMemberships(): void { + $this->mockGraph([ + 'user' => [['A', Member::LEVEL_MEMBER], ['B', Member::LEVEL_ADMIN], ['C', Member::LEVEL_OWNER]], + ]); + + $memberships = $this->generate('user'); + + self::assertEqualsCanonicalizing(['A', 'B', 'C'], array_keys($memberships)); + self::assertSame(Member::LEVEL_ADMIN, $memberships['B']->getLevel()); + self::assertSame(1, $memberships['B']->getInheritanceDepth()); + self::assertSame('user', $memberships['B']->getInheritanceFirst()); + self::assertSame('B', $memberships['B']->getInheritanceLast()); + self::assertSame(['user'], $memberships['B']->getInheritancePath()); + } + + public function testNestedChainKeepsPathsDepthsAndLevelsOfEachHop(): void { + $this->mockGraph([ + 'user' => [['A', Member::LEVEL_ADMIN]], + 'A' => [['B', Member::LEVEL_MEMBER]], + 'B' => [['C', Member::LEVEL_MODERATOR]], + 'C' => [['D', Member::LEVEL_MEMBER]], + ]); + + $memberships = $this->generate('user'); + + self::assertEqualsCanonicalizing(['A', 'B', 'C', 'D'], array_keys($memberships)); + + $expected = [ + 'A' => [Member::LEVEL_ADMIN, 1, 'user', 'A', ['user']], + 'B' => [Member::LEVEL_MEMBER, 2, 'A', 'A', ['A', 'user']], + 'C' => [Member::LEVEL_MODERATOR, 3, 'B', 'A', ['B', 'A', 'user']], + 'D' => [Member::LEVEL_MEMBER, 4, 'C', 'A', ['C', 'B', 'A', 'user']], + ]; + foreach ($expected as $circleId => [$level, $depth, $first, $last, $path]) { + self::assertSame($level, $memberships[$circleId]->getLevel(), "level of $circleId"); + self::assertSame($depth, $memberships[$circleId]->getInheritanceDepth(), "depth of $circleId"); + self::assertSame($first, $memberships[$circleId]->getInheritanceFirst(), "first of $circleId"); + self::assertSame($last, $memberships[$circleId]->getInheritanceLast(), "last of $circleId"); + self::assertSame($path, $memberships[$circleId]->getInheritancePath(), "path of $circleId"); + } + } + + public function testCircularAndSelfMembershipsTerminate(): void { + $this->mockGraph([ + 'user' => [['A', Member::LEVEL_MEMBER]], + 'A' => [['A', Member::LEVEL_MEMBER], ['B', Member::LEVEL_MEMBER]], + 'B' => [['A', Member::LEVEL_MEMBER]], + ]); + + $memberships = $this->generate('user'); + + self::assertEqualsCanonicalizing(['A', 'B'], array_keys($memberships)); + self::assertSame(1, $memberships['A']->getInheritanceDepth()); + self::assertSame(2, $memberships['B']->getInheritanceDepth()); + } + + public function testDiamondIsExpandedOnceAndKeepsFirstPathForDescendants(): void { + $this->mockGraph([ + 'user' => [['A', Member::LEVEL_MEMBER], ['B', Member::LEVEL_MEMBER]], + 'A' => [['D', Member::LEVEL_MEMBER]], + 'B' => [['D', Member::LEVEL_MEMBER]], + 'D' => [['E', Member::LEVEL_MEMBER]], + ]); + + $memberships = $this->generate('user'); + + self::assertEqualsCanonicalizing(['A', 'B', 'D', 'E'], array_keys($memberships)); + self::assertSame([['user'], ['A', 'B'], ['D'], ['E']], $this->queriedSingleIds); + + self::assertSame(2, $memberships['D']->getInheritanceDepth()); + self::assertSame('A', $memberships['D']->getInheritanceFirst()); + self::assertSame(['A', 'user'], $memberships['D']->getInheritancePath()); + + self::assertSame(3, $memberships['E']->getInheritanceDepth()); + self::assertSame('D', $memberships['E']->getInheritanceFirst()); + self::assertSame('A', $memberships['E']->getInheritanceLast()); + self::assertSame(['D', 'A', 'user'], $memberships['E']->getInheritancePath()); + } + + /** + * @param array> $graph single id => [circle id, level] it belongs to + */ + private function mockGraph(array $graph): void { + $membersOf = function (string $singleId) use ($graph): array { + return array_map(function (array $edge) use ($singleId): Member { + $circle = new Circle(); + $circle->setSingleId($edge[0]); + + $member = new Member(); + $member->setSingleId($singleId); + $member->setCircleId($edge[0]); + $member->setLevel($edge[1]); + $member->setCircle($circle); + + return $member; + }, $graph[$singleId] ?? []); + }; + + $this->memberRequest->method('getMembersBySingleIds') + ->willReturnCallback(function (array $singleIds) use ($membersOf): array { + $this->queriedSingleIds[] = $singleIds; + + return array_merge(...array_map($membersOf, $singleIds)); + }); + } + + /** + * @return array + */ + private function generate(string $singleId): array { + return (new \ReflectionMethod($this->service, 'generateMemberships'))->invoke($this->service, $singleId); + } +}