diff --git a/lib/Controller/StackApiController.php b/lib/Controller/StackApiController.php index c8ced0795..4cc6a2f6e 100644 --- a/lib/Controller/StackApiController.php +++ b/lib/Controller/StackApiController.php @@ -64,15 +64,8 @@ class StackApiController extends ApiController { * * Return all of the stacks in the specified board. */ - public function index() { - $boardError = $this->apiHelper->entityHasError($this->request->getParam('boardId'), 'board', $this->boardService); - - if ($boardError) { - return new DataResponse($boardError['message'], $boardError['status']); - } - + public function index() { $stacks = $this->stackService->findAll($this->request->getParam('boardId')); - return new DataResponse($stacks, HTTP::STATUS_OK); } @@ -83,23 +76,8 @@ class StackApiController extends ApiController { * * Return all of the stacks in the specified board. */ - public function get() { - if (is_numeric($this->request->getParam('stackId')) === false) { - return new DataResponse('stack id must be a number', HTTP::STATUS_BAD_REQUEST); - } - - $boardError = $this->apiHelper->entityHasError($this->request->getParam('boardId'), 'board', $this->boardService); - - if ($boardError) { - return new DataResponse($boardError['message'], $boardError['status']); - } - + public function get() { $stack = $this->stackService->find($this->request->getParam('stackId')); - - if ($stack === false || $stack === null) { - return new DataResponse('stack not found', HTTP::STATUS_NOT_FOUND); - } - return new DataResponse($stack, HTTP::STATUS_OK); } @@ -113,24 +91,8 @@ class StackApiController extends ApiController { * * Create a stack with the specified title and order. */ - public function create($title, $order) { - - if ($title === false || $title === null) { - return new DataResponse('title must be provided', HTTP::STATUS_BAD_REQUEST); - } - - if (is_numeric($order) === false) { - return new DataResponse('order must be a number', HTTP::STATUS_BAD_REQUEST); - } - - $boardError = $this->apiHelper->entityHasError( $this->request->getParam('boardId'), 'board', $this->boardService ); - - if ($boardError) { - return new DataResponse($boardError['message'], $boardError['status']); - } - - $stack = $this->stackService->create($title, $this->request->getParam('boardId'), $order); - + public function create($title, $order) { + $stack = $this->stackService->create($title, $this->request->getParam('boardId'), $order); return new DataResponse($stack, HTTP::STATUS_OK); } @@ -144,33 +106,9 @@ class StackApiController extends ApiController { * * Update a stack by the specified stackId and boardId with the values that were put. */ - public function update($title, $order) { - - $boardError = $this->apiHelper->entityHasError( $this->request->getParam('boardId'), 'board', $this->boardService ); - - if ($boardError) { - return new DataResponse($boardError['message'], $boardError['status']); - } - - if (is_numeric($this->request->getParam('stackId')) === false) { - return new DataResponse('stack id must be a number', HTTP::STATUS_BAD_REQUEST); - } - - if (is_numeric($order) === false) { - return new DataResponse('order must be a number', HTTP::STATUS_BAD_REQUEST); - } - - try { - $stack = $this->stackService->update($this->request->getParam('stackId'), $title, $this->request->getParam('boardId'), $order); - - if ($stack === false || $stack === null) { - return new DataResponse('stack not found', HTTP::STATUS_NOT_FOUND); - } - - return new DataResponse($stack, HTTP::STATUS_OK); - } catch (StatusException $e) { - return new DataResponse($e->getMessage(), HTTP::STATUS_INTERNAL_SERVER_ERROR); - } + public function update($title, $order) { + $stack = $this->stackService->update($this->request->getParam('stackId'), $title, $this->request->getParam('boardId'), $order); + return new DataResponse($stack, HTTP::STATUS_OK); } /** @@ -180,24 +118,8 @@ class StackApiController extends ApiController { * * Delete the stack specified by $this->request->getParam('stackId'). */ - public function delete() { - - $boardError = $this->apiHelper->entityHasError( $this->request->getParam('boardId'), 'board', $this->boardService ); - - if ($boardError) { - return new DataResponse($boardError['message'], $boardError['status']); - } - - if (is_numeric($this->request->getParam('stackId')) === false) { - return new DataResponse('stack id must be a number', HTTP::STATUS_BAD_REQUEST); - } - - $stack = $this->stackService->delete($this->request->getParam('stackId')); - - if ($stack == false || $stack == null) { - return new DataResponse('Stack Not Found', HTTP::STATUS_NOT_FOUND); - } - + public function delete() { + $stack = $this->stackService->delete($this->request->getParam('stackId')); return new DataResponse($stack, HTTP::STATUS_OK); } } diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index ae0850dae..152c28258 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\AssignedUsersMapper; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; use OCA\Deck\StatusException; +use OCA\Deck\BadRequestException; use OCP\ICache; use OCP\ICacheFactory; @@ -94,8 +95,13 @@ class StackService { * @return \OCP\AppFramework\Db\Entity * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws BadRequestException */ public function find($stackId) { + if (is_numeric($stackId) === false) { + throw new BadRequestException('stack id must be a number'); + } + $stack = $this->stackMapper->find($stackId); $cards = $this->cardMapper->findAll($stackId); foreach ($cards as $cardIndex => $card) { @@ -111,8 +117,13 @@ class StackService { * @param $boardId * @return array * @throws \OCA\Deck\NoPermissionException + * @throws BadRequestException */ public function findAll($boardId) { + if (is_numeric($boardId) === false) { + throw new BadRequestException('boardId must be a number'); + } + $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ); $stacks = $this->stackMapper->findAll($boardId); $this->enrichStacksWithCards($stacks); @@ -156,8 +167,22 @@ class StackService { * @throws \OCA\Deck\NoPermissionException * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws BadRequestException */ public function create($title, $boardId, $order) { + + if ($title === false || $title === null) { + throw new BadRequestException('title must be provided'); + } + + if (is_numeric($order) === false) { + throw new BadRequestException('order must be a number'); + } + + if (is_numeric($boardId) === false) { + throw new BadRequestException('board id must be a number'); + } + $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE); if ($this->boardService->isArchived(null, $boardId)) { throw new StatusException('Operation not allowed. This board is archived.'); @@ -176,8 +201,14 @@ class StackService { * @throws \OCA\Deck\NoPermissionException * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws BadRequestException */ public function delete($id) { + + if ( is_numeric($id) === false ) { + throw new BadRequestException('stack id must be a number'); + } + $this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE); $stack = $this->stackMapper->find($id); @@ -200,8 +231,26 @@ class StackService { * @throws \OCA\Deck\NoPermissionException * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws BadRequestException */ public function update($id, $title, $boardId, $order, $deletedAt) { + + if (is_numeric($id) === false) { + throw new BadRequestException('stack id must be a number'); + } + + if ($title === false || $title === null) { + throw new BadRequestException('title must be provided'); + } + + if (is_numeric($boardId) === false) { + throw new BadRequestException('board id must be a number'); + } + + if (is_numeric($order) === false) { + throw new BadRequestException('order must be a number'); + } + $this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE); if ($this->boardService->isArchived($this->stackMapper, $id)) { throw new StatusException('Operation not allowed. This board is archived.'); diff --git a/tests/unit/controller/StackApiControllerTest.php b/tests/unit/controller/StackApiControllerTest.php index 047ed88f7..5f844845d 100644 --- a/tests/unit/controller/StackApiControllerTest.php +++ b/tests/unit/controller/StackApiControllerTest.php @@ -67,13 +67,7 @@ class StackApiControllerTest extends \Test\TestCase { $stack->setId($this->exampleStack['id']); $stack->setBoardId($this->exampleStack['boardId']); $stack->setOrder($this->exampleStack['order']); - $stacks = [$stack]; - - $board = new Board(); - $board->setId($this->exampleBoard['boardId']); - $this->boardService->expects($this->once()) - ->method('find') - ->willReturn($board); + $stacks = [$stack]; $this->stackService->expects($this->once()) ->method('findAll') @@ -89,139 +83,28 @@ class StackApiControllerTest extends \Test\TestCase { $this->assertEquals($expected, $actual); } - public function testIndexBadBoardId() { - $this->request->expects($this->any()) - ->method('getParam') - ->with('boardId') - ->will($this->returnValue('bad board id')); - - $expected = new DataResponse('board id must be a number', HTTP::STATUS_BAD_REQUEST); - $actual = $this->controller->index(); - $this->assertEquals($expected, $actual); - } - - public function testIndexBoardNotFound() { - $this->request->expects($this->any()) - ->method('getParam') - ->with('boardId') - ->will($this->returnValue(689)); - - $expected = new DataResponse('board not found', HTTP::STATUS_NOT_FOUND); - $actual = $this->controller->index(); - $this->assertEquals($expected, $actual); - } - public function testGet() { $stack = new Stack(); $stack->setId($this->exampleStack['id']); $stack->setBoardId($this->exampleStack['boardId']); - $stack->setOrder($this->exampleStack['order']); - - $board = new Board(); - $board->setId($this->exampleBoard['boardId']); - $this->boardService->expects($this->once()) - ->method('find') - ->willReturn($board); + $stack->setOrder($this->exampleStack['order']); $this->stackService->expects($this->once()) ->method('find') ->willReturn($stack); - $this->request->expects($this->exactly(3)) + $this->request->expects($this->once()) ->method('getParam') - ->withConsecutive( - ['stackId'], - ['boardId'], - ['stackId'] - ) - ->willReturnOnConsecutiveCalls( - $this->returnValue($this->exampleStack['id']), - $this->returnValue($this->exampleBoard['boardId']), - $this->returnValue($this->exampleStack['id'])); + ->with('stackId') + ->willReturn($this->exampleStack['id']); $expected = new DataResponse($stack, HTTP::STATUS_OK); $actual = $this->controller->get(); $this->assertEquals($expected, $actual); - } - - public function testGetBadBoardId() { - - $this->request->expects($this->exactly(2)) - ->method('getParam') - ->withConsecutive( - ['stackId'], - ['boardId'] - ) - ->willReturnOnConsecutiveCalls( - $this->returnValue(5), - $this->returnValue('not a board id')); - - $expected = new DataResponse('board id must be a number', HTTP::STATUS_BAD_REQUEST); - $actual = $this->controller->get(); - $this->assertEquals($expected, $actual); - } - - public function testGetBoardNotFound() { - $this->request->expects($this->exactly(2)) - ->method('getParam') - ->withConsecutive( - ['stackId'], - ['boardId'] - ) - ->willReturnOnConsecutiveCalls( - $this->returnValue(5), - $this->returnValue($this->exampleBoard['boardId'])); - - - $expected = new DataResponse('board not found', HTTP::STATUS_NOT_FOUND); - $actual = $this->controller->get(); - $this->assertEquals($expected, $actual); - } - - public function testGetStackBadStackId() { - - $this->request->expects($this->any()) - ->method('getParam') - ->with('stackId') - ->will($this->returnValue('not a stack id')); - - $expected = new DataResponse('stack id must be a number', HTTP::STATUS_BAD_REQUEST); - $actual = $this->controller->get(); - $this->assertEquals($expected, $actual); - } - - public function testGetStackNotFound() { - - $board = new Board(); - $board->setId($this->exampleBoard['boardId']); - $this->boardService->expects($this->once()) - ->method('find') - ->willReturn($board); - - $this->request->expects($this->exactly(3)) - ->method('getParam') - ->withConsecutive( - ['stackId'], - ['boardId'], - ['stackId'] - ) - ->willReturnOnConsecutiveCalls( - $this->returnValue(5), - $this->returnValue($this->exampleBoard['boardId']), - $this->returnValue(5)); - - $expected = new DataResponse('stack not found', HTTP::STATUS_NOT_FOUND); - $actual = $this->controller->get(); - $this->assertEquals($expected, $actual); - } + } public function testCreate() { - $board = new Board(); - $board->setId($this->exampleBoard['boardId']); - $this->boardService->expects($this->once()) - ->method('find') - ->willReturn($board); $this->request->expects($this->any()) ->method('getParam') ->with('boardId') @@ -240,29 +123,5 @@ class StackApiControllerTest extends \Test\TestCase { $expected = new DataResponse($stack, HTTP::STATUS_OK); $actual = $this->controller->create($this->exampleStack['title'], $this->exampleStack['order']); $this->assertEquals($expected, $actual); - } - - public function testCreateBadOrder() { - $expected = new DataResponse('order must be a number', HTTP::STATUS_BAD_REQUEST); - $actual = $this->controller->create($this->exampleStack['title'], 'not an order number'); - $this->assertEquals($expected, $actual); - } - - public function testCreateBadTitle() { - $expected = new DataResponse('title must be provided', HTTP::STATUS_BAD_REQUEST); - $actual = $this->controller->create(null, 999); - $this->assertEquals($expected, $actual); - } - - public function testCreateBoardNotFound() { - - $this->request->expects($this->once()) - ->method('getParam') - ->will($this->returnValue(453)); - - $expected = new DataResponse('board not found', HTTP::STATUS_NOT_FOUND); - $actual = $this->controller->create($this->exampleStack['title'], $this->exampleStack['order']); - $this->assertEquals($expected, $actual); - } - + } } \ No newline at end of file