Merge pull request #59753 from nextcloud/work/carl/remove-orderById

perf(share): Remove useless order by id

Author: CarlSchwan

lib/private/Share20/DefaultShareProvider.php

@@ -393,8 +393,7 @@ public function getChildren(IShare $parent): array {
 					], IQueryBuilder::PARAM_INT_ARRAY)
 				)
 			)
-			->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY)))
-			->orderBy('id');
+			->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY)));
 
 		$cursor = $qb->executeQuery();
 		while ($data = $cursor->fetch()) {
@@ -684,8 +683,6 @@ private function getSharesInFolderInternal(?string $userId, Folder $node, ?bool
 			)
 		);
 
-		$qb->orderBy('id');
-
 		$shares = [];
 
 		$chunks = array_chunk($childMountRootIds, 1000);
@@ -744,7 +741,9 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs
 		}
 
 		$qb->setFirstResult($offset);
-		$qb->orderBy('id');
+		if ($offset !== 0 || $limit !== -1) {
+			$qb->orderBy('id');
+		}
 
 		$cursor = $qb->executeQuery();
 		$shares = [];
@@ -815,7 +814,6 @@ public function getSharesByPath(Node $path) {
 			->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($path->getId())))
 			->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter([IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_LINK], IQueryBuilder::PARAM_INT_ARRAY)))
 			->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY)))
-			->orderBy('id', 'ASC')
 			->executeQuery();
 
 		$shares = [];
@@ -910,8 +908,10 @@ private function _getSharedWith(
 				->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
 				->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'));
 
-			// Order by id
-			$qb->orderBy('s.id');
+			if ($offset !== 0 || $limit !== -1) {
+				// Order by id
+				$qb->orderBy('s.id');
+			}
 
 			// Set limit and offset
 			if ($limit !== -1) {
@@ -980,9 +980,12 @@ private function _getSharedWith(
 					->from('share', 's')
 					->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid'))
 					->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id'))
-					->orderBy('s.id')
 					->setFirstResult(0);
 
+				if ($offset !== 0 || $limit !== -1) {
+					$qb->orderBy('s.id');
+				}
+
 				if ($limit !== -1) {
 					$qb->setMaxResults($limit - count($shares));
 				}

tests/lib/Share20/DefaultShareProviderTest.php

@@ -662,6 +662,7 @@ public function testGetChildren(): void {
 		$share->method('getId')->willReturn($id);
 
 		$children = $this->provider->getChildren($share);
+		usort($children, fn (IShare $a, IShare $b) => $a->getId() <=> $b->getId());
 
 		$this->assertCount(2, $children);
 
@@ -2642,6 +2643,7 @@ public function testGetSharesInFolder(): void {
 		$this->assertSame(IShare::TYPE_USER, $file_shares[0]->getShareType());
 
 		$folder_shares = $result[$folder2->getId()];
+		usort($folder_shares, fn (IShare $a, IShare $b) => $a->getId() <=> $b->getId());
 		$this->assertCount(2, $folder_shares);
 		$this->assertSame($folder2->getId(), $folder_shares[0]->getNodeId());
 		$this->assertSame($folder2->getId(), $folder_shares[1]->getNodeId());
@@ -3102,6 +3104,7 @@ public function testGetSharesByPath(): void {
 			->willReturn(1);
 
 		$shares = $this->provider->getSharesByPath($node);
+		usort($shares, fn (IShare $a, IShare $b) => $a->getId() <=> $b->getId());
 		$this->assertCount(3, $shares);
 
 		$this->assertEquals($id1, $shares[0]->getId());