Skip to content

Commit 7af54bf

Browse files
author
Carl Schwan
committed
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 6e9d48b commit 7af54bf

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
@@ -13,7 +13,9 @@
1313
use OCA\DAV\CalDAV\DefaultCalendarValidator;
1414
use OCA\DAV\Connector\Sabre\Directory;
1515
use OCA\DAV\Connector\Sabre\FilesPlugin;
16+
use OCA\Files_Trashbin\Sabre\TrashRoot;
1617
use OCP\DB\QueryBuilder\IQueryBuilder;
18+
use OCP\Files\Folder;
1719
use OCP\IDBConnection;
1820
use OCP\IUser;
1921
use Sabre\DAV\Exception as DavException;
@@ -220,10 +222,16 @@ public function propFind($path, PropFind $propFind) {
220222
}
221223

222224
$node = $this->tree->getNodeForPath($path);
223-
if ($node instanceof Directory && $propFind->getDepth() !== 0) {
224-
$this->cacheDirectory($path, $node);
225+
if (($node instanceof Directory) && $propFind->getDepth() !== 0) {
226+
$this->cacheDirectory($path, $node->getNode());
227+
} else if ($node instanceof TrashRoot) {
228+
$trashNodes = $node->getTrashRoots();
229+
foreach ($trashNodes as $trashNode) {
230+
$this->cacheDirectory($path, $trashNode);
231+
}
225232
}
226233

234+
227235
// First fetch the published properties (set by another user), then get the ones set by
228236
// the current user. If both are set then the latter as priority.
229237
foreach ($this->getPublishedProperties($path, $requestedProps) as $propName => $propValue) {
@@ -344,18 +352,17 @@ private function getPublishedProperties(string $path, array $requestedProperties
344352
/**
345353
* prefetch all user properties in a directory
346354
*/
347-
private function cacheDirectory(string $path, Directory $node): void {
355+
private function cacheDirectory(string $path, Folder $node): void {
348356
$prefix = ltrim($path . '/', '/');
349357
$query = $this->connection->getQueryBuilder();
350358
$query->select('name', 'p.propertypath', 'p.propertyname', 'p.propertyvalue', 'p.valuetype')
351359
->from('filecache', 'f')
352-
->hintShardKey('storage', $node->getNode()->getMountPoint()->getNumericStorageId())
360+
->hintShardKey('storage', $node->getMountPoint()->getNumericStorageId())
353361
->leftJoin('f', 'properties', 'p', $query->expr()->eq('p.propertypath', $query->func()->concat(
354362
$query->createNamedParameter($prefix),
355363
'f.name'
356-
)),
357-
)
358-
->where($query->expr()->eq('parent', $query->createNamedParameter($node->getInternalFileId(), IQueryBuilder::PARAM_INT)))
364+
)))
365+
->where($query->expr()->eq('parent', $query->createNamedParameter($node->getId(), IQueryBuilder::PARAM_INT)))
359366
->andWhere($query->expr()->orX(
360367
$query->expr()->eq('p.userid', $query->createNamedParameter($this->user->getUID())),
361368
$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)