perf(sharing): Move item_type validation to PHP

We do not support anything but file and folder and are not aware of an app doing that, so the assumption is that it can help to move to an index covering share_type + share_with, instead of share_type + item_type and afterwards a second time on share_with Signed-off-by: Joas Schilling <[email protected]>
nextcloud · Apr 10, 2024 · 1256d8b · 1256d8b
1 parent f425841
commit 1256d8b
Showing 1 changed file with 25 additions and 22 deletions.
diff --git a/lib/Share/RoomShareProvider.php b/lib/Share/RoomShareProvider.php
@@ -527,10 +527,6 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
 			'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum'
 		)
 			->from('share', 's')
-			->andWhere($qb->expr()->orX(
-				$qb->expr()->eq('s.item_type', $qb->createNamedParameter('file')),
-				$qb->expr()->eq('s.item_type', $qb->createNamedParameter('folder'))
-			))
 			->andWhere(
 				$qb->expr()->eq('s.share_type', $qb->createNamedParameter(IShare::TYPE_ROOM))
 			);
@@ -562,6 +558,9 @@ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = t
 		$cursor = $qb->executeQuery();
 		$shares = [];
 		while ($data = $cursor->fetch()) {
+			if ($data['item_type'] !== 'file' && $data['item_type'] !== 'folder') {
+				continue;
+			}
 			$shares[$data['fileid']][] = $this->createShareObject($data);
 		}
 		$cursor->closeCursor();
@@ -723,16 +722,16 @@ private function resolveSharesForRecipient(array $shareMap, string $userId, bool
 			->from('share')
 
 			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERROOM)))
-			->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'))
-			));
+			->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($userId)));
 
 		if ($allRoomShares) {
 			$stmt = $query->executeQuery();
 
 			while ($data = $stmt->fetch()) {
+				if ($data['item_type'] !== 'file' && $data['item_type'] !== 'folder') {
+					continue;
+				}
+
 				if (isset($shareMap[$data['parent']])) {
 					$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
 					$shareMap[$data['parent']]->setTarget($data['file_target']);
@@ -749,6 +748,10 @@ private function resolveSharesForRecipient(array $shareMap, string $userId, bool
 				$stmt = $query->executeQuery();
 
 				while ($data = $stmt->fetch()) {
+					if ($data['item_type'] !== 'file' && $data['item_type'] !== 'folder') {
+						continue;
+					}
+
 					$shareMap[$data['parent']]->setPermissions((int)$data['permissions']);
 					$shareMap[$data['parent']]->setTarget($data['file_target']);
 				}
@@ -817,12 +820,12 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset): arra
 			)
 				->selectAlias('st.id', 'storage_string_id')
 				->from('share', 's')
-				->orderBy('s.id', 'ASC')
 				->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
 				->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'));
 
 			if ($limit !== -1) {
-				$qb->setMaxResults($limit);
+				$qb->orderBy('s.id', 'ASC')
+					->setMaxResults($limit);
 			}
 
 			// Filter by node if provided
@@ -834,11 +837,7 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset): arra
 				->andWhere($qb->expr()->in('s.share_with', $qb->createNamedParameter(
 					$rooms,
 					IQueryBuilder::PARAM_STR_ARRAY
-				)))
-				->andWhere($qb->expr()->orX(
-					$qb->expr()->eq('s.item_type', $qb->createNamedParameter('file')),
-					$qb->expr()->eq('s.item_type', $qb->createNamedParameter('folder'))
-				));
+				)));
 
 			$cursor = $qb->executeQuery();
 			while ($data = $cursor->fetch()) {
@@ -863,6 +862,10 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset): arra
 	}
 
 	private function isAccessibleResult(array $data): bool {
+		if ($data['item_type'] !== 'file' && $data['item_type'] !== 'folder') {
+			return false;
+		}
+
 		// exclude shares leading to deleted file entries
 		if ($data['fileid'] === null || $data['path'] === null) {
 			return false;
@@ -971,18 +974,18 @@ public function getAccessList($nodes, $currentAccess): array {
 			$types[] = self::SHARE_TYPE_USERROOM;
 		}
 
-		$qb->select('id', 'parent', 'share_type', 'share_with', 'file_source', 'file_target', 'permissions')
+		$qb->select('id', 'parent', 'share_type', 'share_with', 'file_source', 'file_target', 'permissions', 'item_type')
 			->from('share')
 			->where($qb->expr()->in('share_type', $qb->createNamedParameter($types, IQueryBuilder::PARAM_INT_ARRAY)))
-			->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)))
-			->andWhere($qb->expr()->orX(
-				$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
-				$qb->expr()->eq('item_type', $qb->createNamedParameter('folder'))
-			));
+			->andWhere($qb->expr()->in('file_source', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
 		$cursor = $qb->executeQuery();
 
 		$users = [];
 		while ($row = $cursor->fetch()) {
+			if ($row['item_type'] !== 'file' && $row['item_type'] !== 'folder') {
+				continue;
+			}
+
 			$type = (int)$row['share_type'];
 			if ($type === IShare::TYPE_ROOM) {
 				$roomToken = $row['share_with'];