From 9ac556cdf5559c53838f15ccb8aa802c9157fc6a Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Apr 2026 01:44:56 +0200 Subject: [PATCH 1/3] perf(share): Remove useless order by id This should improve a bit the performance in some particular cases Signed-off-by: Carl Schwan --- lib/private/Share20/DefaultShareProvider.php | 21 +++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index da0e179fe28..a6a942f9831 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -393,8 +393,7 @@ class DefaultShareProvider implements ], 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 @@ class DefaultShareProvider implements ) ); - $qb->orderBy('id'); - $shares = []; $chunks = array_chunk($childMountRootIds, 1000); @@ -744,7 +741,9 @@ class DefaultShareProvider implements } $qb->setFirstResult($offset); - $qb->orderBy('id'); + if ($offset !== 0 || $limit !== -1) { + $qb->orderBy('id'); + } $cursor = $qb->executeQuery(); $shares = []; @@ -815,7 +814,6 @@ class DefaultShareProvider implements ->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 @@ class DefaultShareProvider implements ->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('id'); + } // Set limit and offset if ($limit !== -1) { @@ -980,9 +980,12 @@ class DefaultShareProvider implements ->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('id'); + } + if ($limit !== -1) { $qb->setMaxResults($limit - count($shares)); } From d470ad108f3b69adfc8a514858edfb6d4812cb12 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 21 Apr 2026 14:33:49 +0200 Subject: [PATCH 2/3] fix: use correct variable for order by Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Carl Schwan --- lib/private/Share20/DefaultShareProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index a6a942f9831..1e064ec821e 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -910,7 +910,7 @@ class DefaultShareProvider implements if ($offset !== 0 || $limit !== -1) { // Order by id - $qb->orderBy('id'); + $qb->orderBy('s.id'); } // Set limit and offset @@ -983,7 +983,7 @@ class DefaultShareProvider implements ->setFirstResult(0); if ($offset !== 0 || $limit !== -1) { - $qb->orderBy('id'); + $qb->orderBy('s.id'); } if ($limit !== -1) { From d307557c566ca2c9b82b62ebe4982d539639d792 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 7 May 2026 19:19:23 +0200 Subject: [PATCH 3/3] test: adjust tests to unsorted share listing Signed-off-by: Robin Appelman --- tests/lib/Share20/DefaultShareProviderTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index e21dddfca4b..470a48adde2 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -662,6 +662,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { $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 @@ class DefaultShareProviderTest extends \Test\TestCase { ->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());