diff options
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | businesslayer/businesslayerconflictexception.php (renamed from businesslayer/businesslayerexistsexception.php) | 2 | ||||
-rw-r--r-- | businesslayer/businesslayervalidationexception.php | 39 | ||||
-rw-r--r-- | businesslayer/feedbusinesslayer.php | 4 | ||||
-rw-r--r-- | businesslayer/folderbusinesslayer.php | 20 | ||||
-rw-r--r-- | external/feedapi.php | 4 | ||||
-rw-r--r-- | external/folderapi.php | 16 | ||||
-rw-r--r-- | tests/unit/businesslayer/FolderBusinessLayerTest.php | 32 | ||||
-rw-r--r-- | tests/unit/controller/FolderControllerTest.php | 6 | ||||
-rw-r--r-- | tests/unit/external/FeedAPITest.php | 4 | ||||
-rw-r--r-- | tests/unit/external/FolderAPITest.php | 64 |
11 files changed, 169 insertions, 23 deletions
@@ -5,6 +5,7 @@ owncloud-news (1.401) * Add an option route for the API which handles the CORS headers to allow webapplications to access the API * Also allow youtube and vimeo embeds that start with // and https:// instead of only allowing http * ownCloud 6 compability fixes +* Throw proper error codes when creating invalid folders through the API owncloud-news (1.206) * Also handle URLErrors in updater script that are thrown when the domain of a feed is not found diff --git a/businesslayer/businesslayerexistsexception.php b/businesslayer/businesslayerconflictexception.php index 2e2fd2665..a65bbf2d6 100644 --- a/businesslayer/businesslayerexistsexception.php +++ b/businesslayer/businesslayerconflictexception.php @@ -26,7 +26,7 @@ namespace OCA\News\BusinessLayer; -class BusinessLayerExistsException extends BusinessLayerException { +class BusinessLayerConflictException extends BusinessLayerException { /** * Constructor diff --git a/businesslayer/businesslayervalidationexception.php b/businesslayer/businesslayervalidationexception.php new file mode 100644 index 000000000..c4f2c2387 --- /dev/null +++ b/businesslayer/businesslayervalidationexception.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 dev@bernhard-posselt.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 BusinessLayerValidationException 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 cf79d0b22..034209add 100644 --- a/businesslayer/feedbusinesslayer.php +++ b/businesslayer/feedbusinesslayer.php @@ -88,7 +88,7 @@ class FeedBusinessLayer extends BusinessLayer { * @param string $feedUrl the url to the feed * @param int $folderId the folder where it should be put into, 0 for root folder * @param string $userId for which user the feed should be created - * @throws BusinessLayerExistsException if the feed exists already + * @throws BusinessLayerConflictException if the feed exists already * @throws BusinessLayerException if the url points to an invalid feed * @return Feed the newly created feed */ @@ -100,7 +100,7 @@ class FeedBusinessLayer extends BusinessLayer { // try again if feed exists depending on the reported link try { $this->mapper->findByUrlHash($feed->getUrlHash(), $userId); - throw new BusinessLayerExistsException( + throw new BusinessLayerConflictException( $this->api->getTrans()->t('Can not add feed: Exists already')); } catch(DoesNotExistException $ex){} diff --git a/businesslayer/folderbusinesslayer.php b/businesslayer/folderbusinesslayer.php index ee0f8c8af..e43c705c6 100644 --- a/businesslayer/folderbusinesslayer.php +++ b/businesslayer/folderbusinesslayer.php @@ -58,13 +58,17 @@ class FolderBusinessLayer extends BusinessLayer { } - private function allowNoNameTwice($folderName, $userId){ + private function validateFolder($folderName, $userId){ $existingFolders = $this->mapper->findByName($folderName, $userId); if(count($existingFolders) > 0){ - throw new BusinessLayerExistsException( + throw new BusinessLayerConflictException( $this->api->getTrans()->t('Can not add folder: Exists already')); } + + if(mb_strlen($folderName) === 0) { + throw new BusinessLayerValidationException('Folder name can not be empty'); + } } @@ -73,11 +77,12 @@ class FolderBusinessLayer extends BusinessLayer { * @param string $folderName the name of the folder * @param string $userId the name of the user for whom it should be created * @param int $parentId the parent folder id, deprecated we dont nest folders - * @throws BusinessLayerExistsException if name exists already + * @throws BusinessLayerConflictException if name exists already + * @throws BusinessLayerValidationException if the folder has invalid parameters * @return Folder the newly created folder */ public function create($folderName, $userId, $parentId=0) { - $this->allowNoNameTwice($folderName, $userId); + $this->validateFolder($folderName, $userId); $folder = new Folder(); $folder->setName($folderName); @@ -87,6 +92,7 @@ class FolderBusinessLayer extends BusinessLayer { return $this->mapper->insert($folder); } + /** * @throws BusinessLayerException if the folder does not exist */ @@ -102,13 +108,13 @@ class FolderBusinessLayer extends BusinessLayer { * @param int $folderId the id of the folder that should be deleted * @param string $folderName the new name of the folder * @param string $userId the name of the user for security reasons - * @throws BusinessLayerExistsException if name exists already - * @throws BusinessLayerInvalidParameterException if the foldername is invalid + * @throws BusinessLayerConflictException if name exists already + * @throws BusinessLayerValidationException if the folder has invalid parameters * @throws BusinessLayerException if the folder does not exist * @return Folder the updated folder */ public function rename($folderId, $folderName, $userId){ - $this->allowNoNameTwice($folderName, $userId); + $this->validateFolder($folderName, $userId); $folder = $this->find($folderId, $userId); $folder->setName($folderName); diff --git a/external/feedapi.php b/external/feedapi.php index a9fdb4c70..be228d490 100644 --- a/external/feedapi.php +++ b/external/feedapi.php @@ -35,7 +35,7 @@ use \OCA\News\BusinessLayer\FeedBusinessLayer; use \OCA\News\BusinessLayer\FolderBusinessLayer; use \OCA\News\BusinessLayer\ItemBusinessLayer; use \OCA\News\BusinessLayer\BusinessLayerException; -use \OCA\News\BusinessLayer\BusinessLayerExistsException; +use \OCA\News\BusinessLayer\BusinessLayerConflictException; class FeedAPI extends Controller { @@ -112,7 +112,7 @@ class FeedAPI extends Controller { return new JSONResponse($result); - } catch(BusinessLayerExistsException $ex) { + } catch(BusinessLayerConflictException $ex) { return new JSONResponse(array('message' => $ex->getMessage()), Http::STATUS_CONFLICT); } catch(BusinessLayerException $ex) { diff --git a/external/folderapi.php b/external/folderapi.php index 31a151c0d..5d2555e10 100644 --- a/external/folderapi.php +++ b/external/folderapi.php @@ -34,7 +34,8 @@ use \OCA\AppFramework\Http\Http; use \OCA\News\BusinessLayer\FolderBusinessLayer; use \OCA\News\BusinessLayer\ItemBusinessLayer; use \OCA\News\BusinessLayer\BusinessLayerException; -use \OCA\News\BusinessLayer\BusinessLayerExistsException; +use \OCA\News\BusinessLayer\BusinessLayerConflictException; +use \OCA\News\BusinessLayer\BusinessLayerValidationException; class FolderAPI extends Controller { @@ -93,7 +94,12 @@ class FolderAPI extends Controller { array_push($result['folders'], $folder->toAPI()); return new JSONResponse($result); - } catch(BusinessLayerExistsException $ex) { + + } catch(BusinessLayerValidationException $ex) { + return new JSONResponse(array('message' => $ex->getMessage()), + Http::STATUS_UNPROCESSABLE_ENTITY); + + } catch(BusinessLayerConflictException $ex) { return new JSONResponse(array('message' => $ex->getMessage()), Http::STATUS_CONFLICT); } @@ -137,7 +143,11 @@ class FolderAPI extends Controller { $this->folderBusinessLayer->rename($folderId, $folderName, $userId); return new JSONResponse(); - } catch(BusinessLayerExistsException $ex) { + } catch(BusinessLayerValidationException $ex) { + return new JSONResponse(array('message' => $ex->getMessage()), + Http::STATUS_UNPROCESSABLE_ENTITY); + + } catch(BusinessLayerConflictException $ex) { return new JSONResponse(array('message' => $ex->getMessage()), Http::STATUS_CONFLICT); diff --git a/tests/unit/businesslayer/FolderBusinessLayerTest.php b/tests/unit/businesslayer/FolderBusinessLayerTest.php index d28c078a5..7f512920e 100644 --- a/tests/unit/businesslayer/FolderBusinessLayerTest.php +++ b/tests/unit/businesslayer/FolderBusinessLayerTest.php @@ -115,6 +115,22 @@ class FolderBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility { } + public function testCreateThrowsExWhenFolderNameEmpty(){ + $folderName = ''; + $rows = array( + array('id' => 1) + ); + + $this->folderMapper->expects($this->once()) + ->method('findByName') + ->with($this->equalTo($folderName)) + ->will($this->returnValue(array())); + + $this->setExpectedException('\OCA\News\BusinessLayer\BusinessLayerValidationException'); + $result = $this->folderBusinessLayer->create($folderName, 'john', 3); + } + + public function testOpen(){ $folder = new Folder(); @@ -175,6 +191,22 @@ class FolderBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility { } + public function testRenameThrowsExWhenFolderNameEmpty(){ + $folderName = ''; + $rows = array( + array('id' => 1) + ); + + $this->folderMapper->expects($this->once()) + ->method('findByName') + ->with($this->equalTo($folderName)) + ->will($this->returnValue(array())); + + $this->setExpectedException('\OCA\News\BusinessLayer\BusinessLayerException'); + $result = $this->folderBusinessLayer->rename(3, $folderName, 'john'); + } + + public function testMarkDeleted() { $id = 3; $folder = new Folder(); diff --git a/tests/unit/controller/FolderControllerTest.php b/tests/unit/controller/FolderControllerTest.php index f273c9ec7..e4295583b 100644 --- a/tests/unit/controller/FolderControllerTest.php +++ b/tests/unit/controller/FolderControllerTest.php @@ -34,7 +34,7 @@ use \OCA\AppFramework\Db\MultipleObjectsReturnedException; use \OCA\News\Db\Folder; use \OCA\News\Db\Feed; use \OCA\News\BusinessLayer\BusinessLayerException; -use \OCA\News\BusinessLayer\BusinessLayerExistsException; +use \OCA\News\BusinessLayer\BusinessLayerConflictException; require_once(__DIR__ . "/../../classloader.php"); @@ -256,7 +256,7 @@ class FolderControllerTest extends ControllerTestUtility { public function testCreateReturnsErrorForInvalidCreate(){ $msg = 'except'; - $ex = new BusinessLayerExistsException($msg); + $ex = new BusinessLayerConflictException($msg); $this->api->expects($this->once()) ->method('getUserId') ->will($this->returnValue($this->user)); @@ -342,7 +342,7 @@ class FolderControllerTest extends ControllerTestUtility { public function testRenameReturnsErrorForInvalidCreate(){ $msg = 'except'; - $ex = new BusinessLayerExistsException($msg); + $ex = new BusinessLayerConflictException($msg); $this->folderBusinessLayer->expects($this->once()) ->method('rename') ->will($this->throwException($ex)); diff --git a/tests/unit/external/FeedAPITest.php b/tests/unit/external/FeedAPITest.php index fbc608cac..ec6aedf19 100644 --- a/tests/unit/external/FeedAPITest.php +++ b/tests/unit/external/FeedAPITest.php @@ -31,7 +31,7 @@ use \OCA\AppFramework\Http\Http; use \OCA\AppFramework\Utility\ControllerTestUtility; use \OCA\News\BusinessLayer\BusinessLayerException; -use \OCA\News\BusinessLayer\BusinessLayerExistsException; +use \OCA\News\BusinessLayer\BusinessLayerConflictException; use \OCA\News\Db\Folder; use \OCA\News\Db\Feed; use \OCA\News\Db\Item; @@ -343,7 +343,7 @@ class FeedAPITest extends ControllerTestUtility { ->with($this->equalTo($this->user), $this->equalTo(false)); $this->feedBusinessLayer->expects($this->once()) ->method('create') - ->will($this->throwException(new BusinessLayerExistsException($this->msg))); + ->will($this->throwException(new BusinessLayerConflictException($this->msg))); $response = $this->feedAPI->create(); diff --git a/tests/unit/external/FolderAPITest.php b/tests/unit/external/FolderAPITest.php index 3e5bbabb6..145cc6720 100644 --- a/tests/unit/external/FolderAPITest.php +++ b/tests/unit/external/FolderAPITest.php @@ -31,7 +31,8 @@ use \OCA\AppFramework\Utility\ControllerTestUtility; use \OCA\AppFramework\Http\Http; use \OCA\News\BusinessLayer\BusinessLayerException; -use \OCA\News\BusinessLayer\BusinessLayerExistsException; +use \OCA\News\BusinessLayer\BusinessLayerConflictException; +use \OCA\News\BusinessLayer\BusinessLayerValidationException; use \OCA\News\Db\Folder; use \OCA\News\Db\Feed; @@ -177,7 +178,7 @@ class FolderAPITest extends ControllerTestUtility { ->with($this->equalTo($this->user), $this->equalTo(false)); $this->folderBusinessLayer->expects($this->once()) ->method('create') - ->will($this->throwException(new BusinessLayerExistsException($msg))); + ->will($this->throwException(new BusinessLayerConflictException($msg))); $response = $this->folderAPI->create(); @@ -187,6 +188,27 @@ class FolderAPITest extends ControllerTestUtility { } + public function testCreateInvalidFolderName() { + $msg = 'exists'; + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->folderBusinessLayer->expects($this->once()) + ->method('purgeDeleted') + ->with($this->equalTo($this->user), $this->equalTo(false)); + $this->folderBusinessLayer->expects($this->once()) + ->method('create') + ->will($this->throwException(new BusinessLayerValidationException($msg))); + + $response = $this->folderAPI->create(); + + $data = $response->getData(); + $this->assertEquals($msg, $data['message']); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + } + + public function testDelete() { $folderId = 23; @@ -337,7 +359,7 @@ class FolderAPITest extends ControllerTestUtility { ->will($this->returnValue($this->user)); $this->folderBusinessLayer->expects($this->once()) ->method('rename') - ->will($this->throwException(new BusinessLayerExistsException($this->msg))); + ->will($this->throwException(new BusinessLayerConflictException($this->msg))); $response = $this->folderAPI->update(); @@ -347,6 +369,42 @@ class FolderAPITest extends ControllerTestUtility { } + public function testUpdateInvalidFolderName() { + $folderId = 23; + $folderName = ''; + + $this->folderAPI = new FolderAPI( + $this->api, + new Request( + array( + 'urlParams' => array( + 'folderId' => $folderId + ), + + 'params' => array( + 'name' => $folderName + ) + ) + ), + $this->folderBusinessLayer, + $this->itemBusinessLayer + ); + + $this->api->expects($this->once()) + ->method('getUserId') + ->will($this->returnValue($this->user)); + $this->folderBusinessLayer->expects($this->once()) + ->method('rename') + ->will($this->throwException(new BusinessLayerValidationException($this->msg))); + + $response = $this->folderAPI->update(); + + $data = $response->getData(); + $this->assertEquals($this->msg, $data['message']); + $this->assertEquals(Http::STATUS_UNPROCESSABLE_ENTITY, $response->getStatus()); + } + + public function testRead() { $request = new Request(array( 'urlParams' => array( |