Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
use efficient tag retrieval on DAV report request
- uses DAV search approach against valid files joined by systemtag selector
- reduced table join for tag/systemtag search
- supports pagination
- no changes to the output formats or similar

Example request body:

<?xml version="1.0"?>
<oc:filter-files xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns" xmlns:nc="http://nextcloud.org/ns" xmlns:ocs="http://open-collaboration-services.org/ns">
  <d:prop>
    <d:getcontentlength/>
    <d:getcontenttype/>
    <d:getetag/>
    <d:getlastmodified/>
    <d:resourcetype/>
    <nc:face-detections/>
    <nc:file-metadata-size/>
    <nc:has-preview/>
    <nc:realpath/>
    <oc:favorite/>
    <oc:fileid/>
    <oc:permissions/>
    <nc:nbItems/>
  </d:prop>
  <oc:filter-rules>
    <oc:systemtag>32</oc:systemtag>
  </oc:filter-rules>
  <d:limit>
    <d:nresults>50</d:nresults>
    <nc:firstresult>0</nc:firstresult>
  </d:limit>
</oc:filter-files>

Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
blizzz committed May 2, 2023
commit 72d1488b02e1e2f07e2321aa8a3802a6465b5bef
78 changes: 56 additions & 22 deletions apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ 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,24 @@ public function onReport($reportName, $report, $uri) {

// gather all file ids matching filter
try {
$resultFileIds = $this->processFilterRules($filterRules);
$resultFileIds = $this->processFilterRulesForFileIDs($filterRules);
$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 (!empty($additionalNodes)) {
$results = array_intersect($results, $additionalNodes);
}

$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
$responses = $this->prepareResponses($filesUri, $requestedProps, $results);
Expand Down Expand Up @@ -264,16 +285,12 @@ private function getFilesBaseUri(string $uri, string $subPath): string {
*
* @throws TagNotFoundException whenever a tag was not found
*/
protected function processFilterRules($filterRules) {
protected function processFilterRulesForFileIDs($filterRules) {
$ns = '{' . $this::NS_OWNCLOUD . '}';
$resultFileIds = null;
$systemTagIds = [];
$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 +306,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,6 +318,25 @@ protected function processFilterRules($filterRules) {
return $resultFileIds;
}

protected function processFilterRulesForFileNodes(array $filterRules, ?int $limit, ?int $offset): array {
$systemTagIds = [];
foreach ($filterRules as $filterRule) {
if ($filterRule['name'] === self::SYSTEMTAG_PROPERTYNAME) {
$systemTagIds[] = $filterRule['value'];
}
}

$nodes = [];

if (!empty($systemTagIds)) {
$tags = $this->tagManager->getTagsByIds($systemTagIds);
$tagName = (current($tags))->getName();
$nodes = $this->userFolder->searchBySystemTag($tagName, $this->userSession->getUser()->getUID(), $limit ?? 0, $offset ?? 0);

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value

Check failure

Code scanning / Psalm

UndefinedInterfaceMethod

Method OCP\Files\Folder::searchBySystemTag does not exist
}

return $nodes;
}

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

Expand Down Expand Up @@ -406,7 +433,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($rootNode, $fileIds): array {
if (empty($fileIds)) {
return [];
}
$folder = $this->userFolder;
if (trim($rootNode->getPath(), '/') !== '') {
$folder = $folder->get($rootNode->getPath());
Expand All @@ -417,17 +447,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
37 changes: 21 additions & 16 deletions lib/private/Files/Cache/QuerySearchHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\IMimeTypeLoader;
use OCP\Files\Search\ISearchBinaryOperator;
use OCP\Files\Search\ISearchComparison;
use OCP\Files\Search\ISearchQuery;
use OCP\IDBConnection;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -145,22 +146,26 @@ public function searchInCaches(ISearchQuery $searchQuery, array $caches): array
if ($user === null) {
throw new \InvalidArgumentException("Searching by tag requires the user to be set in the query");
}
$query
->leftJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid'))
->leftJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX(
$builder->expr()->eq('tagmap.type', 'tag.type'),
$builder->expr()->eq('tagmap.categoryid', 'tag.id'),
$builder->expr()->eq('tag.type', $builder->createNamedParameter('files')),
$builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))
))
->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $builder->expr()->andX(
$builder->expr()->eq('file.fileid', $builder->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
$builder->expr()->eq('systemtagmap.objecttype', $builder->createNamedParameter('files'))
))
->leftJoin('systemtagmap', 'systemtag', 'systemtag', $builder->expr()->andX(
$builder->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
$builder->expr()->eq('systemtag.visibility', $builder->createNamedParameter(true))
));
if ($searchQuery->getSearchOperation() instanceof ISearchComparison && $searchQuery->getSearchOperation()->getField() === 'systemtag') {

Check failure

Code scanning / Psalm

UndefinedInterfaceMethod

Method OCP\Files\Search\ISearchOperator::getField does not exist
$query
->leftJoin('file', 'systemtag_object_mapping', 'systemtagmap', $builder->expr()->andX(
$builder->expr()->eq('file.fileid', $builder->expr()->castColumn('systemtagmap.objectid', IQueryBuilder::PARAM_INT)),
$builder->expr()->eq('systemtagmap.objecttype', $builder->createNamedParameter('files'))
))
->leftJoin('systemtagmap', 'systemtag', 'systemtag', $builder->expr()->andX(
$builder->expr()->eq('systemtag.id', 'systemtagmap.systemtagid'),
$builder->expr()->eq('systemtag.visibility', $builder->createNamedParameter(true))
));
} else {
$query
->leftJoin('file', 'vcategory_to_object', 'tagmap', $builder->expr()->eq('file.fileid', 'tagmap.objid'))
->leftJoin('tagmap', 'vcategory', 'tag', $builder->expr()->andX(
$builder->expr()->eq('tagmap.type', 'tag.type'),
$builder->expr()->eq('tagmap.categoryid', 'tag.id'),
$builder->expr()->eq('tag.type', $builder->createNamedParameter('files')),
$builder->expr()->eq('tag.uid', $builder->createNamedParameter($user->getUID()))
));
}
}

$this->applySearchConstraints($query, $searchQuery, $caches);
Expand Down
7 changes: 6 additions & 1 deletion lib/private/Files/Node/Folder.php
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,12 @@ public function searchByMime($mimetype) {
* @return Node[]
*/
public function searchByTag($tag, $userId) {
$query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tagname', $tag), $userId);
$query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'tag', $tag), $userId);
return $this->search($query);
}

public function searchBySystemTag(string $tag, string $userId, int $limit = 0, int $offset = 0): array {
$query = $this->queryFromOperator(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'systemtag', $tag), $userId, $limit, $offset);
return $this->search($query);
}

Expand Down
1 change: 1 addition & 0 deletions lib/private/SystemTag/SystemTagObjectMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public function getObjectIdsForTags($tagIds, string $objectType, int $limit = 0,
while ($row = $result->fetch()) {
$objectIds[] = $row['objectid'];
}
$result->closeCursor();

return $objectIds;
}
Expand Down