Merge pull request #25902 from nextcloud/techdept/psalm/loggerinterface/part2

LoggerInterface for provisioning API Controllers
This commit is contained in:
Roeland Jago Douma 2021-03-03 12:53:15 +01:00 committed by GitHub
commit b87e54bb1c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 49 additions and 43 deletions

View File

@ -42,16 +42,16 @@ use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\IConfig; use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\IUserSession; use OCP\IUserSession;
use OCP\L10N\IFactory; use OCP\L10N\IFactory;
use Psr\Log\LoggerInterface;
class GroupsController extends AUserData { class GroupsController extends AUserData {
/** @var ILogger */ /** @var LoggerInterface */
private $logger; private $logger;
public function __construct(string $appName, public function __construct(string $appName,
@ -62,7 +62,7 @@ class GroupsController extends AUserData {
IUserSession $userSession, IUserSession $userSession,
AccountManager $accountManager, AccountManager $accountManager,
IFactory $l10nFactory, IFactory $l10nFactory,
ILogger $logger) { LoggerInterface $logger) {
parent::__construct($appName, parent::__construct($appName,
$request, $request,
$userManager, $userManager,

View File

@ -60,7 +60,6 @@ use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\IConfig; use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
use OCP\IUser; use OCP\IUser;
@ -70,6 +69,7 @@ use OCP\L10N\IFactory;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventDispatcher;
use Psr\Log\LoggerInterface;
class UsersController extends AUserData { class UsersController extends AUserData {
@ -77,7 +77,7 @@ class UsersController extends AUserData {
private $appManager; private $appManager;
/** @var IURLGenerator */ /** @var IURLGenerator */
protected $urlGenerator; protected $urlGenerator;
/** @var ILogger */ /** @var LoggerInterface */
private $logger; private $logger;
/** @var IFactory */ /** @var IFactory */
protected $l10nFactory; protected $l10nFactory;
@ -101,7 +101,7 @@ class UsersController extends AUserData {
IUserSession $userSession, IUserSession $userSession,
AccountManager $accountManager, AccountManager $accountManager,
IURLGenerator $urlGenerator, IURLGenerator $urlGenerator,
ILogger $logger, LoggerInterface $logger,
IFactory $l10nFactory, IFactory $l10nFactory,
NewUserMailHelper $newUserMailHelper, NewUserMailHelper $newUserMailHelper,
FederatedShareProviderFactory $federatedShareProviderFactory, FederatedShareProviderFactory $federatedShareProviderFactory,
@ -417,36 +417,40 @@ class UsersController extends AUserData {
} catch (\Exception $e) { } catch (\Exception $e) {
// Mail could be failing hard or just be plain not configured // Mail could be failing hard or just be plain not configured
// Logging error as it is the hardest of the two // Logging error as it is the hardest of the two
$this->logger->logException($e, [ $this->logger->error("Unable to send the invitation mail to $email",
'message' => "Unable to send the invitation mail to $email", [
'level' => ILogger::ERROR, 'app' => 'ocs_api',
'app' => 'ocs_api', 'exception' => $e,
]); ]
);
} }
} }
} }
return new DataResponse(['id' => $userid]); return new DataResponse(['id' => $userid]);
} catch (HintException $e) { } catch (HintException $e) {
$this->logger->logException($e, [ $this->logger->warning('Failed addUser attempt with hint exception.',
'message' => 'Failed addUser attempt with hint exception.', [
'level' => ILogger::WARN, 'app' => 'ocs_api',
'app' => 'ocs_api', 'exception' => $e,
]); ]
);
throw new OCSException($e->getHint(), 107); throw new OCSException($e->getHint(), 107);
} catch (OCSException $e) { } catch (OCSException $e) {
$this->logger->logException($e, [ $this->logger->warning('Failed addUser attempt with ocs exeption.',
'message' => 'Failed addUser attempt with ocs exeption.', [
'level' => ILogger::ERROR, 'app' => 'ocs_api',
'app' => 'ocs_api', 'exception' => $e,
]); ]
);
throw $e; throw $e;
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->logException($e, [ $this->logger->error('Failed addUser attempt with exception.',
'message' => 'Failed addUser attempt with exception.', [
'level' => ILogger::ERROR, 'app' => 'ocs_api',
'app' => 'ocs_api', 'exception' => $e
]); ]
);
throw new OCSException('Bad request', 101); throw new OCSException('Bad request', 101);
} }
} }
@ -1047,11 +1051,12 @@ class UsersController extends AUserData {
$emailTemplate = $this->newUserMailHelper->generateTemplate($targetUser, false); $emailTemplate = $this->newUserMailHelper->generateTemplate($targetUser, false);
$this->newUserMailHelper->sendMail($targetUser, $emailTemplate); $this->newUserMailHelper->sendMail($targetUser, $emailTemplate);
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->logException($e, [ $this->logger->error("Can't send new user mail to $email",
'message' => "Can't send new user mail to $email", [
'level' => ILogger::ERROR, 'app' => 'settings',
'app' => 'settings', 'exception' => $e,
]); ]
);
throw new OCSException('Sending email failed', 102); throw new OCSException('Sending email failed', 102);
} }

View File

@ -37,13 +37,13 @@ use OC\User\NoUserException;
use OCA\Provisioning_API\Controller\GroupsController; use OCA\Provisioning_API\Controller\GroupsController;
use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountManager;
use OCP\IConfig; use OCP\IConfig;
use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\IUserSession; use OCP\IUserSession;
use OCP\L10N\IFactory; use OCP\L10N\IFactory;
use OCP\UserInterface; use OCP\UserInterface;
use Psr\Log\LoggerInterface;
class GroupsControllerTest extends \Test\TestCase { class GroupsControllerTest extends \Test\TestCase {
@ -59,7 +59,7 @@ class GroupsControllerTest extends \Test\TestCase {
protected $userSession; protected $userSession;
/** @var AccountManager|\PHPUnit\Framework\MockObject\MockObject */ /** @var AccountManager|\PHPUnit\Framework\MockObject\MockObject */
protected $accountManager; protected $accountManager;
/** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
protected $logger; protected $logger;
/** @var SubAdmin|\PHPUnit\Framework\MockObject\MockObject */ /** @var SubAdmin|\PHPUnit\Framework\MockObject\MockObject */
protected $subAdminManager; protected $subAdminManager;
@ -78,7 +78,7 @@ class GroupsControllerTest extends \Test\TestCase {
$this->userSession = $this->createMock(IUserSession::class); $this->userSession = $this->createMock(IUserSession::class);
$this->accountManager = $this->createMock(AccountManager::class); $this->accountManager = $this->createMock(AccountManager::class);
$this->l10nFactory = $this->createMock(IFactory::class); $this->l10nFactory = $this->createMock(IFactory::class);
$this->logger = $this->createMock(ILogger::class); $this->logger = $this->createMock(LoggerInterface::class);
$this->subAdminManager = $this->createMock(SubAdmin::class); $this->subAdminManager = $this->createMock(SubAdmin::class);

View File

@ -56,7 +56,6 @@ use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig; use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IL10N; use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest; use OCP\IRequest;
use OCP\IURLGenerator; use OCP\IURLGenerator;
use OCP\IUser; use OCP\IUser;
@ -68,6 +67,7 @@ use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\ISecureRandom; use OCP\Security\ISecureRandom;
use OCP\UserInterface; use OCP\UserInterface;
use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase; use Test\TestCase;
class UsersControllerTest extends TestCase { class UsersControllerTest extends TestCase {
@ -82,7 +82,7 @@ class UsersControllerTest extends TestCase {
protected $groupManager; protected $groupManager;
/** @var IUserSession|MockObject */ /** @var IUserSession|MockObject */
protected $userSession; protected $userSession;
/** @var ILogger|MockObject */ /** @var LoggerInterface|MockObject */
protected $logger; protected $logger;
/** @var UsersController|MockObject */ /** @var UsersController|MockObject */
protected $api; protected $api;
@ -113,7 +113,7 @@ class UsersControllerTest extends TestCase {
$this->appManager = $this->createMock(IAppManager::class); $this->appManager = $this->createMock(IAppManager::class);
$this->groupManager = $this->createMock(Manager::class); $this->groupManager = $this->createMock(Manager::class);
$this->userSession = $this->createMock(IUserSession::class); $this->userSession = $this->createMock(IUserSession::class);
$this->logger = $this->createMock(ILogger::class); $this->logger = $this->createMock(LoggerInterface::class);
$this->request = $this->createMock(IRequest::class); $this->request = $this->createMock(IRequest::class);
$this->accountManager = $this->createMock(AccountManager::class); $this->accountManager = $this->createMock(AccountManager::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class); $this->urlGenerator = $this->createMock(IURLGenerator::class);
@ -715,12 +715,13 @@ class UsersControllerTest extends TestCase {
->will($this->throwException($exception)); ->will($this->throwException($exception));
$this->logger $this->logger
->expects($this->once()) ->expects($this->once())
->method('logException') ->method('error')
->with($exception, [ ->with('Failed addUser attempt with exception.',
'message' => 'Failed addUser attempt with exception.', [
'level' => ILogger::ERROR, 'app' => 'ocs_api',
'app' => 'ocs_api', 'exception' => $exception
]); ]
);
$loggedInUser = $this->getMockBuilder(IUser::class) $loggedInUser = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();