From f8d1ee5cfac5405713cc3fdc194cb98d748ec79f Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 13 Apr 2021 22:49:42 +0200 Subject: [PATCH] ignore mail shares of related remote share results Signed-off-by: Arthur Schiwon --- .../Collaboration/Collaborators/Search.php | 36 ++++++- .../Collaborators/SearchResult.php | 20 ++++ .../Collaborators/ISearchResult.php | 8 ++ .../Collaborators/SearchTest.php | 97 ++++++++++++++----- 4 files changed, 137 insertions(+), 24 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/Search.php b/lib/private/Collaboration/Collaborators/Search.php index 79dfd5e1d82..87463572944 100644 --- a/lib/private/Collaboration/Collaborators/Search.php +++ b/lib/private/Collaboration/Collaborators/Search.php @@ -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']]); + } + } + } } diff --git a/lib/private/Collaboration/Collaborators/SearchResult.php b/lib/private/Collaboration/Collaborators/SearchResult.php index 8e2c5a1ff39..e9511f869fa 100644 --- a/lib/private/Collaboration/Collaborators/SearchResult.php +++ b/lib/private/Collaboration/Collaborators/SearchResult.php @@ -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; + } } diff --git a/lib/public/Collaboration/Collaborators/ISearchResult.php b/lib/public/Collaboration/Collaborators/ISearchResult.php index d0d61ccfb28..a559892bb47 100644 --- a/lib/public/Collaboration/Collaborators/ISearchResult.php +++ b/lib/public/Collaboration/Collaborators/ISearchResult.php @@ -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 diff --git a/tests/lib/Collaboration/Collaborators/SearchTest.php b/tests/lib/Collaboration/Collaborators/SearchTest.php index 17f5c39aca7..ae93d2786a2 100644 --- a/tests/lib/Collaboration/Collaborators/SearchTest.php +++ b/tests/lib/Collaboration/Collaborators/SearchTest.php @@ -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 + ], ]; } }