Skip to content

[stable14] always query the lookup server in a global scale setup #11800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MorrisJobke merged 4 commits into from
Oct 24, 2018
Merged

[stable14] always query the lookup server in a global scale setup #11800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52c52f5
always query the lookup server in a global scale setup and have a nic…
schiessle Oct 9, 2018
7e608df
adjust tests
schiessle Oct 9, 2018
84e39b5
some small fixes and improvements
schiessle Oct 10, 2018
49b98b5
add more tests
schiessle Oct 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions lib/private/Collaboration/Collaborators/LookupPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@
use OCP\Collaboration\Collaborators\ISearchPlugin;
use OCP\Collaboration\Collaborators\ISearchResult;
use OCP\Collaboration\Collaborators\SearchResultType;
use OCP\Federation\ICloudIdManager;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserSession;
use OCP\Share;

class LookupPlugin implements ISearchPlugin {
Expand All @@ -37,14 +40,31 @@ class LookupPlugin implements ISearchPlugin {
private $config;
/** @var IClientService */
private $clientService;
/** @var string remote part of the current user's cloud id */
private $currentUserRemote;
/** @var ICloudIdManager */
private $cloudIdManager;
/** @var ILogger */
private $logger;

public function __construct(IConfig $config, IClientService $clientService) {
public function __construct(IConfig $config,
IClientService $clientService,
IUserSession $userSession,
ICloudIdManager $cloudIdManager,
ILogger $logger) {
$this->config = $config;
$this->clientService = $clientService;
$this->cloudIdManager = $cloudIdManager;
$currentUserCloudId = $userSession->getUser()->getCloudId();
$this->currentUserRemote = $cloudIdManager->resolveCloudId($currentUserCloudId)->getRemote();
$this->logger = $logger;
}

public function search($search, $limit, $offset, ISearchResult $searchResult) {
if ($this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') !== 'yes') {
$isGlobalScaleEnabled = $this->config->getSystemValue('gs.enabled', false);
$isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes';
// if case of Global Scale we always search the lookup server
if (!$isLookupServerEnabled && !$isGlobalScaleEnabled) {
return false;
}

Expand All @@ -65,8 +85,20 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
$body = json_decode($response->getBody(), true);

foreach ($body as $lookup) {
try {
$remote = $this->cloudIdManager->resolveCloudId($lookup['federationId'])->getRemote();
} catch (\Exception $e) {
$this->logger->error('Can not parse federated cloud ID "' . $lookup['federationId'] . '"');
$this->logger->error($e->getMessage());
continue;
}
if ($this->currentUserRemote === $remote) {
continue;
}
$name = isset($lookup['name']['value']) ? $lookup['name']['value'] : '';
$label = empty($name) ? $lookup['federationId'] : $name . ' (' . $lookup['federationId'] . ')';
$result[] = [
'label' => $lookup['federationId'],
'label' => $label,
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $lookup['federationId'],
Expand Down
274 changes: 272 additions & 2 deletions tests/lib/Collaboration/Collaborators/LookupPluginTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@


use OC\Collaboration\Collaborators\LookupPlugin;
use OC\Federation\CloudId;
use OCP\Collaboration\Collaborators\ISearchResult;
use OCP\Collaboration\Collaborators\SearchResultType;
use OCP\Federation\ICloudId;
use OCP\Federation\ICloudIdManager;
use OCP\Http\Client\IClient;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share;
use Test\TestCase;

Expand All @@ -40,16 +46,45 @@ class LookupPluginTest extends TestCase {
protected $config;
/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
protected $clientService;
/** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */
protected $userSession;
/** @var ICloudIdManager|\PHPUnit_Framework_MockObject_MockObject */
protected $cloudIdManager;
/** @var LookupPlugin */
protected $plugin;
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
protected $logger;

public function setUp() {
parent::setUp();

$this->userSession = $this->createMock(IUserSession::class);
$this->cloudIdManager = $this->createMock(ICloudIdManager::class);
$this->config = $this->createMock(IConfig::class);
$this->logger = $this->createMock(ILogger::class);
$this->clientService = $this->createMock(IClientService::class);
$cloudId = $this->createMock(ICloudId::class);
$cloudId->expects($this->any())->method('getRemote')->willReturn('myNextcloud.net');
$user = $this->createMock(IUser::class);
$user->expects($this->any())->method('getCloudId')->willReturn('[email protected]');
$this->userSession->expects($this->any())->method('getUser')
->willReturn($user);
$this->cloudIdManager->expects($this->any())->method('resolveCloudId')
->willReturnCallback(function($cloudId) {
if ($cloudId === '[email protected]') {
return new CloudId('[email protected]', 'user', 'myNextcloud.net');
}
return new CloudId('[email protected]', 'user', 'someNextcloud.net');
});


$this->plugin = new LookupPlugin($this->config, $this->clientService);
$this->plugin = new LookupPlugin(
$this->config,
$this->clientService,
$this->userSession,
$this->cloudIdManager,
$this->logger
);
}

/**
Expand All @@ -69,7 +104,11 @@ public function testSearch(array $searchParams) {
->method('getAppValue')
->with('files_sharing', 'lookupServerEnabled', 'no')
->willReturn('yes');
$this->config->expects($this->once())
$this->config->expects($this->at(0))
->method('getSystemValue')
->with('gs.enabled', false)
->willReturn(false);
$this->config->expects($this->at(2))
->method('getSystemValue')
->with('lookup_server', 'https://lookup.nextcloud.com')
->willReturn($searchParams['server']);
Expand Down Expand Up @@ -104,6 +143,70 @@ public function testSearch(array $searchParams) {
$this->assertFalse($moreResults);
}


/**
* @dataProvider dataSearchEnableDisableLookupServer
* @param array $searchParams
* @param bool $GSEnabled
* @param bool $LookupEnabled
*/
public function testSearchEnableDisableLookupServer(array $searchParams, $GSEnabled, $LookupEnabled) {
$type = new SearchResultType('lookup');

/** @var ISearchResult|\PHPUnit_Framework_MockObject_MockObject $searchResult */
$searchResult = $this->createMock(ISearchResult::class);

$this->config->expects($this->once())
->method('getAppValue')
->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) {
$searchResult->expects($this->once())
->method('addResultSet')
->with($type, $searchParams['expectedResult'], []);

$this->config->expects($this->at(2))
->method('getSystemValue')
->with('lookup_server', 'https://lookup.nextcloud.com')
->willReturn($searchParams['server']);

$response = $this->createMock(IResponse::class);
$response->expects($this->once())
->method('getBody')
->willReturn(json_encode($searchParams['resultBody']));

$client = $this->createMock(IClient::class);
$client->expects($this->once())
->method('get')
->willReturnCallback(function ($url) use ($searchParams, $response) {
$this->assertSame(strpos($url, $searchParams['server'] . '/users?search='), 0);
$this->assertNotFalse(strpos($url, urlencode($searchParams['search'])));
return $response;
});

$this->clientService->expects($this->once())
->method('newClient')
->willReturn($client);
} else {
$searchResult->expects($this->never())->method('addResultSet');
}
$moreResults = $this->plugin->search(
$searchParams['search'],
$searchParams['limit'],
$searchParams['offset'],
$searchResult
);



$this->assertFalse($moreResults);
}


public function testSearchLookupServerDisabled() {
$this->config->expects($this->once())
->method('getAppValue')
Expand All @@ -120,6 +223,173 @@ public function testSearchLookupServerDisabled() {
$this->assertFalse($this->plugin->search('irr', 10, 0, $searchResult));
}

public function dataSearchEnableDisableLookupServer() {
$fedIDs = [
'[email protected]',
'[email protected]',
'[email protected]',
];

return [
[[
'search' => 'foo',
'limit' => 10,
'offset' => 0,
'server' => 'https://lookup.example.io',
'resultBody' => [
[ 'federationId' => $fedIDs[0] ],
[ 'federationId' => $fedIDs[1] ],
[ 'federationId' => $fedIDs[2] ],
],
'expectedResult' => [
[
'label' => $fedIDs[0],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[0]
],
'extra' => ['federationId' => $fedIDs[0]],
],
[
'label' => $fedIDs[1],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[1]
],
'extra' => ['federationId' => $fedIDs[1]],
],
[
'label' => $fedIDs[2],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[2]
],
'extra' => ['federationId' => $fedIDs[2]],
],
]
],// GS , Lookup
true, true
],
[[
'search' => 'foo',
'limit' => 10,
'offset' => 0,
'server' => 'https://lookup.example.io',
'resultBody' => [
[ 'federationId' => $fedIDs[0] ],
[ 'federationId' => $fedIDs[1] ],
[ 'federationId' => $fedIDs[2] ],
],
'expectedResult' => [
[
'label' => $fedIDs[0],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[0]
],
'extra' => ['federationId' => $fedIDs[0]],
],
[
'label' => $fedIDs[1],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[1]
],
'extra' => ['federationId' => $fedIDs[1]],
],
[
'label' => $fedIDs[2],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[2]
],
'extra' => ['federationId' => $fedIDs[2]],
],
]
],// GS , Lookup
true, false
],
[[
'search' => 'foo',
'limit' => 10,
'offset' => 0,
'server' => 'https://lookup.example.io',
'resultBody' => [
[ 'federationId' => $fedIDs[0] ],
[ 'federationId' => $fedIDs[1] ],
[ 'federationId' => $fedIDs[2] ],
],
'expectedResult' => [
[
'label' => $fedIDs[0],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[0]
],
'extra' => ['federationId' => $fedIDs[0]],
],
[
'label' => $fedIDs[1],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[1]
],
'extra' => ['federationId' => $fedIDs[1]],
],
[
'label' => $fedIDs[2],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[2]
],
'extra' => ['federationId' => $fedIDs[2]],
],
]
],// GS , Lookup
false, true
],
[[
'search' => 'foo',
'limit' => 10,
'offset' => 0,
'server' => 'https://lookup.example.io',
'resultBody' => [
[ 'federationId' => $fedIDs[0] ],
[ 'federationId' => $fedIDs[1] ],
[ 'federationId' => $fedIDs[2] ],
],
'expectedResult' => [
[
'label' => $fedIDs[0],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[0]
],
'extra' => ['federationId' => $fedIDs[0]],
],
[
'label' => $fedIDs[1],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[1]
],
'extra' => ['federationId' => $fedIDs[1]],
],
[
'label' => $fedIDs[2],
'value' => [
'shareType' => Share::SHARE_TYPE_REMOTE,
'shareWith' => $fedIDs[2]
],
'extra' => ['federationId' => $fedIDs[2]],
],
]
],// GS , Lookup
false, false
],
];
}

public function searchDataProvider() {
$fedIDs = [
'[email protected]',
Expand Down