Skip to content

Commit 43eccfb

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

19 files changed

Lines changed: 166 additions & 126 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())
@@ -365,15 +356,16 @@ public function testSearchInvalid($getData, $message): void {
365356
$urlGenerator,
366357
$this->shareManager,
367358
$this->collaboratorSearch,
368-
$this->federatedShareProvider
359+
$this->federatedShareProvider,
360+
$this->appManager,
369361
])
370362
->onlyMethods(['isRemoteSharingAllowed'])
371363
->getMock();
372364
$sharees->expects($this->never())
373365
->method('isRemoteSharingAllowed');
374366

375367
$this->collaboratorSearch->expects($this->never())
376-
->method('search');
368+
->method('filteredSearch');
377369

378370
try {
379371
$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 {

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3092,22 +3092,6 @@
30923092
<code><![CDATA[IToken::PERMANENT_TOKEN]]></code>
30933093
</DeprecatedClass>
30943094
</file>
3095-
<file src="core/Controller/AutoCompleteController.php">
3096-
<DeprecatedClass>
3097-
<code><![CDATA[new AutoCompleteEvent([
3098-
'search' => $search,
3099-
'results' => $results,
3100-
'itemType' => $itemType,
3101-
'itemId' => $itemId,
3102-
'sorter' => $sorter,
3103-
'shareTypes' => $shareTypes,
3104-
'limit' => $limit,
3105-
])]]></code>
3106-
</DeprecatedClass>
3107-
<DeprecatedMethod>
3108-
<code><![CDATA[dispatch]]></code>
3109-
</DeprecatedMethod>
3110-
</file>
31113095
<file src="core/Controller/ClientFlowLoginV2Controller.php">
31123096
<TypeDoesNotContainType>
31133097
<code><![CDATA[!is_string($stateToken)]]></code>

‎core/Controller/AutoCompleteController.php‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@
1414
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1515
use OCP\AppFramework\Http\DataResponse;
1616
use OCP\AppFramework\OCSController;
17-
use OCP\Collaboration\AutoComplete\AutoCompleteEvent;
18-
use OCP\Collaboration\AutoComplete\AutoCompleteFilterEvent;
1917
use OCP\Collaboration\AutoComplete\IManager;
2018
use OCP\Collaboration\Collaborators\ISearch;
21-
use OCP\EventDispatcher\IEventDispatcher;
2219
use OCP\IRequest;
2320
use OCP\Share\IShare;
2421

@@ -31,7 +28,6 @@ public function __construct(
3128
IRequest $request,
3229
private ISearch $collaboratorSearch,
3330
private IManager $autoCompleteManager,
34-
private IEventDispatcher $dispatcher,
3531
) {
3632
parent::__construct($appName, $request);
3733
}
@@ -55,31 +51,7 @@ public function __construct(
5551
public function get(string $search, ?string $itemType, ?string $itemId, ?string $sorter = null, array $shareTypes = [IShare::TYPE_USER], int $limit = 10): DataResponse {
5652
// if enumeration/user listings are disabled, we'll receive an empty
5753
// 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();
54+
[$results,] = $this->collaboratorSearch->filteredSearch($search, $shareTypes, false, $itemType, $itemId, $limit, 0);
8355

8456
$exactMatches = $results['exact'];
8557
unset($results['exact']);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function __construct(
4141
}
4242

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

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

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function __construct(
3333
}
3434

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function __construct(
3838

3939

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

0 commit comments

Comments
 (0)