Merge pull request #44004 from nextcloud/share-resolve-group-perf

nextcloud · Mar 16, 2024 · 1ecf3b3 · 1ecf3b3
2 parents 8d845c4 + 0e74972
commit 1ecf3b3
Showing 1 changed file with 28 additions and 50 deletions.
diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
@@ -829,7 +829,7 @@ public function getShareById($id, $recipientId = null) {
 
 		// If the recipient is set for a group share resolve to that user
 		if ($recipientId !== null && $share->getShareType() === IShare::TYPE_GROUP) {
-			$share = $this->resolveGroupShares([$share], $recipientId)[0];
+			$share = $this->resolveGroupShares([(int) $share->getId() => $share], $recipientId)[0];
 		}
 
 		return $share;
@@ -1006,7 +1006,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
 					}
 
 					if ($this->isAccessibleResult($data)) {
-						$shares2[] = $this->createShare($data);
+						$share = $this->createShare($data);
+						$shares2[$share->getId()] = $share;
 					}
 				}
 				$cursor->closeCursor();
@@ -1127,61 +1128,38 @@ private function createShare($data) {
 	}
 
 	/**
-	 * @param Share[] $shares
+	 * Update the data from group shares with any per-user modifications
+	 *
+	 * @param array<int, Share> $shareMap shares indexed by share id
 	 * @param $userId
 	 * @return Share[] The updates shares if no update is found for a share return the original
 	 */
-	private function resolveGroupShares($shares, $userId) {
-		$result = [];
-
-		$start = 0;
-		while (true) {
-			/** @var Share[] $shareSlice */
-			$shareSlice = array_slice($shares, $start, 100);
-			$start += 100;
-
-			if ($shareSlice === []) {
-				break;
-			}
-
-			/** @var int[] $ids */
-			$ids = [];
-			/** @var Share[] $shareMap */
-			$shareMap = [];
-
-			foreach ($shareSlice as $share) {
-				$ids[] = (int)$share->getId();
-				$shareMap[$share->getId()] = $share;
-			}
-
-			$qb = $this->dbConn->getQueryBuilder();
-
-			$query = $qb->select('*')
-				->from('share')
-				->where($qb->expr()->in('parent', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)))
-				->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId)))
-				->andWhere($qb->expr()->orX(
-					$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
-					$qb->expr()->eq('item_type', $qb->createNamedParameter('folder'))
-				));
-
-			$stmt = $query->execute();
-
-			while ($data = $stmt->fetch()) {
-				$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
-				$shareMap[$data['parent']]->setStatus((int)$data['accepted']);
-				$shareMap[$data['parent']]->setTarget($data['file_target']);
-				$shareMap[$data['parent']]->setParent($data['parent']);
-			}
+	private function resolveGroupShares($shareMap, $userId) {
+		$qb = $this->dbConn->getQueryBuilder();
+		$query = $qb->select('*')
+			->from('share')
+			->where($qb->expr()->eq('share_with', $qb->createNamedParameter($userId)))
+			->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USERGROUP)))
+			->andWhere($qb->expr()->in('item_type', [$qb->createNamedParameter('file'), $qb->createNamedParameter('folder')]));
+
+		// this is called with either all group shares or one group share.
+		// for all shares it's easier to just only search by share_with,
+		// for a single share it's efficient to filter by parent
+		if (count($shareMap) === 1) {
+			$share = reset($shareMap);
+			$query->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId())));
+		}
 
-			$stmt->closeCursor();
+		$stmt = $query->execute();
 
-			foreach ($shareMap as $share) {
-				$result[] = $share;
-			}
+		while ($data = $stmt->fetch()) {
+			$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
+			$shareMap[$data['parent']]->setStatus((int)$data['accepted']);
+			$shareMap[$data['parent']]->setTarget($data['file_target']);
+			$shareMap[$data['parent']]->setParent($data['parent']);
 		}
 
-		return $result;
+		return array_values($shareMap);
 	}
 
 	/**