Skip to content

Commit c62637d

Browse files
icewind1991skjnldsv
icewind1991
authored and
skjnldsv
committed
Allow filtering the search results to the users home storage
This is done by adding a ```xml <d:eq> <d:prop> <oc:owner-id/> </d:prop> <d:literal>$userId</d:literal> </d:eq> ``` clause to the search query. Searching by `owner-id` can only be done with the current user id and the comparison can not be inside a `<d:not>` or `<d:or>` statement Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 2b19da8 commit c62637d

File tree

7 files changed

+214
-26
lines changed

7 files changed

+214
-26
lines changed

‎apps/dav/lib/Files/FileSearchBackend.php‎

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,14 @@ public function getPropertyDefinitionsForScope($href, $path) {
119119
new SearchPropertyDefinition(FilesPlugin::SIZE_PROPERTYNAME, true, true, true, SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER),
120120
new SearchPropertyDefinition(TagsPlugin::FAVORITE_PROPERTYNAME, true, true, true, SearchPropertyDefinition::DATATYPE_BOOLEAN),
121121
new SearchPropertyDefinition(FilesPlugin::INTERNAL_FILEID_PROPERTYNAME, true, true, false, SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER),
122+
new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, true, true, false),
122123

123124
// select only properties
124125
new SearchPropertyDefinition('{DAV:}resourcetype', false, true, false),
125126
new SearchPropertyDefinition('{DAV:}getcontentlength', false, true, false),
126127
new SearchPropertyDefinition(FilesPlugin::CHECKSUMS_PROPERTYNAME, false, true, false),
127128
new SearchPropertyDefinition(FilesPlugin::PERMISSIONS_PROPERTYNAME, false, true, false),
128129
new SearchPropertyDefinition(FilesPlugin::GETETAG_PROPERTYNAME, false, true, false),
129-
new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, false, true, false),
130130
new SearchPropertyDefinition(FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME, false, true, false),
131131
new SearchPropertyDefinition(FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME, false, true, false),
132132
new SearchPropertyDefinition(FilesPlugin::HAS_PREVIEW_PROPERTYNAME, false, true, false, SearchPropertyDefinition::DATATYPE_BOOLEAN),
@@ -169,10 +169,12 @@ public function search(Query $search) {
169169
return new SearchResult($davNode, $path);
170170
}, $results);
171171

172-
// Sort again, since the result from multiple storages is appended and not sorted
173-
usort($nodes, function (SearchResult $a, SearchResult $b) use ($search) {
174-
return $this->sort($a, $b, $search->orderBy);
175-
});
172+
if (!$query->limitToHome()) {
173+
// Sort again, since the result from multiple storages is appended and not sorted
174+
usort($nodes, function (SearchResult $a, SearchResult $b) use ($search) {
175+
return $this->sort($a, $b, $search->orderBy);
176+
});
177+
}
176178

177179
// If a limit is provided use only return that number of files
178180
if ($search->limit->maxResults !== 0) {
@@ -267,11 +269,29 @@ private function getHrefForNode(Node $node) {
267269
* @param Query $query
268270
* @return ISearchQuery
269271
*/
270-
private function transformQuery(Query $query) {
272+
private function transformQuery(Query $query): ISearchQuery {
271273
// TODO offset
272274
$limit = $query->limit;
273275
$orders = array_map([$this, 'mapSearchOrder'], $query->orderBy);
274-
return new SearchQuery($this->transformSearchOperation($query->where), (int)$limit->maxResults, 0, $orders, $this->user);
276+
277+
$limitHome = false;
278+
$ownerProp = $this->extractWhereValue($query->where, FilesPlugin::OWNER_ID_PROPERTYNAME, Operator::OPERATION_EQUAL);
279+
if ($ownerProp !== null) {
280+
if ($ownerProp === $this->user->getUID()) {
281+
$limitHome = true;
282+
} else {
283+
throw new \InvalidArgumentException("Invalid search value for '{http://owncloud.org/ns}owner-id', only the current user id is allowed");
284+
}
285+
}
286+
287+
return new SearchQuery(
288+
$this->transformSearchOperation($query->where),
289+
(int)$limit->maxResults,
290+
0,
291+
$orders,
292+
$this->user,
293+
$limitHome
294+
);
275295
}
276296

277297
/**
@@ -360,4 +380,52 @@ private function castValue(SearchPropertyDefinition $property, $value) {
360380
return $value;
361381
}
362382
}
383+
384+
/**
385+
* Get a specific property from the were clause
386+
*/
387+
private function extractWhereValue(Operator &$operator, string $propertyName, string $comparison, bool $acceptableLocation = true): ?string {
388+
switch ($operator->type) {
389+
case Operator::OPERATION_AND:
390+
case Operator::OPERATION_OR:
391+
case Operator::OPERATION_NOT:
392+
foreach ($operator->arguments as &$argument) {
393+
$value = $this->extractWhereValue($argument, $propertyName, $comparison, $acceptableLocation && $operator->type === Operator::OPERATION_AND);
394+
if ($value !== null) {
395+
return $value;
396+
}
397+
}
398+
return null;
399+
case Operator::OPERATION_EQUAL:
400+
case Operator::OPERATION_GREATER_OR_EQUAL_THAN:
401+
case Operator::OPERATION_GREATER_THAN:
402+
case Operator::OPERATION_LESS_OR_EQUAL_THAN:
403+
case Operator::OPERATION_LESS_THAN:
404+
case Operator::OPERATION_IS_LIKE:
405+
if ($operator->arguments[0]->name === $propertyName) {
406+
if ($operator->type === $comparison) {
407+
if ($acceptableLocation) {
408+
if ($operator->arguments[1] instanceof Literal) {
409+
$value = $operator->arguments[1]->value;
410+
411+
// to remove the comparison from the query, we replace it with an empty AND
412+
$operator = new Operator(Operator::OPERATION_AND);
413+
414+
return $value;
415+
} else {
416+
throw new \InvalidArgumentException("searching by '$propertyName' is only allowed with a literal value");
417+
}
418+
} else{
419+
throw new \InvalidArgumentException("searching by '$propertyName' is not allowed inside a '{DAV:}or' or '{DAV:}not'");
420+
}
421+
} else {
422+
throw new \InvalidArgumentException("searching by '$propertyName' is only allowed inside a '$comparison'");
423+
}
424+
} else {
425+
return null;
426+
}
427+
default:
428+
return null;
429+
}
430+
}
363431
}

‎apps/dav/tests/unit/Files/FileSearchBackendTest.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@
3535
use OCP\Files\FileInfo;
3636
use OCP\Files\Folder;
3737
use OCP\Files\IRootFolder;
38+
use OCP\Files\Search\ISearchBinaryOperator;
3839
use OCP\Files\Search\ISearchComparison;
40+
use OCP\Files\Search\ISearchQuery;
3941
use OCP\IUser;
4042
use OCP\Share\IManager;
4143
use SearchDAV\Backend\SearchPropertyDefinition;
@@ -308,4 +310,81 @@ public function testSearchNonFolder() {
308310
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
309311
$this->search->search($query);
310312
}
313+
314+
public function testSearchLimitOwnerBasic() {
315+
$this->tree->expects($this->any())
316+
->method('getNodeForPath')
317+
->willReturn($this->davFolder);
318+
319+
/** @var ISearchQuery|null $receivedQuery */
320+
$receivedQuery = null;
321+
$this->searchFolder
322+
->method('search')
323+
->will($this->returnCallback(function ($query) use (&$receivedQuery) {
324+
$receivedQuery = $query;
325+
return [
326+
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
327+
];
328+
}));
329+
330+
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
331+
$this->search->search($query);
332+
333+
$this->assertNotNull($receivedQuery);
334+
$this->assertTrue($receivedQuery->limitToHome());
335+
336+
/** @var ISearchBinaryOperator $operator */
337+
$operator = $receivedQuery->getSearchOperation();
338+
$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
339+
$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
340+
$this->assertEmpty($operator->getArguments());
341+
}
342+
343+
public function testSearchLimitOwnerNested() {
344+
$this->tree->expects($this->any())
345+
->method('getNodeForPath')
346+
->willReturn($this->davFolder);
347+
348+
/** @var ISearchQuery|null $receivedQuery */
349+
$receivedQuery = null;
350+
$this->searchFolder
351+
->method('search')
352+
->will($this->returnCallback(function ($query) use (&$receivedQuery) {
353+
$receivedQuery = $query;
354+
return [
355+
new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
356+
];
357+
}));
358+
359+
$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
360+
$query->where = new \SearchDAV\Query\Operator(
361+
\SearchDAV\Query\Operator::OPERATION_AND,
362+
[
363+
new \SearchDAV\Query\Operator(
364+
\SearchDAV\Query\Operator::OPERATION_EQUAL,
365+
[new SearchPropertyDefinition('{DAV:}getcontenttype', true, true, true), new \SearchDAV\Query\Literal('image/png')]
366+
),
367+
new \SearchDAV\Query\Operator(
368+
\SearchDAV\Query\Operator::OPERATION_EQUAL,
369+
[new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, true, true, true), new \SearchDAV\Query\Literal($this->user->getUID())]
370+
)
371+
]
372+
);
373+
$this->search->search($query);
374+
375+
$this->assertNotNull($receivedQuery);
376+
$this->assertTrue($receivedQuery->limitToHome());
377+
378+
/** @var ISearchBinaryOperator $operator */
379+
$operator = $receivedQuery->getSearchOperation();
380+
$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
381+
$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
382+
$this->assertCount(2, $operator->getArguments());
383+
384+
/** @var ISearchBinaryOperator $operator */
385+
$operator = $operator->getArguments()[1];
386+
$this->assertInstanceOf(ISearchBinaryOperator::class, $operator);
387+
$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
388+
$this->assertEmpty($operator->getArguments());
389+
}
311390
}

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,10 @@ public function searchQuery(ISearchQuery $searchQuery) {
793793
->andWhere($builder->expr()->eq('tag.uid', $builder->createNamedParameter($searchQuery->getUser()->getUID())));
794794
}
795795

796-
$query->andWhere($this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation()));
796+
$searchExpr = $this->querySearchHelper->searchOperatorToDBExpr($builder, $searchQuery->getSearchOperation());
797+
if ($searchExpr) {
798+
$query->andWhere($searchExpr);
799+
}
797800

798801
$this->querySearchHelper->addSearchOrdersToQuery($query, $searchQuery->getOrder());
799802

‎lib/private/Files/Cache/QuerySearchHelper.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,18 @@ public function shouldJoinTags(ISearchOperator $operator) {
8888
* @param ISearchOperator $operator
8989
*/
9090
public function searchOperatorArrayToDBExprArray(IQueryBuilder $builder, array $operators) {
91-
return array_map(function ($operator) use ($builder) {
91+
return array_filter(array_map(function ($operator) use ($builder) {
9292
return $this->searchOperatorToDBExpr($builder, $operator);
93-
}, $operators);
93+
}, $operators));
9494
}
9595

9696
public function searchOperatorToDBExpr(IQueryBuilder $builder, ISearchOperator $operator) {
9797
$expr = $builder->expr();
9898
if ($operator instanceof ISearchBinaryOperator) {
99+
if (count($operator->getArguments()) === 0) {
100+
return null;
101+
}
102+
99103
switch ($operator->getType()) {
100104
case ISearchBinaryOperator::OPERATOR_NOT:
101105
$negativeOperator = $operator->getArguments()[0];
@@ -121,6 +125,11 @@ public function searchOperatorToDBExpr(IQueryBuilder $builder, ISearchOperator $
121125
private function searchComparisonToDBExpr(IQueryBuilder $builder, ISearchComparison $comparison, array $operatorMap) {
122126
$this->validateComparison($comparison);
123127

128+
// "owner" search is done by limiting the storages queries and should not be put in the sql
129+
if ($comparison->getField() === 'owner') {
130+
return null;
131+
}
132+
124133
list($field, $value, $type) = $this->getOperatorFieldAndValue($comparison);
125134
if (isset($operatorMap[$type])) {
126135
$queryOperator = $operatorMap[$type];

‎lib/private/Files/Node/Folder.php‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
use OCP\Files\Mount\IMountPoint;
3535
use OCP\Files\NotFoundException;
3636
use OCP\Files\NotPermittedException;
37-
use OCP\Files\Search\ISearchOperator;
37+
use OCP\Files\Search\ISearchQuery;
3838

3939
class Folder extends Node implements \OCP\Files\Folder {
4040
/**
@@ -191,7 +191,7 @@ public function newFile($path) {
191191
/**
192192
* search for files with the name matching $query
193193
*
194-
* @param string|ISearchOperator $query
194+
* @param string|ISearchQuery $query
195195
* @return \OC\Files\Node\Node[]
196196
*/
197197
public function search($query) {
@@ -229,6 +229,11 @@ public function searchByTag($tag, $userId) {
229229
* @return \OC\Files\Node\Node[]
230230
*/
231231
private function searchCommon($method, $args) {
232+
$limitToHome = ($method === 'searchQuery')? $args[0]->limitToHome(): false;
233+
if ($limitToHome && count(explode('/', $this->path)) !== 3) {
234+
throw new \InvalidArgumentException('searching by owner is only allows on the users home folder');
235+
}
236+
232237
$files = array();
233238
$rootLength = strlen($this->path);
234239
$mount = $this->root->getMount($this->path);
@@ -252,19 +257,22 @@ private function searchCommon($method, $args) {
252257
}
253258
}
254259

255-
$mounts = $this->root->getMountsIn($this->path);
256-
foreach ($mounts as $mount) {
257-
$storage = $mount->getStorage();
258-
if ($storage) {
259-
$cache = $storage->getCache('');
260-
261-
$relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/');
262-
$results = call_user_func_array(array($cache, $method), $args);
263-
foreach ($results as $result) {
264-
$result['internalPath'] = $result['path'];
265-
$result['path'] = $relativeMountPoint . $result['path'];
266-
$result['storage'] = $storage;
267-
$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount);
260+
if (!$limitToHome) {
261+
$mounts = $this->root->getMountsIn($this->path);
262+
foreach ($mounts as $mount) {
263+
$storage = $mount->getStorage();
264+
if ($storage) {
265+
$cache = $storage->getCache('');
266+
267+
$relativeMountPoint = ltrim(substr($mount->getMountPoint(), $rootLength), '/');
268+
$results = call_user_func_array([$cache, $method], $args);
269+
foreach ($results as $result) {
270+
$result['internalPath'] = $result['path'];
271+
$result['path'] = $relativeMountPoint . $result['path'];
272+
$result['storage'] = $storage;
273+
$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage,
274+
$result['internalPath'], $result, $mount);
275+
}
268276
}
269277
}
270278
}

‎lib/private/Files/Search/SearchQuery.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class SearchQuery implements ISearchQuery {
3939
private $order;
4040
/** @var IUser */
4141
private $user;
42+
private $limitToHome;
4243

4344
/**
4445
* SearchQuery constructor.
@@ -48,13 +49,22 @@ class SearchQuery implements ISearchQuery {
4849
* @param int $offset
4950
* @param array $order
5051
* @param IUser $user
52+
* @param bool $limitToHome
5153
*/
52-
public function __construct(ISearchOperator $searchOperation, $limit, $offset, array $order, IUser $user) {
54+
public function __construct(
55+
ISearchOperator $searchOperation,
56+
int $limit,
57+
int $offset,
58+
array $order,
59+
IUser $user,
60+
bool $limitToHome = false
61+
) {
5362
$this->searchOperation = $searchOperation;
5463
$this->limit = $limit;
5564
$this->offset = $offset;
5665
$this->order = $order;
5766
$this->user = $user;
67+
$this->limitToHome = $limitToHome;
5868
}
5969

6070
/**
@@ -91,4 +101,8 @@ public function getOrder() {
91101
public function getUser() {
92102
return $this->user;
93103
}
104+
105+
public function limitToHome(): bool {
106+
return $this->limitToHome;
107+
}
94108
}

‎lib/public/Files/Search/ISearchQuery.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,11 @@ public function getOrder();
6666
* @since 12.0.0
6767
*/
6868
public function getUser();
69+
70+
/**
71+
* Whether or not the search should be limited to the users home storage
72+
*
73+
* @return bool
74+
*/
75+
public function limitToHome(): bool;
6976
}

0 commit comments

Comments
 (0)