Skip to content

Commit

Permalink
Allow getting the share list filtered by share type via API
Browse files Browse the repository at this point in the history
  • Loading branch information
@jvillafanez @phil-davis
jvillafanez authored and phil-davis committed Oct 20, 2020
1 parent 0d2e3d0 commit f748fcd
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 23 deletions.
7 changes: 7 additions & 0 deletions apps/files_sharing/js/sharedfilelist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@
if (this._sharedWithUser) {
requestData.shared_with_me = true;
requestData.state = 'all';
requestData.share_types = [
OC.Share.SHARE_TYPE_USER,
OC.Share.SHARE_TYPE_GROUP,
OC.Share.SHARE_TYPE_REMOTE
].join(',');
} else if (this._linksOnly) {
requestData.share_types = [OC.Share.SHARE_TYPE_LINK].join(',');
}
requestData.include_tags = true;
var shares = $.ajax({
Expand Down
54 changes: 46 additions & 8 deletions apps/files_sharing/lib/Controller/Share20OcsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,10 @@ private function getSharesInDir($folder) {

/**
* The getShares function.
* For the share type filter, if it isn't provided or is an empty string,
* all the share types will be returned, otherwise just the requested ones.
* Invalid share types will be ignored. If only invalid share types are requested,
* the function will return an empty list.
*
* @NoCSRFRequired
* @NoAdminRequired
Expand All @@ -640,6 +644,7 @@ private function getSharesInDir($folder) {
* - Get shares with the current user (?shared_with_me=true)
* - Get shares for a specific path (?path=...)
* - Get all shares in a folder (?subfiles=true&path=..)
* - Filter by share type (?share_types=0,1,3,6)
*
* @return Result
* @throws LockedException
Expand All @@ -655,6 +660,31 @@ public function getShares() {
$path = $this->request->getParam('path', null);

$includeTags = $this->request->getParam('include_tags', false);
$shareTypes = $this->request->getParam('share_types', '');
if ($shareTypes === '') {
$shareTypes = [
Share::SHARE_TYPE_USER,
Share::SHARE_TYPE_GROUP,
Share::SHARE_TYPE_LINK,
Share::SHARE_TYPE_REMOTE,
];
} else {
$shareTypes = \explode(',', $shareTypes);
}

$requestedShareTypes = [
Share::SHARE_TYPE_USER => false,
Share::SHARE_TYPE_GROUP => false,
Share::SHARE_TYPE_LINK => false,
Share::SHARE_TYPE_REMOTE => false,
];
foreach ($shareTypes as $shareType) {
if (isset($requestedShareTypes[$shareType])) {
$requestedShareTypes[$shareType] = true;
}
}
// requestedShareTypes now contains as keys the share type that has been requested
// (with "true" value), without duplicate elements, and only valid share types

if ($path !== null) {
$userFolder = $this->rootFolder->getUserFolder($this->userSession->getUser()->getUID());
Expand Down Expand Up @@ -698,15 +728,23 @@ public function getShares() {
$reshares = false;
}

// Get all shares
$userShares = $this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), Share::SHARE_TYPE_USER, $path, $reshares, -1, 0);
$groupShares = $this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), Share::SHARE_TYPE_GROUP, $path, $reshares, -1, 0);
$linkShares = $this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), Share::SHARE_TYPE_LINK, $path, $reshares, -1, 0);
$shares = \array_merge($userShares, $groupShares, $linkShares);
$shares = [];
foreach ($requestedShareTypes as $shareType => $requested) {
if (!$requested) {
continue;
}

if ($this->shareManager->outgoingServer2ServerSharesAllowed()) {
$federatedShares = $this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), Share::SHARE_TYPE_REMOTE, $path, $reshares, -1, 0);
$shares = \array_merge($shares, $federatedShares);
if ($shareType !== Share::SHARE_TYPE_REMOTE) {
$shares = \array_merge(
$shares,
$this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), $shareType, $path, $reshares, -1, 0)
);
} elseif ($shareType === Share::SHARE_TYPE_REMOTE && $this->shareManager->outgoingServer2ServerSharesAllowed()) {
$shares = \array_merge(
$shares,
$this->shareManager->getSharesBy($this->userSession->getUser()->getUID(), $shareType, $path, $reshares, -1, 0)
);
}
}

$formatted = [];
Expand Down
69 changes: 59 additions & 10 deletions apps/files_sharing/tests/Controller/Share20OcsControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2850,27 +2850,58 @@ public function providesGetSharesAll() {
[
null,
false,
false,
[],
],
[
'/requested/path',
true,
false,
[],
],
[
'/requested/path',
false,
false,
[],
],
[
'/requested/path',
true,
true,
[],
],
[
null,
false,
false,
[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_REMOTE],
],
[
'/requested/path',
true,
true
false,
[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_REMOTE],
],
[
'/requested/path',
false,
false,
[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_REMOTE],
],
[
'/requested/path',
true,
true,
[\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_REMOTE],
],
];
}

/**
* @dataProvider providesGetSharesAll
*/
public function testGetSharesAll($requestedPath, $requestedReshares, $fedAllowed = false) {
public function testGetSharesAll($requestedPath, $requestedReshares, $fedAllowed, $shareTypes) {
$userShare = $this->newShare();
$groupShare = $this->newShare();
$linkShare = $this->newShare();
Expand Down Expand Up @@ -2910,6 +2941,7 @@ public function testGetSharesAll($requestedPath, $requestedReshares, $fedAllowed
->will($this->returnValueMap([
['path', null, $requestedPath],
['reshares', null, $requestedReshares ? 'true' : 'false'],
['share_types', '', \implode(',', $shareTypes)],
]));

$this->shareManager->method('outgoingServer2ServerSharesAllowed')->willReturn($fedAllowed);
Expand All @@ -2918,16 +2950,33 @@ public function testGetSharesAll($requestedPath, $requestedReshares, $fedAllowed
$ocs->expects($this->any())->method('formatShare')->will($this->returnArgument(0));
$result = $ocs->getShares();

if ($fedAllowed) {
$this->assertCount(4, $result->getData());
$this->assertContains($federatedShare, $result->getData(), 'result contains federated share');
} else {
$this->assertCount(3, $result->getData());
if (\count($shareTypes) === 0) {
$shareTypes = [
\OCP\Share::SHARE_TYPE_USER,
\OCP\Share::SHARE_TYPE_GROUP,
\OCP\Share::SHARE_TYPE_LINK,
\OCP\Share::SHARE_TYPE_REMOTE,
];
}

$this->assertContains($userShare, $result->getData(), 'result contains user share');
$this->assertContains($groupShare, $result->getData(), 'result contains group share');
$this->assertContains($linkShare, $result->getData(), 'result contains link share');
foreach ($shareTypes as $shareType) {
switch ($shareType) {
case \OCP\Share::SHARE_TYPE_USER:
$this->assertContains($userShare, $result->getData(), 'result contains user share');
break;
case \OCP\Share::SHARE_TYPE_GROUP:
$this->assertContains($groupShare, $result->getData(), 'result contains group share');
break;
case \OCP\Share::SHARE_TYPE_LINK:
$this->assertContains($linkShare, $result->getData(), 'result contains link share');
break;
case \OCP\Share::SHARE_TYPE_REMOTE:
if ($fedAllowed) {
$this->assertContains($federatedShare, $result->getData(), 'result contains federated share');
}
break;
}
}
}

public function providesGetSharesSharedWithMe() {
Expand Down
10 changes: 5 additions & 5 deletions apps/files_sharing/tests/js/sharedfilelistSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe('OCA.Sharing.FileList tests', function() {
expect(fakeServer.requests.length).toEqual(2);
expect(fakeServer.requests[0].url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
'shares?format=json&shared_with_me=true&state=all&include_tags=true'
'shares?format=json&shared_with_me=true&state=all&share_types=0%2C1%2C6&include_tags=true'
);

expect(fakeServer.requests[1].url).toEqual(
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('OCA.Sharing.FileList tests', function() {
expect(fakeServer.requests.length).toEqual(2);
expect(fakeServer.requests[0].url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
'shares?format=json&shared_with_me=true&state=all&include_tags=true'
'shares?format=json&shared_with_me=true&state=all&share_types=0%2C1%2C6&include_tags=true'
);
expect(fakeServer.requests[1].url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
Expand Down Expand Up @@ -308,7 +308,7 @@ describe('OCA.Sharing.FileList tests', function() {
expect(fakeServer.requests.length).toEqual(2);
expect(fakeServer.requests[0].url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
'shares?format=json&shared_with_me=true&state=all&include_tags=true'
'shares?format=json&shared_with_me=true&state=all&share_types=0%2C1%2C6&include_tags=true'
);
expect(fakeServer.requests[1].url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
Expand Down Expand Up @@ -679,7 +679,7 @@ describe('OCA.Sharing.FileList tests', function() {
request = fakeServer.requests[0];
expect(request.url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
'shares?format=json&include_tags=true'
'shares?format=json&share_types=3&include_tags=true'
);

fakeServer.requests[0].respond(
Expand Down Expand Up @@ -732,7 +732,7 @@ describe('OCA.Sharing.FileList tests', function() {
request = fakeServer.requests[0];
expect(request.url).toEqual(
OC.linkToOCS('apps/files_sharing/api/v1') +
'shares?format=json&include_tags=true'
'shares?format=json&share_types=3&include_tags=true'
);

fakeServer.requests[0].respond(
Expand Down
11 changes: 11 additions & 0 deletions changelog/unreleased/38000
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Enhancement: Allow getting the share list filtered by share type via API

Previously, the share API returned all the shares. There were some filters,
but you weren't able to filter by share type. You couldn't get only your
link shares.

Now the API allows filtering by share type, along with the filters previously
available. The web UI is using this filtering now.

https://github.com/owncloud/core/pull/38000

0 comments on commit f748fcd

Please sign in to comment.