Skip to content

Commit c2fca99

Browse files
blizzzblizzz
authored
Merge pull request #51424 from nextcloud/backport/51407/stable24
[stable24] fix(lookup-server): disable when not using global scale
2 parents bb73ec2 + 97e29cc commit c2fca99

File tree

9 files changed

+97
-93
lines changed

9 files changed

+97
-93
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,8 +1065,10 @@ public function isLookupServerQueriesEnabled() {
10651065
if ($this->gsConfig->isGlobalScaleEnabled()) {
10661066
return true;
10671067
}
1068-
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes');
1069-
return ($result === 'yes');
1068+
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
1069+
// TODO: Reenable if lookup server is used again
1070+
// return $result;
1071+
return false;
10701072
}
10711073

10721074

@@ -1080,8 +1082,10 @@ public function isLookupServerUploadEnabled() {
10801082
if ($this->gsConfig->isGlobalScaleEnabled()) {
10811083
return false;
10821084
}
1083-
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes');
1084-
return ($result === 'yes');
1085+
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
1086+
// TODO: Reenable if lookup server is used again
1087+
// return $result;
1088+
return false;
10851089
}
10861090

10871091
/**

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
845845
$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
846846
->willReturn($gsEnabled);
847847
$this->config->expects($this->any())->method('getAppValue')
848-
->with('files_sharing', 'lookupServerEnabled', 'yes')
848+
->with('files_sharing', 'lookupServerEnabled', 'no')
849849
->willReturn($isEnabled);
850850

851851
$this->assertSame($expected,
@@ -856,10 +856,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
856856

857857
public function dataTestIsLookupServerQueriesEnabled() {
858858
return [
859-
[false, 'yes', true],
860-
[false, 'no', false],
861859
[true, 'yes', true],
862860
[true, 'no', true],
861+
// TODO: reenable if we use the lookup server for non-global scale
862+
// [false, 'yes', true],
863+
// [false, 'no', false],
864+
[false, 'no', false],
865+
[false, 'yes', false],
863866
];
864867
}
865868

@@ -873,7 +876,7 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
873876
$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
874877
->willReturn($gsEnabled);
875878
$this->config->expects($this->any())->method('getAppValue')
876-
->with('files_sharing', 'lookupServerUploadEnabled', 'yes')
879+
->with('files_sharing', 'lookupServerUploadEnabled', 'no')
877880
->willReturn($isEnabled);
878881

879882
$this->assertSame($expected,
@@ -883,10 +886,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
883886

884887
public function dataTestIsLookupServerUploadEnabled() {
885888
return [
886-
[false, 'yes', true],
887-
[false, 'no', false],
888889
[true, 'yes', false],
889890
[true, 'no', false],
891+
// TODO: reenable if we use the lookup server again
892+
// [false, 'yes', true],
893+
// [false, 'no', false],
894+
[false, 'yes', false],
895+
[false, 'no', false],
890896
];
891897
}
892898

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
*/
3838
namespace OCA\Files_Sharing\Controller;
3939

40-
use OCP\Constants;
4140
use function array_slice;
4241
use function array_values;
4342
use Generator;
@@ -48,6 +47,8 @@
4847
use OCP\Collaboration\Collaborators\ISearch;
4948
use OCP\Collaboration\Collaborators\ISearchResult;
5049
use OCP\Collaboration\Collaborators\SearchResultType;
50+
use OCP\Constants;
51+
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
5152
use OCP\IConfig;
5253
use OCP\IRequest;
5354
use OCP\IURLGenerator;
@@ -217,15 +218,11 @@ public function search(string $search = '', string $itemType = null, int $page =
217218
$this->limit = $perPage;
218219
$this->offset = $perPage * ($page - 1);
219220

220-
// In global scale mode we always search the loogup server
221-
if ($this->config->getSystemValueBool('gs.enabled', false)) {
222-
$lookup = true;
223-
$this->result['lookupEnabled'] = true;
224-
} else {
225-
$this->result['lookupEnabled'] = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
226-
}
221+
// In global scale mode we always search the lookup server
222+
$this->result['lookupEnabled'] = \OC::$server->get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
223+
// TODO: Reconsider using lookup server for non-global-scale federation
227224

228-
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $lookup, $this->limit, $this->offset);
225+
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset);
229226

230227
// extra treatment for 'exact' subarray, with a single merge expected keys might be lost
231228
if (isset($result['exact'])) {

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OCP\AppFramework\Http;
3636
use OCP\AppFramework\OCS\OCSBadRequestException;
3737
use OCP\Collaboration\Collaborators\ISearch;
38+
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
3839
use OCP\IConfig;
3940
use OCP\IRequest;
4041
use OCP\IURLGenerator;
@@ -240,14 +241,14 @@ public function testSearch(array $getData, string $apiSetting, string $enumSetti
240241
$perPage = $getData['perPage'] ?? 200;
241242
$shareType = $getData['shareType'] ?? null;
242243

244+
$globalConfig = $this->createMock(GlobalScaleIConfig::class);
245+
$globalConfig->expects(self::once())
246+
->method('isGlobalScaleEnabled')
247+
->willReturn(true);
248+
$this->overwriteService(GlobalScaleIConfig::class, $globalConfig);
249+
243250
/** @var IConfig|MockObject $config */
244251
$config = $this->createMock(IConfig::class);
245-
$config->expects($this->exactly(1))
246-
->method('getAppValue')
247-
->with($this->anything(), $this->anything(), $this->anything())
248-
->willReturnMap([
249-
['files_sharing', 'lookupServerEnabled', 'yes', 'yes'],
250-
]);
251252

252253
$this->shareManager->expects($this->once())
253254
->method('allowGroupSharing')

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,12 @@ public function execute(IJobList $jobList, ILogger $logger = null): void {
128128
* @return bool
129129
*/
130130
protected function shouldRemoveBackgroundJob(): bool {
131-
return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
132-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
133-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
134-
$this->retries >= 5;
131+
// TODO: Remove global scale condition once lookup server is used for non-global scale federation
132+
// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
133+
return !$this->config->getSystemValueBool('gs.enabled', false)
134+
|| $this->config->getSystemValueBool('has_internet_connection', true) === false
135+
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
136+
|| $this->retries >= 5;
135137
}
136138

137139
protected function shouldRun(): bool {
@@ -193,7 +195,7 @@ protected function run($argument): void {
193195
$user->getUID(),
194196
'lookup_server_connector',
195197
'update_retries',
196-
$this->retries + 1
198+
(string)($this->retries + 1),
197199
);
198200
}
199201
}

‎apps/lookup_server_connector/lib/UpdateLookupServer.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,9 @@ public function userUpdated(IUser $user): void {
8282
* @return bool
8383
*/
8484
private function shouldUpdateLookupServer(): bool {
85-
return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
86-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes' &&
87-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
85+
// TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
86+
return $this->config->getSystemValueBool('gs.enabled', false)
87+
&& $this->config->getSystemValueBool('has_internet_connection', true)
88+
&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
8889
}
8990
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,11 @@ protected function verifyWebsite(array $argument) {
169169
}
170170

171171
protected function verifyViaLookupServer(array $argument, string $dataType): bool {
172-
if (empty($this->lookupServerUrl) ||
173-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
174-
$this->config->getSystemValue('has_internet_connection', true) === false) {
172+
// TODO: Consider to enable for non-global-scale setups by checking 'files_sharing', 'lookupServerUploadEnabled'
173+
if (!$this->config->getSystemValueBool('gs.enabled', false)
174+
|| empty($this->lookupServerUrl)
175+
|| $this->config->getSystemValue('has_internet_connection', true) === false
176+
) {
175177
return true;
176178
}
177179

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,14 @@ public function __construct(IConfig $config,
6464
}
6565

6666
public function search($search, $limit, $offset, ISearchResult $searchResult) {
67-
$isGlobalScaleEnabled = $this->config->getSystemValue('gs.enabled', false);
68-
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
67+
$isGlobalScaleEnabled = $this->config->getSystemValueBool('gs.enabled', false);
68+
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
6969
$hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true);
7070

71-
// if case of Global Scale we always search the lookup server
72-
if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection)) {
71+
// If case of Global Scale we always search the lookup server
72+
// TODO: Reconsider using the lookup server for non-global scale
73+
// if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection || $disableLookupServer)) {
74+
if (!$isGlobalScaleEnabled) {
7375
return false;
7476
}
7577

0 commit comments

Comments
 (0)