Skip to content

Commit 8495993

Browse files
Carl SchwanCarlSchwan
authored andcommitted
fix(trashbin): Fix n+1 issue in propfind in trash root
CustomPropertiesBackend is already able to cache the custom properties of a folder content. But the trash root is not a Directory but instead just a ICollection, so extend CacheEntry to also handle a TrashEntry. Signed-off-by: Carl Schwan <carl.schwan@nextclound.com>
1 parent ebfdbf8 commit 8495993

6 files changed

Lines changed: 50 additions & 7 deletions

File tree

apps/dav/lib/DAV/CustomPropertiesBackend.php

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919
use OCA\DAV\CalDAV\Trashbin\TrashbinHome;
2020
use OCA\DAV\Connector\Sabre\Directory;
2121
use OCA\DAV\Db\PropertyMapper;
22+
use OCA\DAV\Connector\Sabre\FilesPlugin;
23+
use OCA\Files_Trashbin\Sabre\TrashRoot;
2224
use OCP\DB\QueryBuilder\IQueryBuilder;
25+
use OCP\Files\Folder;
2326
use OCP\IDBConnection;
2427
use OCP\IUser;
2528
use Override;
@@ -208,8 +211,13 @@ public function propFind($path, PropFind $propFind): void {
208211
}
209212

210213
$node = $this->tree->getNodeForPath($path);
211-
if ($node instanceof Directory && $propFind->getDepth() !== 0) {
212-
$this->cacheDirectory($path, $node);
214+
if (($node instanceof Directory) && $propFind->getDepth() !== 0) {
215+
$this->cacheDirectory($path, $node->getNode());
216+
} else if ($node instanceof TrashRoot) {
217+
$trashNodes = $node->getTrashRoots();
218+
foreach ($trashNodes as $trashNode) {
219+
$this->cacheDirectory($path, $trashNode);
220+
}
213221
}
214222

215223
if ($node instanceof CalendarHome && $propFind->getDepth() !== 0) {
@@ -356,18 +364,17 @@ private function getPublishedProperties(string $path, array $requestedProperties
356364
/**
357365
* Prefetch all user properties in a directory
358366
*/
359-
private function cacheDirectory(string $path, Directory $node): void {
367+
private function cacheDirectory(string $path, Folder $node): void {
360368
$prefix = ltrim($path . '/', '/');
361369
$query = $this->connection->getQueryBuilder();
362370
$query->select('name', 'p.propertypath', 'p.propertyname', 'p.propertyvalue', 'p.valuetype')
363371
->from('filecache', 'f')
364-
->hintShardKey('storage', $node->getNode()->getMountPoint()->getNumericStorageId())
372+
->hintShardKey('storage', $node->getMountPoint()->getNumericStorageId())
365373
->leftJoin('f', 'properties', 'p', $query->expr()->eq('p.propertypath', $query->func()->concat(
366374
$query->createNamedParameter($prefix),
367375
'f.name'
368-
)),
369-
)
370-
->where($query->expr()->eq('parent', $query->createNamedParameter($node->getInternalFileId(), IQueryBuilder::PARAM_INT)))
376+
)))
377+
->where($query->expr()->eq('parent', $query->createNamedParameter($node->getId(), IQueryBuilder::PARAM_INT)))
371378
->andWhere($query->expr()->orX(
372379
$query->expr()->eq('p.userid', $query->createNamedParameter($this->user->getUID())),
373380
$query->expr()->isNull('p.userid'),

apps/files_trashbin/lib/Sabre/TrashRoot.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,18 @@
1313
use OCA\Files_Trashbin\Trash\ITrashManager;
1414
use OCA\Files_Trashbin\Trashbin;
1515
use OCP\Files\FileInfo;
16+
use OCP\Files\Folder;
17+
use OCP\Files\IRootFolder;
18+
use OCP\Files\NotFoundException;
19+
use OCP\Files\NotPermittedException;
1620
use OCP\IUser;
21+
use OCP\Server;
1722
use Sabre\DAV\Exception\Forbidden;
1823
use Sabre\DAV\Exception\NotFound;
1924
use Sabre\DAV\ICollection;
2025

2126
class TrashRoot implements ICollection {
27+
private ?Folder $trashFilesRoot = null;
2228

2329
public function __construct(
2430
private IUser $user,
@@ -90,4 +96,11 @@ public function childExists($name): bool {
9096
public function getLastModified(): int {
9197
return 0;
9298
}
99+
100+
/**
101+
* @return Folder[]
102+
*/
103+
public function getTrashRoots(): array {
104+
return $this->trashManager->getTrashRootsForUser($this->user);
105+
}
93106
}

apps/files_trashbin/lib/Trash/ITrashBackend.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
namespace OCA\Files_Trashbin\Trash;
88

9+
use OCP\Files\Folder;
910
use OCP\Files\Node;
1011
use OCP\Files\Storage\IStorage;
1112
use OCP\IUser;
@@ -64,4 +65,10 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool;
6465
* @return Node|null
6566
*/
6667
public function getTrashNodeById(IUser $user, int $fileId);
68+
69+
/**
70+
* @return Folder[]
71+
* @since 32.0.0
72+
*/
73+
public function getTrashRootsForUser(IUser $user): array;
6774
}

apps/files_trashbin/lib/Trash/ITrashManager.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,9 @@ public function pauseTrash();
3838
* @since 15.0.0
3939
*/
4040
public function resumeTrash();
41+
42+
/**
43+
* @since 32.0.0
44+
*/
45+
public function getTrashRootsForUser(IUser $user): array;
4146
}

apps/files_trashbin/lib/Trash/LegacyTrashBackend.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,9 @@ public function getTrashNodeById(IUser $user, int $fileId) {
118118
return null;
119119
}
120120
}
121+
122+
public function getTrashRootsForUser(IUser $user): array {
123+
$userFolder = $this->rootFolder->getUserFolder($user->getUID());
124+
return [$userFolder->getParent()->get('files_trashbin/files')];
125+
}
121126
}

apps/files_trashbin/lib/Trash/TrashManager.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,10 @@ public function pauseTrash() {
108108
public function resumeTrash() {
109109
$this->trashPaused = false;
110110
}
111+
112+
public function getTrashRootsForUser(IUser $user): array {
113+
return array_reduce($this->getBackends(), function (array $items, ITrashBackend $backend) use ($user) {
114+
return array_merge($items, $backend->getTrashRootsForUser($user));
115+
}, []);
116+
}
111117
}

0 commit comments

Comments
 (0)