Skip to content

Commit 6840f54

Browse files
CarlSchwanCarlSchwan
committed
fix: Use AutoCompleteFilterEvent when returning list of sharees
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
1 parent 37e6ee9 commit 6840f54

18 files changed

Lines changed: 164 additions & 107 deletions

File tree

‎apps/files_sharing/lib/Controller/ShareesAPIController.php‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ public function __construct(
7474
protected IManager $shareManager,
7575
protected ISearch $collaboratorSearch,
7676
protected FederatedShareProvider $federatedShareProvider,
77+
protected IAppManager $appManager,
7778
) {
7879
parent::__construct($appName, $request);
7980
}
@@ -161,7 +162,7 @@ public function search(string $search = '', ?string $itemType = null, int $page
161162
}
162163

163164
// FIXME: DI
164-
if (Server::get(IAppManager::class)->isEnabledForUser('circles') && class_exists('\OCA\Circles\ShareByCircleProvider')) {
165+
if ($this->appManager->isEnabledForUser('circles') && class_exists('\OCA\Circles\ShareByCircleProvider')) {
165166
$shareTypes[] = IShare::TYPE_CIRCLE;
166167
}
167168

@@ -183,25 +184,26 @@ public function search(string $search = '', ?string $itemType = null, int $page
183184
$this->result['lookupEnabled'] = Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
184185
// TODO: Reconsider using lookup server for non-global-scale federation
185186

186-
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset);
187+
[$result, $hasMoreResults] = $this->collaboratorSearch->filteredSearch($search, $shareTypes, $this->result['lookupEnabled'], $itemType, null, $this->limit, $this->offset);
187188

188189
// extra treatment for 'exact' subarray, with a single merge expected keys might be lost
189190
if (isset($result['exact'])) {
190191
$result['exact'] = array_merge($this->result['exact'], $result['exact']);
191192
}
192-
$this->result = array_merge($this->result, $result);
193-
$response = new DataResponse($this->result);
193+
/** @var Files_SharingShareesSearchResult $result */
194+
$result = array_merge($this->result, $result);
194195

196+
$headers = [];
195197
if ($hasMoreResults) {
196-
$response->setHeaders(['Link' => $this->getPaginationLink($page, [
198+
$headers['Link'] = $this->getPaginationLink($page, [
197199
'search' => $search,
198200
'itemType' => $itemType,
199201
'shareType' => $shareTypes,
200202
'perPage' => $perPage,
201-
])]);
203+
]);
202204
}
203205

204-
return $response;
206+
return new DataResponse($result, Http::STATUS_OK, $headers);
205207
}
206208

207209
/**

‎apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCA\FederatedFileSharing\FederatedShareProvider;
1111
use OCA\Files_Sharing\Controller\ShareesAPIController;
1212
use OCA\Files_Sharing\Tests\TestCase;
13+
use OCP\App\IAppManager;
1314
use OCP\AppFramework\Http\DataResponse;
1415
use OCP\AppFramework\OCS\OCSBadRequestException;
1516
use OCP\Collaboration\Collaborators\ISearch;
@@ -29,26 +30,14 @@
2930
*/
3031
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
3132
class ShareesAPIControllerTest extends TestCase {
32-
/** @var ShareesAPIController */
33-
protected $sharees;
34-
35-
/** @var string */
36-
protected $uid;
37-
38-
/** @var IRequest|MockObject */
39-
protected $request;
40-
41-
/** @var IManager|MockObject */
33+
protected ShareesAPIController $sharees;
34+
protected string $uid;
35+
protected IRequest&MockObject $request;
4236
protected $shareManager;
43-
44-
/** @var ISearch|MockObject */
45-
protected $collaboratorSearch;
46-
47-
/** @var IConfig|MockObject */
48-
protected $config;
49-
50-
/** @var FederatedShareProvider|MockObject */
51-
protected $federatedShareProvider;
37+
protected ISearch&MockObject $collaboratorSearch;
38+
protected IConfig&MockObject $config;
39+
protected FederatedShareProvider&MockObject $federatedShareProvider;
40+
protected IAppManager&MockObject $appManager;
5241

5342
protected function setUp(): void {
5443
parent::setUp();
@@ -58,11 +47,11 @@ protected function setUp(): void {
5847
$this->shareManager = $this->createMock(IManager::class);
5948
$this->config = $this->createMock(IConfig::class);
6049

61-
/** @var IURLGenerator|MockObject $urlGeneratorMock */
6250
$urlGeneratorMock = $this->createMock(IURLGenerator::class);
6351

6452
$this->collaboratorSearch = $this->createMock(ISearch::class);
6553
$this->federatedShareProvider = $this->createMock(FederatedShareProvider::class);
54+
$this->appManager = $this->createMock(IAppManager::class);
6655

6756
$this->sharees = new ShareesAPIController(
6857
'files_sharing',
@@ -72,7 +61,8 @@ protected function setUp(): void {
7261
$urlGeneratorMock,
7362
$this->shareManager,
7463
$this->collaboratorSearch,
75-
$this->federatedShareProvider
64+
$this->federatedShareProvider,
65+
$this->appManager,
7666
);
7767
}
7868

@@ -267,7 +257,8 @@ public function testSearch(
267257
$urlGenerator,
268258
$this->shareManager,
269259
$this->collaboratorSearch,
270-
$this->federatedShareProvider
260+
$this->federatedShareProvider,
261+
$this->appManager,
271262
])
272263
->onlyMethods(['isRemoteSharingAllowed', 'isRemoteGroupSharingAllowed'])
273264
->getMock();
@@ -276,8 +267,8 @@ public function testSearch(
276267
sort($expectedShareTypes);
277268

278269
$this->collaboratorSearch->expects($this->once())
279-
->method('search')
280-
->with($search, $expectedShareTypes, $this->anything(), $perPage, $perPage * ($page - 1))
270+
->method('filteredSearch')
271+
->with($search, $expectedShareTypes, $this->anything(), $itemType, null, $perPage, $perPage * ($page - 1))
281272
->willReturn([[], false]);
282273

283274
$sharees->expects($this->any())
@@ -367,15 +358,16 @@ public function testSearchInvalid($getData, $message): void {
367358
$urlGenerator,
368359
$this->shareManager,
369360
$this->collaboratorSearch,
370-
$this->federatedShareProvider
361+
$this->federatedShareProvider,
362+
$this->appManager,
371363
])
372364
->onlyMethods(['isRemoteSharingAllowed'])
373365
->getMock();
374366
$sharees->expects($this->never())
375367
->method('isRemoteSharingAllowed');
376368

377369
$this->collaboratorSearch->expects($this->never())
378-
->method('search');
370+
->method('filteredSearch');
379371

380372
try {
381373
$sharees->search('', null, $page, $perPage, null);

‎apps/user_status/lib/Controller/StatusesController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public function findAll(?int $limit = null, ?int $offset = null): DataResponse {
6565

6666
if ($users !== $event->getUsers()) {
6767
$removedUsers = $event->getFilteredOutUsers();
68-
$allStatuses = array_filter($allStatuses, fn (UserStatus $userStatus): bool => in_array($userStatus->getUserId(), $removedUsers, true));
68+
$allStatuses = array_filter($allStatuses, fn (UserStatus $userStatus): bool => !in_array($userStatus->getUserId(), $removedUsers, true));
6969
}
7070

7171
return new DataResponse(array_values(array_map(function ($userStatus) {

‎apps/user_status/tests/Unit/Controller/StatusesControllerTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,24 @@
1313
use OCA\UserStatus\Service\StatusService;
1414
use OCP\AppFramework\Db\DoesNotExistException;
1515
use OCP\AppFramework\OCS\OCSNotFoundException;
16+
use OCP\EventDispatcher\IEventDispatcher;
1617
use OCP\IRequest;
1718
use PHPUnit\Framework\MockObject\MockObject;
1819
use Test\TestCase;
1920

2021
class StatusesControllerTest extends TestCase {
2122
private StatusService&MockObject $service;
23+
private IEventDispatcher&MockObject $eventDispatcher;
2224
private StatusesController $controller;
2325

2426
protected function setUp(): void {
2527
parent::setUp();
2628

2729
$request = $this->createMock(IRequest::class);
2830
$this->service = $this->createMock(StatusService::class);
31+
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
2932

30-
$this->controller = new StatusesController('user_status', $request, $this->service);
33+
$this->controller = new StatusesController('user_status', $request, $this->service, $this->eventDispatcher);
3134
}
3235

3336
public function testFindAll(): void {

‎core/Controller/AutoCompleteController.php‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@
1414
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1515
use OCP\AppFramework\Http\DataResponse;
1616
use OCP\AppFramework\OCSController;
17-
use OCP\Collaboration\AutoComplete\AutoCompleteEvent;
1817
use OCP\Collaboration\AutoComplete\AutoCompleteFilterEvent;
1918
use OCP\Collaboration\AutoComplete\IManager;
2019
use OCP\Collaboration\Collaborators\ISearch;
21-
use OCP\EventDispatcher\IEventDispatcher;
2220
use OCP\IRequest;
2321
use OCP\Share\IShare;
2422

@@ -31,7 +29,6 @@ public function __construct(
3129
IRequest $request,
3230
private ISearch $collaboratorSearch,
3331
private IManager $autoCompleteManager,
34-
private IEventDispatcher $dispatcher,
3532
) {
3633
parent::__construct($appName, $request);
3734
}
@@ -55,31 +52,7 @@ public function __construct(
5552
public function get(string $search, ?string $itemType, ?string $itemId, ?string $sorter = null, array $shareTypes = [IShare::TYPE_USER], int $limit = 10): DataResponse {
5653
// if enumeration/user listings are disabled, we'll receive an empty
5754
// result from search() – thus nothing else to do here.
58-
[$results,] = $this->collaboratorSearch->search($search, $shareTypes, false, $limit, 0);
59-
60-
$event = new AutoCompleteEvent([
61-
'search' => $search,
62-
'results' => $results,
63-
'itemType' => $itemType,
64-
'itemId' => $itemId,
65-
'sorter' => $sorter,
66-
'shareTypes' => $shareTypes,
67-
'limit' => $limit,
68-
]);
69-
$this->dispatcher->dispatch(IManager::class . '::filterResults', $event);
70-
$results = $event->getResults();
71-
72-
$event = new AutoCompleteFilterEvent(
73-
$results,
74-
$search,
75-
$itemType,
76-
$itemId,
77-
$sorter,
78-
$shareTypes,
79-
$limit,
80-
);
81-
$this->dispatcher->dispatchTyped($event);
82-
$results = $event->getResults();
55+
[$results,] = $this->collaboratorSearch->filteredSearch($search, $shareTypes, false, $itemType, $itemId, $limit, 0);
8356

8457
$exactMatches = $results['exact'];
8558
unset($results['exact']);

‎lib/private/Collaboration/Collaborators/GroupPlugin.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ public function __construct(
4040
}
4141
}
4242

43-
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
43+
#[\Override]
44+
public function search(string $search, int $limit, int $offset, ISearchResult $searchResult): bool {
4445
if ($this->groupSharingDisabled) {
4546
return false;
4647
}

‎lib/private/Collaboration/Collaborators/MailPlugin.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,8 @@ public function __construct(
5959
}
6060
}
6161

62-
/**
63-
* {@inheritdoc}
64-
*/
65-
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
62+
#[\Override]
63+
public function search(string $search, int $limit, int $offset, ISearchResult $searchResult): bool {
6664
if ($this->shareeEnumerationFullMatch && !$this->shareeEnumerationFullMatchEmail) {
6765
return false;
6866
}

‎lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ public function __construct(
3232
}
3333
}
3434

35-
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
35+
#[\Override]
36+
public function search(string $search, int $limit, int $offset, ISearchResult $searchResult): bool {
3637
$result = ['wide' => [], 'exact' => []];
3738
$resultType = new SearchResultType('remote_groups');
3839

‎lib/private/Collaboration/Collaborators/RemotePlugin.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ public function __construct(
3737
}
3838

3939

40-
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
40+
#[\Override]
41+
public function search(string $search, int $limit, int $offset, ISearchResult $searchResult): bool {
4142
$result = ['wide' => [], 'exact' => []];
4243
$resultType = new SearchResultType('remotes');
4344

‎lib/private/Collaboration/Collaborators/Search.php‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,34 @@
66
*/
77
namespace OC\Collaboration\Collaborators;
88

9+
use OCP\Collaboration\AutoComplete\AutoCompleteFilterEvent;
10+
use OCP\Collaboration\AutoComplete\IManager;
911
use OCP\Collaboration\Collaborators\ISearch;
1012
use OCP\Collaboration\Collaborators\ISearchPlugin;
1113
use OCP\Collaboration\Collaborators\ISearchResult;
1214
use OCP\Collaboration\Collaborators\SearchResultType;
15+
use OCP\EventDispatcher\IEventDispatcher;
1316
use OCP\IContainer;
1417
use OCP\Share\IShare;
1518

1619
class Search implements ISearch {
20+
/** @var array<IShare::TYPE_*, list<class-string<ISearchPlugin>>> $pluginList */
1721
protected array $pluginList = [];
1822

1923
public function __construct(
20-
private IContainer $container,
24+
private readonly IContainer $container,
25+
private readonly IEventDispatcher $eventDispatcher,
2126
) {
2227
}
2328

24-
/**
25-
* @param string $search
26-
* @param bool $lookup
27-
* @param int|null $limit
28-
* @param int|null $offset
29-
* @throws \OCP\AppFramework\QueryException
30-
*/
31-
public function search($search, array $shareTypes, $lookup, $limit, $offset): array {
29+
#[\Override]
30+
public function search(string $search, array $shareTypes, bool $lookup, int $limit, int $offset): array {
31+
[$results, $more] = $this->filteredSearch($search, $shareTypes, $lookup, null, null, $limit, $offset, false);
32+
return [$results, $more];
33+
}
34+
35+
#[\Override]
36+
public function filteredSearch(string $search, array $shareTypes, bool $lookup, ?string $itemType, ?string $itemId, int $limit, int $offset, bool $sendFilterEvent = true): array {
3237
$hasMoreResults = false;
3338

3439
// Trim leading and trailing whitespace characters, e.g. when query is copy-pasted
@@ -43,7 +48,7 @@ public function search($search, array $shareTypes, $lookup, $limit, $offset): ar
4348
}
4449
foreach ($this->pluginList[$type] as $plugin) {
4550
/** @var ISearchPlugin $searchPlugin */
46-
$searchPlugin = $this->container->resolve($plugin);
51+
$searchPlugin = $this->container->get($plugin);
4752
if ($searchPlugin instanceof UserPlugin && $lookup) {
4853
// we are in GlobalScale, we ignore local accounts and prefer the result from lookup
4954
continue;
@@ -54,7 +59,7 @@ public function search($search, array $shareTypes, $lookup, $limit, $offset): ar
5459

5560
// Get from lookup server, not a separate share type
5661
if ($lookup) {
57-
$searchPlugin = $this->container->resolve(LookupPlugin::class);
62+
$searchPlugin = $this->container->get(LookupPlugin::class);
5863
$hasMoreResults = $searchPlugin->search($search, $limit, $offset, $searchResult) || $hasMoreResults;
5964
}
6065

@@ -81,11 +86,43 @@ public function search($search, array $shareTypes, $lookup, $limit, $offset): ar
8186
$searchResult->unsetResult($emailType);
8287
}
8388

84-
return [$searchResult->asArray(), $hasMoreResults];
89+
$results = $searchResult->asArray();
90+
if ($sendFilterEvent) {
91+
$event = new AutoCompleteEvent([
92+
'search' => $search,
93+
'results' => $results,
94+
'itemType' => $itemType,
95+
'itemId' => $itemId,
96+
'sorter' => $sorter,
97+
'shareTypes' => $shareTypes,
98+
'limit' => $limit,
99+
]);
100+
$this->eventDispatcher->dispatch(IManager::class . '::filterResults', $event);
101+
$results = $event->getResults();
102+
103+
$event = new AutoCompleteFilterEvent(
104+
$results,
105+
$search,
106+
$itemType,
107+
$itemId,
108+
null,
109+
$shareTypes,
110+
$limit,
111+
);
112+
$this->eventDispatcher->dispatchTyped($event);
113+
$results = $event->getResults();
114+
}
115+
116+
return [$results, $hasMoreResults];
85117
}
86118

87119
public function registerPlugin(array $pluginInfo): void {
88-
$shareType = constant(IShare::class . '::' . substr($pluginInfo['shareType'], strlen('SHARE_')));
120+
/** @psalm-suppress InvalidScalarArgument For legacy reasons */
121+
if (str_starts_with($pluginInfo['shareType'], 'SHARE_')) {
122+
$shareType = constant(IShare::class . '::' . substr($pluginInfo['shareType'], strlen('SHARE_')));
123+
} else {
124+
$shareType = $pluginInfo['shareType'];
125+
}
89126
if ($shareType === null) {
90127
throw new \InvalidArgumentException('Provided ShareType is invalid');
91128
}

0 commit comments

Comments
 (0)