diff options
author | Bernhard Posselt <nukeawhale@gmail.com> | 2013-05-02 21:40:10 +0400 |
---|---|---|
committer | Bernhard Posselt <nukeawhale@gmail.com> | 2013-05-02 21:40:10 +0400 |
commit | 5ae697ac9dbaf999d51fa7805078249f33c301dc (patch) | |
tree | a4f408c9462ff4353820e266efad4864043d08f2 | |
parent | e9878cb5b583bc993a9f3a482d0b371bbeea5bd2 (diff) |
added proper exception handling for all controllers and businesslayer
-rw-r--r-- | businesslayer/businesslayerexistsexception.php | 39 | ||||
-rw-r--r-- | businesslayer/feedbusinesslayer.php | 12 | ||||
-rw-r--r-- | businesslayer/folderbusinesslayer.php | 11 | ||||
-rw-r--r-- | businesslayer/itembusinesslayer.php | 28 | ||||
-rw-r--r-- | controller/feedcontroller.php | 19 | ||||
-rw-r--r-- | controller/foldercontroller.php | 25 | ||||
-rw-r--r-- | controller/itemcontroller.php | 37 | ||||
-rw-r--r-- | external/folder.php | 59 | ||||
-rw-r--r-- | tests/unit/businesslayer/ItemBusinessLayerTest.php | 11 | ||||
-rw-r--r-- | tests/unit/controller/FeedControllerTest.php | 49 | ||||
-rw-r--r-- | tests/unit/controller/FolderControllerTest.php | 70 | ||||
-rw-r--r-- | tests/unit/controller/ItemControllerTest.php | 102 |
12 files changed, 360 insertions, 102 deletions
diff --git a/businesslayer/businesslayerexistsexception.php b/businesslayer/businesslayerexistsexception.php new file mode 100644 index 000000000..40a483528 --- /dev/null +++ b/businesslayer/businesslayerexistsexception.php @@ -0,0 +1,39 @@ +<?php + +/** +* ownCloud - News +* +* @author Alessandro Cosentino +* @author Bernhard Posselt +* @copyright 2012 Alessandro Cosentino cosenal@gmail.com +* @copyright 2012 Bernhard Posselt nukeawhale@gmail.com +* +* This library is free software; you can redistribute it and/or +* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE +* License as published by the Free Software Foundation; either +* version 3 of the License, or any later version. +* +* This library is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU AFFERO GENERAL PUBLIC LICENSE for more details. +* +* You should have received a copy of the GNU Affero General Public +* License along with this library. If not, see <http://www.gnu.org/licenses/>. +* +*/ + +namespace OCA\News\BusinessLayer; + + +class BusinessLayerExistsException extends BusinessLayerException { + + /** + * Constructor + * @param string $msg the error message + */ + public function __construct($msg){ + parent::__construct($msg); + } + +}
\ No newline at end of file diff --git a/businesslayer/feedbusinesslayer.php b/businesslayer/feedbusinesslayer.php index aba234989..284147242 100644 --- a/businesslayer/feedbusinesslayer.php +++ b/businesslayer/feedbusinesslayer.php @@ -62,11 +62,15 @@ class FeedBusinessLayer extends BusinessLayer { } + /** + * @throws BusinessLayerExistsException if the feed exists already + * @throws BusinessLayerException if the url points to an invalid feed + */ public function create($feedUrl, $folderId, $userId){ // first try if the feed exists already try { $this->mapper->findByUrlHash(md5($feedUrl), $userId); - throw new BusinessLayerException( + throw new BusinessLayerExistsException( $this->api->getTrans()->t('Can not add feed: Exists already')); } catch(DoesNotExistException $ex){} @@ -123,6 +127,9 @@ class FeedBusinessLayer extends BusinessLayer { } + /** + * @throws BusinessLayerException if the feed does not exist + */ public function update($feedId, $userId){ try { $existingFeed = $this->mapper->find($feedId, $userId); @@ -179,6 +186,9 @@ class FeedBusinessLayer extends BusinessLayer { } + /** + * @throws BusinessLayerException if the feed does not exist + */ public function move($feedId, $folderId, $userId){ $feed = $this->find($feedId, $userId); $feed->setFolderId($folderId); diff --git a/businesslayer/folderbusinesslayer.php b/businesslayer/folderbusinesslayer.php index 916476896..a4c025b18 100644 --- a/businesslayer/folderbusinesslayer.php +++ b/businesslayer/folderbusinesslayer.php @@ -51,13 +51,13 @@ class FolderBusinessLayer extends BusinessLayer { $existingFolders = $this->mapper->findByName($folderName, $userId); if(count($existingFolders) > 0){ - throw new BusinessLayerException( + throw new BusinessLayerExistsException( $this->api->getTrans()->t('Can not add folder: Exists already')); } } /** - * @throws BusinessLayerException if name exists already + * @throws BusinessLayerExistsException if name exists already */ public function create($folderName, $userId, $parentId=0) { $this->allowNoNameTwice($folderName, $userId); @@ -70,7 +70,9 @@ class FolderBusinessLayer extends BusinessLayer { return $this->mapper->insert($folder); } - + /** + * @throws BusinessLayerException if the folder does not exist + */ public function open($folderId, $opened, $userId){ $folder = $this->find($folderId, $userId); $folder->setOpened($opened); @@ -79,7 +81,8 @@ class FolderBusinessLayer extends BusinessLayer { /** - * @throws BusinessLayerException if name exists already + * @throws BusinessLayerExistsException if name exists already + * @throws BusinessLayerException if the folder does not exist */ public function rename($folderId, $folderName, $userId){ $this->allowNoNameTwice($folderName, $userId); diff --git a/businesslayer/itembusinesslayer.php b/businesslayer/itembusinesslayer.php index 72323f413..176669971 100644 --- a/businesslayer/itembusinesslayer.php +++ b/businesslayer/itembusinesslayer.php @@ -93,19 +93,28 @@ class ItemBusinessLayer extends BusinessLayer { } + /** + * @throws BusinessLayerException if the item does not exist + */ public function star($feedId, $guidHash, $isStarred, $userId){ - // FIXME: this can throw two possible exceptions - $item = $this->mapper->findByGuidHash($guidHash, $feedId, $userId); - $item->setLastModified($this->timeFactory->getTime()); - if($isStarred){ - $item->setStarred(); - } else { - $item->setUnstarred(); + try { + $item = $this->mapper->findByGuidHash($guidHash, $feedId, $userId); + $item->setLastModified($this->timeFactory->getTime()); + if($isStarred){ + $item->setStarred(); + } else { + $item->setUnstarred(); + } + $this->mapper->update($item); + } catch(DoesNotExistException $ex) { + throw new BusinessLayerException($ex->getMessage()); } - $this->mapper->update($item); } + /** + * @throws BusinessLayerException if the item does not exist + */ public function read($itemId, $isRead, $userId){ $item = $this->find($itemId, $userId); $item->setLastModified($this->timeFactory->getTime()); @@ -134,6 +143,9 @@ class ItemBusinessLayer extends BusinessLayer { } + /** + * @throws BusinessLayerException if there is no newest item + */ public function getNewestItemId($userId) { try { return $this->mapper->getNewestItemId($userId); diff --git a/controller/feedcontroller.php b/controller/feedcontroller.php index d19008b5a..63a2c3122 100644 --- a/controller/feedcontroller.php +++ b/controller/feedcontroller.php @@ -140,7 +140,6 @@ class FeedController extends Controller { return $this->renderJSON($params); } catch(BusinessLayerException $ex) { - return $this->renderJSON(array(), $ex->getMessage()); } } @@ -155,9 +154,12 @@ class FeedController extends Controller { $feedId = (int) $this->params('feedId'); $userId = $this->api->getUserId(); - $this->feedBusinessLayer->delete($feedId, $userId); - - return $this->renderJSON(); + try { + $this->feedBusinessLayer->delete($feedId, $userId); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -202,9 +204,12 @@ class FeedController extends Controller { $parentFolderId = (int) $this->params('parentFolderId'); $userId = $this->api->getUserId(); - $this->feedBusinessLayer->move($feedId, $parentFolderId, $userId); - - return $this->renderJSON(); + try { + $this->feedBusinessLayer->move($feedId, $parentFolderId, $userId); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } diff --git a/controller/foldercontroller.php b/controller/foldercontroller.php index f7934ae67..f8d99410d 100644 --- a/controller/foldercontroller.php +++ b/controller/foldercontroller.php @@ -71,8 +71,12 @@ class FolderController extends Controller { * @Ajax */ public function open(){ - $this->setOpened(true); - return $this->renderJSON(); + try { + $this->setOpened(true); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -82,8 +86,12 @@ class FolderController extends Controller { * @Ajax */ public function collapse(){ - $this->setOpened(false); - return $this->renderJSON(); + try { + $this->setOpened(false); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -121,9 +129,12 @@ class FolderController extends Controller { $userId = $this->api->getUserId(); $folderId = (int) $this->params('folderId'); - $this->folderBusinessLayer->delete($folderId, $userId); - - return $this->renderJSON(); + try { + $this->folderBusinessLayer->delete($folderId, $userId); + return $this->renderJSON(); + } catch (BusinessLayerException $ex){ + return $this->renderJSON(array(), $ex->getMessage()); + } } diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php index a4d9abf3f..d19707f90 100644 --- a/controller/itemcontroller.php +++ b/controller/itemcontroller.php @@ -104,9 +104,12 @@ class ItemController extends Controller { * @Ajax */ public function star(){ - $this->setStarred(true); - - return $this->renderJSON(); + try { + $this->setStarred(true); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -116,9 +119,12 @@ class ItemController extends Controller { * @Ajax */ public function unstar(){ - $this->setStarred(false); - - return $this->renderJSON(); + try { + $this->setStarred(false); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -129,15 +135,19 @@ class ItemController extends Controller { $this->itemBusinessLayer->read($itemId, $isRead, $userId); } + /** * @IsAdminExemption * @IsSubAdminExemption * @Ajax */ public function read(){ - $this->setRead(true); - - return $this->renderJSON(); + try { + $this->setRead(true); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } @@ -147,9 +157,12 @@ class ItemController extends Controller { * @Ajax */ public function unread(){ - $this->setRead(false); - - return $this->renderJSON(); + try { + $this->setRead(false); + return $this->renderJSON(); + } catch(BusinessLayerException $ex) { + return $this->renderJSON(array(), $ex->getMessage()); + } } diff --git a/external/folder.php b/external/folder.php deleted file mode 100644 index 68c8ff523..000000000 --- a/external/folder.php +++ /dev/null @@ -1,59 +0,0 @@ -<?php - -namespace OCA\News; - -use \OCA\News\Controller\FolderController; - -class FolderApi { - - public function __construct($bl){ - $this->bl = $bl; - } - - public function getAll() { - $folders = $this->bl->getAll(); - $serializedFolders = array(); - - //TODO: check the behaviour for nested folders - foreach ($folders as $folder) { - $serializedFolders[] = $folder->jsonSerialize(); - } - return new \OC_OCS_Result($serializedFolders); - } - - public function create() { - $name = $_POST['name']; - $parentId = $_POST['parentid']; - - $this->bl->create($name, $parentId); - - return new \OC_OCS_Result(); - } - - public function delete($params) { - $id = $params['folderid']; - if(!is_numeric($id)) - return new \OC_OCS_Result(null,999,'Invalid input! folderid must be an integer'); - - if($this->bl->delete($id)) - return new \OC_OCS_Result(); - else - return new \OC_OCS_Result(null,999,'Could not delete folder'); - } - - public function modify($params) { - $id = $params['folderid']; - if(!is_numeric($id)) - return new \OC_OCS_Result(null,999,'Invalid input! folderid must be an integer'.$id); - - $name = $_POST['name']; - $parentId = $_POST['parentid']; - $opened = $_POST['opened']; - - if($this->bl->modify($id, $name, $parentid, $opened)) - return new \OC_OCS_Result(); - else - return new \OC_OCS_Result(null,999,'Could not modify folder'); - } -} - diff --git a/tests/unit/businesslayer/ItemBusinessLayerTest.php b/tests/unit/businesslayer/ItemBusinessLayerTest.php index e32a3b308..2094456b7 100644 --- a/tests/unit/businesslayer/ItemBusinessLayerTest.php +++ b/tests/unit/businesslayer/ItemBusinessLayerTest.php @@ -248,6 +248,17 @@ class ItemBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility { } + public function testStarDoesNotExist(){ + + $this->setExpectedException('\OCA\News\BusinessLayer\BusinessLayerException'); + $this->mapper->expects($this->once()) + ->method('findByGuidHash') + ->will($this->throwException(new DoesNotExistException(''))); + + $this->itemBusinessLayer->star(1, 'hash', true, $this->user); + } + + public function testReadFeed(){ $feedId = 3; $highestItemId = 6; diff --git a/tests/unit/controller/FeedControllerTest.php b/tests/unit/controller/FeedControllerTest.php index 14c31b545..403d9c3c8 100644 --- a/tests/unit/controller/FeedControllerTest.php +++ b/tests/unit/controller/FeedControllerTest.php @@ -348,6 +348,29 @@ class FeedControllerTest extends ControllerTestUtility { } + public function testDeleteDoesNotExist(){ + $url = array( + 'feedId' => 4 + ); + $msg = 'hehe'; + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->feedBusinessLayer->expects($this->once()) + ->method('delete') + ->will($this->throwException(new BusinessLayerException($msg))); + + $response = $this->controller->delete(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testUpdate(){ $feed = new Feed(); $feed->setId(3); @@ -433,6 +456,32 @@ class FeedControllerTest extends ControllerTestUtility { } + public function testMoveDoesNotExist(){ + $post = array( + 'parentFolderId' => 3 + ); + $url = array( + 'feedId' => 4 + ); + $msg = 'john'; + $this->controller = $this->getPostController($post, $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->feedBusinessLayer->expects($this->once()) + ->method('move') + ->will($this->throwException(new BusinessLayerException($msg))); + + $response = $this->controller->move(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testImportGoogleReader() { $feed = new Feed(); diff --git a/tests/unit/controller/FolderControllerTest.php b/tests/unit/controller/FolderControllerTest.php index b71c522e0..4b070a0bb 100644 --- a/tests/unit/controller/FolderControllerTest.php +++ b/tests/unit/controller/FolderControllerTest.php @@ -33,6 +33,7 @@ use \OCA\AppFramework\Db\MultipleObjectsReturnedException; use \OCA\News\Db\Folder; use \OCA\News\BusinessLayer\BusinessLayerException; +use \OCA\News\BusinessLayer\BusinessLayerExistsException; require_once(__DIR__ . "/../../classloader.php"); @@ -43,6 +44,7 @@ class FolderControllerTest extends ControllerTestUtility { private $folderBusinessLayer; private $request; private $controller; + private $msg; /** @@ -57,6 +59,7 @@ class FolderControllerTest extends ControllerTestUtility { $this->controller = new FolderController($this->api, $this->request, $this->folderBusinessLayer); $this->user = 'jack'; + $this->msg = 'ron'; } @@ -143,6 +146,27 @@ class FolderControllerTest extends ControllerTestUtility { } + public function testOpenDoesNotExist(){ + $url = array('folderId' => 5); + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->folderBusinessLayer->expects($this->once()) + ->method('open') + ->will($this->throwException(new BusinessLayerException($this->msg))); + + $response = $this->controller->open(); + + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($this->msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testCollapse(){ $url = array('folderId' => 5); $this->controller = $this->getPostController(array(), $url); @@ -161,6 +185,27 @@ class FolderControllerTest extends ControllerTestUtility { } + public function testCollapseDoesNotExist(){ + $url = array('folderId' => 5); + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->folderBusinessLayer->expects($this->once()) + ->method('open') + ->will($this->throwException(new BusinessLayerException($this->msg))); + + $response = $this->controller->collapse(); + + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($this->msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testCreate(){ $post = array('folderName' => 'tech'); $this->controller = $this->getPostController($post); @@ -186,7 +231,7 @@ class FolderControllerTest extends ControllerTestUtility { public function testCreateReturnsErrorForInvalidCreate(){ $msg = 'except'; - $ex = new BusinessLayerException($msg); + $ex = new BusinessLayerExistsException($msg); $this->folderBusinessLayer->expects($this->once()) ->method('create') ->will($this->throwException($ex)); @@ -218,6 +263,27 @@ class FolderControllerTest extends ControllerTestUtility { } + public function testDeleteDoesNotExist(){ + $url = array('folderId' => 5); + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->folderBusinessLayer->expects($this->once()) + ->method('delete') + ->will($this->throwException(new BusinessLayerException($this->msg))); + + $response = $this->controller->delete(); + + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($this->msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testRename(){ $post = array('folderName' => 'tech'); $url = array('folderId' => 4); @@ -245,7 +311,7 @@ class FolderControllerTest extends ControllerTestUtility { public function testRenameReturnsErrorForInvalidCreate(){ $msg = 'except'; - $ex = new BusinessLayerException($msg); + $ex = new BusinessLayerExistsException($msg); $this->folderBusinessLayer->expects($this->once()) ->method('rename') ->will($this->throwException($ex)); diff --git a/tests/unit/controller/ItemControllerTest.php b/tests/unit/controller/ItemControllerTest.php index b0d2a52bc..16df210e0 100644 --- a/tests/unit/controller/ItemControllerTest.php +++ b/tests/unit/controller/ItemControllerTest.php @@ -128,7 +128,32 @@ class ItemControllerTest extends ControllerTestUtility { ->with($url['itemId'], true, $this->user); - $this->controller->read(); + $result = $this->controller->read(); + $this->assertTrue($result instanceof JSONResponse); + } + + + public function testReadDoesNotExist(){ + $url = array( + 'itemId' => 4 + ); + $msg = 'hi'; + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->itemBusinessLayer->expects($this->once()) + ->method('read') + ->will($this->throwException(new BusinessLayerException($msg))); + + + $response = $this->controller->read(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); } @@ -149,7 +174,32 @@ class ItemControllerTest extends ControllerTestUtility { } - public function testStar(){ + + public function testUnreadDoesNotExist(){ + $url = array( + 'itemId' => 4 + ); + $msg = 'hi'; + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->itemBusinessLayer->expects($this->once()) + ->method('read') + ->will($this->throwException(new BusinessLayerException($msg))); + + + $response = $this->controller->unread(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + + public function testStar(){ $url = array( 'feedId' => 4, 'guidHash' => md5('test') @@ -172,6 +222,30 @@ class ItemControllerTest extends ControllerTestUtility { } + public function testStarDoesNotExist(){ + $url = array( + 'feedId' => 4, + 'guidHash' => md5('test') + ); + $msg = 'ho'; + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->itemBusinessLayer->expects($this->once()) + ->method('star') + ->will($this->throwException(new BusinessLayerException($msg)));; + + $response = $this->controller->star(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testUnstar(){ $url = array( 'feedId' => 4, @@ -195,6 +269,30 @@ class ItemControllerTest extends ControllerTestUtility { } + public function testUnstarDoesNotExist(){ + $url = array( + 'feedId' => 4, + 'guidHash' => md5('test') + ); + $msg = 'ho'; + $this->controller = $this->getPostController(array(), $url); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->itemBusinessLayer->expects($this->once()) + ->method('star') + ->will($this->throwException(new BusinessLayerException($msg)));; + + $response = $this->controller->unstar(); + $params = json_decode($response->render(), true); + + $this->assertEquals('error', $params['status']); + $this->assertEquals($msg, $params['msg']); + $this->assertTrue($response instanceof JSONResponse); + } + + public function testReadFeed(){ $url = array( 'feedId' => 4 |