refactor: Fix psalm issues

- Add typing for most of the services, controllers and mappers
- Add api doc for mappers
- Use vendor-bin for psalm
- Use attributes for controllers
- Fix upload of attachments

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>

tests/unit/controller/BoardControllerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
@@ -26,46 +28,47 @@ namespace OCA\Deck\Controller;
 
 use OCA\Deck\Db\Acl;
 use OCA\Deck\Db\Board;
+use OCA\Deck\Service\BoardService;
+use OCA\Deck\Service\Importer\BoardImportService;
+use OCA\Deck\Service\PermissionService;
+use OCP\IGroupManager;
+use OCP\IL10N;
+use OCP\IRequest;
 use OCP\IUser;
+use OCP\IUserManager;
+use PHPUnit\Framework\MockObject\MockObject;
 
 class BoardControllerTest extends \Test\TestCase {
-	private $l10n;
-	private $controller;
-	private $request;
-	private $userManager;
-	private $groupManager;
-	private $boardService;
-	private $permissionService;
-	private $boardImportService;
+	private IL10N&MockObject $l10n;
+	private BoardController $controller;
+	private IRequest&MockObject $request;
+	private IUserManager&MockObject $userManager;
+	private IGroupManager&MockObject $groupManager;
+	private BoardService&MockObject $boardService;
+	private PermissionService&MockObject $permissionService;
+	private BoardImportService&MockObject $boardImportService;
 	private $userId = 'user';
 
 	public function setUp(): void {
-		$this->l10n = $this->request = $this->getMockBuilder(
-			'\OCP\IL10n')
+		$this->l10n = $this->getMockBuilder(IL10N::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->request = $this->getMockBuilder(
-			'\OCP\IRequest')
+		$this->request = $this->getMockBuilder(IRequest::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->userManager = $this->getMockBuilder(
-			'\OCP\IUserManager')
+		$this->userManager = $this->getMockBuilder(IUserManager::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->groupManager = $this->getMockBuilder(
-			'\OCP\IGroupManager')
+		$this->groupManager = $this->getMockBuilder(IGroupManager::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->boardService = $this->getMockBuilder(
-			'\OCA\Deck\Service\BoardService')
+		$this->boardService = $this->getMockBuilder(BoardService::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->permissionService = $this->getMockBuilder(
-			'\OCA\Deck\Service\PermissionService')
+		$this->permissionService = $this->getMockBuilder(PermissionService::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->boardImportService = $this->getMockBuilder(
-			'\OCA\Deck\Service\Importer\BoardImportService')
+		$this->boardImportService = $this->getMockBuilder(BoardImportService::class)
 			->disableOriginalConstructor()
 			->getMock();
 
@@ -107,35 +110,39 @@ class BoardControllerTest extends \Test\TestCase {
 	}
 
 	public function testCreate() {
+		$board = $this->createMock(Board::class);
 		$this->boardService->expects($this->once())
 			->method('create')
-			->with(1, 'user', 3)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->create(1, 3));
+			->with('abc', 'user', 'green')
+			->willReturn($board);
+		$this->assertEquals($board, $this->controller->create('abc', 'green'));
 	}
 
-	public function testUpdate() {
+	public function testUpdate(): void {
+		$board = $this->createMock(Board::class);
 		$this->boardService->expects($this->once())
 			->method('update')
-			->with(1, 2, 3, false)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 2, 3, false));
+			->with(1, 'abc', 'green', false)
+			->willReturn($board);
+		$this->assertEquals($board, $this->controller->update(1, 'abc', 'green', false));
 	}
 
-	public function testDelete() {
+	public function testDelete(): void {
+		$board = $this->createMock(Board::class);
 		$this->boardService->expects($this->once())
 			->method('delete')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->delete(123));
+			->willReturn($board);
+		$this->assertEquals($board, $this->controller->delete(123));
 	}
 
 	public function testDeleteUndo() {
+		$board = $this->createMock(Board::class);
 		$this->boardService->expects($this->once())
 			->method('deleteUndo')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->deleteUndo(123));
+			->willReturn($board);
+		$this->assertEquals($board, $this->controller->deleteUndo(123));
 	}
 
 	public function testGetUserPermissions() {
@@ -158,20 +165,22 @@ class BoardControllerTest extends \Test\TestCase {
 		$this->assertEquals($expected, $this->controller->getUserPermissions(123));
 	}
 
-	public function testAddAcl() {
+	public function testAddAcl(): void {
+		$acl = $this->createMock(Acl::class);
 		$this->boardService->expects($this->once())
 			->method('addAcl')
-			->with(1, 2, 3, 4, 5, 6)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->addAcl(1, 2, 3, 4, 5, 6));
+			->with(1, 2, 'user1', true, true, true)
+			->willReturn($acl);
+		$this->assertEquals($acl, $this->controller->addAcl(1, 2, 'user1', true, true, true));
 	}
 
-	public function testUpdateAcl() {
+	public function testUpdateAcl(): void {
+		$acl = $this->createMock(Acl::class);
 		$this->boardService->expects($this->once())
 			->method('updateAcl')
-			->with(1, 2, 3, 4)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->updateAcl(1, 2, 3, 4));
+			->with(1, true, true, true)
+			->willReturn($acl);
+		$this->assertEquals($acl, $this->controller->updateAcl(1, true, true, true));
 	}
 
 	public function testDeleteAcl() {

tests/unit/controller/CardApiControllerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2018 Ryan Fletcher <ryan.fletcher@codepassion.ca>
  *
@@ -30,15 +32,16 @@ use OCP\AppFramework\Http;
 
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IRequest;
+use PHPUnit\Framework\MockObject\MockObject;
 
 class CardApiControllerTest extends \Test\TestCase {
-	private $controller;
-	private $request;
-	private $cardService;
-	private $userId = 'admin';
-	private $cardExample;
-	private $stackExample;
-	private $assignmentService;
+	private CardApiController $controller;
+	private IRequest&MockObject $request;
+	private CardService&MockObject $cardService;
+	private string $userId = 'admin';
+	private array $cardExample;
+	private array $stackExample;
+	private AssignmentService&MockObject $assignmentService;
 
 	public function setUp(): void {
 		parent::setUp();
@@ -51,7 +54,7 @@ class CardApiControllerTest extends \Test\TestCase {
 		$this->stackExample['id'] = 1;
 
 		$this->controller = new CardApiController(
-			$appName = 'deck',
+			'deck',
 			$this->request,
 			$this->cardService,
 			$this->assignmentService,
@@ -59,7 +62,7 @@ class CardApiControllerTest extends \Test\TestCase {
 		);
 	}
 
-	public function testGet() {
+	public function testGet(): void {
 		$card = new Card();
 		$card->setId($this->cardExample['id']);
 
@@ -116,7 +119,7 @@ class CardApiControllerTest extends \Test\TestCase {
 			->willReturn($card);
 
 		$expected = new DataResponse($card, HTTP::STATUS_OK);
-		$actual = $this->controller->update('title', 'plain', 0, 'description', $this->userId, null);
+		$actual = $this->controller->update('title', 'plain', $this->userId, 'description', 0, null);
 		$this->assertEquals($expected, $actual);
 	}
 

tests/unit/controller/CardControllerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
@@ -24,6 +26,7 @@
 
 namespace OCA\Deck\Controller;
 
+use OCA\Deck\Db\Card;
 use OCA\Deck\Service\AssignmentService;
 use OCA\Deck\Service\CardService;
 use OCP\IRequest;
@@ -32,17 +35,11 @@ use Test\TestCase;
 
 class CardControllerTest extends TestCase {
 
-	/** @var CardController|MockObject */
-	private $controller;
-	/** @var IRequest|MockObject */
-	private $request;
-	/** @var CardService|MockObject */
-	private $cardService;
-	/** @var AssignmentService|MockObject */
-	private $assignmentService;
-	/** @var string */
-	private $userId = 'user';
-
+	private CardController $controller;
+	private IRequest&MockObject $request;
+	private CardService&MockObject $cardService;
+	private AssignmentService&MockObject $assignmentService;
+	private string $userId = 'user';
 
 	public function setUp(): void {
 		$this->request = $this->createMock(IRequest::class);
@@ -58,39 +55,43 @@ class CardControllerTest extends TestCase {
 	}
 
 	public function testRead() {
+		$card = $this->createMock(Card::class);
 		$this->cardService->expects($this->once())
 			->method('find')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->read(123));
+			->willReturn($card);
+		$this->assertEquals($card, $this->controller->read(123));
 	}
 
-	public function testCreate() {
+	public function testCreate(): void {
+		$card = $this->createMock(Card::class);
 		$this->cardService->expects($this->once())
 			->method('create')
 			->with('foo', 1, 'text', 3, $this->userId)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->create('foo', 1, 'text', 3));
+			->willReturn($card);
+		$this->assertEquals($card, $this->controller->create('foo', 1, 'text', 3));
 	}
 
-	public function testUpdate() {
+	public function testUpdate(): void {
+		$card = $this->createMock(Card::class);
 		$this->cardService->expects($this->once())
 			->method('update')
 			->with(1, 'title', 3, 'text', $this->userId, 'foo', 5, '2017-01-01 00:00:00')
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 'title', 3, 'text', 5, 'foo', '2017-01-01 00:00:00', null));
+			->willReturn($card);
+		$this->assertEquals($card, $this->controller->update(1, 'title', 3, 'text', 5, 'foo', '2017-01-01 00:00:00', null));
 	}
 
-	public function testDelete() {
+	public function testDelete(): void {
+		$card = $this->createMock(Card::class);
 		$this->cardService->expects($this->once())
 			->method('delete')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->delete(123));
+			->willReturn($card);
+		$this->assertEquals($card, $this->controller->delete(123));
 	}
 
 	public function testArchive() {
-		$this->cardService->expects($this->once())->method('archive')->willReturn(true);
+		$this->cardService->expects($this->once())->method('archive');
 		$this->controller->archive(1);
 	}
 	public function testUnarchive() {

tests/unit/controller/LabelControllerTest.php

@@ -24,6 +24,7 @@
 
 namespace OCA\Deck\Controller;
 
+use OCA\Deck\Db\Label;
 use OCA\Deck\Service\LabelService;
 use OCP\AppFramework\Controller;
 use OCP\IRequest;
@@ -56,27 +57,30 @@ class LabelControllerTest extends \Test\TestCase {
 	}
 
 
-	public function testCreate() {
+	public function testCreate(): void {
+		$label = $this->createMock(Label::class);
 		$this->labelService->expects($this->once())
 			->method('create')
 			->with(1, 2, 3)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->create(1, 2, 3));
+			->willReturn($label);
+		$this->assertEquals($label, $this->controller->create(1, 2, 3));
 	}
 
-	public function testUpdate() {
+	public function testUpdate(): void {
+		$label = $this->createMock(Label::class);
 		$this->labelService->expects($this->once())
 			->method('update')
 			->with(1, 2, 3)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 2, 3));
+			->willReturn($label);
+		$this->assertEquals($label, $this->controller->update(1, 2, 3));
 	}
 
-	public function testDelete() {
+	public function testDelete(): void {
+		$label = $this->createMock(Label::class);
 		$this->labelService->expects($this->once())
 			->method('delete')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->delete(123));
+			->willReturn($label);
+		$this->assertEquals($label, $this->controller->delete(123));
 	}
 }

tests/unit/controller/StackControllerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Julius Härtl <jus@bitgrid.net>
  *
@@ -24,6 +26,7 @@
 
 namespace OCA\Deck\Controller;
 
+use OCA\Deck\Db\Stack;
 use OCA\Deck\Service\StackService;
 use OCP\AppFramework\Controller;
 use OCP\IRequest;
@@ -67,34 +70,38 @@ class StackControllerTest extends \Test\TestCase {
 	}
 
 	public function testCreate() {
+		$stack = $this->createMock(Stack::class);
 		$this->stackService->expects($this->once())
 			->method('create')
-			->with(1, 2, 3)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->create(1, 2, 3));
+			->with('abc', 2, 3)
+			->willReturn($stack);
+		$this->assertEquals($stack, $this->controller->create('abc', 2, 3));
 	}
 
 	public function testUpdate() {
+		$stack = $this->createMock(Stack::class);
 		$this->stackService->expects($this->once())
 			->method('update')
-			->with(1, 2, 3, 4)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->update(1, 2, 3, 4, null));
+			->with(1, 'abc', 3, 4)
+			->willReturn($stack);
+		$this->assertEquals($stack, $this->controller->update(1, 'abc', 3, 4, null));
 	}
 
 	public function testReorder() {
+		$stack = $this->createMock(Stack::class);
 		$this->stackService->expects($this->once())
 			->method('reorder')
 			->with(1, 2)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->reorder(1, 2));
+			->willReturn([$stack]);
+		$this->assertEquals([$stack], $this->controller->reorder(1, 2));
 	}
 
 	public function testDelete() {
+		$stack = $this->createMock(Stack::class);
 		$this->stackService->expects($this->once())
 			->method('delete')
 			->with(123)
-			->willReturn(1);
-		$this->assertEquals(1, $this->controller->delete(123));
+			->willReturn($stack);
+		$this->assertEquals($stack, $this->controller->delete(123));
 	}
 }