Skip to content

Commit 1d58ce8

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #51404 from nextcloud/backport/51378/stable31
[stable31] fix(lookup-server): disable when not using global scale
2 parents c4a17a6 + e60f6fe commit 1d58ce8

File tree

13 files changed

+89
-74
lines changed

13 files changed

+89
-74
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -998,8 +998,10 @@ public function isLookupServerQueriesEnabled(): bool {
998998
if ($this->gsConfig->isGlobalScaleEnabled()) {
999999
return true;
10001000
}
1001-
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes');
1002-
return $result === 'yes';
1001+
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
1002+
// TODO: Reenable if lookup server is used again
1003+
// return $result;
1004+
return false;
10031005
}
10041006

10051007

@@ -1011,8 +1013,10 @@ public function isLookupServerUploadEnabled(): bool {
10111013
if ($this->gsConfig->isGlobalScaleEnabled()) {
10121014
return false;
10131015
}
1014-
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes');
1015-
return $result === 'yes';
1016+
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
1017+
// TODO: Reenable if lookup server is used again
1018+
// return $result;
1019+
return false;
10161020
}
10171021

10181022
/**

‎apps/federatedfilesharing/src/components/AdminSettings.vue‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,23 @@
3232
{{ t('federatedfilesharing', 'Allow people on this server to receive group shares from other servers') }}
3333
</NcCheckboxRadioSwitch>
3434

35-
<NcCheckboxRadioSwitch type="switch"
36-
:checked.sync="lookupServerEnabled"
37-
@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
38-
{{ t('federatedfilesharing', 'Search global and public address book for people') }}
39-
</NcCheckboxRadioSwitch>
35+
<fieldset>
36+
<legend>{{ t('federatedfilesharing', 'The lookup server is only available for global scale.') }}</legend>
4037

41-
<NcCheckboxRadioSwitch type="switch"
42-
:checked.sync="lookupServerUploadEnabled"
43-
@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
44-
{{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }}
45-
</NcCheckboxRadioSwitch>
38+
<NcCheckboxRadioSwitch type="switch"
39+
:checked.sync="lookupServerEnabled"
40+
disabled
41+
@update:checked="update('lookupServerEnabled', lookupServerEnabled)">
42+
{{ t('federatedfilesharing', 'Search global and public address book for people') }}
43+
</NcCheckboxRadioSwitch>
44+
45+
<NcCheckboxRadioSwitch type="switch"
46+
:checked.sync="lookupServerUploadEnabled"
47+
disabled
48+
@update:checked="update('lookupServerUploadEnabled', lookupServerUploadEnabled)">
49+
{{ t('federatedfilesharing', 'Allow people to publish their data to a global and public address book') }}
50+
</NcCheckboxRadioSwitch>
51+
</fieldset>
4652

4753
<!-- Trusted server handling -->
4854
<div class="settings-subsection">

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
824824
$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
825825
->willReturn($gsEnabled);
826826
$this->config->expects($this->any())->method('getAppValue')
827-
->with('files_sharing', 'lookupServerEnabled', 'yes')
827+
->with('files_sharing', 'lookupServerEnabled', 'no')
828828
->willReturn($isEnabled);
829829

830830
$this->assertSame($expected,
@@ -835,10 +835,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
835835

836836
public function dataTestIsLookupServerQueriesEnabled() {
837837
return [
838-
[false, 'yes', true],
839-
[false, 'no', false],
840838
[true, 'yes', true],
841839
[true, 'no', true],
840+
// TODO: reenable if we use the lookup server for non-global scale
841+
// [false, 'yes', true],
842+
// [false, 'no', false],
843+
[false, 'no', false],
844+
[false, 'yes', false],
842845
];
843846
}
844847

@@ -852,7 +855,7 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
852855
$this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled')
853856
->willReturn($gsEnabled);
854857
$this->config->expects($this->any())->method('getAppValue')
855-
->with('files_sharing', 'lookupServerUploadEnabled', 'yes')
858+
->with('files_sharing', 'lookupServerUploadEnabled', 'no')
856859
->willReturn($isEnabled);
857860

858861
$this->assertSame($expected,
@@ -862,10 +865,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
862865

863866
public function dataTestIsLookupServerUploadEnabled() {
864867
return [
865-
[false, 'yes', true],
866-
[false, 'no', false],
867868
[true, 'yes', false],
868869
[true, 'no', false],
870+
// TODO: reenable if we use the lookup server again
871+
// [false, 'yes', true],
872+
// [false, 'no', false],
873+
[false, 'yes', false],
874+
[false, 'no', false],
869875
];
870876
}
871877

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCP\Collaboration\Collaborators\ISearchResult;
2222
use OCP\Collaboration\Collaborators\SearchResultType;
2323
use OCP\Constants;
24+
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
2425
use OCP\IConfig;
2526
use OCP\IRequest;
2627
use OCP\IURLGenerator;
@@ -173,15 +174,11 @@ public function search(string $search = '', ?string $itemType = null, int $page
173174
$this->limit = $perPage;
174175
$this->offset = $perPage * ($page - 1);
175176

176-
// In global scale mode we always search the loogup server
177-
if ($this->config->getSystemValueBool('gs.enabled', false)) {
178-
$lookup = true;
179-
$this->result['lookupEnabled'] = true;
180-
} else {
181-
$this->result['lookupEnabled'] = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes';
182-
}
177+
// In global scale mode we always search the lookup server
178+
$this->result['lookupEnabled'] = \OCP\Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
179+
// TODO: Reconsider using lookup server for non-global-scale federation
183180

184-
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $lookup, $this->limit, $this->offset);
181+
[$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset);
185182

186183
// extra treatment for 'exact' subarray, with a single merge expected keys might be lost
187184
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
@@ -11,6 +11,7 @@
1111
use OCP\AppFramework\Http\DataResponse;
1212
use OCP\AppFramework\OCS\OCSBadRequestException;
1313
use OCP\Collaboration\Collaborators\ISearch;
14+
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
1415
use OCP\IConfig;
1516
use OCP\IRequest;
1617
use OCP\IURLGenerator;
@@ -229,14 +230,14 @@ public function testSearch(
229230
$perPage = $getData['perPage'] ?? 200;
230231
$shareType = $getData['shareType'] ?? null;
231232

233+
$globalConfig = $this->createMock(GlobalScaleIConfig::class);
234+
$globalConfig->expects(self::once())
235+
->method('isGlobalScaleEnabled')
236+
->willReturn(true);
237+
$this->overwriteService(GlobalScaleIConfig::class, $globalConfig);
238+
232239
/** @var IConfig|MockObject $config */
233240
$config = $this->createMock(IConfig::class);
234-
$config->expects($this->exactly(1))
235-
->method('getAppValue')
236-
->with($this->anything(), $this->anything(), $this->anything())
237-
->willReturnMap([
238-
['files_sharing', 'lookupServerEnabled', 'yes', 'yes'],
239-
]);
240241

241242
$this->shareManager->expects($this->once())
242243
->method('allowGroupSharing')

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,12 @@ public function start(IJobList $jobList): void {
8484
* - max retries are reached (set to 5)
8585
*/
8686
protected function shouldRemoveBackgroundJob(): bool {
87-
return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
88-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
89-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
90-
$this->retries >= 5;
87+
// TODO: Remove global scale condition once lookup server is used for non-global scale federation
88+
// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
89+
return !$this->config->getSystemValueBool('gs.enabled', false)
90+
|| $this->config->getSystemValueBool('has_internet_connection', true) === false
91+
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
92+
|| $this->retries >= 5;
9193
}
9294

9395
protected function shouldRun(): bool {
@@ -149,7 +151,7 @@ protected function run($argument): void {
149151
$user->getUID(),
150152
'lookup_server_connector',
151153
'update_retries',
152-
$this->retries + 1
154+
(string)($this->retries + 1),
153155
);
154156
}
155157
}

‎apps/lookup_server_connector/lib/UpdateLookupServer.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ public function userUpdated(IUser $user): void {
5656
* @return bool
5757
*/
5858
private function shouldUpdateLookupServer(): bool {
59-
return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
60-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes' &&
61-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
59+
// TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
60+
return $this->config->getSystemValueBool('gs.enabled', false)
61+
&& $this->config->getSystemValueBool('has_internet_connection', true)
62+
&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
6263
}
6364
}

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

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

122122
protected function verifyViaLookupServer(array $argument, string $dataType): bool {
123-
if (empty($this->lookupServerUrl) ||
124-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' ||
125-
$this->config->getSystemValue('has_internet_connection', true) === false) {
123+
// TODO: Consider to enable for non-global-scale setups by checking 'files_sharing', 'lookupServerUploadEnabled'
124+
if (!$this->config->getSystemValueBool('gs.enabled', false)
125+
|| empty($this->lookupServerUrl)
126+
|| $this->config->getSystemValue('has_internet_connection', true) === false
127+
) {
126128
return true;
127129
}
128130

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -988,11 +988,6 @@
988988
<code><![CDATA[$storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')]]></code>
989989
</RedundantCondition>
990990
</file>
991-
<file src="apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php">
992-
<InvalidArgument>
993-
<code><![CDATA[$this->retries + 1]]></code>
994-
</InvalidArgument>
995-
</file>
996991
<file src="apps/oauth2/lib/Controller/OauthApiController.php">
997992
<NoInterfaceProperties>
998993
<code><![CDATA[$this->request->server]]></code>

0 commit comments

Comments
 (0)