Merge pull request #29610 from nextcloud/fix/user_ldap-logger

Get rid of user_ldap LogWrapper and use LoggerInterface

apps/user_ldap/ajax/wizard.php

@@ -55,7 +55,7 @@ $con->setIgnoreValidation(true);
 $userManager = new \OCA\User_LDAP\User\Manager(
 	\OC::$server->getConfig(),
 	new \OCA\User_LDAP\FilesystemHelper(),
-	new \OCA\User_LDAP\LogWrapper(),
+	\OC::$server->get(\Psr\Log\LoggerInterface::class),
 	\OC::$server->getAvatarManager(),
 	new \OCP\Image(),
 	\OC::$server->getUserManager(),

apps/user_ldap/composer/composer/autoload_classmap.php

@@ -49,7 +49,6 @@ return array(
     'OCA\\User_LDAP\\LDAPProvider' => $baseDir . '/../lib/LDAPProvider.php',
     'OCA\\User_LDAP\\LDAPProviderFactory' => $baseDir . '/../lib/LDAPProviderFactory.php',
     'OCA\\User_LDAP\\LDAPUtility' => $baseDir . '/../lib/LDAPUtility.php',
-    'OCA\\User_LDAP\\LogWrapper' => $baseDir . '/../lib/LogWrapper.php',
     'OCA\\User_LDAP\\Mapping\\AbstractMapping' => $baseDir . '/../lib/Mapping/AbstractMapping.php',
     'OCA\\User_LDAP\\Mapping\\GroupMapping' => $baseDir . '/../lib/Mapping/GroupMapping.php',
     'OCA\\User_LDAP\\Mapping\\UserMapping' => $baseDir . '/../lib/Mapping/UserMapping.php',

apps/user_ldap/composer/composer/autoload_static.php

@@ -64,7 +64,6 @@ class ComposerStaticInitUser_LDAP
         'OCA\\User_LDAP\\LDAPProvider' => __DIR__ . '/..' . '/../lib/LDAPProvider.php',
         'OCA\\User_LDAP\\LDAPProviderFactory' => __DIR__ . '/..' . '/../lib/LDAPProviderFactory.php',
         'OCA\\User_LDAP\\LDAPUtility' => __DIR__ . '/..' . '/../lib/LDAPUtility.php',
-        'OCA\\User_LDAP\\LogWrapper' => __DIR__ . '/..' . '/../lib/LogWrapper.php',
         'OCA\\User_LDAP\\Mapping\\AbstractMapping' => __DIR__ . '/..' . '/../lib/Mapping/AbstractMapping.php',
         'OCA\\User_LDAP\\Mapping\\GroupMapping' => __DIR__ . '/..' . '/../lib/Mapping/GroupMapping.php',
         'OCA\\User_LDAP\\Mapping\\UserMapping' => __DIR__ . '/..' . '/../lib/Mapping/UserMapping.php',

apps/user_ldap/composer/composer/installed.php

@@ -5,7 +5,7 @@
         'type' => 'library',
         'install_path' => __DIR__ . '/../',
         'aliases' => array(),
-        'reference' => 'c6429e6cd19c57582364338362e543580821cf99',
+        'reference' => '6b960de47cabaa7a231e72479012ba4dcbc2e882',
         'name' => '__root__',
         'dev' => false,
     ),
@@ -16,7 +16,7 @@
             'type' => 'library',
             'install_path' => __DIR__ . '/../',
             'aliases' => array(),
-            'reference' => 'c6429e6cd19c57582364338362e543580821cf99',
+            'reference' => '6b960de47cabaa7a231e72479012ba4dcbc2e882',
             'dev_requirement' => false,
         ),
     ),

apps/user_ldap/lib/LogWrapper.php

@@ -1,39 +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;
-
-/**
- * @brief wraps around static Nextcloud core methods
- */
-class LogWrapper {
-	protected $app = 'user_ldap';
-
-	/**
-	 * @brief states whether the filesystem was loaded
-	 * @return bool
-	 */
-	public function log($msg, $level) {
-		\OCP\Util::writeLog($this->app, $msg, $level);
-	}
-}

apps/user_ldap/lib/Proxy.php

@@ -75,7 +75,6 @@ abstract class Proxy {
 		if ($fs === null) {
 			$ocConfig = \OC::$server->getConfig();
 			$fs = new FilesystemHelper();
-			$log = new LogWrapper();
 			$avatarM = \OC::$server->getAvatarManager();
 			$db = \OC::$server->getDatabaseConnection();
 			$userMap = new UserMapping($db);
@@ -86,7 +85,7 @@ abstract class Proxy {
 			$logger = \OC::$server->get(LoggerInterface::class);
 		}
 		$userManager =
-			new Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image(),
+			new Manager($ocConfig, $fs, $logger, $avatarM, new \OCP\Image(),
 				$coreUserManager, $coreNotificationManager, $shareManager);
 		$connector = new Connection($this->ldap, $configPrefix);
 		$access = new Access($connector, $this->ldap, $userManager, new Helper($ocConfig, \OC::$server->getDatabaseConnection()), $ocConfig, $coreUserManager, $logger);

apps/user_ldap/lib/User/Manager.php

@@ -31,7 +31,6 @@ namespace OCA\User_LDAP\User;
 use OC\Cache\CappedMemoryCache;
 use OCA\User_LDAP\Access;
 use OCA\User_LDAP\FilesystemHelper;
-use OCA\User_LDAP\LogWrapper;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -39,6 +38,7 @@ use OCP\Image;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
 
 /**
  * Manager
@@ -65,8 +65,8 @@ class Manager {
 	/** @var FilesystemHelper */
 	protected $ocFilesystem;
 
-	/** @var LogWrapper */
-	protected $ocLog;
+	/** @var LoggerInterface */
+	protected $logger;
 
 	/** @var Image */
 	protected $image;
@@ -88,7 +88,7 @@ class Manager {
 	public function __construct(
 		IConfig $ocConfig,
 		FilesystemHelper $ocFilesystem,
-		LogWrapper $ocLog,
+		LoggerInterface $logger,
 		IAvatarManager $avatarManager,
 		Image $image,
 		IUserManager $userManager,
@@ -97,7 +97,7 @@ class Manager {
 	) {
 		$this->ocConfig = $ocConfig;
 		$this->ocFilesystem = $ocFilesystem;
-		$this->ocLog = $ocLog;
+		$this->logger = $logger;
 		$this->avatarManager = $avatarManager;
 		$this->image = $image;
 		$this->userManager = $userManager;
@@ -126,7 +126,7 @@ class Manager {
 	private function createAndCache($dn, $uid) {
 		$this->checkAccess();
 		$user = new User($uid, $dn, $this->access, $this->ocConfig,
-			$this->ocFilesystem, clone $this->image, $this->ocLog,
+			$this->ocFilesystem, clone $this->image, $this->logger,
 			$this->avatarManager, $this->userManager,
 			$this->notificationManager);
 		$this->usersByDN[$dn] = $user;

apps/user_ldap/lib/User/User.php

@@ -35,7 +35,6 @@ use OCA\User_LDAP\Access;
 use OCA\User_LDAP\Connection;
 use OCA\User_LDAP\Exceptions\AttributeNotSet;
 use OCA\User_LDAP\FilesystemHelper;
-use OCA\User_LDAP\LogWrapper;
 use OCP\IAvatarManager;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -43,6 +42,7 @@ use OCP\Image;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
+use Psr\Log\LoggerInterface;
 
 /**
  * User
@@ -71,9 +71,9 @@ class User {
 	 */
 	protected $image;
 	/**
-	 * @var LogWrapper
+	 * @var LoggerInterface
 	 */
-	protected $log;
+	protected $logger;
 	/**
 	 * @var IAvatarManager
 	 */
@@ -112,24 +112,16 @@ class User {
 	 * @brief constructor, make sure the subclasses call this one!
 	 * @param string $username the internal username
 	 * @param string $dn the LDAP DN
-	 * @param Access $access
-	 * @param IConfig $config
-	 * @param FilesystemHelper $fs
-	 * @param Image $image any empty instance
-	 * @param LogWrapper $log
-	 * @param IAvatarManager $avatarManager
-	 * @param IUserManager $userManager
-	 * @param INotificationManager $notificationManager
 	 */
 	public function __construct($username, $dn, Access $access,
 		IConfig $config, FilesystemHelper $fs, Image $image,
-		LogWrapper $log, IAvatarManager $avatarManager, IUserManager $userManager,
+		LoggerInterface $logger, IAvatarManager $avatarManager, IUserManager $userManager,
 		INotificationManager $notificationManager) {
 		if ($username === null) {
-			$log->log("uid for '$dn' must not be null!", ILogger::ERROR);
+			$logger->error("uid for '$dn' must not be null!", ['app' => 'user_ldap']);
 			throw new \InvalidArgumentException('uid must not be null!');
 		} elseif ($username === '') {
-			$log->log("uid for '$dn' must not be an empty string", ILogger::ERROR);
+			$logger->error("uid for '$dn' must not be an empty string", ['app' => 'user_ldap']);
 			throw new \InvalidArgumentException('uid must not be an empty string!');
 		}
 
@@ -140,7 +132,7 @@ class User {
 		$this->dn = $dn;
 		$this->uid = $username;
 		$this->image = $image;
-		$this->log = $log;
+		$this->logger = $logger;
 		$this->avatarManager = $avatarManager;
 		$this->userManager = $userManager;
 		$this->notificationManager = $notificationManager;
@@ -493,19 +485,19 @@ class User {
 			if ($aQuota && (count($aQuota) > 0) && $this->verifyQuotaValue($aQuota[0])) {
 				$quota = $aQuota[0];
 			} elseif (is_array($aQuota) && isset($aQuota[0])) {
-				$this->log->log('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ILogger::DEBUG);
+				$this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $aQuota[0] . ']', ['app' => 'user_ldap']);
 			}
 		} elseif ($this->verifyQuotaValue($valueFromLDAP)) {
 			$quota = $valueFromLDAP;
 		} else {
-			$this->log->log('no suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ILogger::DEBUG);
+			$this->logger->debug('no suitable LDAP quota found for user ' . $this->uid . ': [' . $valueFromLDAP . ']', ['app' => 'user_ldap']);
 		}
 
 		if ($quota === false && $this->verifyQuotaValue($defaultQuota)) {
 			// quota not found using the LDAP attribute (or not parseable). Try the default quota
 			$quota = $defaultQuota;
 		} elseif ($quota === false) {
-			$this->log->log('no suitable default quota found for user ' . $this->uid . ': [' . $defaultQuota . ']', ILogger::DEBUG);
+			$this->logger->debug('no suitable default quota found for user ' . $this->uid . ': [' . $defaultQuota . ']', ['app' => 'user_ldap']);
 			return;
 		}
 
@@ -513,7 +505,7 @@ class User {
 		if ($targetUser instanceof IUser) {
 			$targetUser->setQuota($quota);
 		} else {
-			$this->log->log('trying to set a quota for user ' . $this->uid . ' but the user is missing', ILogger::INFO);
+			$this->logger->info('trying to set a quota for user ' . $this->uid . ' but the user is missing', ['app' => 'user_ldap']);
 		}
 	}
 
@@ -573,7 +565,7 @@ class User {
 	 */
 	private function setOwnCloudAvatar() {
 		if (!$this->image->valid()) {
-			$this->log->log('avatar image data from LDAP invalid for '.$this->dn, ILogger::ERROR);
+			$this->logger->error('avatar image data from LDAP invalid for '.$this->dn, ['app' => 'user_ldap']);
 			return false;
 		}
 
@@ -581,7 +573,7 @@ class User {
 		//make sure it is a square and not bigger than 128x128
 		$size = min([$this->image->width(), $this->image->height(), 128]);
 		if (!$this->image->centerCrop($size)) {
-			$this->log->log('croping image for avatar failed for '.$this->dn, ILogger::ERROR);
+			$this->logger->error('croping image for avatar failed for '.$this->dn, ['app' => 'user_ldap']);
 			return false;
 		}
 

apps/user_ldap/tests/AccessTest.php

@@ -39,7 +39,6 @@ use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\ILDAPWrapper;
 use OCA\User_LDAP\LDAP;
-use OCA\User_LDAP\LogWrapper;
 use OCA\User_LDAP\Mapping\GroupMapping;
 use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
@@ -119,7 +118,7 @@ class AccessTest extends TestCase {
 			->setConstructorArgs([
 				$this->createMock(IConfig::class),
 				$this->createMock(FilesystemHelper::class),
-				$this->createMock(LogWrapper::class),
+				$this->createMock(LoggerInterface::class),
 				$this->createMock(IAvatarManager::class),
 				$this->createMock(Image::class),
 				$this->createMock(IUserManager::class),

apps/user_ldap/tests/Integration/AbstractIntegrationTest.php

@@ -33,10 +33,10 @@ use OCA\User_LDAP\FilesystemHelper;
 use OCA\User_LDAP\GroupPluginManager;
 use OCA\User_LDAP\Helper;
 use OCA\User_LDAP\LDAP;
-use OCA\User_LDAP\LogWrapper;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\UserPluginManager;
 use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
 
 abstract class AbstractIntegrationTest {
 	/** @var  LDAP */
@@ -123,7 +123,7 @@ abstract class AbstractIntegrationTest {
 		$this->userManager = new Manager(
 			\OC::$server->getConfig(),
 			new FilesystemHelper(),
-			new LogWrapper(),
+			\OC::$server->get(LoggerInterface::class),
 			\OC::$server->getAvatarManager(),
 			new \OCP\Image(),
 			\OC::$server->getUserManager(),

apps/user_ldap/tests/Integration/Lib/User/IntegrationTestUserAvatar.php

@@ -28,7 +28,6 @@
 namespace OCA\User_LDAP\Tests\Integration\Lib\User;
 
 use OCA\User_LDAP\FilesystemHelper;
-use OCA\User_LDAP\LogWrapper;
 use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\Tests\Integration\AbstractIntegrationTest;
 use OCA\User_LDAP\User\Manager;
@@ -36,6 +35,7 @@ use OCA\User_LDAP\User\User;
 use OCA\User_LDAP\User_LDAP;
 use OCA\User_LDAP\UserPluginManager;
 use OCP\Image;
+use Psr\Log\LoggerInterface;
 
 require_once __DIR__ . '/../../Bootstrap.php';
 
@@ -134,7 +134,7 @@ class IntegrationTestUserAvatar extends AbstractIntegrationTest {
 		$this->userManager = new Manager(
 			\OC::$server->getConfig(),
 			new FilesystemHelper(),
-			new LogWrapper(),
+			\OC::$server->get(LoggerInterface::class),
 			\OC::$server->getAvatarManager(),
 			new Image(),
 			\OC::$server->getDatabaseConnection(),

apps/user_ldap/tests/User/ManagerTest.php

@@ -32,7 +32,6 @@ 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\Manager;
 use OCA\User_LDAP\User\User;
 use OCP\IAvatarManager;
@@ -42,6 +41,7 @@ use OCP\Image;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class Test_User_Manager
@@ -60,8 +60,8 @@ class ManagerTest extends \Test\TestCase {
 	/** @var FilesystemHelper|\PHPUnit\Framework\MockObject\MockObject */
 	protected $fileSystemHelper;
 
-	/** @var LogWrapper|\PHPUnit\Framework\MockObject\MockObject */
-	protected $log;
+	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
+	protected $logger;
 
 	/** @var IAvatarManager|\PHPUnit\Framework\MockObject\MockObject */
 	protected $avatarManager;
@@ -95,7 +95,7 @@ class ManagerTest extends \Test\TestCase {
 		$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->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
 		$this->ncUserManager = $this->createMock(IUserManager::class);
@@ -113,7 +113,7 @@ class ManagerTest extends \Test\TestCase {
 		$this->manager = new Manager(
 			$this->config,
 			$this->fileSystemHelper,
-			$this->log,
+			$this->logger,
 			$this->avatarManager,
 			$this->image,
 			$this->ncUserManager,

apps/user_ldap/tests/User/UserTest.php

@@ -31,7 +31,6 @@ 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\LogWrapper;
 use OCA\User_LDAP\User\User;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
@@ -41,6 +40,7 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Notification\INotification;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class UserTest
@@ -66,8 +66,8 @@ class UserTest extends \Test\TestCase {
 	protected $image;
 	/** @var IAvatarManager|\PHPUnit\Framework\MockObject\MockObject */
 	protected $avatarManager;
-	/** @var LogWrapper|\PHPUnit\Framework\MockObject\MockObject */
-	protected $log;
+	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
+	protected $logger;
 	/** @var string */
 	protected $uid = 'alice';
 	/** @var string */
@@ -88,7 +88,7 @@ class UserTest extends \Test\TestCase {
 
 		$this->config = $this->createMock(IConfig::class);
 		$this->filesystemhelper = $this->createMock(FilesystemHelper::class);
-		$this->log = $this->createMock(LogWrapper::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->avatarManager = $this->createMock(IAvatarManager::class);
 		$this->image = $this->createMock(Image::class);
 		$this->userManager = $this->createMock(IUserManager::class);
@@ -101,7 +101,7 @@ class UserTest extends \Test\TestCase {
 			$this->config,
 			$this->filesystemhelper,
 			$this->image,
-			$this->log,
+			$this->logger,
 			$this->avatarManager,
 			$this->userManager,
 			$this->notificationManager
@@ -935,7 +935,7 @@ class UserTest extends \Test\TestCase {
 				$this->config,
 				$this->filesystemhelper,
 				$this->image,
-				$this->log,
+				$this->logger,
 				$this->avatarManager,
 				$this->userManager,
 				$this->notificationManager