From 8b3e3890628fa00d50c94af1131cbe3cd63e4a12 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 8 Dec 2014 15:32:59 +0100 Subject: [PATCH] Add statuscodes --- settings/controller/groupscontroller.php | 18 ++++--- settings/controller/userscontroller.php | 22 +++++--- .../controller/groupscontrollertest.php | 43 +++++++++++++-- .../controller/userscontrollertest.php | 53 +++++++++++++++++-- 4 files changed, 113 insertions(+), 23 deletions(-) diff --git a/settings/controller/groupscontroller.php b/settings/controller/groupscontroller.php index 6e6ab89460..8236d5507f 100644 --- a/settings/controller/groupscontroller.php +++ b/settings/controller/groupscontroller.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; +use OC\AppFramework\Http; use \OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; use OCP\IGroupManager; @@ -85,7 +86,8 @@ class GroupsController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Group already exists.') ) - ) + ), + Http::STATUS_CONFLICT ); } if($this->groupManager->createGroup($id)) { @@ -95,7 +97,8 @@ class GroupsController extends Controller { 'data' => array( 'groupname' => $id ) - ) + ), + Http::STATUS_CREATED ); } @@ -105,7 +108,8 @@ class GroupsController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to add group.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -123,7 +127,8 @@ class GroupsController extends Controller { 'data' => array( 'groupname' => $id ) - ) + ), + Http::STATUS_NO_CONTENT ); } } @@ -132,8 +137,9 @@ class GroupsController extends Controller { 'status' => 'error', 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete group.') - ) - ) + ), + ), + Http::STATUS_FORBIDDEN ); } diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index d61d19f8cb..aa16574221 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; +use OC\AppFramework\Http; use OC\User\User; use \OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; @@ -164,7 +165,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to create user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -187,7 +189,8 @@ class UsersController extends Controller { 'groups' => $this->groupManager->getUserGroupIds($user), 'storageLocation' => $user->getHome() ) - ) + ), + Http::STATUS_CREATED ); } @@ -208,7 +211,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } @@ -218,8 +222,10 @@ class UsersController extends Controller { array( 'status' => 'error', 'data' => array( - 'message' => (string)$this->l10n->t('Authentication error')) - ) + 'message' => (string)$this->l10n->t('Authentication error') + ) + ), + Http::STATUS_FORBIDDEN ); } @@ -232,7 +238,8 @@ class UsersController extends Controller { 'data' => array( 'username' => $id ) - ) + ), + Http::STATUS_NO_CONTENT ); } } @@ -243,7 +250,8 @@ class UsersController extends Controller { 'data' => array( 'message' => (string)$this->l10n->t('Unable to delete user.') ) - ) + ), + Http::STATUS_FORBIDDEN ); } diff --git a/tests/settings/controller/groupscontrollertest.php b/tests/settings/controller/groupscontrollertest.php index ac4619cfdb..003fb70b81 100644 --- a/tests/settings/controller/groupscontrollertest.php +++ b/tests/settings/controller/groupscontrollertest.php @@ -11,6 +11,7 @@ namespace OC\Settings\Controller; use OC\Group\Group; use \OC\Settings\Application; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; /** @@ -145,7 +146,15 @@ class GroupsControllerTest extends \Test\TestCase { ->with('ExistingGroup') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Group already exists.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Group already exists.' + ) + ), + Http::STATUS_CONFLICT + ); $response = $this->groupsController->create('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } @@ -162,7 +171,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('NewGroup') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'NewGroup'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array('groupname' => 'NewGroup') + ), + Http::STATUS_CREATED + ); $response = $this->groupsController->create('NewGroup'); $this->assertEquals($expectedResponse, $response); } @@ -179,7 +194,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('NewGroup') ->will($this->returnValue(false)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to add group.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array('message' => 'Unable to add group.') + ), + Http::STATUS_FORBIDDEN + ); $response = $this->groupsController->create('NewGroup'); $this->assertEquals($expectedResponse, $response); } @@ -197,7 +218,13 @@ class GroupsControllerTest extends \Test\TestCase { ->method('delete') ->will($this->returnValue(true)); - $expectedResponse = new DataResponse(array('status' => 'success', 'data' => array('groupname' => 'ExistingGroup'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'success', + 'data' => array('groupname' => 'ExistingGroup') + ), + Http::STATUS_NO_CONTENT + ); $response = $this->groupsController->destroy('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } @@ -209,7 +236,13 @@ class GroupsControllerTest extends \Test\TestCase { ->with('ExistingGroup') ->will($this->returnValue(null)); - $expectedResponse = new DataResponse(array('status' => 'error', 'data' => array('message' => 'Unable to delete group.'))); + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array('message' => 'Unable to delete group.') + ), + Http::STATUS_FORBIDDEN + ); $response = $this->groupsController->destroy('ExistingGroup'); $this->assertEquals($expectedResponse, $response); } diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index 8aee2b2dd5..0f1dfb4e68 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -10,6 +10,7 @@ namespace OC\Settings\Controller; use \OC\Settings\Application; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; /** @@ -158,7 +159,8 @@ class UsersControllerTest extends \Test\TestCase { 'groups' => null, 'storageLocation' => '/home/user' ) - ) + ), + Http::STATUS_CREATED ); $response = $this->usersController->create('foo', 'password', array()); $this->assertEquals($expectedResponse, $response); @@ -217,7 +219,8 @@ class UsersControllerTest extends \Test\TestCase { 'groups' => array('NewGroup', 'ExistingGroup'), 'storageLocation' => '/home/user' ) - ) + ), + Http::STATUS_CREATED ); $response = $this->usersController->create('foo', 'password', array('NewGroup', 'ExistingGroup')); $this->assertEquals($expectedResponse, $response); @@ -238,7 +241,8 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'message' => 'Unable to create user.' ) - ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->create('foo', 'password', array()); $this->assertEquals($expectedResponse, $response); @@ -265,7 +269,8 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'message' => 'Unable to delete user.' ) - ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->destroy('myself'); $this->assertEquals($expectedResponse, $response); @@ -302,7 +307,45 @@ class UsersControllerTest extends \Test\TestCase { 'data' => array( 'username' => 'UserToDelete' ) - ) + ), + Http::STATUS_NO_CONTENT + ); + $response = $this->usersController->destroy('UserToDelete'); + $this->assertEquals($expectedResponse, $response); + } + /** + * TODO: Since the function uses the static OC_Subadmin class it can't be mocked + * to test for subadmins. Thus the test always assumes you have admin permissions... + */ + public function testDestroyUnsuccessful() { + $user = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $user + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('Admin')); + $toDeleteUser = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $toDeleteUser + ->expects($this->once()) + ->method('delete') + ->will($this->returnValue(false)); + $this->container['UserSession'] + ->method('getUser') + ->will($this->returnValue($user)); + $this->container['UserManager'] + ->method('get') + ->with('UserToDelete') + ->will($this->returnValue($toDeleteUser)); + + $expectedResponse = new DataResponse( + array( + 'status' => 'error', + 'data' => array( + 'message' => 'Unable to delete user.' + ) + ), + Http::STATUS_FORBIDDEN ); $response = $this->usersController->destroy('UserToDelete'); $this->assertEquals($expectedResponse, $response);