mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #26550 from nextcloud/fix/noid/prefer-cloudshare-over-mail-take2 

hide mail share if a cloud share is possible – backend only solution
This commit is contained in:
Morris Jobke 2021-04-20 14:26:14 +02:00 committed by GitHub
parent 9d1e2c5082 92ca49d21d
commit 94322971a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 137 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
lib/private/Collaboration/Collaborators/Search.php
View File

@ -81,8 +81,8 @@ class Search implements ISearch {
// sanitizing, could go into the plugins as well
// if we have a exact match, either for the federated cloud id or for the
// email address we only return the exact match. It is highly unlikely
// if we have an exact match, either for the federated cloud id or for the
// email address, we only return the exact match. It is highly unlikely
// that the exact same email address and federated cloud id exists
$emailType = new SearchResultType('emails');
$remoteType = new SearchResultType('remotes');
@ -92,6 +92,8 @@ class Search implements ISearch {
$searchResult->unsetResult($emailType);
}
$this->dropMailSharesWhereRemoteShareIsPossible($searchResult);
// if we have an exact local user match with an email-a-like query,
// there is no need to show the remote and email matches.
$userType = new SearchResultType('users');
@ -110,4 +112,34 @@ class Search implements ISearch {
}
$this->pluginList[$shareType][] = $pluginInfo['class'];
}
protected function dropMailSharesWhereRemoteShareIsPossible(ISearchResult $searchResult): void {
$allResults = $searchResult->asArray();
$emailType = new SearchResultType('emails');
$remoteType = new SearchResultType('remotes');
if (!isset($allResults[$remoteType->getLabel()])
|| !isset($allResults[$emailType->getLabel()])) {
return;
}
$mailIdMap = [];
foreach ($allResults[$emailType->getLabel()] as $mailRow) {
// sure, array_reduce looks nicer, but foreach needs less resources and is faster
if (!isset($mailRow['uuid'])) {
continue;
}
$mailIdMap[$mailRow['uuid']] = $mailRow['value']['shareWith'];
}
foreach ($allResults[$remoteType->getLabel()] as $resultRow) {
if (!isset($resultRow['uuid'])) {
continue;
}
if (isset($mailIdMap[$resultRow['uuid']])) {
$searchResult->removeCollaboratorResult($emailType, $mailIdMap[$resultRow['uuid']]);
}
}
}
}

20
lib/private/Collaboration/Collaborators/SearchResult.php
View File

@ -85,4 +85,24 @@ class SearchResult implements ISearchResult {
$this->result['exact'][$type] = [];
}
}
public function removeCollaboratorResult(SearchResultType $type, string $collaboratorId): bool {
$type = $type->getLabel();
if (!isset($this->result[$type])) {
return false;
}
$actionDone = false;
$resultArrays = [&$this->result['exact'][$type], &$this->result[$type]];
foreach ($resultArrays as &$resultArray) {
foreach ($resultArray as $k => $result) {
if ($result['value']['shareWith'] === $collaboratorId) {
unset($resultArray[$k]);
$actionDone = true;
}
}
}
return $actionDone;
}
}

8
lib/public/Collaboration/Collaborators/ISearchResult.php
View File

@ -45,6 +45,14 @@ interface ISearchResult {
*/
public function hasResult(SearchResultType $type, $collaboratorId);
/**
* Removes all result where $collaborationId exactly matches shareWith of
* any of wide and exact result matches of the given type.
*
* @since 22.0.0
*/
public function removeCollaboratorResult(SearchResultType $type, string $collaboratorId): bool;
/**
* @param SearchResultType $type
* @since 13.0.0

97
tests/lib/Collaboration/Collaborators/SearchTest.php
View File

@ -48,27 +48,18 @@ class SearchTest extends TestCase {
/**
* @dataProvider dataSearchSharees
*
* @param string $searchTerm
* @param array $shareTypes
* @param int $page
* @param int $perPage
* @param array $mockedUserResult
* @param array $mockedGroupsResult
* @param array $mockedRemotesResult
* @param array $expected
* @param bool $expectedMoreResults
*/
public function testSearch(
$searchTerm,
string $searchTerm,
array $shareTypes,
$page,
$perPage,
int $page,
int $perPage,
array $mockedUserResult,
array $mockedGroupsResult,
array $mockedRemotesResult,
array $mockedMailResult,
array $expected,
$expectedMoreResults
bool $expectedMoreResults
) {
$searchResult = new SearchResult();
@ -104,9 +95,18 @@ class SearchTest extends TestCase {
return $expectedMoreResults;
});
$mailPlugin = $this->createMock(ISearchPlugin::class);
$mailPlugin->expects($this->any())
->method('search')
->willReturnCallback(function () use ($searchResult, $mockedMailResult, $expectedMoreResults) {
$type = new SearchResultType('emails');
$searchResult->addResultSet($type, $mockedMailResult);
return $expectedMoreResults;
});
$this->container->expects($this->any())
->method('resolve')
->willReturnCallback(function ($class) use ($searchResult, $userPlugin, $groupPlugin, $remotePlugin) {
->willReturnCallback(function ($class) use ($searchResult, $userPlugin, $groupPlugin, $remotePlugin, $mailPlugin) {
if ($class === SearchResult::class) {
return $searchResult;
} elseif ($class === $userPlugin) {
@ -115,6 +115,8 @@ class SearchTest extends TestCase {
return $groupPlugin;
} elseif ($class === $remotePlugin) {
return $remotePlugin;
} elseif ($class === $mailPlugin) {
return $mailPlugin;
}
return null;
});
@ -122,6 +124,7 @@ class SearchTest extends TestCase {
$this->search->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => $userPlugin]);
$this->search->registerPlugin(['shareType' => 'SHARE_TYPE_GROUP', 'class' => $groupPlugin]);
$this->search->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE', 'class' => $remotePlugin]);
$this->search->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => $mailPlugin]);
[$results, $moreResults] = $this->search->search($searchTerm, $shareTypes, false, $perPage, $perPage * ($page - 1));
@ -131,24 +134,31 @@ class SearchTest extends TestCase {
public function dataSearchSharees() {
return [
// #0
[
'test', [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE], 1, 2, [], [], ['results' => [], 'exact' => [], 'exactIdMatch' => false],
[],
[
'exact' => ['users' => [], 'groups' => [], 'remotes' => []],
'users' => [],
'groups' => [],
'remotes' => [],
], false
],
false
],
// #1
[
'test', [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE], 1, 2, [], [], ['results' => [], 'exact' => [], 'exactIdMatch' => false],
[],
[
'exact' => ['users' => [], 'groups' => [], 'remotes' => []],
'users' => [],
'groups' => [],
'remotes' => [],
], false
],
false
],
// #2
[
'test', [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE], 1, 2, [
['label' => 'test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
@ -157,6 +167,7 @@ class SearchTest extends TestCase {
], [
'results' => [['label' => 'testz@remote', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'testz@remote']]], 'exact' => [], 'exactIdMatch' => false,
],
[],
[
'exact' => ['users' => [], 'groups' => [], 'remotes' => []],
'users' => [
@ -170,13 +181,14 @@ class SearchTest extends TestCase {
],
], true,
],
// No groups requested
// #3 No groups requested
[
'test', [IShare::TYPE_USER, IShare::TYPE_REMOTE], 1, 2, [
['label' => 'test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
], [], [
'results' => [['label' => 'testz@remote', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'testz@remote']]], 'exact' => [], 'exactIdMatch' => false
],
[],
[
'exact' => ['users' => [], 'remotes' => []],
'users' => [
@ -187,11 +199,11 @@ class SearchTest extends TestCase {
],
], false,
],
// Share type restricted to user - Only one user
// #4 Share type restricted to user - Only one user
[
'test', [IShare::TYPE_USER], 1, 2, [
['label' => 'test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
], [], [],
], [], [], [],
[
'exact' => ['users' => []],
'users' => [
@ -199,12 +211,12 @@ class SearchTest extends TestCase {
],
], false,
],
// Share type restricted to user - Multipage result
// #5 Share type restricted to user - Multipage result
[
'test', [IShare::TYPE_USER], 1, 2, [
['label' => 'test 1', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
['label' => 'test 2', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test2']],
], [], [],
], [], [], [],
[
'exact' => ['users' => []],
'users' => [
@ -213,6 +225,47 @@ class SearchTest extends TestCase {
],
], true,
],
// #6 Mail shares filtered out in favor of remote shares
[
'test', // search term
[IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_REMOTE, IShare::TYPE_EMAIL], // plugins
1, // page
10, // per page
[ // user result
['label' => 'test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
],
[ // group result
['label' => 'testgroup1', 'value' => ['shareType' => IShare::TYPE_GROUP, 'shareWith' => 'testgroup1']],
],
[ // remote result
'results' => [
['label' => 'testz@remote.tld', 'uuid' => 'f3d78140-abcc-46df-b58d-c7cc1176aadf','value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'testz@remote.tld']]
],
'exact' => [],
'exactIdMatch' => false,
],
[ // mail result
['label' => 'test Two', 'uuid' => 'b2321e9e-31af-43ac-a406-583fb26d1964', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test2@remote.tld']],
['label' => 'test One', 'uuid' => 'f3d78140-abcc-46df-b58d-c7cc1176aadf', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'testz@remote.tld']],
],
[ // expected result
'exact' => ['users' => [], 'groups' => [], 'remotes' => [], 'emails' => []],
'users' => [
['label' => 'test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1']],
],
'groups' => [
['label' => 'testgroup1', 'value' => ['shareType' => IShare::TYPE_GROUP, 'shareWith' => 'testgroup1']],
],
'remotes' => [
['label' => 'testz@remote.tld', 'uuid' => 'f3d78140-abcc-46df-b58d-c7cc1176aadf', 'value' => ['shareType' => IShare::TYPE_REMOTE, 'shareWith' => 'testz@remote.tld']],
],
'emails' => [
// one passes, another is filtered out
['label' => 'test Two', 'uuid' => 'b2321e9e-31af-43ac-a406-583fb26d1964', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test2@remote.tld']]
]
],
false, // expected more results indicator
],
];
}
}