added proper exception handling for all controllers and businesslayer

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);
+	}
+
+}

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);

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);

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);

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());
+		}
 	}
 
 

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());
+		}
 	}
 
 

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());
+		}
 	}
 
 

external/folder.php

@@ -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');
-	}
-}
-

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;

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();
 

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));

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