From 2a1e5d8bebf2e64db2f0f37c032277c4a8cd039a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 26 Oct 2020 13:43:53 +0100 Subject: [PATCH 1/3] add test for active shares flag Signed-off-by: Arthur Schiwon --- apps/user_ldap/tests/User/OfflineUserTest.php | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 apps/user_ldap/tests/User/OfflineUserTest.php diff --git a/apps/user_ldap/tests/User/OfflineUserTest.php b/apps/user_ldap/tests/User/OfflineUserTest.php new file mode 100644 index 0000000000..8c8835e968 --- /dev/null +++ b/apps/user_ldap/tests/User/OfflineUserTest.php @@ -0,0 +1,93 @@ + + * + * @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 Doctrine\DBAL\Driver\Statement; +use OCA\User_LDAP\Mapping\UserMapping; +use OCA\User_LDAP\User\OfflineUser; +use OCP\IConfig; +use OCP\IDBConnection; +use Test\TestCase; + +class OfflineUserTest extends TestCase { + + /** @var OfflineUser */ + protected $offlineUser; + /** @var UserMapping|\PHPUnit\Framework\MockObject\MockObject */ + protected $mapping; + /** @var string */ + protected $uid; + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + protected $config; + /** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */ + protected $dbc; + + public function setUp(): void { + $this->uid = 'deborah'; + $this->config = $this->createMock(IConfig::class); + $this->dbc = $this->createMock(IDBConnection::class); + $this->mapping = $this->createMock(UserMapping::class); + + $this->offlineUser = new OfflineUser( + $this->uid, + $this->config, + $this->dbc, + $this->mapping + ); + } + + public function shareOwnerProvider(): array { + // tests for none, one, many + return [ + [ 0, 0, false], + [ 1, 0, true], + [ 0, 1, true], + [ 1, 1, true], + [ 2, 0, true], + [ 0, 2, true], + [ 2, 2, true], + ]; + } + + /** + * @dataProvider shareOwnerProvider + */ + public function testHasActiveShares(int $internalOwnerships, int $externalOwnerships, bool $expected) { + $queryMock = $this->createMock(Statement::class); + $queryMock->expects($this->atLeastOnce()) + ->method('fetchColumn') + ->willReturnOnConsecutiveCalls( + (string)$internalOwnerships, + (string)$externalOwnerships + ); + + $this->dbc->expects($this->atLeastOnce()) + ->method('prepare') + ->willReturn($queryMock); + + $this->assertSame($expected, $this->offlineUser->getHasActiveShares()); + } +} From 71c34876a6f2039c716917fae339961f5c4f4676 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 26 Oct 2020 13:54:03 +0100 Subject: [PATCH 2/3] split instantiation from business logic in OfflineUser Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User/OfflineUser.php | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/User/OfflineUser.php b/apps/user_ldap/lib/User/OfflineUser.php index 72d29dd544..77863da6ac 100644 --- a/apps/user_ldap/lib/User/OfflineUser.php +++ b/apps/user_ldap/lib/User/OfflineUser.php @@ -90,7 +90,6 @@ class OfflineUser { $this->config = $config; $this->db = $db; $this->mapping = $mapping; - $this->fetchDetails(); } /** @@ -132,6 +131,9 @@ class OfflineUser { * @return string */ public function getUID() { + if (!isset($this->uid)) { + $this->fetchDetails(); + } return $this->uid; } @@ -140,6 +142,9 @@ class OfflineUser { * @return string */ public function getDN() { + if (!isset($this->dn)) { + $this->fetchDetails(); + } return $this->dn; } @@ -148,6 +153,9 @@ class OfflineUser { * @return string */ public function getDisplayName() { + if (!isset($this->displayName)) { + $this->fetchDetails(); + } return $this->displayName; } @@ -156,6 +164,9 @@ class OfflineUser { * @return string */ public function getEmail() { + if (!isset($this->email)) { + $this->fetchDetails(); + } return $this->email; } @@ -164,6 +175,9 @@ class OfflineUser { * @return string */ public function getHomePath() { + if (!isset($this->homePath)) { + $this->fetchDetails(); + } return $this->homePath; } @@ -172,6 +186,9 @@ class OfflineUser { * @return int */ public function getLastLogin() { + if (!isset($this->lastLogin)) { + $this->fetchDetails(); + } return (int)$this->lastLogin; } @@ -180,6 +197,9 @@ class OfflineUser { * @return int */ public function getDetectedOn() { + if (!isset($this->foundDeleted)) { + $this->fetchDetails(); + } return (int)$this->foundDeleted; } @@ -188,6 +208,9 @@ class OfflineUser { * @return bool */ public function getHasActiveShares() { + if (!isset($this->hasActiveShares)) { + $this->fetchDetails(); + } return $this->hasActiveShares; } From 37301a5378fb59323fa8d1c49a2a2df619422198 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 26 Oct 2020 14:00:57 +0100 Subject: [PATCH 3/3] fixes determining whether former user is a share owner Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/User/OfflineUser.php | 15 ++++----------- apps/user_ldap/tests/User/OfflineUserTest.php | 8 +++++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/apps/user_ldap/lib/User/OfflineUser.php b/apps/user_ldap/lib/User/OfflineUser.php index 77863da6ac..2f201ca4a4 100644 --- a/apps/user_ldap/lib/User/OfflineUser.php +++ b/apps/user_ldap/lib/User/OfflineUser.php @@ -243,29 +243,22 @@ class OfflineUser { */ protected function determineShares() { $query = $this->db->prepare(' - SELECT COUNT(`uid_owner`) + SELECT `uid_owner` FROM `*PREFIX*share` WHERE `uid_owner` = ? ', 1); $query->execute([$this->ocName]); - $sResult = $query->fetchColumn(0); - if ((int)$sResult === 1) { + if ($query->rowCount() > 0) { $this->hasActiveShares = true; return; } $query = $this->db->prepare(' - SELECT COUNT(`owner`) + SELECT `owner` FROM `*PREFIX*share_external` WHERE `owner` = ? ', 1); $query->execute([$this->ocName]); - $sResult = $query->fetchColumn(0); - if ((int)$sResult === 1) { - $this->hasActiveShares = true; - return; - } - - $this->hasActiveShares = false; + $this->hasActiveShares = $query->rowCount() > 0; } } diff --git a/apps/user_ldap/tests/User/OfflineUserTest.php b/apps/user_ldap/tests/User/OfflineUserTest.php index 8c8835e968..298e1708a5 100644 --- a/apps/user_ldap/tests/User/OfflineUserTest.php +++ b/apps/user_ldap/tests/User/OfflineUserTest.php @@ -78,10 +78,12 @@ class OfflineUserTest extends TestCase { public function testHasActiveShares(int $internalOwnerships, int $externalOwnerships, bool $expected) { $queryMock = $this->createMock(Statement::class); $queryMock->expects($this->atLeastOnce()) - ->method('fetchColumn') + ->method('execute'); + $queryMock->expects($this->atLeastOnce()) + ->method('rowCount') ->willReturnOnConsecutiveCalls( - (string)$internalOwnerships, - (string)$externalOwnerships + $internalOwnerships > 0 ? 1 : 0, + $externalOwnerships > 0 ? 1 : 0 ); $this->dbc->expects($this->atLeastOnce())