Merge pull request #10034 from nextcloud/fix/noid/ldap-silence-quota-logmsgs

lower log level for quota manipulation cases

apps/user_ldap/lib/Connection.php

@@ -57,6 +57,9 @@ use OCP\ILogger;
  * @property string ldapUuidGroupAttribute
  * @property string ldapExpertUUIDUserAttr
  * @property string ldapExpertUUIDGroupAttr
+ * @property string ldapQuotaAttribute
+ * @property string ldapQuotaDefault
+ * @property string ldapEmailAttribute
  */
 class Connection extends LDAPUtility {
 	private $ldapConnectionRes = null;

apps/user_ldap/lib/User/User.php

@@ -37,8 +37,8 @@ use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\Image;
+use OCP\IUser;
 use OCP\IUserManager;
-use OCP\Util;
 use OCP\Notification\IManager as INotificationManager;
 
 /**
@@ -499,44 +499,39 @@ class User {
 			return;
 		}
 
-		$quota = false;
+		$quotaAttribute = $this->connection->ldapQuotaAttribute;
-		if(is_null($valueFromLDAP)) {
+		$defaultQuota = $this->connection->ldapQuotaDefault;
-			$quotaAttribute = $this->connection->ldapQuotaAttribute;
+		if($quotaAttribute === '' && $defaultQuota === '') {
-			if ($quotaAttribute !== '') {
+			return;
-				$aQuota = $this->access->readAttribute($this->dn, $quotaAttribute);
-				if($aQuota && (count($aQuota) > 0)) {
-					if ($this->verifyQuotaValue($aQuota[0])) {
-						$quota = $aQuota[0];
-					} else {
-						$this->log->log('not suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ILogger::WARN);
-					}
-				}
-			}
-		} else {
-			if ($this->verifyQuotaValue($valueFromLDAP)) {
-				$quota = $valueFromLDAP;
-			} else {
-				$this->log->log('not suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ILogger::WARN);
-			}
 		}
 
-		if ($quota === false) {
+		$quota = false;
-			// quota not found using the LDAP attribute (or not parseable). Try the default quota
+		if(is_null($valueFromLDAP) && $quotaAttribute !== '') {
-			$defaultQuota = $this->connection->ldapQuotaDefault;
+			$aQuota = $this->access->readAttribute($this->dn, $quotaAttribute);
-			if ($this->verifyQuotaValue($defaultQuota)) {
+			if($aQuota && (count($aQuota) > 0) && $this->verifyQuotaValue($aQuota[0])) {
-				$quota = $defaultQuota;
+				$quota = $aQuota[0];
+			} else if(is_array($aQuota) && isset($aQuota[0])) {
+				$this->log->log('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ILogger::DEBUG);
 			}
+		} else if ($this->verifyQuotaValue($valueFromLDAP)) {
+			$quota = $valueFromLDAP;
+		} else {
+			$this->log->log('no suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ILogger::DEBUG);
+		}
+
+		if ($quota === false && $this->verifyQuotaValue($defaultQuota)) {
+			// quota not found using the LDAP attribute (or not parseable). Try the default quota
+			$quota = $defaultQuota;
+		} else if($quota === false) {
+			$this->log->log('no suitable default quota found for user ' . $this->uid . ': [' . $defaultQuota . ']', ILogger::DEBUG);
+			return;
 		}
 
 		$targetUser = $this->userManager->get($this->uid);
-		if ($targetUser) {
+		if ($targetUser instanceof IUser) {
-			if($quota !== false) {
+			$targetUser->setQuota($quota);
-				$targetUser->setQuota($quota);
-			} else {
-				$this->log->log('not suitable default quota found for user ' . $this->uid . ': [' . $defaultQuota . ']', ILogger::WARN);
-			}
 		} else {
-			$this->log->log('trying to set a quota for user ' . $this->uid . ' but the user is missing', ILogger::ERROR);
+			$this->log->log('trying to set a quota for user ' . $this->uid . ' but the user is missing', ILogger::INFO);
 		}
 	}
 

apps/user_ldap/tests/User/UserTest.php

@@ -32,14 +32,12 @@ 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\User;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
 use OCP\IConfig;
-use OCP\IDBConnection;
 use OCP\Image;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -79,15 +77,14 @@ class UserTest extends \Test\TestCase {
 		$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);
 
-		return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr);
+		return array($access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
 	}
 
 	public function testGetDNandUsername() {
-		list($access, $config, $filesys, $image, $log, $avaMgr, $db, $userMgr, $notiMgr) =
+		list($access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$uid = 'alice';
@@ -101,7 +98,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateEmailProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->once())
@@ -135,7 +132,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateEmailNotProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->once())
@@ -161,7 +158,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateEmailNotConfigured() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->once())
@@ -185,15 +182,15 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaAllProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
+		$this->connection->expects($this->exactly(2))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
+			->willReturnMap([
-			->will($this->returnValue('myquota'));
+				['ldapQuotaAttribute', 'myquota'],
-		$this->connection->expects($this->exactly(1))
+				['ldapQuotaDefault', '']
-			->method('__get');
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -206,7 +203,7 @@ class UserTest extends \Test\TestCase {
 			->method('setQuota')
 			->with('42 GB');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->atLeastOnce())
 			->method('get')
 			->with('alice')
 			->will($this->returnValue($user));
@@ -221,15 +218,15 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaToDefaultAllProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
+		$this->connection->expects($this->exactly(2))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
+			->willReturnMap([
-			->will($this->returnValue('myquota'));
+				['ldapQuotaAttribute', 'myquota'],
-		$this->connection->expects($this->exactly(1))
+				['ldapQuotaDefault', '']
-			->method('__get');
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -257,15 +254,15 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaToNoneAllProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
+		$this->connection->expects($this->exactly(2))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
+			->willReturnMap([
-			->will($this->returnValue('myquota'));
+				['ldapQuotaAttribute', 'myquota'],
-		$this->connection->expects($this->exactly(1))
+				['ldapQuotaDefault', '']
-			->method('__get');
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -293,7 +290,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaDefaultProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->at(0))
@@ -333,15 +330,15 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaIndividualProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
+		$this->connection->expects($this->exactly(2))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
+			->willReturnMap([
-			->will($this->returnValue('myquota'));
+				['ldapQuotaAttribute', 'myquota'],
-		$this->connection->expects($this->exactly(1))
+				['ldapQuotaDefault', '']
-			->method('__get');
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -369,19 +366,15 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaNoneProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue(''));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', 'myquota'],
+				['ldapQuotaDefault', '']
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -393,10 +386,9 @@ class UserTest extends \Test\TestCase {
 		$user->expects($this->never())
 			->method('setQuota');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->never())
 			->method('get')
-			->with('alice')
+			->with('alice');
-			->will($this->returnValue($user));
 
 		$config->expects($this->never())
 			->method('setUserValue');
@@ -411,28 +403,22 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaNoneConfigured() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue(''));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue(''));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', ''],
+				['ldapQuotaDefault', '']
+			]);
 
 		$user = $this->createMock('\OCP\IUser');
 		$user->expects($this->never())
 			->method('setQuota');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->never())
-			->method('get')
+			->method('get');
-			->with('alice')
-			->will($this->returnValue($user));
 
 		$this->access->expects($this->never())
 			->method('readAttribute');
@@ -450,14 +436,17 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateQuotaFromValue() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$readQuota = '19 GB';
 
-		$this->connection->expects($this->never())
+		$this->connection->expects($this->exactly(2))
 			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'));
+			->willReturnMap([
+				['ldapQuotaAttribute', 'myquota'],
+				['ldapQuotaDefault', '']
+			]);
 
 		$this->access->expects($this->never())
 			->method('readAttribute');
@@ -485,19 +474,15 @@ class UserTest extends \Test\TestCase {
 	 * Unparseable quota will fallback to use the LDAP default
 	 */
 	public function testUpdateWrongQuotaAllProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('23 GB'));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', 'myquota'],
+				['ldapQuotaDefault', '23 GB']
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -528,19 +513,15 @@ class UserTest extends \Test\TestCase {
 	 * No user quota and wrong default will set 'default' as quota
 	 */
 	public function testUpdateWrongDefaultQuotaProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('23 GBwowowo'));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', 'myquota'],
+				['ldapQuotaDefault', '23 GBwowowo']
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -552,10 +533,8 @@ class UserTest extends \Test\TestCase {
 		$user->expects($this->never())
 			->method('setQuota');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->never())
-			->method('get')
+			->method('get');
-			->with('alice')
-			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -570,19 +549,15 @@ class UserTest extends \Test\TestCase {
 	 * Wrong user quota and wrong default will set 'default' as quota
 	 */
 	public function testUpdateWrongQuotaAndDefaultAllProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue('myquota'));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('23 GBwowowo'));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', 'myquota'],
+				['ldapQuotaDefault', '23 GBwowowo']
+			]);
 
 		$this->access->expects($this->once())
 			->method('readAttribute')
@@ -594,10 +569,8 @@ class UserTest extends \Test\TestCase {
 		$user->expects($this->never())
 			->method('setQuota');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->never())
-			->method('get')
+			->method('get');
-			->with('alice')
-			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -612,19 +585,15 @@ class UserTest extends \Test\TestCase {
 	 * No quota attribute set and wrong default will set 'default' as quota
 	 */
 	public function testUpdateWrongDefaultQuotaNotProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
-		$this->connection->expects($this->at(0))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaAttribute'))
-			->will($this->returnValue(''));
-		$this->connection->expects($this->at(1))
-			->method('__get')
-			->with($this->equalTo('ldapQuotaDefault'))
-			->will($this->returnValue('23 GBwowowo'));
 		$this->connection->expects($this->exactly(2))
-			->method('__get');
+			->method('__get')
+			->willReturnMap([
+				['ldapQuotaAttribute', ''],
+				['ldapQuotaDefault', '23 GBwowowo']
+			]);
 
 		$this->access->expects($this->never())
 			->method('readAttribute');
@@ -633,10 +602,8 @@ class UserTest extends \Test\TestCase {
 		$user->expects($this->never())
 			->method('setQuota');
 
-		$userMgr->expects($this->once())
+		$userMgr->expects($this->never())
-			->method('get')
+			->method('get');
-			->with('alice')
-			->will($this->returnValue($user));
 
 		$uid = 'alice';
 		$dn  = 'uid=alice,dc=foo,dc=bar';
@@ -649,7 +616,7 @@ class UserTest extends \Test\TestCase {
 
 	//the testUpdateAvatar series also implicitely tests getAvatarImage
 	public function testUpdateAvatarJpegPhotoProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->access->expects($this->once())
@@ -695,7 +662,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateAvatarThumbnailPhotoProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->access->expects($this->any())
@@ -750,7 +717,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateAvatarNotProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->access->expects($this->any())
@@ -793,7 +760,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateBeforeFirstLogin() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->at(0))
@@ -823,7 +790,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateAfterFirstLogin() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->at(0))
@@ -857,7 +824,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testUpdateNoRefresh() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->at(0))
@@ -894,7 +861,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testMarkLogin() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $db, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->once())
@@ -915,7 +882,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testGetAvatarImageProvided() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $db, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->access->expects($this->once())
@@ -938,7 +905,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testProcessAttributes() {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$uid = 'alice';
@@ -954,7 +921,8 @@ class UserTest extends \Test\TestCase {
 			'updateAvatar'
 		);
 
-		$userMock = $this->getMockBuilder('OCA\User_LDAP\User\User')
+		/** @var User|\PHPUnit_Framework_MockObject_MockObject $userMock */
+		$userMock = $this->getMockBuilder(User::class)
 			->setConstructorArgs(array($uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr))
 			->setMethods($requiredMethods)
 			->getMock();
@@ -1002,7 +970,7 @@ class UserTest extends \Test\TestCase {
 	 * @dataProvider emptyHomeFolderAttributeValueProvider
 	 */
 	public function testGetHomePathNotConfigured($attributeValue) {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->any())
@@ -1027,7 +995,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testGetHomePathConfiguredNotAvailableAllowed() {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->any())
@@ -1059,7 +1027,7 @@ class UserTest extends \Test\TestCase {
 	 * @expectedException \Exception
 	 */
 	public function testGetHomePathConfiguredNotAvailableNotAllowed() {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$this->connection->expects($this->any())
@@ -1097,7 +1065,7 @@ class UserTest extends \Test\TestCase {
 	 * @dataProvider displayNameProvider
 	 */
 	public function testComposeAndStoreDisplayName($part1, $part2, $expected) {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$config->expects($this->once())
@@ -1111,7 +1079,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testHandlePasswordExpiryWarningDefaultPolicy() {
-		list(, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$uid = 'alice';
@@ -1181,7 +1149,7 @@ class UserTest extends \Test\TestCase {
 	}
 
 	public function testHandlePasswordExpiryWarningCustomPolicy() {
-		list(, $config, $filesys, $image, $log, $avaMgr, , $userMgr, $notiMgr) =
+		list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
 			$this->getTestInstances();
 
 		$uid = 'alice';