From 52c52f555d9d75a465e3b990c201960a02825d1f Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 9 Oct 2018 14:30:25 +0200 Subject: always query the lookup server in a global scale setup and have a nicer label Signed-off-by: Bjoern Schiessle --- .../Collaboration/Collaborators/LookupPlugin.php | 25 +++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 3a6a0943772..6355500de0c 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -27,8 +27,10 @@ namespace OC\Collaboration\Collaborators; 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\IUserSession; use OCP\Share; class LookupPlugin implements ISearchPlugin { @@ -37,14 +39,27 @@ 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; - public function __construct(IConfig $config, IClientService $clientService) { + public function __construct(IConfig $config, + IClientService $clientService, + IUserSession $userSession, + ICloudIdManager $cloudIdManager) { $this->config = $config; $this->clientService = $clientService; + $this->cloudIdManager = $cloudIdManager; + $currentUserCloudId = $userSession->getUser()->getCloudId(); + $this->currentUserRemote = $cloudIdManager->resolveCloudId($currentUserCloudId)->getRemote(); } 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'); + // if case of Global Scale we always search the lookup server + if ($isLookupServerEnabled !== 'yes' && !$isGlobalScaleEnabled) { return false; } @@ -65,8 +80,12 @@ class LookupPlugin implements ISearchPlugin { $body = json_decode($response->getBody(), true); foreach ($body as $lookup) { + $remote = $this->cloudIdManager->resolveCloudId($lookup['federationId'])->getRemote(); + if ($this->currentUserRemote === $remote) continue; + $name = $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'], -- cgit v1.2.3 From 7e608df5987e7158693bc4dd2ef81922cfbc6c0a Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 9 Oct 2018 18:31:33 +0200 Subject: adjust tests Signed-off-by: Bjoern Schiessle --- .../Collaboration/Collaborators/LookupPlugin.php | 2 +- .../Collaborators/LookupPluginTest.php | 33 ++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 6355500de0c..6f983b66b08 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -82,7 +82,7 @@ class LookupPlugin implements ISearchPlugin { foreach ($body as $lookup) { $remote = $this->cloudIdManager->resolveCloudId($lookup['federationId'])->getRemote(); if ($this->currentUserRemote === $remote) continue; - $name = $lookup['name']['value']; + $name = isset($lookup['name']['value']) ? $lookup['name']['value'] : ''; $label = empty($name) ? $lookup['federationId'] : $name . ' (' . $lookup['federationId'] . ')'; $result[] = [ 'label' => $label, diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 83d366cf467..20a1f962306 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -25,12 +25,17 @@ namespace Test\Collaboration\Collaborators; 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\IUser; +use OCP\IUserSession; use OCP\Share; use Test\TestCase; @@ -40,16 +45,36 @@ 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; public function setUp() { parent::setUp(); + $this->userSession = $this->createMock(IUserSession::class); + $this->cloudIdManager = $this->createMock(ICloudIdManager::class); $this->config = $this->createMock(IConfig::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('user@myNextcloud.net'); + $this->userSession->expects($this->any())->method('getUser') + ->willReturn($user); + $this->cloudIdManager->expects($this->any())->method('resolveCloudId') + ->willReturnCallback(function($cloudId) { + if ($cloudId === 'user@myNextcloud.net') { + return new CloudId('user@myNextcloud.net', 'user', 'myNextcloud.net'); + } + return new CloudId('user@someNextcloud.net', 'user', 'someNextcloud.net'); + }); + - $this->plugin = new LookupPlugin($this->config, $this->clientService); + $this->plugin = new LookupPlugin($this->config, $this->clientService, $this->userSession, $this->cloudIdManager); } /** @@ -69,7 +94,11 @@ class LookupPluginTest extends TestCase { ->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']); -- cgit v1.2.3 From 84e39b522c41f2e638169034a9a65b35e847bc6f Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 10 Oct 2018 16:07:01 +0200 Subject: some small fixes and improvements Signed-off-by: Bjoern Schiessle --- .../Collaboration/Collaborators/LookupPlugin.php | 23 +++++++++++++++++----- .../Collaborators/LookupPluginTest.php | 12 ++++++++++- 2 files changed, 29 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/private/Collaboration/Collaborators/LookupPlugin.php b/lib/private/Collaboration/Collaborators/LookupPlugin.php index 6f983b66b08..ae5f7fd0cbc 100644 --- a/lib/private/Collaboration/Collaborators/LookupPlugin.php +++ b/lib/private/Collaboration/Collaborators/LookupPlugin.php @@ -30,6 +30,7 @@ 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; @@ -43,23 +44,27 @@ class LookupPlugin implements ISearchPlugin { private $currentUserRemote; /** @var ICloudIdManager */ private $cloudIdManager; + /** @var ILogger */ + private $logger; public function __construct(IConfig $config, IClientService $clientService, IUserSession $userSession, - ICloudIdManager $cloudIdManager) { + 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) { $isGlobalScaleEnabled = $this->config->getSystemValue('gs.enabled', false); - $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no'); + $isLookupServerEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no') === 'yes'; // if case of Global Scale we always search the lookup server - if ($isLookupServerEnabled !== 'yes' && !$isGlobalScaleEnabled) { + if (!$isLookupServerEnabled && !$isGlobalScaleEnabled) { return false; } @@ -80,8 +85,16 @@ class LookupPlugin implements ISearchPlugin { $body = json_decode($response->getBody(), true); foreach ($body as $lookup) { - $remote = $this->cloudIdManager->resolveCloudId($lookup['federationId'])->getRemote(); - if ($this->currentUserRemote === $remote) continue; + 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[] = [ diff --git a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php index 20a1f962306..a2707016fb8 100644 --- a/tests/lib/Collaboration/Collaborators/LookupPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/LookupPluginTest.php @@ -34,6 +34,7 @@ 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; @@ -51,6 +52,8 @@ class LookupPluginTest extends TestCase { protected $cloudIdManager; /** @var LookupPlugin */ protected $plugin; + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + protected $logger; public function setUp() { parent::setUp(); @@ -58,6 +61,7 @@ class LookupPluginTest extends TestCase { $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'); @@ -74,7 +78,13 @@ class LookupPluginTest extends TestCase { }); - $this->plugin = new LookupPlugin($this->config, $this->clientService, $this->userSession, $this->cloudIdManager); + $this->plugin = new LookupPlugin( + $this->config, + $this->clientService, + $this->userSession, + $this->cloudIdManager, + $this->logger + ); } /** -- cgit v1.2.3