remove app specific IUserTools and consolidate test

Just some house keeping. IUserTools with used in even older days for
easier creation of Access instances…

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php
index 98a1bbfa1b76f37c873f8829f3d6ca2b4f8f8d69..e25b7ee3126d0f0c299bb10f241fd36421ec4c53 100644 (file)
--- a/apps/user_ldap/composer/composer/autoload_classmap.php
+++ b/apps/user_ldap/composer/composer/autoload_classmap.php
@@ -56,7 +56,6 @@ return array(
     'OCA\\User_LDAP\\Settings\\Section' => $baseDir . '/../lib/Settings/Section.php',
     'OCA\\User_LDAP\\UserPluginManager' => $baseDir . '/../lib/UserPluginManager.php',
     'OCA\\User_LDAP\\User\\DeletedUsersIndex' => $baseDir . '/../lib/User/DeletedUsersIndex.php',
-    'OCA\\User_LDAP\\User\\IUserTools' => $baseDir . '/../lib/User/IUserTools.php',
     'OCA\\User_LDAP\\User\\Manager' => $baseDir . '/../lib/User/Manager.php',
     'OCA\\User_LDAP\\User\\OfflineUser' => $baseDir . '/../lib/User/OfflineUser.php',
     'OCA\\User_LDAP\\User\\User' => $baseDir . '/../lib/User/User.php',

diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php
index 83e49daf0f3b96ef00b452f95c7517543aab0578..23819055be47db09ab593b8161c50ced5a731a28 100644 (file)
--- a/apps/user_ldap/composer/composer/autoload_static.php
+++ b/apps/user_ldap/composer/composer/autoload_static.php
@@ -71,7 +71,6 @@ class ComposerStaticInitUser_LDAP
         'OCA\\User_LDAP\\Settings\\Section' => __DIR__ . '/..' . '/../lib/Settings/Section.php',
         'OCA\\User_LDAP\\UserPluginManager' => __DIR__ . '/..' . '/../lib/UserPluginManager.php',
         'OCA\\User_LDAP\\User\\DeletedUsersIndex' => __DIR__ . '/..' . '/../lib/User/DeletedUsersIndex.php',
-        'OCA\\User_LDAP\\User\\IUserTools' => __DIR__ . '/..' . '/../lib/User/IUserTools.php',
         'OCA\\User_LDAP\\User\\Manager' => __DIR__ . '/..' . '/../lib/User/Manager.php',
         'OCA\\User_LDAP\\User\\OfflineUser' => __DIR__ . '/..' . '/../lib/User/OfflineUser.php',
         'OCA\\User_LDAP\\User\\User' => __DIR__ . '/..' . '/../lib/User/User.php',

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index d0d51ae8c85b8c1b09911e8fda495e3ee7022ef6..fb2582e82662f0873336b0cd13b4dac5c19dd138 100644 (file)
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -46,7 +46,6 @@ namespace OCA\User_LDAP;
 use OC\HintException;
 use OC\Hooks\PublicEmitter;
 use OCA\User_LDAP\Exceptions\ConstraintViolationException;
-use OCA\User_LDAP\User\IUserTools;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
 use OCA\User_LDAP\Mapping\AbstractMapping;
@@ -59,7 +58,7 @@ use OCP\IUserManager;
  * Class Access
  * @package OCA\User_LDAP
  */
-class Access extends LDAPUtility implements IUserTools {
+class Access extends LDAPUtility {
        const UUID_ATTRIBUTES = ['entryuuid', 'nsuniqueid', 'objectguid', 'guid', 'ipauniqueid'];
 
        /** @var \OCA\User_LDAP\Connection */

diff --git a/apps/user_ldap/lib/User/IUserTools.php b/apps/user_ldap/lib/User/IUserTools.php
deleted file mode 100644 (file)
index 4ba9ceb..0000000
--- a/apps/user_ldap/lib/User/IUserTools.php
+++ /dev/null
@@ -1,42 +0,0 @@
-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Arthur Schiwon <blizzz@arthur-schiwon.de>
- * @author Joas Schilling <coding@schilljs.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * 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, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-namespace OCA\User_LDAP\User;
-
-/**
- * IUserTools
- *
- * defines methods that are required by User class for LDAP interaction
- */
-interface IUserTools {
-       public function getConnection();
-
-       public function readAttribute($dn, $attr, $filter = 'objectClass=*');
-
-       public function stringResemblesDN($string);
-
-       public function dn2username($dn, $ldapname = null);
-
-       public function username2dn($name);
-}

diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php
index 9f2f3649777dc4ce96807a2f82c002120d70e8b8..13555f9e31b90ec1c9390cab0df7849e66cc5057 100644 (file)
--- a/apps/user_ldap/lib/User/Manager.php
+++ b/apps/user_ldap/lib/User/Manager.php
@@ -45,7 +45,7 @@ use OCP\Notification\IManager as INotificationManager;
  * cache
  */
 class Manager {
-       /** @var IUserTools */
+       /** @var Access */
        protected $access;
 
        /** @var IConfig */
@@ -110,11 +110,11 @@ class Manager {
        }
 
        /**
-        * @brief binds manager to an instance of IUserTools (implemented by
-        * Access). It needs to be assigned first before the manager can be used.
-        * @param IUserTools
+        * Binds manager to an instance of Access.
+        * It needs to be assigned first before the manager can be used.
+        * @param Access
         */
-       public function setLdapAccess(IUserTools $access) {
+       public function setLdapAccess(Access $access) {
                $this->access = $access;
        }
 

diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php
index f4be19a7ad532f81495665c5d7e68a76c9cfb038..706424d31898c13e6f9bdb21853415b8ee75fa75 100644 (file)
--- a/apps/user_ldap/lib/User/User.php
+++ b/apps/user_ldap/lib/User/User.php
@@ -30,6 +30,7 @@
 
 namespace OCA\User_LDAP\User;
 
+use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\LogWrapper;
@@ -48,7 +49,7 @@ use OCP\Notification\IManager as INotificationManager;
  */
 class User {
        /**
-        * @var IUserTools
+        * @var Access
         */
        protected $access;
        /**
@@ -110,8 +111,7 @@ class User {
         * @brief constructor, make sure the subclasses call this one!
         * @param string $username the internal username
         * @param string $dn the LDAP DN
-        * @param IUserTools $access an instance that implements IUserTools for
-        * LDAP interaction
+        * @param Access $access
         * @param IConfig $config
         * @param FilesystemHelper $fs
         * @param Image $image any empty instance
@@ -120,7 +120,7 @@ class User {
         * @param IUserManager $userManager
         * @param INotificationManager $notificationManager
         */
-       public function __construct($username, $dn, IUserTools $access,
+       public function __construct($username, $dn, Access $access,
                IConfig $config, FilesystemHelper $fs, Image $image,
                LogWrapper $log, IAvatarManager $avatarManager, IUserManager $userManager,
                INotificationManager $notificationManager) {

diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php
index 104a70ff70024ce462cfffd992bde65d5bdb8303..5c111abdc4e2723ca85e105f661252d81fbb6edf 100644 (file)
--- a/apps/user_ldap/tests/User/ManagerTest.php
+++ b/apps/user_ldap/tests/User/ManagerTest.php
@@ -28,11 +28,13 @@
 
 namespace OCA\User_LDAP\Tests\User;
 
+use OCA\User_LDAP\Access;
+use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\LogWrapper;
-use OCA\User_LDAP\User\IUserTools;
 use OCA\User_LDAP\User\Manager;
+use OCA\User_LDAP\User\User;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -48,200 +50,181 @@ use OCP\Notification\IManager as INotificationManager;
  * @package OCA\User_LDAP\Tests\User
  */
 class ManagerTest extends \Test\TestCase {
+       /** @var Access|\PHPUnit_Framework_MockObject_MockObject */
+       protected $access;
 
-       private function getTestInstances() {
-               $access = $this->createMock(IUserTools::class);
-               $config = $this->createMock(IConfig::class);
-               $filesys = $this->createMock(FilesystemHelper::class);
-               $log = $this->createMock(LogWrapper::class);
-               $avaMgr = $this->createMock(IAvatarManager::class);
-               $image = $this->createMock(Image::class);
-               $dbc = $this->createMock(IDBConnection::class);
-               $userMgr = $this->createMock(IUserManager::class);
-               $notiMgr  = $this->createMock(INotificationManager::class);
-
-               $connection = new \OCA\User_LDAP\Connection(
-                       $lw  = $this->createMock(ILDAPWrapper::class),
-                       '',
-                       null
-               );
+       /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+       protected $config;
 
-               $access->expects($this->any())
-                       ->method('getConnection')
-                       ->will($this->returnValue($connection));
+       /** @var FilesystemHelper|\PHPUnit_Framework_MockObject_MockObject */
+       protected $fileSystemHelper;
 
-               return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr);
-       }
+       /** @var LogWrapper|\PHPUnit_Framework_MockObject_MockObject */
+       protected $log;
 
-       public function testGetByDNExisting() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
+       /** @var IAvatarManager|\PHPUnit_Framework_MockObject_MockObject */
+       protected $avatarManager;
 
-               $inputDN = 'cn=foo,dc=foobar,dc=bar';
-               $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e';
+       /** @var Image|\PHPUnit_Framework_MockObject_MockObject */
+       protected $image;
 
-               $access->expects($this->once())
-                       ->method('stringResemblesDN')
-                       ->with($this->equalTo($inputDN))
-                       ->will($this->returnValue(true));
+       /** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */
+       protected $dbc;
 
-               $access->expects($this->once())
-                       ->method('dn2username')
-                       ->with($this->equalTo($inputDN))
-                       ->will($this->returnValue($uid));
+       /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+       protected $ncUserManager;
 
-               $access->expects($this->never())
-                       ->method('username2dn');
+       /** @var INotificationManager|\PHPUnit_Framework_MockObject_MockObject */
+       protected $notificationManager;
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($inputDN);
+       /** @var ILDAPWrapper|\PHPUnit_Framework_MockObject_MockObject */
+       protected $ldapWrapper;
 
-               // Now we fetch the user again. If this leads to a failing test,
-               // runtime caching the manager is broken.
-               $user = $manager->get($inputDN);
-
-               $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user);
-       }
+       /** @var Connection */
+       protected $connection;
 
-       public function testGetByEDirectoryDN() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
+       /** @var Manager */
+       protected $manager;
 
-               $inputDN = 'uid=foo,o=foobar,c=bar';
-               $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e';
+       public function setUp() {
+               parent::setUp();
 
-               $access->expects($this->once())
-                       ->method('stringResemblesDN')
-                       ->with($this->equalTo($inputDN))
-                       ->will($this->returnValue(true));
-
-               $access->expects($this->once())
-                       ->method('dn2username')
-                       ->with($this->equalTo($inputDN))
-                       ->will($this->returnValue($uid));
+               $this->access = $this->createMock(Access::class);
+               $this->config = $this->createMock(IConfig::class);
+               $this->fileSystemHelper = $this->createMock(FilesystemHelper::class);
+               $this->log = $this->createMock(LogWrapper::class);
+               $this->avatarManager = $this->createMock(IAvatarManager::class);
+               $this->image = $this->createMock(Image::class);
+               $this->dbc = $this->createMock(IDBConnection::class);
+               $this->ncUserManager = $this->createMock(IUserManager::class);
+               $this->notificationManager = $this->createMock(INotificationManager::class);
 
-               $access->expects($this->never())
-                       ->method('username2dn');
+               $this->ldapWrapper = $this->createMock(ILDAPWrapper::class);
+               $this->connection = new Connection($this->ldapWrapper, '', null);
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($inputDN);
+               $this->access->expects($this->any())
+                       ->method('getConnection')
+                       ->will($this->returnValue($this->connection));
+
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $this->manager = new Manager(
+                       $this->config,
+                       $this->fileSystemHelper,
+                       $this->log,
+                       $this->avatarManager,
+                       $this->image,
+                       $this->dbc,
+                       $this->ncUserManager,
+                       $this->notificationManager
+               );
 
-               $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user);
+               $this->manager->setLdapAccess($this->access);
        }
 
-       public function testGetByExoticDN() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
+       public function dnProvider() {
+               return [
+                       ['cn=foo,dc=foobar,dc=bar'],
+                       ['uid=foo,o=foobar,c=bar'],
+                       ['ab=cde,f=ghei,mno=pq'],
+               ];
+       }
 
-               $inputDN = 'ab=cde,f=ghei,mno=pq';
+       /**
+        * @dataProvider dnProvider
+        */
+       public function testGetByDNExisting(string $inputDN) {
                $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e';
 
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('stringResemblesDN')
                        ->with($this->equalTo($inputDN))
                        ->will($this->returnValue(true));
-
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('dn2username')
                        ->with($this->equalTo($inputDN))
                        ->will($this->returnValue($uid));
-
-               $access->expects($this->never())
+               $this->access->expects($this->never())
                        ->method('username2dn');
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($inputDN);
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $this->manager->get($inputDN);
 
-               $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user);
+               // Now we fetch the user again. If this leads to a failing test,
+               // runtime caching the manager is broken.
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $user = $this->manager->get($inputDN);
+
+               $this->assertInstanceOf(User::class, $user);
        }
 
        public function testGetByDNNotExisting() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
-
                $inputDN = 'cn=gone,dc=foobar,dc=bar';
 
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('stringResemblesDN')
                        ->with($this->equalTo($inputDN))
                        ->will($this->returnValue(true));
-
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('dn2username')
                        ->with($this->equalTo($inputDN))
                        ->will($this->returnValue(false));
-
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('username2dn')
                        ->with($this->equalTo($inputDN))
                        ->will($this->returnValue(false));
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($inputDN);
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $user = $this->manager->get($inputDN);
 
                $this->assertNull($user);
        }
 
        public function testGetByUidExisting() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
-
                $dn = 'cn=foo,dc=foobar,dc=bar';
                $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e';
 
-               $access->expects($this->never())
+               $this->access->expects($this->never())
                        ->method('dn2username');
-
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('username2dn')
                        ->with($this->equalTo($uid))
                        ->will($this->returnValue($dn));
-
-               $access->expects($this->once())
+               $this->access->expects($this->once())
                        ->method('stringResemblesDN')
                        ->with($this->equalTo($uid))
                        ->will($this->returnValue(false));
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($uid);
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $this->manager->get($uid);
 
                // Now we fetch the user again. If this leads to a failing test,
                // runtime caching the manager is broken.
-               $user = $manager->get($uid);
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $user = $this->manager->get($uid);
 
-               $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user);
+               $this->assertInstanceOf(User::class, $user);
        }
 
        public function testGetByUidNotExisting() {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
-
                $uid = 'gone';
 
-               $access->expects($this->never())
+               $this->access->expects($this->never())
                        ->method('dn2username');
-
-               $access->expects($this->exactly(1))
+               $this->access->expects($this->exactly(1))
                        ->method('username2dn')
                        ->with($this->equalTo($uid))
                        ->will($this->returnValue(false));
 
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-               $user = $manager->get($uid);
+               /** @noinspection PhpUnhandledExceptionInspection */
+               $user = $this->manager->get($uid);
 
                $this->assertNull($user);
        }
 
        public function attributeRequestProvider() {
                return [
-                       [ false ],
-                       [ true ],
+                       [false],
+                       [true],
                ];
        }
 
@@ -249,23 +232,16 @@ class ManagerTest extends \Test\TestCase {
         * @dataProvider attributeRequestProvider
         */
        public function testGetAttributes($minimal) {
-               list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
-                       $this->getTestInstances();
-
-               $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr);
-               $manager->setLdapAccess($access);
-
-               $connection = $access->getConnection();
-               $connection->setConfiguration([
+               $this->connection->setConfiguration([
                        'ldapEmailAttribute' => 'mail',
                        'ldapUserAvatarRule' => 'default',
                        'ldapQuotaAttribute' => '',
                ]);
 
-               $attributes = $manager->getAttributes($minimal);
+               $attributes = $this->manager->getAttributes($minimal);
 
                $this->assertTrue(in_array('dn', $attributes));
-               $this->assertTrue(in_array($access->getConnection()->ldapEmailAttribute, $attributes));
+               $this->assertTrue(in_array($this->access->getConnection()->ldapEmailAttribute, $attributes));
                $this->assertFalse(in_array('', $attributes));
                $this->assertSame(!$minimal, in_array('jpegphoto', $attributes));
                $this->assertSame(!$minimal, in_array('thumbnailphoto', $attributes));