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

[stable25] use more efficient tag retrieval on DAV report request #39233

Merged
merged 15 commits into from
Jul 11, 2023
Merged

[stable25] use more efficient tag retrieval on DAV report request #39233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7e2ad76
use efficient tag retrieval on DAV report request
blizzz Apr 28, 2023
5086dff
fix: change if with conditionless else to switch; and a parameter value
blizzz May 11, 2023
837fd9f
fix: favorites view and universal search against tags
blizzz May 11, 2023
03590e8
fix: ensure searchBySystemTag() is available
blizzz May 11, 2023
2b513e9
chore: cleanup unused code
blizzz May 11, 2023
8d25273
fix: no search when LazyFolder was provided
blizzz May 12, 2023
5c0a1a4
fix: search with more than one search tags
blizzz Jun 2, 2023
d21b251
fix: include invisible tags for admins
blizzz Jun 15, 2023
1ffae9d
fix: cominbation of small fixes
blizzz Jun 21, 2023
9f54901
fix: obey offset and limit for results from favs and circles
blizzz Jun 21, 2023
2c16269
fix: use array_unitersect against objects
blizzz Jun 21, 2023
5cd984b
refactor: save unnecessary method_exists
blizzz Jun 21, 2023
6256da1
fix: cannot apply limit+offset on multi-tag-search
blizzz Jun 27, 2023
33b9ba5
refactor: adjust to unexposed searchBySystemTag
blizzz Jul 6, 2023
2ca8c71
fix: PHP 7.4 compatibility
blizzz Jul 10, 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
142 changes: 84 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);
} 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) && 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

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

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,25 @@ 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;
}

/**
* @param \OCP\Files\File|\OCP\Files\Folder $node
* @return File|Directory
*/
protected function wrapNode($node): \OCA\DAV\Connector\Sabre\Node {
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