Skip to content

always query the lookup server in a global scale setup #11714

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
schiessle merged 4 commits into from
Oct 15, 2018
Merged

always query the lookup server in a global scale setup #11714

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3fa13e7
always query the lookup server in a global scale setup and have a nic…
schiessle Oct 9, 2018
9e76d40
adjust tests
schiessle Oct 9, 2018
fd5fc24
some small fixes and improvements
schiessle Oct 10, 2018
1b0b159
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
Next Next commit
always query the lookup server in a global scale setup and have a nic… 
…er label

Signed-off-by: Bjoern Schiessle <[email protected]>
  • Loading branch information
@schiessle
schiessle committed Oct 9, 2018
commit 3fa13e791e825c765cf808ff7aefcabd64ddcf2b
25 changes: 22 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,10 @@
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 {
Expand All @@ -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) {
Copy link
Member

@MorrisJobke MorrisJobke Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be a lot more readable if the === 'yes' is added to line 60 and then the if statement only holds !$isLookupServerEnabled && !$isGlobalScaleEnabled.

Sorry for nitpicking, but readable code is worth it IMO.

Copy link
Member Author

@schiessle schiessle Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea... will add it.

return false;
}

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

foreach ($body as $lookup) {
$remote = $this->cloudIdManager->resolveCloudId($lookup['federationId'])->getRemote();
Copy link
Member

@nickvergessen nickvergessen Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try-catch

if ($this->currentUserRemote === $remote) continue;
Copy link
Member

@nickvergessen nickvergessen Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline + brackets

$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'],
Expand Down