Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use more efficient tag retrieval on DAV report request #37969

Merged
merged 14 commits into from
Jun 28, 2023
Merged

use more efficient tag retrieval on DAV report request #37969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
58f7fd2
use efficient tag retrieval on DAV report request
blizzz Apr 28, 2023
b0d1cf5
fix: change if with conditionless else to switch; and a parameter value
blizzz May 11, 2023
783e32a
fix: favorites view and universal search against tags
blizzz May 11, 2023
be50bd1
fix: ensure searchBySystemTag() is available
blizzz May 11, 2023
ddb65be
chore: cleanup unused code
blizzz May 11, 2023
30028bb
fix: no search when LazyFolder was provided
blizzz May 12, 2023
221562d
feat: add searchBySystemTag as PHP API
blizzz Jun 7, 2023
29f59a5
fix: search with more than one search tags
blizzz Jun 2, 2023
49db546
fix: include invisible tags for admins
blizzz Jun 15, 2023
a0f9556
fix: cominbation of small fixes
blizzz Jun 21, 2023
27f6f86
fix: obey offset and limit for results from favs and circles
blizzz Jun 21, 2023
a0b6834
fix: use array_unitersect against objects
blizzz Jun 21, 2023
00b396a
refactor: save unnecessary method_exists
blizzz Jun 21, 2023
d948710
fix: cannot apply limit+offset on multi-tag-search
blizzz Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 80 additions & 58 deletions apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use OC\Files\View;
use OCP\App\IAppManager;
use OCP\Files\Folder;
use OCP\Files\Node as INode;
use OCP\IGroupManager;
use OCP\ITagManager;
use OCP\IUserSession;
Expand All @@ -45,9 +46,9 @@
use Sabre\DAV\Xml\Response\MultiStatus;

class FilesReportPlugin extends ServerPlugin {

// namespace
public const NS_OWNCLOUD = 'http://owncloud.org/ns';
public const NS_NEXTCLOUD = 'http://nextcloud.org/ns';
public const REPORT_NAME = '{http://owncloud.org/ns}filter-files';
public const SYSTEMTAG_PROPERTYNAME = '{http://owncloud.org/ns}systemtag';
public const CIRCLE_PROPERTYNAME = '{http://owncloud.org/ns}circle';
Expand Down Expand Up @@ -186,6 +187,7 @@ public function onReport($reportName, $report, $uri) {
}

$ns = '{' . $this::NS_OWNCLOUD . '}';
$ncns = '{' . $this::NS_NEXTCLOUD . '}';
$requestedProps = [];
$filterRules = [];

Expand All @@ -199,6 +201,14 @@ public function onReport($reportName, $report, $uri) {
foreach ($reportProps['value'] as $propVal) {
$requestedProps[] = $propVal['name'];
}
} elseif ($name === '{DAV:}limit') {
foreach ($reportProps['value'] as $propVal) {
if ($propVal['name'] === '{DAV:}nresults') {
$limit = (int)$propVal['value'];
} elseif ($propVal['name'] === $ncns . 'firstresult') {
$offset = (int)$propVal['value'];
}
}
}
}

Expand All @@ -209,13 +219,32 @@ public function onReport($reportName, $report, $uri) {

// gather all file ids matching filter
try {
$resultFileIds = $this->processFilterRules($filterRules);
$resultFileIds = $this->processFilterRulesForFileIDs($filterRules);
// no logic in circles and favorites for paging, we always have all results, and slice later on
$resultFileIds = array_slice($resultFileIds, $offset ?? 0, $limit ?? null);
// fetching nodes has paging on DB level – therefore we cannot mix and slice the results, similar
// to user backends. I.e. the final result may return more results than requested.
$resultNodes = $this->processFilterRulesForFileNodes($filterRules, $limit ?? null, $offset ?? null);
blizzz marked this conversation as resolved.
Show resolved Hide resolved
} catch (TagNotFoundException $e) {
throw new PreconditionFailed('Cannot filter by non-existing tag', 0, $e);
}

$results = [];
foreach ($resultNodes as $entry) {
if ($entry) {
$results[] = $this->wrapNode($entry);
}
}

// find sabre nodes by file id, restricted to the root node path
$results = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
$additionalNodes = $this->findNodesByFileIds($reportTargetNode, $resultFileIds);
if ($additionalNodes && $results) {
$results = array_uintersect($results, $additionalNodes, function (Node $a, Node $b): int {
return $a->getId() - $b->getId();
});
} elseif (!$results && $additionalNodes) {
$results = $additionalNodes;
}

$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
$responses = $this->prepareResponses($filesUri, $requestedProps, $results);
Expand Down Expand Up @@ -261,19 +290,13 @@ private function getFilesBaseUri(string $uri, string $subPath): string {
*
* @param array $filterRules
* @return array array of unique file id results
*
* @throws TagNotFoundException whenever a tag was not found
*/
protected function processFilterRules($filterRules) {
protected function processFilterRulesForFileIDs(array $filterRules): array {
$ns = '{' . $this::NS_OWNCLOUD . '}';
$resultFileIds = null;
$systemTagIds = [];
$resultFileIds = [];
$circlesIds = [];
$favoriteFilter = null;
foreach ($filterRules as $filterRule) {
if ($filterRule['name'] === $ns . 'systemtag') {
$systemTagIds[] = $filterRule['value'];
}
if ($filterRule['name'] === self::CIRCLE_PROPERTYNAME) {
$circlesIds[] = $filterRule['value'];
}
Expand All @@ -289,15 +312,6 @@ protected function processFilterRules($filterRules) {
}
}

if (!empty($systemTagIds)) {
$fileIds = $this->getSystemTagFileIds($systemTagIds);
if (empty($resultFileIds)) {
$resultFileIds = $fileIds;
} else {
$resultFileIds = array_intersect($fileIds, $resultFileIds);
}
}

if (!empty($circlesIds)) {
$fileIds = $this->getCirclesFileIds($circlesIds);
if (empty($resultFileIds)) {
Expand All @@ -310,47 +324,48 @@ protected function processFilterRules($filterRules) {
return $resultFileIds;
}

private function getSystemTagFileIds($systemTagIds) {
$resultFileIds = null;

// check user permissions, if applicable
if (!$this->isAdmin()) {
// check visibility/permission
$tags = $this->tagManager->getTagsByIds($systemTagIds);
$unknownTagIds = [];
foreach ($tags as $tag) {
if (!$tag->isUserVisible()) {
$unknownTagIds[] = $tag->getId();
}
}

if (!empty($unknownTagIds)) {
throw new TagNotFoundException('Tag with ids ' . implode(', ', $unknownTagIds) . ' not found');
protected function processFilterRulesForFileNodes(array $filterRules, ?int $limit, ?int $offset): array {
$systemTagIds = [];
foreach ($filterRules as $filterRule) {
if ($filterRule['name'] === self::SYSTEMTAG_PROPERTYNAME) {
$systemTagIds[] = $filterRule['value'];
}
}

// fetch all file ids and intersect them
foreach ($systemTagIds as $systemTagId) {
$fileIds = $this->tagMapper->getObjectIdsForTags($systemTagId, 'files');
$nodes = [];

if (empty($fileIds)) {
// This tag has no files, nothing can ever show up
return [];
}
// type check to ensure searchBySystemTag is available, it is not
// exposed in API yet
if (!empty($systemTagIds)) {
$tags = $this->tagManager->getTagsByIds($systemTagIds, $this->userSession->getUser());

// first run ?
if ($resultFileIds === null) {
$resultFileIds = $fileIds;
} else {
$resultFileIds = array_intersect($resultFileIds, $fileIds);
// For we run DB queries per tag and require intersection, we cannot apply limit and offset for DB queries on multi tag search.
$oneTagSearch = count($tags) === 1;
$dbLimit = $oneTagSearch ? $limit ?? 0 : 0;
$dbOffset = $oneTagSearch ? $offset ?? 0 : 0;

foreach ($tags as $tag) {
$tagName = $tag->getName();
$tmpNodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $dbLimit, $dbOffset);

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
if (count($nodes) === 0) {
marcelklehr marked this conversation as resolved.
Show resolved Hide resolved
$nodes = $tmpNodes;
} else {
$nodes = array_uintersect($nodes, $tmpNodes, function (INode $a, INode $b): int {
return $a->getId() - $b->getId();
});
}
if ($nodes === []) {
// there cannot be a common match when nodes are empty early.
return $nodes;
}
}

if (empty($resultFileIds)) {
// Empty intersection, nothing can show up anymore
return [];
if (!$oneTagSearch && ($limit !== null || $offset !== null)) {
$nodes = array_slice($nodes, $offset, $limit);

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 2 of array_slice cannot be null, possibly null value provided
}
}
return $resultFileIds;

return $nodes;
}

/**
Expand Down Expand Up @@ -406,7 +421,10 @@ public function prepareResponses($filesUri, $requestedProps, $nodes) {
* @param array $fileIds file ids
* @return Node[] array of Sabre nodes
*/
public function findNodesByFileIds($rootNode, $fileIds) {
public function findNodesByFileIds(Node $rootNode, array $fileIds): array {
if (empty($fileIds)) {
return [];
}
$folder = $this->userFolder;
if (trim($rootNode->getPath(), '/') !== '') {
$folder = $folder->get($rootNode->getPath());
Expand All @@ -417,17 +435,21 @@ public function findNodesByFileIds($rootNode, $fileIds) {
$entry = $folder->getById($fileId);
if ($entry) {
$entry = current($entry);
if ($entry instanceof \OCP\Files\File) {
$results[] = new File($this->fileView, $entry);
} elseif ($entry instanceof \OCP\Files\Folder) {
$results[] = new Directory($this->fileView, $entry);
}
$results[] = $this->wrapNode($entry);
}
}

return $results;
}

protected function wrapNode(\OCP\Files\File|\OCP\Files\Folder $node): File|Directory {
if ($node instanceof \OCP\Files\File) {
return new File($this->fileView, $node);
} else {
return new Directory($this->fileView, $node);
}
}

/**
* Returns whether the currently logged in user is an administrator
*/
Expand Down
Loading