Skip to content
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

Use BackendUserIterator with uid filter for user:sync --uid #37398

Merged
merged 1 commit into from
May 19, 2020
Merged

Use BackendUserIterator with uid filter for user:sync --uid #37398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions changelog/unreleased/37398
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Bugfix: Adjust user:sync --uid to use user backend iterator

It fixes the behavior for user:sync --uid that attempts to retrieve all
user backend users without limit at offset, that is not supported by LDAP backend.
Instead, proper iterator and search query has been used

https://github.com/owncloud/core/pull/37398
https://github.com/owncloud/enterprise/issues/3981
13 changes: 8 additions & 5 deletions core/Command/User/SyncBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

use OC\User\Account;
use OC\User\AccountMapper;
use OC\User\Sync\AllUsersIterator;
use OC\User\Sync\BackendUsersIterator;
use OC\User\Sync\SeenUsersIterator;
use OC\User\SyncService;
use OCP\IConfig;
Expand Down Expand Up @@ -214,7 +214,7 @@ private function syncMultipleUsers(
$iterator = new SeenUsersIterator($this->accountMapper, $backendClass);
} else {
$output->writeln("Inserting new and updating all known users from $backendClass ...");
$iterator = new AllUsersIterator($backend);
$iterator = new BackendUsersIterator($backend);
}

$p = new ProgressBar($output);
Expand Down Expand Up @@ -251,10 +251,11 @@ private function syncSingleUser(
$uid,
$missingAccountsAction
) {
$output->writeln("Syncing $uid ...");
$userUids = $backend->getUsers('', null);
$output->writeln("Searching for $uid ...");

$iterator = new BackendUsersIterator($backend, $uid);
$userToSync = null;
foreach ($userUids as $userUid) {
foreach ($iterator as $userUid) {
if ($userUid === $uid) {
if ($userToSync === null) {
$userToSync = $userUid;
Expand All @@ -268,9 +269,11 @@ private function syncSingleUser(

if ($userToSync !== null) {
// Run the sync using the internal username if mapped
$output->writeln("Syncing $uid ...");
$syncService->run($backend, new \ArrayIterator([$userToSync]));
} else {
// Not found
$output->writeln("Exact match for user $uid not found in the backend.");
$this->handleRemovedUsers([$uid => $dummy], $input, $output, $missingAccountsAction);
}

Expand Down
4 changes: 2 additions & 2 deletions core/Migrations/Version20170221114437.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use OC\User\AccountMapper;
use OC\User\AccountTermMapper;
use OC\User\Database;
use OC\User\Sync\AllUsersIterator;
use OC\User\Sync\BackendUsersIterator;
use OC\User\SyncService;
use OCP\Migration\ISimpleMigration;
use OCP\Migration\IOutput;
Expand All @@ -25,7 +25,7 @@ public function run(IOutput $out) {
// insert/update known users
$out->info("Insert new users ...");
$out->startProgress($backend->countUsers());
$syncService->run($backend, new AllUsersIterator($backend), function () use ($out) {
$syncService->run($backend, new BackendUsersIterator($backend), function () use ($out) {
$out->advance();
});
$out->finishProgress();
Expand Down
12 changes: 8 additions & 4 deletions lib/private/User/Sync/AllUsersIterator.php → ...rivate/User/Sync/BackendUsersIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

use OCP\UserInterface;

class AllUsersIterator extends UsersIterator {
class BackendUsersIterator extends UsersIterator {
/**
* @var UserInterface
*/
Expand All @@ -41,13 +41,17 @@ class AllUsersIterator extends UsersIterator {
/** @var bool false if the backend returned less than LIMIT results */
private $hasMoreData = false;

public function __construct(UserInterface $backend) {
/** @var string search for the uid string in backend */
private $search;

public function __construct(UserInterface $backend, $filterUID = '') {
$this->backend = $backend;
$this->search = $filterUID;
}

public function rewind() {
parent::rewind();
$this->data = $this->backend->getUsers('', self::LIMIT, 0);
$this->data = $this->backend->getUsers($this->search, self::LIMIT, 0);
$this->dataPos = 0;
$this->endPos = \count($this->data);
$this->hasMoreData = $this->endPos >= self::LIMIT;
Expand All @@ -59,7 +63,7 @@ public function next() {
if ($this->hasMoreData && $this->dataPos >= $this->endPos) {
$this->page++;
$offset = $this->page * self::LIMIT;
$this->data = $this->backend->getUsers('', self::LIMIT, $offset);
$this->data = $this->backend->getUsers($this->search, self::LIMIT, $offset);
$this->dataPos = 0;
$this->endPos = \count($this->data);
$this->hasMoreData = $this->endPos >= self::LIMIT;
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/Command/User/SyncBackendTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public function testInvalidAccountActionShowsError() {

public function executeProvider() {
return [
['foo', 'Syncing foo ...'],
['foo', 'Searching for foo ...'],
[null, 'Analysing known accounts ...'],
];
}
Expand Down
6 changes: 3 additions & 3 deletions tests/lib/User/Sync/AllUsersIteratorTest.php → ...ib/User/Sync/BackendUsersIteratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@
use Test\TestCase;

/**
* Class AllUsersIteratorTest
* Class BackendUsersIteratorTest
*
* @package OC\User\Sync
*
* @see http://php.net/manual/en/class.iterator.php for the order of calls on an iterator
*/
class AllUsersIteratorTest extends TestCase {
class BackendUsersIteratorTest extends TestCase {

/**
* @var UserInterface|\PHPUnit\Framework\MockObject\MockObject
Expand All @@ -47,7 +47,7 @@ public function setUp(): void {

$this->backend = $this->createMock(UserInterface::class);

$this->iterator = new AllUsersIterator($this->backend);
$this->iterator = new BackendUsersIterator($this->backend);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions tests/lib/User/SyncServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
use OC\User\Account;
use OC\User\AccountMapper;
use OC\User\Database;
use OC\User\Sync\AllUsersIterator;
use OC\User\Sync\BackendUsersIterator;
use OC\User\SyncService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
Expand Down Expand Up @@ -119,7 +119,7 @@ public function testSetupNewAccount() {
// Ignore state flag

$s = new SyncService($this->config, $this->logger, $this->mapper);
$s->run($backend, new AllUsersIterator($backend));
$s->run($backend, new BackendUsersIterator($backend));

static::invokePrivate($s, 'syncHome', [$account, $backend]);
}
Expand All @@ -142,7 +142,7 @@ public function testSetupNewAccountLogsErrorOnException() {
$this->logger->expects($this->at(1))->method('logException');

$s = new SyncService($this->config, $this->logger, $this->mapper);
$s->run($backend, new AllUsersIterator($backend));
$s->run($backend, new BackendUsersIterator($backend));
}

public function testSyncHomeLogsWhenBackendDiffersFromExisting() {
Expand Down