Pimp ActionProviderStore

* Fix paradoxale condition
* Add types
* Move away from ILogger

Makes psalm happy, and thus me happy

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

lib/private/Contacts/ContactsMenu/ActionProviderStore.php

@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
  *
@@ -28,9 +30,9 @@ use OC\App\AppManager;
 use OC\Contacts\ContactsMenu\Providers\EMailProvider;
 use OCP\AppFramework\QueryException;
 use OCP\Contacts\ContactsMenu\IProvider;
-use OCP\ILogger;
 use OCP\IServerContainer;
 use OCP\IUser;
+use Psr\Log\LoggerInterface;
 
 class ActionProviderStore {
 
@@ -40,15 +42,10 @@ class ActionProviderStore {
 	/** @var AppManager */
 	private $appManager;
 
-	/** @var ILogger */
+	/** @var LoggerInterface */
 	private $logger;
 
-	/**
-	 * @param IServerContainer $serverContainer
-	 * @param AppManager $appManager
-	 * @param ILogger $logger
-	 */
-	public function __construct(IServerContainer $serverContainer, AppManager $appManager, ILogger $logger) {
+	public function __construct(IServerContainer $serverContainer, AppManager $appManager, LoggerInterface $logger) {
 		$this->serverContainer = $serverContainer;
 		$this->appManager = $appManager;
 		$this->logger = $logger;
@@ -59,7 +56,7 @@ class ActionProviderStore {
 	 * @return IProvider[]
 	 * @throws Exception
 	 */
-	public function getProviders(IUser $user) {
+	public function getProviders(IUser $user): array {
 		$appClasses = $this->getAppProviderClasses($user);
 		$providerClasses = $this->getServerProviderClasses();
 		$allClasses = array_merge($providerClasses, $appClasses);
@@ -69,11 +66,13 @@ class ActionProviderStore {
 			try {
 				$providers[] = $this->serverContainer->query($class);
 			} catch (QueryException $ex) {
-				$this->logger->logException($ex, [
-					'message' => "Could not load contacts menu action provider $class",
-					'app' => 'core',
-				]);
-				throw new Exception("Could not load contacts menu action provider");
+				$this->logger->error('Could not load contacts menu action provider ' . $class,
+					[
+						'app' => 'core',
+						'exception' => $ex,
+					]
+				);
+				throw new Exception('Could not load contacts menu action provider');
 			}
 		}
 
@@ -83,7 +82,7 @@ class ActionProviderStore {
 	/**
 	 * @return string[]
 	 */
-	private function getServerProviderClasses() {
+	private function getServerProviderClasses(): array {
 		return [
 			EMailProvider::class,
 		];
@@ -93,11 +92,11 @@ class ActionProviderStore {
 	 * @param IUser $user
 	 * @return string[]
 	 */
-	private function getAppProviderClasses(IUser $user) {
+	private function getAppProviderClasses(IUser $user): array {
 		return array_reduce($this->appManager->getEnabledAppsForUser($user), function ($all, $appId) {
 			$info = $this->appManager->getAppInfo($appId);
 
-			if (!isset($info['contactsmenu']) || !isset($info['contactsmenu'])) {
+			if (!isset($info['contactsmenu'])) {
 				// Nothing to add
 				return $all;
 			}

tests/lib/Contacts/ContactsMenu/ActionProviderStoreTest.php

@@ -30,9 +30,9 @@ use OC\Contacts\ContactsMenu\Providers\EMailProvider;
 use OCP\App\IAppManager;
 use OCP\AppFramework\QueryException;
 use OCP\Contacts\ContactsMenu\IProvider;
-use OCP\ILogger;
 use OCP\IServerContainer;
 use OCP\IUser;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class ActionProviderStoreTest extends TestCase {
@@ -43,7 +43,7 @@ class ActionProviderStoreTest extends TestCase {
 	/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
 	private $appManager;
 
-	/** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
 	private $logger;
 
 	/** @var ActionProviderStore */
@@ -54,7 +54,7 @@ class ActionProviderStoreTest extends TestCase {
 
 		$this->serverContainer = $this->createMock(IServerContainer::class);
 		$this->appManager = $this->createMock(AppManager::class);
-		$this->logger = $this->createMock(ILogger::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->actionProviderStore = new ActionProviderStore($this->serverContainer, $this->appManager, $this->logger);
 	}