diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index d5fa99ef2319f..e59f23b4532e6 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -1065,8 +1065,10 @@ public function isLookupServerQueriesEnabled() { if ($this->gsConfig->isGlobalScaleEnabled()) { return true; } - $result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes'); - return ($result === 'yes'); + $result = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; + // TODO: Reenable if lookup server is used again + // return $result; + return false; } @@ -1080,8 +1082,10 @@ public function isLookupServerUploadEnabled() { if ($this->gsConfig->isGlobalScaleEnabled()) { return false; } - $result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes'); - return ($result === 'yes'); + $result = $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') === 'yes'; + // TODO: Reenable if lookup server is used again + // return $result; + return false; } /** diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index a3253323769de..bdc87be3a16fb 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -844,7 +844,7 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect $this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled') ->willReturn($gsEnabled); $this->config->expects($this->any())->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') + ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn($isEnabled); $this->assertSame($expected, @@ -855,10 +855,13 @@ public function testIsLookupServerQueriesEnabled($gsEnabled, $isEnabled, $expect public function dataTestIsLookupServerQueriesEnabled() { return [ - [false, 'yes', true], - [false, 'no', false], [true, 'yes', true], [true, 'no', true], + // TODO: reenable if we use the lookup server for non-global scale + // [false, 'yes', true], + // [false, 'no', false], + [false, 'no', false], + [false, 'yes', false], ]; } @@ -872,7 +875,7 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte $this->gsConfig->expects($this->once())->method('isGlobalScaleEnabled') ->willReturn($gsEnabled); $this->config->expects($this->any())->method('getAppValue') - ->with('files_sharing', 'lookupServerUploadEnabled', 'yes') + ->with('files_sharing', 'lookupServerUploadEnabled', 'no') ->willReturn($isEnabled); $this->assertSame($expected, @@ -882,10 +885,13 @@ public function testIsLookupServerUploadEnabled($gsEnabled, $isEnabled, $expecte public function dataTestIsLookupServerUploadEnabled() { return [ - [false, 'yes', true], - [false, 'no', false], [true, 'yes', false], [true, 'no', false], + // TODO: reenable if we use the lookup server again + // [false, 'yes', true], + // [false, 'no', false], + [false, 'yes', false], + [false, 'no', false], ]; } diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 399cd4e00be2c..8cdeaf345413a 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -37,7 +37,6 @@ */ namespace OCA\Files_Sharing\Controller; -use OCP\Constants; use function array_slice; use function array_values; use Generator; @@ -48,6 +47,8 @@ use OCP\Collaboration\Collaborators\ISearch; use OCP\Collaboration\Collaborators\ISearchResult; use OCP\Collaboration\Collaborators\SearchResultType; +use OCP\Constants; +use OCP\GlobalScale\IConfig as GlobalScaleIConfig; use OCP\IConfig; use OCP\IRequest; use OCP\IURLGenerator; @@ -217,15 +218,11 @@ public function search(string $search = '', string $itemType = null, int $page = $this->limit = $perPage; $this->offset = $perPage * ($page - 1); - // In global scale mode we always search the loogup server - if ($this->config->getSystemValueBool('gs.enabled', false)) { - $lookup = true; - $this->result['lookupEnabled'] = true; - } else { - $this->result['lookupEnabled'] = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes'; - } + // In global scale mode we always search the lookup server + $this->result['lookupEnabled'] = \OC::$server->get(GlobalScaleIConfig::class)->isGlobalScaleEnabled(); + // TODO: Reconsider using lookup server for non-global-scale federation - [$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $lookup, $this->limit, $this->offset); + [$result, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, $this->result['lookupEnabled'], $this->limit, $this->offset); // extra treatment for 'exact' subarray, with a single merge expected keys might be lost if (isset($result['exact'])) { diff --git a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php index 860d5796e56dc..60cb079ea5a75 100644 --- a/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareesAPIControllerTest.php @@ -35,6 +35,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\Collaboration\Collaborators\ISearch; +use OCP\GlobalScale\IConfig as GlobalScaleIConfig; use OCP\IConfig; use OCP\IRequest; use OCP\IURLGenerator; @@ -240,14 +241,14 @@ public function testSearch(array $getData, string $apiSetting, string $enumSetti $perPage = $getData['perPage'] ?? 200; $shareType = $getData['shareType'] ?? null; + $globalConfig = $this->createMock(GlobalScaleIConfig::class); + $globalConfig->expects(self::once()) + ->method('isGlobalScaleEnabled') + ->willReturn(true); + $this->overwriteService(GlobalScaleIConfig::class, $globalConfig); + /** @var IConfig|MockObject $config */ $config = $this->createMock(IConfig::class); - $config->expects($this->exactly(1)) - ->method('getAppValue') - ->with($this->anything(), $this->anything(), $this->anything()) - ->willReturnMap([ - ['files_sharing', 'lookupServerEnabled', 'yes', 'yes'], - ]); $this->shareManager->expects($this->once()) ->method('allowGroupSharing') diff --git a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php index f905f9173b658..4b310ad75ef8f 100644 --- a/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php +++ b/apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php @@ -128,10 +128,12 @@ public function execute(IJobList $jobList, ILogger $logger = null): void { * @return bool */ protected function shouldRemoveBackgroundJob(): bool { - return $this->config->getSystemValueBool('has_internet_connection', true) === false || - $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' || - $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' || - $this->retries >= 5; + // TODO: Remove global scale condition once lookup server is used for non-global scale federation + // return $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'no') !== 'yes' + return !$this->config->getSystemValueBool('gs.enabled', false) + || $this->config->getSystemValueBool('has_internet_connection', true) === false + || $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') === '' + || $this->retries >= 5; } protected function shouldRun(): bool { @@ -193,7 +195,7 @@ protected function run($argument): void { $user->getUID(), 'lookup_server_connector', 'update_retries', - $this->retries + 1 + (string)($this->retries + 1), ); } } diff --git a/apps/lookup_server_connector/lib/UpdateLookupServer.php b/apps/lookup_server_connector/lib/UpdateLookupServer.php index ec528e6effa7f..4e58a67733c29 100644 --- a/apps/lookup_server_connector/lib/UpdateLookupServer.php +++ b/apps/lookup_server_connector/lib/UpdateLookupServer.php @@ -82,8 +82,9 @@ public function userUpdated(IUser $user): void { * @return bool */ private function shouldUpdateLookupServer(): bool { - return $this->config->getSystemValueBool('has_internet_connection', true) === true && - $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') === 'yes' && - $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== ''; + // TODO: Consider reenable for non-global-scale setups by checking "'files_sharing', 'lookupServerUploadEnabled'" instead of "gs.enabled" + return $this->config->getSystemValueBool('gs.enabled', false) + && $this->config->getSystemValueBool('has_internet_connection', true) + && $this->config->getSystemValueString('lookup_server', 'https://lookup.nextcloud.com') !== ''; } } diff --git a/apps/settings/lib/BackgroundJobs/VerifyUserData.php b/apps/settings/lib/BackgroundJobs/VerifyUserData.php index ec75f4243bec2..30afe1f7edab7 100644 --- a/apps/settings/lib/BackgroundJobs/VerifyUserData.php +++ b/apps/settings/lib/BackgroundJobs/VerifyUserData.php @@ -169,9 +169,11 @@ protected function verifyWebsite(array $argument) { } protected function verifyViaLookupServer(array $argument, string $dataType): bool { - if (empty($this->lookupServerUrl) || - $this->config->getAppValue('files_sharing', 'lookupServerUploadEnabled', 'yes') !== 'yes' || - $this->config->getSystemValue('has_internet_connection', true) === false) { + // TODO: Consider to enable for non-global-scale setups by checking 'files_sharing', 'lookupServerUploadEnabled' + if (!$this->config->getSystemValueBool('gs.enabled', false) + || empty($this->lookupServerUrl) + || $this->config->getSystemValue('has_internet_connection', true) === false + ) { return true; } diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 72cbfd4de4b88..34113aa0c3ee7 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -64,12 +64,14 @@ public function __construct(IConfig $config, } public function search($search, $limit, $offset, ISearchResult $searchResult) { - $isGlobalScaleEnabled = $this->config->getSystemValue('gs.enabled', false); - $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'yes') === 'yes'; + $isGlobalScaleEnabled = $this->config->getSystemValueBool('gs.enabled', false); + $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; $hasInternetConnection = $this->config->getSystemValueBool('has_internet_connection', true); - // if case of Global Scale we always search the lookup server - if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection)) { + // If case of Global Scale we always search the lookup server + // TODO: Reconsider using the lookup server for non-global scale + // if (!$isGlobalScaleEnabled && (!$isLookupServerEnabled || !$hasInternetConnection || $disableLookupServer)) { + if (!$isGlobalScaleEnabled) { return false; } diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 1d856252745ac..a61c1186c6f2d 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -88,20 +88,17 @@ protected function setUp(): void { } public function testSearchNoLookupServerURI() { - $this->config->expects($this->once()) + $this->config->expects($this->atMost(1)) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') + ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->at(0)) - ->method('getSystemValue') - ->with('gs.enabled', false) - ->willReturn(false); - - $this->config->expects($this->at(2)) + $this->config->expects($this->exactly(2)) ->method('getSystemValueBool') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config->expects($this->at(3)) + ->willReturnMap([ + ['gs.enabled', false, true], + ['has_internet_connection', true, true], + ]); + $this->config->expects($this->once()) ->method('getSystemValue') ->with('lookup_server', 'https://lookup.nextcloud.com') ->willReturn(''); @@ -118,17 +115,15 @@ public function testSearchNoLookupServerURI() { public function testSearchNoInternet() { $this->config->expects($this->once()) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') + ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->at(0)) - ->method('getSystemValue') - ->with('gs.enabled', false) - ->willReturn(false); - $this->config->expects($this->at(2)) + $this->config->expects($this->exactly(2)) ->method('getSystemValueBool') - ->with('has_internet_connection', true) - ->willReturn(false); + ->willReturnMap([ + ['has_internet_connection', true, false], + ['gs.enabled', false, true], + ]); $this->clientService->expects($this->never()) ->method('newClient'); @@ -154,21 +149,18 @@ public function testSearch(array $searchParams) { $this->config->expects($this->once()) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') + ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn('yes'); - $this->config->expects($this->at(0)) - ->method('getSystemValue') - ->with('gs.enabled', false) - ->willReturn(false); - - $this->config->expects($this->at(2)) - ->method('getSystemValueBool') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config->expects($this->at(3)) + $this->config->expects($this->once()) ->method('getSystemValue') ->with('lookup_server', 'https://lookup.nextcloud.com') ->willReturn($searchParams['server']); + $this->config->expects($this->exactly(2)) + ->method('getSystemValueBool') + ->willReturnMap([ + ['gs.enabled', false, true], + ['has_internet_connection', true, true], + ]); $response = $this->createMock(IResponse::class); $response->expects($this->once()) @@ -213,22 +205,21 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab $this->config->expects($this->once()) ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') + ->with('files_sharing', 'lookupServerEnabled', 'no') ->willReturn($LookupEnabled ? 'yes' : 'no'); - $this->config->expects($this->at(0)) - ->method('getSystemValue') - ->with('gs.enabled', false) - ->willReturn($GSEnabled); - if ($GSEnabled || $LookupEnabled) { + + $this->config->expects($this->exactly(2)) + ->method('getSystemValueBool') + ->willReturnMap([ + ['has_internet_connection', true, true], + ['gs.enabled', false, $GSEnabled], + ]); + if ($GSEnabled) { $searchResult->expects($this->once()) ->method('addResultSet') ->with($type, $searchParams['expectedResult'], []); - $this->config->expects($this->at(2)) - ->method('getSystemValueBool') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config->expects($this->at(3)) + $this->config->expects($this->once()) ->method('getSystemValue') ->with('lookup_server', 'https://lookup.nextcloud.com') ->willReturn($searchParams['server']); @@ -263,12 +254,13 @@ public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnab $this->assertFalse($moreResults); } - - public function testSearchLookupServerDisabled() { - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('files_sharing', 'lookupServerEnabled', 'yes') - ->willReturn('no'); + public function testSearchGSDisabled(): void { + $this->config->expects($this->atLeastOnce()) + ->method('getSystemValueBool') + ->willReturnMap([ + ['has_internet_connection', true, true], + ['gs.enabled', false, false], + ]); /** @var ISearchResult|MockObject $searchResult */ $searchResult = $this->createMock(ISearchResult::class); @@ -387,7 +379,6 @@ public function dataSearchEnableDisableLookupServer() { 'label' => $fedIDs[0], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, 'shareWith' => $fedIDs[0] ], 'extra' => ['federationId' => $fedIDs[0]], @@ -396,7 +387,6 @@ public function dataSearchEnableDisableLookupServer() { 'label' => $fedIDs[1], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, 'shareWith' => $fedIDs[1] ], 'extra' => ['federationId' => $fedIDs[1]], @@ -405,7 +395,6 @@ public function dataSearchEnableDisableLookupServer() { 'label' => $fedIDs[2], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, 'shareWith' => $fedIDs[2] ], 'extra' => ['federationId' => $fedIDs[2]], @@ -480,7 +469,7 @@ public function searchDataProvider() { 'label' => $fedIDs[0], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, + 'globalScale' => true, 'shareWith' => $fedIDs[0] ], 'extra' => ['federationId' => $fedIDs[0]], @@ -489,7 +478,7 @@ public function searchDataProvider() { 'label' => $fedIDs[1], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, + 'globalScale' => true, 'shareWith' => $fedIDs[1] ], 'extra' => ['federationId' => $fedIDs[1]], @@ -498,7 +487,7 @@ public function searchDataProvider() { 'label' => $fedIDs[2], 'value' => [ 'shareType' => IShare::TYPE_REMOTE, - 'globalScale' => false, + 'globalScale' => true, 'shareWith' => $fedIDs[2] ], 'extra' => ['federationId' => $fedIDs[2]],