From 21bc4b3cad588391a03375567c216eee403d694c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 15 Jun 2020 09:29:30 +0200 Subject: [PATCH] Avoid duplicate matches in wide and exact results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../Collaborators/UserPlugin.php | 56 ++++++++----------- .../Collaborators/UserPluginTest.php | 41 ++++++++++---- 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 4809585dca..7c02e3c79a 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -71,7 +71,6 @@ class UserPlugin implements ISearchPlugin { public function search($search, $limit, $offset, ISearchResult $searchResult) { $result = ['wide' => [], 'exact' => []]; $users = []; - $autoCompleteUsers = []; $hasMoreResults = false; $userGroups = []; @@ -91,28 +90,6 @@ class UserPlugin implements ISearchPlugin { foreach ($usersTmp as $user) { if ($user->isEnabled()) { // Don't keep deactivated users $users[$user->getUID()] = $user; - - $addToWideResults = false; - if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) { - $addToWideResults = true; - } - - if ($this->shareeEnumerationInGroupOnly) { - $commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user)); - if (!empty($commonGroups)) { - $addToWideResults = true; - } - } - - if ($addToWideResults) { - $autoCompleteUsers[] = [ - 'label' => $user->getDisplayName(), - 'value' => [ - 'shareType' => IShare::TYPE_USER, - 'shareWith' => (string)$user->getUID(), - ], - ]; - } } } } @@ -145,13 +122,27 @@ class UserPlugin implements ISearchPlugin { ], ]; } else { - $result['wide'][] = [ - 'label' => $userDisplayName, - 'value' => [ - 'shareType' => Share::SHARE_TYPE_USER, - 'shareWith' => $uid, - ], - ]; + $addToWideResults = false; + if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) { + $addToWideResults = true; + } + + if ($this->shareeEnumerationInGroupOnly) { + $commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user)); + if (!empty($commonGroups)) { + $addToWideResults = true; + } + } + + if ($addToWideResults) { + $result['wide'][] = [ + 'label' => $userDisplayName, + 'value' => [ + 'shareType' => IShare::TYPE_USER, + 'shareWith' => $uid, + ], + ]; + } } } @@ -183,10 +174,7 @@ class UserPlugin implements ISearchPlugin { } } - // overwrite wide matches if they are limited - if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly) { - $result['wide'] = $autoCompleteUsers; - } + $type = new SearchResultType('users'); $searchResult->addResultSet($type, $result['wide'], $result['exact']); diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index 07bcce9352..0256c1eb15 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -522,7 +522,16 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => ['groupA']], ['uid' => 'test2', 'groups' => ['groupB']], ], - ['test1'], + ['exact' => [], 'wide' => ['test1']], + ], + [ + 'test1', + ['groupA'], + [ + ['uid' => 'test1', 'groups' => ['groupA']], + ['uid' => 'test2', 'groups' => ['groupB']], + ], + ['exact' => ['test1'], 'wide' => []], ], [ 'test', @@ -531,7 +540,7 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => ['groupA']], ['uid' => 'test2', 'groups' => ['groupB', 'groupA']], ], - ['test1', 'test2'], + ['exact' => [], 'wide' => ['test1', 'test2']], ], [ 'test', @@ -540,7 +549,7 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => ['groupA', 'groupC']], ['uid' => 'test2', 'groups' => ['groupB', 'groupA']], ], - ['test1', 'test2'], + ['exact' => [], 'wide' => ['test1', 'test2']], ], [ 'test', @@ -549,7 +558,7 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => ['groupA', 'groupC']], ['uid' => 'test2', 'groups' => ['groupB', 'groupA']], ], - ['test1', 'test2'], + ['exact' => [], 'wide' => ['test1', 'test2']], ], [ 'test', @@ -558,7 +567,7 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => ['groupA']], ['uid' => 'test2', 'groups' => ['groupB', 'groupA']], ], - [], + ['exact' => [], 'wide' => []], ], [ 'test', @@ -567,7 +576,16 @@ class UserPluginTest extends TestCase { ['uid' => 'test1', 'groups' => []], ['uid' => 'test2', 'groups' => []], ], - [], + ['exact' => [], 'wide' => []], + ], + [ + 'test', + ['groupC', 'groupB'], + [ + ['uid' => 'test1', 'groups' => []], + ['uid' => 'test2', 'groups' => []], + ], + ['exact' => [], 'wide' => []], ], ]; } @@ -582,9 +600,12 @@ class UserPluginTest extends TestCase { return $this->getUserMock($user['uid'], $user['uid']); }, $matchingUsers); - $mappedResult = array_map(function ($user) { + $mappedResultExact = array_map(function ($user) { return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user]]; - }, $result); + }, $result['exact']); + $mappedResultWide = array_map(function ($user) { + return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user]]; + }, $result['wide']); $this->userManager->expects($this->once()) ->method('searchDisplayName') @@ -615,7 +636,7 @@ class UserPluginTest extends TestCase { $this->plugin->search($search, $this->limit, $this->offset, $this->searchResult); $result = $this->searchResult->asArray(); - $this->assertEquals([], $result['exact']['users']); - $this->assertEquals($mappedResult, $result['users']); + $this->assertEquals($mappedResultExact, $result['exact']['users']); + $this->assertEquals($mappedResultWide, $result['users']); } }