From fbd4e9e6513426b0809fcc6fc1faec72e6b52d2d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 17 Dec 2018 14:55:00 +0100 Subject: add tests for the DUI as they are interact with the DB they are more integraiton than unit tests Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User/DeletedUsersIndex.php | 12 +-- .../user_ldap/tests/User/DeletedUsersIndexTest.php | 118 +++++++++++++++++++++ 2 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 apps/user_ldap/tests/User/DeletedUsersIndexTest.php (limited to 'apps') diff --git a/apps/user_ldap/lib/User/DeletedUsersIndex.php b/apps/user_ldap/lib/User/DeletedUsersIndex.php index 9ec95a01b58..3473398f415 100644 --- a/apps/user_ldap/lib/User/DeletedUsersIndex.php +++ b/apps/user_ldap/lib/User/DeletedUsersIndex.php @@ -70,7 +70,7 @@ class DeletedUsersIndex { $deletedUsers = $this->config->getUsersForUserValue( 'user_ldap', 'isDeleted', '1'); - $userObjects = array(); + $userObjects = []; foreach($deletedUsers as $user) { $userObjects[] = new OfflineUser($user, $this->config, $this->db, $this->mapping); } @@ -95,20 +95,20 @@ class DeletedUsersIndex { * @return bool */ public function hasUsers() { - if($this->deletedUsers === false) { + if(!is_array($this->deletedUsers)) { $this->fetchDeletedUsers(); } - if(is_array($this->deletedUsers) && count($this->deletedUsers) > 0) { - return true; - } - return false; + return is_array($this->deletedUsers) && (count($this->deletedUsers) > 0); } /** * marks a user as deleted + * * @param string $ocName + * @throws \OCP\PreConditionNotMetException */ public function markUser($ocName) { $this->config->setUserValue($ocName, 'user_ldap', 'isDeleted', '1'); + $this->deletedUsers = null; } } diff --git a/apps/user_ldap/tests/User/DeletedUsersIndexTest.php b/apps/user_ldap/tests/User/DeletedUsersIndexTest.php new file mode 100644 index 00000000000..b3cf076e973 --- /dev/null +++ b/apps/user_ldap/tests/User/DeletedUsersIndexTest.php @@ -0,0 +1,118 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\User_LDAP\Tests\User; + +use OCA\User_LDAP\Mapping\UserMapping; +use OCA\User_LDAP\User\DeletedUsersIndex; +use OCP\IConfig; +use OCP\IDBConnection; + +/** + * Class DeletedUsersIndexTest + * + * @group DB + * + * @package OCA\User_LDAP\Tests\User + */ +class DeletedUsersIndexTest extends \Test\TestCase { + /** @var DeletedUsersIndex */ + protected $dui; + + /** @var IConfig */ + protected $config; + + /** @var IDBConnection */ + protected $db; + + /** @var UserMapping|\PHPUnit_Framework_MockObject_MockObject */ + protected $mapping; + + public function setUp() { + parent::setUp(); + + // no mocks for those as tests go against DB + $this->config = \OC::$server->getConfig(); + $this->db = \OC::$server->getDatabaseConnection(); + + $this->mapping = $this->createMock(UserMapping::class); + + $this->dui = new DeletedUsersIndex($this->config, $this->db, $this->mapping); + } + + public function tearDown() { + $this->config->deleteAppFromAllUsers('user_ldap'); + return parent::tearDown(); + } + + public function testMarkAndFetchUser() { + $uids = [ + 'cef3775c-71d2-48eb-8984-39a4051b0b95', + '8c4bbb40-33ed-42d0-9b14-85b0ab76c1cc', + ]; + + // ensure test works on a pristine state + $this->assertFalse($this->dui->hasUsers()); + + $this->dui->markUser($uids[0]); + + $this->assertTrue($this->dui->hasUsers()); + + $this->dui->markUser($uids[1]); + + $deletedUsers = $this->dui->getUsers(); + $this->assertSame(2, count($deletedUsers)); + + // ensure the different uids were used + foreach($deletedUsers as $deletedUser) { + $this->assertTrue(in_array($deletedUser->getOCName(), $uids)); + $i = array_search($deletedUser->getOCName(), $uids); + $this->assertNotFalse($i); + unset($uids[$i]); + } + $this->assertEmpty($uids); + } + + public function testUnmarkUser() { + $uids = [ + '22a162c7-a9ee-487c-9f33-0563795583fb', + '1fb4e0da-4a75-47f3-8fa7-becc7e35c9c5', + ]; + + // we know this works, because of "testMarkAndFetchUser" + $this->dui->markUser($uids[0]); + // this returns a working instance of OfflineUser + $testUser = $this->dui->getUsers()[0]; + $testUser->unmark(); + + // the DUI caches the users, to clear mark someone else + $this->dui->markUser($uids[1]); + + $deletedUsers = $this->dui->getUsers(); + foreach ($deletedUsers as $deletedUser) { + $this->assertNotSame($testUser->getOCName(), $deletedUser->getOCName()); + } + } + + +} -- cgit v1.2.3 From 85f14bc591e764d36c570cec0b594a4f818d675a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 17 Dec 2018 17:37:45 +0100 Subject: LDAP: extend remnants output with "detected on" field Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Command/ShowRemnants.php | 47 +++++++++++++++++---------- apps/user_ldap/lib/User/DeletedUsersIndex.php | 6 ++++ apps/user_ldap/lib/User/OfflineUser.php | 30 ++++++++++++----- 3 files changed, 57 insertions(+), 26 deletions(-) (limited to 'apps') diff --git a/apps/user_ldap/lib/Command/ShowRemnants.php b/apps/user_ldap/lib/Command/ShowRemnants.php index 365c8967ee0..6fb207c4e2d 100644 --- a/apps/user_ldap/lib/Command/ShowRemnants.php +++ b/apps/user_ldap/lib/Command/ShowRemnants.php @@ -55,38 +55,49 @@ class ShowRemnants extends Command { $this ->setName('ldap:show-remnants') ->setDescription('shows which users are not available on LDAP anymore, but have remnants in Nextcloud.') - ->addOption('json', null, InputOption::VALUE_NONE, 'return JSON array instead of pretty table.'); + ->addOption('json', null, InputOption::VALUE_NONE, 'return JSON array instead of pretty table.') + ->addOption('short-date', null, InputOption::VALUE_NONE, 'show dates in Y-m-d format'); + } + + protected function formatDate(int $timestamp, string $default, bool $showShortDate) { + if (!($timestamp > 0)) { + return $default; + } + if ($showShortDate) { + return date('Y-m-d', $timestamp); + } + return $this->dateFormatter->formatDate($timestamp); } /** - * executes the command, i.e. creeates and outputs a table of LDAP users marked as deleted + * executes the command, i.e. creates and outputs a table of LDAP users marked as deleted * * {@inheritdoc} */ protected function execute(InputInterface $input, OutputInterface $output) { /** @var \Symfony\Component\Console\Helper\Table $table */ $table = new Table($output); - $table->setHeaders(array( + $table->setHeaders([ 'Nextcloud name', 'Display Name', 'LDAP UID', 'LDAP DN', 'Last Login', - 'Dir', 'Sharer')); - $rows = array(); + 'Detected on', 'Dir', 'Sharer' + ]); + $rows = []; $resultSet = $this->dui->getUsers(); - foreach($resultSet as $user) { - $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; - $lastLogin = ($user->getLastLogin() > 0) ? - $this->dateFormatter->formatDate($user->getLastLogin()) : '-'; - $rows[] = array('ocName' => $user->getOCName(), - 'displayName' => $user->getDisplayName(), - 'uid' => $user->getUID(), - 'dn' => $user->getDN(), - 'lastLogin' => $lastLogin, - 'homePath' => $user->getHomePath(), - 'sharer' => $hAS - ); + foreach ($resultSet as $user) { + $rows[] = [ + 'ocName' => $user->getOCName(), + 'displayName' => $user->getDisplayName(), + 'uid' => $user->getUID(), + 'dn' => $user->getDN(), + 'lastLogin' => $this->formatDate($user->getLastLogin(), '-', (bool)$input->getOption('short-date')), + 'detectedOn' => $this->formatDate($user->getDetectedOn(), 'unknown', (bool)$input->getOption('short-date')), + 'homePath' => $user->getHomePath(), + 'sharer' => $user->getHasActiveShares() ? 'Y' : 'N', + ]; } if ($input->getOption('json')) { - $output->writeln(json_encode($rows)); + $output->writeln(json_encode($rows)); } else { $table->setRows($rows); $table->render($output); diff --git a/apps/user_ldap/lib/User/DeletedUsersIndex.php b/apps/user_ldap/lib/User/DeletedUsersIndex.php index 3473398f415..0f0d4859c23 100644 --- a/apps/user_ldap/lib/User/DeletedUsersIndex.php +++ b/apps/user_ldap/lib/User/DeletedUsersIndex.php @@ -108,7 +108,13 @@ class DeletedUsersIndex { * @throws \OCP\PreConditionNotMetException */ public function markUser($ocName) { + $curValue = $this->config->getUserValue($ocName, 'user_ldap', 'isDeleted', '0'); + if($curValue === '1') { + // the user is already marked, do not write to DB again + return; + } $this->config->setUserValue($ocName, 'user_ldap', 'isDeleted', '1'); + $this->config->setUserValue($ocName, 'user_ldap', 'foundDeleted', (string)time()); $this->deletedUsers = null; } } diff --git a/apps/user_ldap/lib/User/OfflineUser.php b/apps/user_ldap/lib/User/OfflineUser.php index 9576b49d5a2..5ed158cc105 100644 --- a/apps/user_ldap/lib/User/OfflineUser.php +++ b/apps/user_ldap/lib/User/OfflineUser.php @@ -53,6 +53,10 @@ class OfflineUser { * @var string $lastLogin the timestamp of the last login */ protected $lastLogin; + /** + * @var string $foundDeleted the timestamp when the user was detected as unavailable + */ + protected $foundDeleted; /** * @var string $email */ @@ -92,7 +96,8 @@ class OfflineUser { * remove the Delete-flag from the user. */ public function unmark() { - $this->config->setUserValue($this->ocName, 'user_ldap', 'isDeleted', '0'); + $this->config->deleteUserValue($this->ocName, 'user_ldap', 'isDeleted'); + $this->config->deleteUserValue($this->ocName, 'user_ldap', 'foundDeleted'); } /** @@ -169,6 +174,14 @@ class OfflineUser { return (int)$this->lastLogin; } + /** + * getter for the detection timestamp + * @return int + */ + public function getDetectedOn() { + return (int)$this->foundDeleted; + } + /** * getter for having active shares * @return bool @@ -181,13 +194,14 @@ class OfflineUser { * reads the user details */ protected function fetchDetails() { - $properties = array ( - 'displayName' => 'user_ldap', - 'uid' => 'user_ldap', - 'homePath' => 'user_ldap', - 'email' => 'settings', - 'lastLogin' => 'login' - ); + $properties = [ + 'displayName' => 'user_ldap', + 'uid' => 'user_ldap', + 'homePath' => 'user_ldap', + 'foundDeleted' => 'user_ldap', + 'email' => 'settings', + 'lastLogin' => 'login', + ]; foreach($properties as $property => $app) { $this->$property = $this->config->getUserValue($this->ocName, $app, $property, ''); } -- cgit v1.2.3 From 925043c60cb178939bfa4bb58e5ae3260b1df11e Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 4 Jan 2019 22:10:22 +0100 Subject: ensure db is pristine before starting the tests Signed-off-by: Arthur Schiwon --- apps/user_ldap/tests/User/DeletedUsersIndexTest.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'apps') diff --git a/apps/user_ldap/tests/User/DeletedUsersIndexTest.php b/apps/user_ldap/tests/User/DeletedUsersIndexTest.php index b3cf076e973..916b5119ce0 100644 --- a/apps/user_ldap/tests/User/DeletedUsersIndexTest.php +++ b/apps/user_ldap/tests/User/DeletedUsersIndexTest.php @@ -55,6 +55,9 @@ class DeletedUsersIndexTest extends \Test\TestCase { $this->config = \OC::$server->getConfig(); $this->db = \OC::$server->getDatabaseConnection(); + // ensure a clean database + $this->config->deleteAppFromAllUsers('user_ldap'); + $this->mapping = $this->createMock(UserMapping::class); $this->dui = new DeletedUsersIndex($this->config, $this->db, $this->mapping); -- cgit v1.2.3