Skip to content

Commit 97db4b1

Browse files
susnuxsusnux
committed
fix(lookup-server): disable lookup server for non-global scale setups
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 7d3047f commit 97db4b1

File tree

11 files changed

+79
-62
lines changed

11 files changed

+79
-62
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,8 +1008,10 @@ public function isLookupServerQueriesEnabled() {
10081008
if ($this->gsConfig->isGlobalScaleEnabled()) {
10091009
return true;
10101010
}
1011-
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no');
1012-
return $result === 'yes';
1011+
$result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
1012+
// TODO: Reenable if lookup server is used again
1013+
// return $result;
1014+
return false;
10131015
}
10141016

10151017

@@ -1023,8 +1025,10 @@ public function isLookupServerUploadEnabled() {
10231025
if ($this->gsConfig->isGlobalScaleEnabled()) {
10241026
return false;
10251027
}
1026-
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no');
1027-
return $result === 'yes';
1028+
$result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes';
1029+
// TODO: Reenable if lookup server is used again
1030+
// return $result;
1031+
return false;
10281032
}
10291033

10301034
/**

‎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
</NcSettingsSection>
4753
</template>
4854

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -834,10 +834,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect
834834

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

@@ -861,10 +864,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte
861864

862865
public function dataTestIsLookupServerUploadEnabled() {
863866
return [
864-
[false, 'yes', true],
865-
[false, 'no', false],
866867
[true, 'yes', false],
867868
[true, 'no', false],
869+
// TODO: reenable if we use the lookup server again
870+
// [false, 'yes', true],
871+
// [false, 'no', false],
872+
[false, 'yes', false],
873+
[false, 'no', false],
868874
];
869875
}
870876

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use OCP\Collaboration\Collaborators\ISearchResult;
2121
use OCP\Collaboration\Collaborators\SearchResultType;
2222
use OCP\Constants;
23+
use OCP\GlobalScale\IConfig as GlobalScaleIConfig;
2324
use OCP\IConfig;
2425
use OCP\IRequest;
2526
use OCP\IURLGenerator;
@@ -181,12 +182,11 @@ public function search(string $search = '', ?string $itemType = null, int $page
181182
$this->limit = $perPage;
182183
$this->offset = $perPage * ($page - 1);
183184

184-
// In global scale mode we always search the loogup server
185-
$lookup = $this->config->getSystemValueBool('gs.enabled', false)
186-
|| $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
187-
$this->result['lookupEnabled'] = $lookup;
185+
// In global scale mode we always search the lookup server
186+
$this->result['lookupEnabled'] = Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();
187+
// TODO: Reconsider using lookup server for non-global-scale federation
188188

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

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

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

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

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,12 @@ public function start(IJobList $jobList): void {
9090
* - max retries are reached (set to 5)
9191
*/
9292
protected function shouldRemoveBackgroundJob(): bool {
93-
return $this->config->getSystemValueBool('has_internet_connection', true) === false ||
94-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' ||
95-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes' ||
96-
$this->retries >= 5;
93+
// TODO: Remove global scale condition once lookup server is used for non-global scale federation
94+
// return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes'
95+
return !$this->config->getSystemValueBool('gs.enabled', false)
96+
|| $this->config->getSystemValueBool('has_internet_connection', true) === false
97+
|| $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === ''
98+
|| $this->retries >= 5;
9799
}
98100

99101
protected function shouldRun(): bool {
@@ -155,7 +157,7 @@ protected function run($argument): void {
155157
$user->getUID(),
156158
'lookup_server_connector',
157159
'update_retries',
158-
$this->retries + 1
160+
(string)($this->retries + 1),
159161
);
160162
}
161163
}

‎apps/lookup_server_connector/lib/UpdateLookupServer.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ public function userUpdated(IUser $user): void {
6161
* @return bool
6262
*/
6363
private function shouldUpdateLookupServer(): bool {
64-
return $this->config->getSystemValueBool('has_internet_connection', true) === true &&
65-
$this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes' &&
66-
$this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
64+
// TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled"
65+
return $this->config->getSystemValueBool('gs.enabled', false)
66+
&& $this->config->getSystemValueBool('has_internet_connection', true)
67+
&& $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== '';
6768
}
6869
}

‎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', 'no') !== '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
@@ -1027,11 +1027,6 @@
10271027
<code><![CDATA[$storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')]]></code>
10281028
</RedundantCondition>
10291029
</file>
1030-
<file src="apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php">
1031-
<InvalidArgument>
1032-
<code><![CDATA[$this->retries + 1]]></code>
1033-
</InvalidArgument>
1034-
</file>
10351030
<file src="apps/oauth2/lib/Controller/OauthApiController.php">
10361031
<NoInterfaceProperties>
10371032
<code><![CDATA[$this->request->server]]></code>

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
3535
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
3636
$hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true);
3737

38-
// if case of Global Scale we always search the lookup server
39-
if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection)) {
38+
// If case of Global Scale we always search the lookup server
39+
// TODO: Reconsider using the lookup server for non-global scale
40+
// if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection || $disableLookupServer)) {
41+
if (!$isGlobalScaleEnabled) {
4042
return false;
4143
}
4244

0 commit comments

Comments
 (0)