Skip to content

Commit 7335eaf

Browse files
CarlSchwanCarlSchwan
committed
refactor: Use GlobalScale/IConfig everywhere
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
1 parent 58dcd3d commit 7335eaf

6 files changed

Lines changed: 66 additions & 52 deletions

File tree

‎apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCP\AppFramework\Utility\ITimeFactory;
1515
use OCP\BackgroundJob\IJobList;
1616
use OCP\BackgroundJob\Job;
17+
use OCP\GlobalScale\IConfig as GlobalScaleConfig;
1718
use OCP\Http\Client\IClientService;
1819
use OCP\IConfig;
1920
use OCP\IUser;
@@ -39,6 +40,7 @@ public function __construct(
3940
private IUserManager $userManager,
4041
private IAccountManager $accountManager,
4142
private Signer $signer,
43+
private GlobalScaleConfig $globalScaleConfig,
4244
) {
4345
parent::__construct($time);
4446

@@ -86,7 +88,7 @@ public function start(IJobList $jobList): void {
8688
protected function shouldRemoveBackgroundJob(): bool {
8789
// TODO: Remove global scale condition once lookup server is used for non-global scale federation
8890
// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
89-
return !$this->config->getSystemValueBool('gs.enabled', false)
91+
return !$this->globalScaleConfig->isGlobalScaleEnabled()
9092
|| $this->config->getSystemValueBool('has_internet_connection', true) === false
9193
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
9294
|| $this->retries >= 5;

‎apps/lookup_server_connector/lib/UpdateLookupServer.php‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
use OCA\LookupServerConnector\BackgroundJobs\RetryJob;
1313
use OCP\BackgroundJob\IJobList;
14+
use OCP\Config\IUserConfig;
15+
use OCP\GlobalScale\IConfig as GlobalScaleConfig;
1416
use OCP\IConfig;
1517
use OCP\IUser;
1618

@@ -20,26 +22,21 @@
2022
* @package OCA\LookupServerConnector
2123
*/
2224
class UpdateLookupServer {
23-
/**
24-
* @param IJobList $jobList
25-
* @param IConfig $config
26-
*/
2725
public function __construct(
28-
private IJobList $jobList,
29-
private IConfig $config,
26+
private readonly IJobList $jobList,
27+
private readonly IConfig $config,
28+
private readonly IUserConfig $userConfig,
29+
private readonly GlobalScaleConfig $globalScaleConfig,
3030
) {
3131
}
3232

33-
/**
34-
* @param IUser $user
35-
*/
3633
public function userUpdated(IUser $user): void {
3734
if (!$this->shouldUpdateLookupServer()) {
3835
return;
3936
}
4037

4138
// Reset retry counter
42-
$this->config->deleteUserValue(
39+
$this->userConfig->deleteUserConfig(
4340
$user->getUID(),
4441
'lookup_server_connector',
4542
'update_retries'
@@ -48,17 +45,15 @@ public function userUpdated(IUser $user): void {
4845
}
4946

5047
/**
51-
* check if we should update the lookup server, we only do it if
48+
* Check if we should update the lookup server, we only do it if
5249
*
5350
* + we have an internet connection
5451
* + the lookup server update was not disabled by the admin
5552
* + we have a valid lookup server URL
56-
*
57-
* @return bool
5853
*/
5954
private function shouldUpdateLookupServer(): bool {
6055
// TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
61-
return $this->config->getSystemValueBool('gs.enabled', false)
56+
return $this->globalScaleConfig->isGlobalScaleEnabled()
6257
&& $this->config->getSystemValueBool('has_internet_connection', true)
6358
&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
6459
}

‎apps/settings/lib/BackgroundJobs/VerifyUserData.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCP\AppFramework\Utility\ITimeFactory;
1616
use OCP\BackgroundJob\IJobList;
1717
use OCP\BackgroundJob\Job;
18+
use OCP\GlobalScale\IConfig as GlobalScaleConfig;
1819
use OCP\Http\Client\IClientService;
1920
use OCP\IConfig;
2021
use OCP\IUserManager;
@@ -38,6 +39,7 @@ public function __construct(
3839
private LoggerInterface $logger,
3940
ITimeFactory $timeFactory,
4041
private IConfig $config,
42+
private GlobalScaleConfig $globalScaleConfig,
4143
) {
4244
parent::__construct($timeFactory);
4345

@@ -124,7 +126,7 @@ protected function verifyWebsite(array $argument) {
124126

125127
protected function verifyViaLookupServer(array $argument, string $dataType): bool {
126128
// TODO: Consider to enable for non-global-scale setups by checking 'files_sharing', 'lookupServerUploadEnabled'
127-
if (!$this->config->getSystemValueBool('gs.enabled', false)
129+
if (!$this->globalScaleConfig->isGlobalScaleEnabled()
128130
|| empty($this->lookupServerUrl)
129131
|| $this->config->getSystemValue('has_internet_connection', true) === false
130132
) {

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,11 +2209,6 @@
22092209
<code><![CDATA[setUserValue]]></code>
22102210
</DeprecatedMethod>
22112211
</file>
2212-
<file src="apps/lookup_server_connector/lib/UpdateLookupServer.php">
2213-
<DeprecatedMethod>
2214-
<code><![CDATA[deleteUserValue]]></code>
2215-
</DeprecatedMethod>
2216-
</file>
22172212
<file src="apps/oauth2/lib/Controller/LoginRedirectorController.php">
22182213
<DeprecatedMethod>
22192214
<code><![CDATA[generate]]></code>

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCP\Collaboration\Collaborators\ISearchResult;
1313
use OCP\Collaboration\Collaborators\SearchResultType;
1414
use OCP\Federation\ICloudIdManager;
15+
use OCP\GlobalScale\IConfig as GlobalScaleConfig;
1516
use OCP\Http\Client\IClientService;
1617
use OCP\IConfig;
1718
use OCP\IUserSession;
@@ -23,20 +24,21 @@ class LookupPlugin implements ISearchPlugin {
2324
private string $currentUserRemote;
2425

2526
public function __construct(
26-
private IConfig $config,
27-
private IClientService $clientService,
27+
private readonly IConfig $config,
28+
private readonly IClientService $clientService,
2829
IUserSession $userSession,
29-
private ICloudIdManager $cloudIdManager,
30-
private LoggerInterface $logger,
31-
private ?TrustedServers $trustedServers,
30+
private readonly ICloudIdManager $cloudIdManager,
31+
private readonly LoggerInterface $logger,
32+
private readonly ?TrustedServers $trustedServers,
33+
private readonly GlobalScaleConfig $globalScaleConfig,
3234
) {
3335
$currentUserCloudId = $userSession->getUser()->getCloudId();
3436
$this->currentUserRemote = $cloudIdManager->resolveCloudId($currentUserCloudId)->getRemote();
3537
}
3638

3739
#[\Override]
3840
public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
39-
$isGlobalScaleEnabled = $this->config->getSystemValueBool('gs.enabled', false);
41+
$isGlobalScaleEnabled = $this->globalScaleConfig->isGlobalScaleEnabled();
4042
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
4143
$hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true);
4244

‎tests/lib/Collaboration/Collaborators/LookupPluginTest.php‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-License-Identifier: AGPL-3.0-or-later
@@ -13,6 +15,7 @@
1315
use OCP\Collaboration\Collaborators\SearchResultType;
1416
use OCP\Federation\ICloudId;
1517
use OCP\Federation\ICloudIdManager;
18+
use OCP\GlobalScale\IConfig as GlobalScaleConfig;
1619
use OCP\Http\Client\IClient;
1720
use OCP\Http\Client\IClientService;
1821
use OCP\Http\Client\IResponse;
@@ -25,18 +28,13 @@
2528
use Test\TestCase;
2629

2730
class LookupPluginTest extends TestCase {
28-
/** @var IConfig|MockObject */
29-
protected $config;
30-
/** @var IClientService|MockObject */
31-
protected $clientService;
32-
/** @var IUserSession|MockObject */
33-
protected $userSession;
34-
/** @var ICloudIdManager|MockObject */
35-
protected $cloudIdManager;
36-
/** @var LookupPlugin */
37-
protected $plugin;
38-
/** @var LoggerInterface|MockObject */
39-
protected $logger;
31+
protected IConfig&MockObject $config;
32+
protected GlobalScaleConfig&MockObject $globalScaleConfig;
33+
protected IClientService&MockObject $clientService;
34+
protected IUserSession&MockObject $userSession;
35+
protected ICloudIdManager&MockObject $cloudIdManager;
36+
protected LookupPlugin $plugin;
37+
protected LoggerInterface&MockObject $logger;
4038

4139
#[\Override]
4240
protected function setUp(): void {
@@ -45,6 +43,7 @@ protected function setUp(): void {
4543
$this->userSession = $this->createMock(IUserSession::class);
4644
$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
4745
$this->config = $this->createMock(IConfig::class);
46+
$this->globalScaleConfig = $this->createMock(GlobalScaleConfig::class);
4847
$this->logger = $this->createMock(LoggerInterface::class);
4948
$this->clientService = $this->createMock(IClientService::class);
5049
$cloudId = $this->createMock(ICloudId::class);
@@ -71,7 +70,8 @@ protected function setUp(): void {
7170
$this->userSession,
7271
$this->cloudIdManager,
7372
$this->logger,
74-
null
73+
null,
74+
$this->globalScaleConfig,
7575
);
7676
}
7777

@@ -80,13 +80,16 @@ public function testSearchNoLookupServerURI(): void {
8080
->method('getAppValue')
8181
->with('files_sharing', 'lookupServerEnabled', 'no')
8282
->willReturn('yes');
83-
$this->config->expects($this->exactly(2))
83+
$this->config->expects($this->once())
8484
->method('getSystemValueBool')
8585
->willReturnMap([
86-
['gs.enabled', false, true],
8786
['has_internet_connection', true, true],
8887
]);
8988

89+
$this->globalScaleConfig->expects($this->once())
90+
->method('isGlobalScaleEnabled')
91+
->willReturn(true);
92+
9093
$this->config->expects($this->once())
9194
->method('getSystemValueString')
9295
->with('lookup_server', 'https://lookup.nextcloud.com')
@@ -106,13 +109,16 @@ public function testSearchNoInternet(): void {
106109
->method('getAppValue')
107110
->with('files_sharing', 'lookupServerEnabled', 'no')
108111
->willReturn('yes');
109-
$this->config->expects($this->exactly(2))
112+
$this->config->expects($this->exactly(1))
110113
->method('getSystemValueBool')
111114
->willReturnMap([
112-
['gs.enabled', false, false],
113115
['has_internet_connection', true, false],
114116
]);
115117

118+
$this->globalScaleConfig->expects($this->exactly(1))
119+
->method('isGlobalScaleEnabled')
120+
->willReturn(false);
121+
116122
$this->clientService->expects($this->never())
117123
->method('newClient');
118124

@@ -139,13 +145,16 @@ public function testSearch(array $searchParams): void {
139145
->method('getAppValue')
140146
->with('files_sharing', 'lookupServerEnabled', 'no')
141147
->willReturn('yes');
142-
$this->config->expects($this->exactly(2))
148+
$this->config->expects($this->once())
143149
->method('getSystemValueBool')
144150
->willReturnMap([
145-
['gs.enabled', false, true],
146151
['has_internet_connection', true, true],
147152
]);
148153

154+
$this->globalScaleConfig->expects($this->once())
155+
->method('isGlobalScaleEnabled')
156+
->willReturn(true);
157+
149158
$this->config->expects($this->once())
150159
->method('getSystemValueString')
151160
->with('lookup_server', 'https://lookup.nextcloud.com')
@@ -200,12 +209,15 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab
200209
->method('addResultSet')
201210
->with($type, $searchParams['expectedResult'], []);
202211

203-
$this->config->expects($this->exactly(2))
212+
$this->config->expects($this->once())
204213
->method('getSystemValueBool')
205214
->willReturnMap([
206-
['gs.enabled', false, $GSEnabled],
207215
['has_internet_connection', true, true],
208216
]);
217+
218+
$this->globalScaleConfig->expects($this->once())
219+
->method('isGlobalScaleEnabled')
220+
->willReturn($GSEnabled);
209221
$this->config->expects($this->once())
210222
->method('getSystemValueString')
211223
->with('lookup_server', 'https://lookup.nextcloud.com')
@@ -230,12 +242,15 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab
230242
->willReturn($client);
231243
} else {
232244
$searchResult->expects($this->never())->method('addResultSet');
233-
$this->config->expects($this->exactly(2))
245+
$this->config->expects($this->once())
234246
->method('getSystemValueBool')
235247
->willReturnMap([
236-
['gs.enabled', false, $GSEnabled],
237248
['has_internet_connection', true, true],
238249
]);
250+
251+
$this->globalScaleConfig->expects($this->once())
252+
->method('isGlobalScaleEnabled')
253+
->willReturn($GSEnabled);
239254
}
240255
$moreResults = $this->plugin->search(
241256
$searchParams['search'],
@@ -248,13 +263,16 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab
248263
}
249264

250265
public function testSearchGSDisabled(): void {
251-
$this->config->expects($this->atLeastOnce())
266+
$this->config->expects($this->once())
252267
->method('getSystemValueBool')
253268
->willReturnMap([
254269
['has_internet_connection', true, true],
255-
['gs.enabled', false, false],
256270
]);
257271

272+
$this->globalScaleConfig->expects($this->once())
273+
->method('isGlobalScaleEnabled')
274+
->willReturn(false);
275+
258276
/** @var ISearchResult|MockObject $searchResult */
259277
$searchResult = $this->createMock(ISearchResult::class);
260278
$searchResult->expects($this->never())

0 commit comments

Comments
 (0)