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

[stable27] use more efficient tag retrieval on DAV report request #39202

Merged
merged 14 commits into from Jul 7, 2023
Merged

[stable27] use more efficient tag retrieval on DAV report request #39202

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
7bfe6f2
use efficient tag retrieval on DAV report request
blizzz Apr 28, 2023
7d8d586
fix: change if with conditionless else to switch; and a parameter value
blizzz May 11, 2023
99442b6
fix: favorites view and universal search against tags
blizzz May 11, 2023
36a0d14
fix: ensure searchBySystemTag() is available
blizzz May 11, 2023
f872867
chore: cleanup unused code
blizzz May 11, 2023
4e5c3bd
fix: no search when LazyFolder was provided
blizzz May 12, 2023
4ac77f0
fix: search with more than one search tags
blizzz Jun 2, 2023
66a7064
fix: include invisible tags for admins
blizzz Jun 15, 2023
b55be6f
fix: cominbation of small fixes
blizzz Jun 21, 2023
a5a1345
fix: obey offset and limit for results from favs and circles
blizzz Jun 21, 2023
653812d
fix: use array_unitersect against objects
blizzz Jun 21, 2023
d05d185
refactor: save unnecessary method_exists
blizzz Jun 21, 2023
35de0b5
fix: cannot apply limit+offset on multi-tag-search
blizzz Jun 27, 2023
a7b4f50
refactor: adjust to unexposed searchBySystemTag
blizzz Jul 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
138 changes: 80 additions & 58 deletions apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
View file
Open in desktop
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 @@
}

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

Expand All @@ -199,6 +201,14 @@
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 @@

// 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);
} 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 @@
*
* @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 @@
}
}

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 @@
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) && method_exists($this->userFolder, 'searchBySystemTag')) {
$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 Note

Cannot call method getUID on possibly null value
if (count($nodes) === 0) {
$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 Note

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

return $nodes;
}

/**
Expand Down Expand Up @@ -406,7 +421,10 @@
* @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 @@
$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