Skip to content

Commit 5cad74f

Browse files
authored
Merge pull request #51423 from nextcloud/backport/51407/stable25
[stable25] fix(lookup-server): disable when not using global scale
2 parents a9c3b72 + 93234d5 commit 5cad74f

12 files changed

Lines changed: 119 additions & 117 deletions

File tree

apps/federatedfilesharing/lib/FederatedShareProvider.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,8 +1070,10 @@ public function isLookupServerQueriesEnabled() {
10701070
if ($this->gsConfig->isGlobalScaleEnabled()) {
10711071
return true;
10721072
}
1073-
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes');
1074-
return ($result === 'yes');
1073+
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
1074+
// TODO: Reenable if lookup server is used again
1075+
// return $result;
1076+
return false;
10751077
}
10761078

10771079

@@ -1085,8 +1087,10 @@ public function isLookupServerUploadEnabled() {
10851087
if ($this->gsConfig->isGlobalScaleEnabled()) {
10861088
return false;
10871089
}
1088-
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes');
1089-
return ($result === 'yes');
1090+
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
1091+
// TODO: Reenable if lookup server is used again
1092+
// return $result;
1093+
return false;
10901094
}
10911095

10921096
/**

apps/federatedfilesharing/src/components/AdminSettings.vue

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,22 @@
5050
{{ t('federatedfilesharing', 'Allow users on this server to receive group shares from other servers') }}
5151
</NcCheckboxRadioSwitch>
5252

53-
<NcCheckboxRadioSwitch type="switch"
54-
:checked.sync="lookupServerEnabled"
55-
@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
56-
{{ t('federatedfilesharing', 'Search global and public address book for users') }}
57-
</NcCheckboxRadioSwitch>
53+
<fieldset>
54+
<legend>{{ t('federatedfilesharing', 'The lookup server is only available for global scale.') }}</legend>
55+
<NcCheckboxRadioSwitch type="switch"
56+
:checked.sync="lookupServerEnabled"
57+
disabled
58+
@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
59+
{{ t('federatedfilesharing', 'Search global and public address book for users') }}
60+
</NcCheckboxRadioSwitch>
5861

59-
<NcCheckboxRadioSwitch type="switch"
60-
:checked.sync="lookupServerUploadEnabled"
61-
@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
62-
{{ t('federatedfilesharing', 'Allow users to publish their data to a global and public address book') }}
63-
</NcCheckboxRadioSwitch>
62+
<NcCheckboxRadioSwitch type="switch"
63+
:checked.sync="lookupServerUploadEnabled"
64+
disabled
65+
@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
66+
{{ t('federatedfilesharing', 'Allow users to publish their data to a global and public address book') }}
67+
</NcCheckboxRadioSwitch>
68+
</fieldset>
6469
</NcSettingsSection>
6570
</template>
6671

apps/federatedfilesharing/tests/FederatedShareProviderTest.php

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

855855
$this->assertSame($expected,
@@ -860,10 +860,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
860860

861861
public function dataTestIsLookupServerQueriesEnabled() {
862862
return [
863-
[false, 'yes', true],
864-
[false, 'no', false],
865863
[true, 'yes', true],
866864
[true, 'no', true],
865+
// TODO: reenable if we use the lookup server for non-global scale
866+
// [false, 'yes', true],
867+
// [false, 'no', false],
868+
[false, 'no', false],
869+
[false, 'yes', false],
867870
];
868871
}
869872

@@ -877,7 +880,7 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
877880
$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
878881
->willReturn($gsEnabled);
879882
$this->config->expects($this->any())->method('getAppValue')
880-
->with('files_sharing', 'lookupServerUploadEnabled', 'yes')
883+
->with('files_sharing', 'lookupServerUploadEnabled', 'no')
881884
->willReturn($isEnabled);
882885

883886
$this->assertSame($expected,
@@ -887,10 +890,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
887890

888891
public function dataTestIsLookupServerUploadEnabled() {
889892
return [
890-
[false, 'yes', true],
891-
[false, 'no', false],
892893
[true, 'yes', false],
893894
[true, 'no', false],
895+
// TODO: reenable if we use the lookup server again
896+
// [false, 'yes', true],
897+
// [false, 'no', false],
898+
[false, 'yes', false],
899+
[false, 'no', false],
894900
];
895901
}
896902

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;
@@ -225,15 +226,11 @@ public function search(string $search = '', string $itemType = null, int $page =
225226
$this->limit = $perPage;
226227
$this->offset = $perPage * ($page - 1);
227228

228-
// In global scale mode we always search the loogup server
229-
if ($this->config->getSystemValueBool('gs.enabled', false)) {
230-
$lookup = true;
231-
$this->result['lookupEnabled'] = true;
232-
} else {
233-
$this->result['lookupEnabled'] = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
234-
}
229+
// In global scale mode we always search the lookup server
230+
$this->result['lookupEnabled'] = \OCP\Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
231+
// TODO: Reconsider using lookup server for non-global-scale federation
235232

236-
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $lookup, $this->limit, $this->offset);
233+
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset);
237234

238235
// extra treatment for 'exact' subarray, with a single merge expected keys might be lost
239236
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
@@ -115,10 +115,12 @@ public function start(IJobList $jobList): void {
115115
* - max retries are reached (set to 5)
116116
*/
117117
protected function shouldRemoveBackgroundJob(): bool {
118-
return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
119-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
120-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
121-
$this->retries >= 5;
118+
// TODO: Remove global scale condition once lookup server is used for non-global scale federation
119+
// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
120+
return !$this->config->getSystemValueBool('gs.enabled', false)
121+
|| $this->config->getSystemValueBool('has_internet_connection', true) === false
122+
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
123+
|| $this->retries >= 5;
122124
}
123125

124126
protected function shouldRun(): bool {
@@ -180,7 +182,7 @@ protected function run($argument): void {
180182
$user->getUID(),
181183
'lookup_server_connector',
182184
'update_retries',
183-
$this->retries + 1
185+
(string)($this->retries + 1),
184186
);
185187
}
186188
}

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

dist/federatedfilesharing-vue-settings-admin.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/federatedfilesharing-vue-settings-admin.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)