Merge pull request #3983 from nextcloud/perf/sharing

Simpify query for getting shared files
2022-08-29 11:02:09 +02:00
parent 15c48b919d b81f55057a
commit 56e3215785
4 changed files with 56 additions and 50 deletions
--- a/.github/workflows/integration.yml
+++ b/.github/workflows/integration.yml
@@ -62,7 +62,7 @@ jobs:
          auth_header="$(git config --local --get http.https://github.com/.extraheader)"
          git submodule sync --recursive
          git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1
-          cd build/integration && composer require --dev phpunit/phpunit:~8
+          cd build/integration && composer require --dev phpunit/phpunit:~9

      - name: Checkout app
        uses: actions/checkout@v3
--- a/lib/Db/BoardMapper.php
+++ b/lib/Db/BoardMapper.php
@@ -107,6 +107,47 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
 		return $this->boardCache[$id];
 	}

+	public function findBoardIds(string $userId): array {
+		$qb = $this->db->getQueryBuilder();
+		$qb->selectDistinct('b.id')
+			->from($this->getTableName(), 'b')
+			->leftJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id'));
+
+		// Owned by the user
+		$qb->where($qb->expr()->andX(
+			$qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)),
+		));
+
+		// Shared to the user
+		$qb->orWhere($qb->expr()->andX(
+			$qb->expr()->eq('acl.participant', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)),
+			$qb->expr()->eq('acl.type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_USER, IQueryBuilder::PARAM_INT)),
+		));
+
+		// Shared to user groups of the user
+		$groupIds = $this->groupManager->getUserGroupIds($this->userManager->get($userId));
+		if (count($groupIds) !== 0) {
+			$qb->orWhere($qb->expr()->andX(
+					$qb->expr()->in('acl.participant', $qb->createNamedParameter($groupIds, IQueryBuilder::PARAM_STR_ARRAY)),
+					$qb->expr()->eq('acl.type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_GROUP, IQueryBuilder::PARAM_INT)),
+				));
+		}
+
+		// Shared to circles of the user
+		$circles = $this->circlesService->getUserCircles($userId);
+		if (count($circles) !== 0) {
+			$qb->orWhere($qb->expr()->andX(
+				$qb->expr()->in('acl.participant', $qb->createNamedParameter($circles, IQueryBuilder::PARAM_STR_ARRAY)),
+				$qb->expr()->eq('acl.type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT)),
+			));
+		}
+
+		$result = $qb->executeQuery();
+		return array_map(function (string $id) {
+			return (int)$id;
+		}, $result->fetchAll(\PDO::FETCH_COLUMN));
+	}
+
 	public function findAllForUser(string $userId, ?int $since = null, bool $includeArchived = true, ?int $before = null,
 								   ?string $term = null): array {
 		$useCache = ($since === -1 && $includeArchived === true && $before === null && $term === null);
--- a/lib/Service/OverviewService.php
+++ b/lib/Service/OverviewService.php
@@ -32,29 +32,18 @@ use OCA\Deck\Db\Card;
 use OCA\Deck\Db\CardMapper;
 use OCA\Deck\Model\CardDetails;
 use OCP\Comments\ICommentsManager;
-use OCP\IGroupManager;
 use OCA\Deck\Db\BoardMapper;
 use OCA\Deck\Db\LabelMapper;
 use OCP\IUserManager;

 class OverviewService {
-
-	/** @var BoardMapper */
-	private $boardMapper;
-	/** @var LabelMapper */
-	private $labelMapper;
-	/** @var CardMapper */
-	private $cardMapper;
-	/** @var AssignmentMapper */
-	private $assignedUsersMapper;
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var ICommentsManager */
-	private $commentsManager;
-	/** @var AttachmentService */
-	private $attachmentService;
+	private BoardMapper $boardMapper;
+	private LabelMapper $labelMapper;
+	private CardMapper $cardMapper;
+	private AssignmentMapper $assignedUsersMapper;
+	private IUserManager $userManager;
+	private ICommentsManager $commentsManager;
+	private AttachmentService $attachmentService;

 	public function __construct(
 		BoardMapper $boardMapper,
@@ -62,7 +51,6 @@ class OverviewService {
 		CardMapper $cardMapper,
 		AssignmentMapper $assignedUsersMapper,
 		IUserManager $userManager,
-		IGroupManager $groupManager,
 		ICommentsManager $commentsManager,
 		AttachmentService $attachmentService
 	) {
@@ -71,7 +59,6 @@ class OverviewService {
 		$this->cardMapper = $cardMapper;
 		$this->assignedUsersMapper = $assignedUsersMapper;
 		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
 		$this->commentsManager = $commentsManager;
 		$this->attachmentService = $attachmentService;
 	}
@@ -93,7 +80,7 @@ class OverviewService {
 	}

 	public function findAllWithDue(string $userId): array {
-		$userBoards = $this->findAllBoardsFromUser($userId);
+		$userBoards = $this->boardMapper->findAllForUser($userId);
 		$allDueCards = [];
 		foreach ($userBoards as $userBoard) {
 			$allDueCards[] = array_map(function ($card) use ($userBoard, $userId) {
@@ -105,7 +92,7 @@ class OverviewService {
 	}

 	public function findUpcomingCards(string $userId): array {
-		$userBoards = $this->findAllBoardsFromUser($userId);
+		$userBoards = $this->boardMapper->findAllForUser($userId);
 		$foundCards = [];
 		foreach ($userBoards as $userBoard) {
 			if (count($userBoard->getAcl()) === 0) {
@@ -126,22 +113,4 @@ class OverviewService {
 		}
 		return array_merge(...$foundCards);
 	}
-
-	// FIXME: This is duplicate code with the board service
-	private function findAllBoardsFromUser(string $userId): array {
-		$userInfo = $this->getBoardPrerequisites($userId);
-		$userBoards = $this->boardMapper->findAllByUser($userInfo['user'], null, null);
-		$groupBoards = $this->boardMapper->findAllByGroups($userInfo['user'], $userInfo['groups'], null, null);
-		$circleBoards = $this->boardMapper->findAllByCircles($userInfo['user'], null, null);
-		return array_unique(array_merge($userBoards, $groupBoards, $circleBoards));
-	}
-
-	private function getBoardPrerequisites($userId): array {
-		$user = $this->userManager->get($userId);
-		$groups = $user !== null ? $this->groupManager->getUserGroupIds($user) : [];
-		return [
-			'user' => $userId,
-			'groups' => $groups
-		];
-	}
 }
--- a/lib/Sharing/DeckShareProvider.php
+++ b/lib/Sharing/DeckShareProvider.php
@@ -634,8 +634,8 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 		$start = 0;
 		while (true) {
 			/** @var IShare[] $shareSlice */
-			$shareSlice = array_slice($shares, $start, 100);
-			$start += 100;
+			$shareSlice = array_slice($shares, $start, 1000);
+			$start += 1000;

 			if ($shareSlice === []) {
 				break;
@@ -714,15 +714,15 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 	 * @return IShare[]
 	 */
 	public function getSharedWith($userId, $shareType, $node, $limit, $offset): array {
-		$allBoards = $this->boardMapper->findAllForUser($userId);
+		$allBoards = $this->boardMapper->findBoardIds($userId);

 		/** @var IShare[] $shares */
 		$shares = [];

 		$start = 0;
 		while (true) {
-			$boards = array_slice($allBoards, $start, 100);
-			$start += 100;
+			$boards = array_slice($allBoards, $start, 1000);
+			$start += 1000;

 			if ($boards === []) {
 				break;
@@ -752,10 +752,6 @@ class DeckShareProvider implements \OCP\Share\IShareProvider {
 				$qb->andWhere($qb->expr()->eq('s.file_source', $qb->createNamedParameter($node->getId())));
 			}

-			$boards = array_map(function (Board $board) {
-				return $board->getId();
-			}, $boards);
-
 			$qb->andWhere($qb->expr()->eq('s.share_type', $qb->createNamedParameter(IShare::TYPE_DECK)))
 				->andWhere($qb->expr()->in('db.id', $qb->createNamedParameter(
 					$boards,