Skip to content

Search API based on webDAV report#31873

Merged
PVince81 merged 1 commit into
masterfrom
search_api
Jun 28, 2018
Merged

Search API based on webDAV report#31873
PVince81 merged 1 commit into
masterfrom
search_api

Conversation

@jvillafanez

Copy link
Copy Markdown
Member

Description

Add support to search files through webDAV

Related Issue

#25373

Motivation and Context

Allow search file using the REPORT webDAV method. The request must be made to the base of the user's directory for files, usually "/remote.php/webdav/" or "/remote.php/dav/files/myuser/". Trying to search in inner directories will return an NotImplemented exception.

The minimum xml body request is:

<?xml version="1.0" encoding="utf-8" ?>
<oc:search-files xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns" >
        <oc:search>
                <oc:pattern>search-pattern</oc:pattern>
        </oc:search>
</oc:search-files>

The base xml body request must be:

<?xml version="1.0" encoding="utf-8" ?>
<oc:search-files xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns" >
        <a:prop>
                <oc:fileid/>
                <oc:permissions/>
                <a:getlastmodified/>
                <a:getetag/>
                <a:getcontenttype/>
                <oc:size/>
                <oc:owner-id/>
                <oc:owner-display-name/>
                <oc:size/>
        </a:prop>
        <oc:search>
                <oc:pattern>gif</oc:pattern>
                <oc:limit>5</oc:limit>
        </oc:search>
</oc:search-files>

The list of properties can be adjusted to get what you need as any PROPFIND request.
The base search limit is hardcoded to 30 results if no explicit limit is set in the request.

How Has This Been Tested?

Manually tested

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jvillafanez jvillafanez added this to the development milestone Jun 22, 2018
@jvillafanez jvillafanez self-assigned this Jun 22, 2018

@DeepDiver1975 DeepDiver1975 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good over app - please add tests- THX

$this->server->on('report', [$this, 'onReport']);
}

/**

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this slipped by... I'll fix this during the squash


/**
* @param string $filesUri the base uri for this user's files directory, usually /files/username
* @param \OC\Search\Result\File[] $searchResults the results coming from the search service, within the files app

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add use statement - 80 chars per line please

$limit = $searchInfo['limit'];
}

$searchResults = $this->searchService->searchPaged($searchInfo['pattern'], ['files'], 1, $limit);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 1 page one ? strange that it doesn't start at zero...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the phpdocs in the function, it starts at 1. The default value is also 1. 🤷‍♂️

*/
private function getSearchResultIterator($filesUri, $searchResults, $requestedProps) {
$paths = \array_map(function ($searchResult) use ($filesUri) {
return $filesUri . $searchResult->path;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sad that the search result doesn't contain node ids as it would spare us a step

but for the sake of "keep it simple" and not touching too many things, this is ok

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for a new API for searches. Something that needs to be planned in any case.

@codecov

codecov Bot commented Jun 27, 2018

Copy link
Copy Markdown

Codecov Report

Merging #31873 into master will increase coverage by 0.01%.
The diff coverage is 70%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #31873      +/-   ##
============================================
+ Coverage     63.44%   63.45%   +0.01%     
- Complexity    18502    18523      +21     
============================================
  Files          1165     1167       +2     
  Lines         69410    69499      +89     
  Branches       1264     1264              
============================================
+ Hits          44039    44104      +65     
- Misses        25002    25026      +24     
  Partials        369      369
Flag Coverage Δ Complexity Δ
#javascript 52.58% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 64.7% <70%> (+0.01%) 18523 <21> (+21) ⬆️
Impacted Files Coverage Δ Complexity Δ
apps/dav/lib/Files/Xml/SearchRequest.php 0% <0%> (ø) 8 <8> (?)
apps/dav/lib/Server.php 50.34% <0%> (-1.08%) 24 <0> (ø)
apps/dav/lib/Connector/Sabre/ServerFactory.php 94.36% <100%> (+0.24%) 11 <0> (ø) ⬇️
...av/lib/Connector/Sabre/FilesSearchReportPlugin.php 96.77% <96.77%> (ø) 13 <13> (?)
lib/private/Search/Result/File.php 0% <0%> (ø) 3% <0%> (ø) ⬇️
lib/private/Server.php 85.26% <0%> (+0.23%) 253% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a49adcc...7deb54c. Read the comment docs.

@jvillafanez

Copy link
Copy Markdown
Member Author

squashed (with minor fixes)

@PVince81 PVince81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@PVince81
PVince81 merged commit 1e1718c into master Jun 28, 2018
@PVince81
PVince81 deleted the search_api branch June 28, 2018 09:07
@PVince81

Copy link
Copy Markdown
Contributor

@jvillafanez please backport

@phil-davis

Copy link
Copy Markdown
Contributor

Backport PR #31946 has been merged.

settermjd added a commit to owncloud/docs that referenced this pull request Nov 13, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 13, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 14, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 14, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 15, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 15, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 16, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
settermjd added a commit to owncloud/docs that referenced this pull request Nov 19, 2018
This relates to owncloud/core#31873 and
documents the search-files report of ownCloud's Search API.
@lock lock Bot locked as resolved and limited conversation to collaborators Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants