Merge pull request #259 from nextcloud/fix/noid/29-adjustments

use promoted arguments, switching to IAppConfig
nextcloud · Mar 13, 2024 · 688ccf3 · 688ccf3
2 parents 741be16 + a893c35
commit 688ccf3
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 89 deletions.
diff --git a/lib/Controller/LogoutController.php b/lib/Controller/LogoutController.php
@@ -14,38 +14,25 @@
 use Psr\Log\LoggerInterface;
 
 class LogoutController extends Controller {
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IUserSession */
-	private $userSession;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var ISession */
-	private $session;
-	/** @var IEventDispatcher */
-	private $eventDispatcher;
-
 	/**
 	 * @param string $appName
 	 * @param IRequest $request
 	 * @param IUserManager $userManager
 	 * @param IUserSession $userSession
 	 * @param ISession $session
 	 * @param LoggerInterface $logger
+	 * @param IEventDispatcher $eventDispatcher
 	 */
-	public function __construct($appName,
-		IRequest $request,
-		IUserManager $userManager,
-		IUserSession $userSession,
-		ISession $session,
-		LoggerInterface $logger,
-		IEventDispatcher $eventDispatcher) {
+	public function __construct(
+		string                   $appName,
+		IRequest                 $request,
+		private IUserManager     $userManager,
+		private IUserSession     $userSession,
+		private ISession         $session,
+		private LoggerInterface  $logger,
+		private IEventDispatcher $eventDispatcher
+	) {
 		parent::__construct($appName, $request);
-		$this->userManager = $userManager;
-		$this->userSession = $userSession;
-		$this->session = $session;
-		$this->logger = $logger;
-		$this->eventDispatcher = $eventDispatcher;
 	}
 
 	/**

diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php
@@ -11,13 +11,13 @@
 
 namespace OCA\Impersonate\Controller;
 
-use OC\Group\Manager;
 use OCA\Impersonate\Events\BeginImpersonateEvent;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\IConfig;
+use OCP\Exceptions\AppConfigTypeConflictException;
+use OCP\IAppConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -28,47 +28,25 @@
 use Psr\Log\LoggerInterface;
 
 class SettingsController extends Controller {
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IGroupManager|Manager */
-	private $groupManager;
-	/** @var IUserSession */
-	private $userSession;
-	/** @var ISession */
-	private $session;
-	/** @var IConfig */
-	private $config;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var IL10N */
-	private $l;
-	/** @var IEventDispatcher */
-	private $eventDispatcher;
-
-	public function __construct(string $appName,
+	public function __construct(
+		string $appName,
 		IRequest $request,
-		IUserManager $userManager,
-		IGroupManager $groupManager,
-		IUserSession $userSession,
-		ISession $session,
-		IConfig $config,
-		LoggerInterface $logger,
-		IL10N $l,
-		IEventDispatcher $eventDispatcher) {
+		protected IUserManager $userManager,
+		protected IGroupManager $groupManager,
+		protected IUserSession $userSession,
+		protected ISession $session,
+		protected IAppConfig $config,
+		protected LoggerInterface $logger,
+		protected IL10N $l,
+		protected IEventDispatcher $eventDispatcher
+	) {
 		parent::__construct($appName, $request);
-		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
-		$this->userSession = $userSession;
-		$this->session = $session;
-		$this->config = $config;
-		$this->logger = $logger;
-		$this->l = $l;
-		$this->eventDispatcher = $eventDispatcher;
 	}
 
 	/**
 	 * @UseSession
 	 * @NoAdminRequired
+	 * @throws AppConfigTypeConflictException
 	 */
 	public function impersonate(string $userId): JSONResponse {
 		/** @var IUser $impersonator */
@@ -105,7 +83,7 @@ public function impersonate(string $userId): JSONResponse {
 			);
 		}
 
-		$authorized = json_decode($this->config->getAppValue('impersonate', 'authorized', '["admin"]'));
+		$authorized = json_decode($this->config->getValueString('impersonate', 'authorized', '["admin"]'));
 		if (!empty($authorized)) {
 			$userGroups = $this->groupManager->getUserGroupIds($impersonator);
 

diff --git a/tests/unit/Controller/SettingsControllerTest.php b/tests/unit/Controller/SettingsControllerTest.php
@@ -18,7 +18,7 @@
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\EventDispatcher\IEventDispatcher;
-use OCP\IConfig;
+use OCP\IAppConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -36,41 +36,29 @@
  */
 
 class SettingsControllerTest extends TestCase {
-	/** @var string */
-	private $appName;
-	/** @var IRequest|MockObject */
-	private $request;
-	/** @var IUserManager|MockObject */
-	private $userManager;
-	/** @var IGroupManager|MockObject */
-	private $groupManager;
-	/** @var SubAdmin|MockObject */
-	private $subadmin;
-	/** @var IUserSession|MockObject */
-	private $userSession;
-	/** @var ISession|MockObject */
-	private $session;
-	/** @var IConfig|MockObject */
-	private $config;
-	/** @var LoggerInterface|MockObject */
-	private $logger;
-	/** @var IL10N|MockObject */
-	private $l;
-	/** @var SettingsController */
-	private $controller;
+	private string $appName;
+	private IUserManager|MockObject $userManager;
+	private IGroupManager|MockObject $groupManager;
+	private SubAdmin|MockObject $subadmin;
+	private IUserSession|MockObject $userSession;
+	private ISession|MockObject $session;
+	private IAppConfig|MockObject $config;
+	private LoggerInterface|MockObject $logger;
+	private IL10N|MockObject $l;
+	private SettingsController $controller;
 	/** @var IEventDispatcher|IEventDispatcher&MockObject|MockObject */
 	private $eventDispatcher;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->appName = 'impersonate';
-		$this->request = $this->createMock(IRequest::class);
+		$request = $this->createMock(IRequest::class);
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->groupManager = $this->createMock(Manager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->session = $this->createMock(ISession::class);
-		$this->config = $this->createMock(IConfig::class);
+		$this->config = $this->createMock(IAppConfig::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->l = $this->createMock(IL10N::class);
 		$this->l->expects($this->any())
@@ -86,7 +74,7 @@ protected function setUp(): void {
 
 		$this->controller = new SettingsController(
 			$this->appName,
-			$this->request,
+			$request,
 			$this->userManager,
 			$this->groupManager,
 			$this->userSession,
@@ -165,7 +153,7 @@ public function testAdminImpersonate($query, $uid) {
 			->willReturn(['admin']);
 
 		$this->config->expects($this->once())
-			->method('getAppValue')
+			->method('getValueString')
 			->with('impersonate', 'authorized', '["admin"]')
 			->willReturnArgument(2);
 
@@ -227,7 +215,7 @@ public function testSubAdminImpersonate($query, $uid) {
 			->willReturn(['subadmin']);
 
 		$this->config->expects($this->once())
-			->method('getAppValue')
+			->method('getValueString')
 			->with('impersonate', 'authorized', '["admin"]')
 			->willReturn(json_encode(['admin', 'subadmin']));
 
@@ -289,7 +277,7 @@ public function testSubAdminImpersonateNotAllowed($query, $uid) {
 			->willReturn(['subadmin']);
 
 		$this->config->expects($this->once())
-			->method('getAppValue')
+			->method('getValueString')
 			->with('impersonate', 'authorized', '["admin"]')
 			->willReturnArgument(2);