Skip to content

Commit

Permalink
use efficient tag retrieval on DAV report request
Browse files Browse the repository at this point in the history 
- 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 <blizzz@arthur-schiwon.de>
  • Loading branch information
@blizzz
blizzz committed Apr 28, 2023
1 parent c174172 commit d40537b
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 39 deletions.
75 changes: 53 additions & 22 deletions apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
View file
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,21 @@ 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, $offset);
} 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);
$results = array_merge($this->findNodesByFileIds($reportTargetNode, $resultFileIds), $results);

$filesUri = $this->getFilesBaseUri($uri, $reportTargetNode->getPath());
$responses = $this->prepareResponses($filesUri, $requestedProps, $results);
Expand Down Expand Up @@ -264,16 +282,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 +303,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 +315,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);
}

return $nodes;
}

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

Expand Down Expand Up @@ -406,7 +430,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 +444,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
View file
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 @@ -144,22 +145,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') {
$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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,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
View file
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

0 comments on commit d40537b

Please sign in to comment.