diff options
author | Roeland Jago Douma <rullzer@users.noreply.github.com> | 2016-08-10 22:36:25 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-10 22:36:25 +0200 |
commit | ba922c9f73c8b61998686d2273fbaccc7f88ed97 (patch) | |
tree | bb9b6985d64358b68ea5168e7a12b6dd08d58a9c | |
parent | 67d016833d637982a197bb848836c7afcea88358 (diff) | |
parent | 9544c97ffe888a16cdf58013b5a97056771eb856 (diff) | |
download | nextcloud-server-ba922c9f73c8b61998686d2273fbaccc7f88ed97.tar.gz nextcloud-server-ba922c9f73c8b61998686d2273fbaccc7f88ed97.zip |
Merge pull request #807 from nextcloud/ocs_dataresponse
OCSController requires DataResponse
-rw-r--r-- | apps/files_sharing/lib/API/Share20OCS.php | 14 | ||||
-rw-r--r-- | apps/files_sharing/tests/API/Share20OCSTest.php | 31 | ||||
-rw-r--r-- | apps/files_sharing/tests/ApiTest.php | 68 | ||||
-rw-r--r-- | core/Controller/OCSController.php | 4 | ||||
-rw-r--r-- | lib/public/AppFramework/OCSController.php | 15 | ||||
-rw-r--r-- | tests/lib/AppFramework/Controller/OCSControllerTest.php | 57 |
6 files changed, 67 insertions, 122 deletions
diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index 0cce05c3b17..2c0ed03f880 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -203,7 +203,7 @@ class Share20OCS extends OCSController { if ($this->canAccessShare($share)) { try { $share = $this->formatShare($share); - return new DataResponse(['data' => [$share]]); + return new DataResponse([$share]); } catch (NotFoundException $e) { //Fall trough } @@ -339,7 +339,7 @@ class Share20OCS extends OCSController { */ $existingShares = $this->shareManager->getSharesBy($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_LINK, $path, false, 1, 0); if (!empty($existingShares)) { - return new DataResponse(['data' => $this->formatShare($existingShares[0])]); + return new DataResponse($this->formatShare($existingShares[0])); } $publicUpload = $this->request->getParam('publicUpload', null); @@ -408,7 +408,7 @@ class Share20OCS extends OCSController { $output = $this->formatShare($share); - return new DataResponse(['data' => $output]); + return new DataResponse($output); } /** @@ -432,7 +432,7 @@ class Share20OCS extends OCSController { } } - return new DataResponse(['data' => $formatted]); + return new DataResponse($formatted); } /** @@ -466,7 +466,7 @@ class Share20OCS extends OCSController { } } - return new DataResponse(['data' => $formatted]); + return new DataResponse($formatted); } /** @@ -537,7 +537,7 @@ class Share20OCS extends OCSController { } } - return new DataResponse(['data' => $formatted]); + return new DataResponse($formatted); } /** @@ -671,7 +671,7 @@ class Share20OCS extends OCSController { throw new OCSBadRequestException($e->getMessage()); } - return new DataResponse(['data' => $this->formatShare($share)]); + return new DataResponse($this->formatShare($share)); } /** diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index b32684ef547..a89bdb065c4 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -24,7 +24,6 @@ namespace OCA\Files_Sharing\Tests\API; use OCP\AppFramework\Http\DataResponse; -use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\IL10N; use OCA\Files_Sharing\API\Share20OCS; use OCP\Files\NotFoundException; @@ -35,7 +34,6 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\Files\IRootFolder; use OCP\Lock\LockedException; -use Punic\Data; /** * Class Share20OCSTest @@ -485,7 +483,7 @@ class Share20OCSTest extends \Test\TestCase { ['group', $group], ])); - $this->assertEquals($result, $ocs->getShare($share->getId())->getData()['data'][0]); + $this->assertEquals($result, $ocs->getShare($share->getId())->getData()[0]); } /** @@ -706,6 +704,7 @@ class Share20OCSTest extends \Test\TestCase { $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); + /** @var \OCA\Files_Sharing\API\Share20OCS $ocs */ $ocs = $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') ->setConstructorArgs([ $this->appName, @@ -766,7 +765,7 @@ class Share20OCSTest extends \Test\TestCase { })) ->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->createShare(); $this->assertInstanceOf(get_class($expected), $result); @@ -879,7 +878,7 @@ class Share20OCSTest extends \Test\TestCase { })) ->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->createShare(); $this->assertInstanceOf(get_class($expected), $result); @@ -1049,7 +1048,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->createShare(); $this->assertInstanceOf(get_class($expected), $result); @@ -1093,7 +1092,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->createShare(); $this->assertInstanceOf(get_class($expected), $result); @@ -1140,7 +1139,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->createShare(); $this->assertInstanceOf(get_class($expected), $result); @@ -1337,7 +1336,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1377,7 +1376,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1415,7 +1414,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1562,7 +1561,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1605,7 +1604,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1645,7 +1644,7 @@ class Share20OCSTest extends \Test\TestCase { }) )->will($this->returnArgument(0)); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1687,7 +1686,7 @@ class Share20OCSTest extends \Test\TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); @@ -1754,7 +1753,7 @@ class Share20OCSTest extends \Test\TestCase { $this->shareManager->method('getSharedWith')->willReturn([]); - $expected = new DataResponse(['data' => null]); + $expected = new DataResponse(null); $result = $ocs->updateShare(42); $this->assertInstanceOf(get_class($expected), $result); diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 5f8b17eb5c6..f39ddeb5485 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -140,7 +140,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals(19, $data['permissions']); $this->assertEmpty($data['expiration']); @@ -164,7 +164,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals(31, $data['permissions']); $this->assertEmpty($data['expiration']); @@ -189,7 +189,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals(19, $data['permissions']); $this->assertEmpty($data['expiration']); @@ -212,7 +212,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals(31, $data['permissions']); $this->assertEmpty($data['expiration']); @@ -235,7 +235,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals(1, $data['permissions']); $this->assertEmpty($data['expiration']); $this->assertTrue(is_string($data['token'])); @@ -263,7 +263,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals( \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | @@ -332,7 +332,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); // setting new password should succeed $data2 = [ @@ -382,7 +382,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->shareManager->getShareById('ocinternal:'.$data['id']); @@ -405,7 +405,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->shareManager->getShareById('ocinternal:' . $data['id']); @@ -453,7 +453,7 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $this->assertTrue(count($result->getData()['data']) === 1); + $this->assertTrue(count($result->getData()) === 1); $this->shareManager->deleteShare($share); } @@ -482,7 +482,7 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $this->assertTrue(count($result->getData()['data']) === 2); + $this->assertTrue(count($result->getData()) === 2); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); @@ -501,7 +501,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); // check if we have a token $this->assertTrue(is_string($data['token'])); @@ -517,7 +517,7 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals($url, current($data)['url']); // check for path @@ -526,7 +526,7 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals($url, current($data)['url']); // check in share id @@ -535,7 +535,7 @@ class ApiTest extends TestCase { $result = $ocs->getShare($id); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals($url, current($data)['url']); $request = $this->createRequest([]); @@ -572,7 +572,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share created from testCreateShare() - $this->assertTrue(count($result->getData()['data']) === 2); + $this->assertTrue(count($result->getData()) === 2); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); @@ -607,7 +607,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share - $this->assertTrue(count($result->getData()['data']) === 1); + $this->assertTrue(count($result->getData()) === 1); // now also ask for the reshares $request = $this->createRequest(['path' => $this->filename, 'reshares' => 'true']); @@ -616,7 +616,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // now we should get two shares, the initial share and the reshare - $this->assertCount(2, $result->getData()['data']); + $this->assertCount(2, $result->getData()); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); @@ -643,7 +643,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share created from testCreateShare() - $this->assertEquals(1, count($result->getData()['data'])); + $this->assertEquals(1, count($result->getData())); $this->shareManager->deleteShare($share1); } @@ -676,7 +676,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $this->assertTrue(count($result->getData()['data']) === 1); + $this->assertTrue(count($result->getData()) === 1); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); @@ -749,7 +749,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertEquals($value['expectedResult'], $data[0]['path']); } @@ -788,7 +788,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); // we should get exactly one result $this->assertCount(1, $data); @@ -838,7 +838,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); // we should get exactly one result $this->assertCount(1, $data); @@ -853,7 +853,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); // we should get exactly one result $this->assertCount(1, $data); @@ -868,7 +868,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); // we should get exactly one result $this->assertCount(1, $data); @@ -915,7 +915,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data1 = $result1->getData()['data']; + $data1 = $result1->getData(); $this->assertCount(1, $data1); $s1 = reset($data1); @@ -925,7 +925,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data2 = $result2->getData()['data']; + $data2 = $result2->getData(); $this->assertCount(1, $data2); $s2 = reset($data2); @@ -976,7 +976,7 @@ class ApiTest extends TestCase { $ocs->cleanup(); // test should return one share within $this->folder - $data = $result->getData()['data']; + $data = $result->getData(); // we should get exactly one result $this->assertCount(1, $data); @@ -1491,7 +1491,7 @@ class ApiTest extends TestCase { } $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertTrue(is_string($data['token'])); $this->assertEquals($date, substr($data['expiration'], 0, 10)); @@ -1525,7 +1525,7 @@ class ApiTest extends TestCase { $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $this->assertTrue(is_string($data['token'])); $this->assertEquals($date->format('Y-m-d') . ' 00:00:00', $data['expiration']); @@ -1613,7 +1613,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1); $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $topId = $data['id']; @@ -1637,7 +1637,7 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $this->assertEmpty($result->getData()['data']); + $this->assertEmpty($result->getData()); } /** @@ -1654,7 +1654,7 @@ class ApiTest extends TestCase { $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1); $result = $ocs->createShare(); $ocs->cleanup(); - $data = $result->getData()['data']; + $data = $result->getData(); $topId = $data['id']; @@ -1678,6 +1678,6 @@ class ApiTest extends TestCase { $result = $ocs->getShares(); $ocs->cleanup(); - $this->assertEmpty($result->getData()['data']); + $this->assertEmpty($result->getData()); } } diff --git a/core/Controller/OCSController.php b/core/Controller/OCSController.php index d5783ae32e9..8eee52096c2 100644 --- a/core/Controller/OCSController.php +++ b/core/Controller/OCSController.php @@ -69,7 +69,7 @@ class OCSController extends \OCP\AppFramework\OCSController { $result['capabilities'] = $this->capabilitiesManager->getCapabilities(); - return new DataResponse(['data' => $result]); + return new DataResponse($result); } /** @@ -83,6 +83,6 @@ class OCSController extends \OCP\AppFramework\OCSController { 'display-name' => $userObject->getDisplayName(), 'email' => $userObject->getEMailAddress(), ]; - return new DataResponse(['data' => $data]); + return new DataResponse($data); } } diff --git a/lib/public/AppFramework/OCSController.php b/lib/public/AppFramework/OCSController.php index bd50f0a4017..6036fc6a5a8 100644 --- a/lib/public/AppFramework/OCSController.php +++ b/lib/public/AppFramework/OCSController.php @@ -88,26 +88,19 @@ abstract class OCSController extends ApiController { /** * Unwrap data and build ocs response * @param string $format json or xml - * @param array|DataResponse $data the data which should be transformed + * @param DataResponse $data the data which should be transformed * @since 8.1.0 + * @return OCSResponse */ - private function buildOCSResponse($format, $data) { - if ($data instanceof DataResponse) { - $data = $data->getData(); - } - + private function buildOCSResponse($format, DataResponse $data) { $params = [ 'statuscode' => 100, 'message' => 'OK', - 'data' => [], + 'data' => $data->getData(), 'itemscount' => '', 'itemsperpage' => '' ]; - foreach ($data as $key => $value) { - $params[$key] = $value; - } - return new OCSResponse( $format, $params['statuscode'], $params['message'], $params['data'], diff --git a/tests/lib/AppFramework/Controller/OCSControllerTest.php b/tests/lib/AppFramework/Controller/OCSControllerTest.php index 7dcbd189cd5..9c9214181a4 100644 --- a/tests/lib/AppFramework/Controller/OCSControllerTest.php +++ b/tests/lib/AppFramework/Controller/OCSControllerTest.php @@ -75,8 +75,8 @@ class OCSControllerTest extends \Test\TestCase { $expected = "<?xml version=\"1.0\"?>\n" . "<ocs>\n" . " <meta>\n" . - " <status>failure</status>\n" . - " <statuscode>400</statuscode>\n" . + " <status>ok</status>\n" . + " <statuscode>100</statuscode>\n" . " <message>OK</message>\n" . " <totalitems></totalitems>\n" . " <itemsperpage></itemsperpage>\n" . @@ -86,54 +86,12 @@ class OCSControllerTest extends \Test\TestCase { " </data>\n" . "</ocs>\n"; - $params = [ - 'data' => [ - 'test' => 'hi' - ], - 'statuscode' => 400 - ]; + $params = new DataResponse(['test' => 'hi']); $out = $controller->buildResponse($params, 'xml')->render(); $this->assertEquals($expected, $out); } - - public function testXMLDataResponse() { - $controller = new ChildOCSController('app', new Request( - [], - $this->getMockBuilder('\OCP\Security\ISecureRandom') - ->disableOriginalConstructor() - ->getMock(), - $this->getMockBuilder('\OCP\IConfig') - ->disableOriginalConstructor() - ->getMock() - )); - $expected = "<?xml version=\"1.0\"?>\n" . - "<ocs>\n" . - " <meta>\n" . - " <status>failure</status>\n" . - " <statuscode>400</statuscode>\n" . - " <message>OK</message>\n" . - " <totalitems></totalitems>\n" . - " <itemsperpage></itemsperpage>\n" . - " </meta>\n" . - " <data>\n" . - " <test>hi</test>\n" . - " </data>\n" . - "</ocs>\n"; - - $params = new DataResponse([ - 'data' => [ - 'test' => 'hi' - ], - 'statuscode' => 400 - ]); - - $out = $controller->buildResponse($params, 'xml')->render(); - $this->assertEquals($expected, $out); - } - - public function testJSON() { $controller = new ChildOCSController('app', new Request( [], @@ -144,14 +102,9 @@ class OCSControllerTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock() )); - $expected = '{"ocs":{"meta":{"status":"failure","statuscode":400,"message":"OK",' . + $expected = '{"ocs":{"meta":{"status":"ok","statuscode":100,"message":"OK",' . '"totalitems":"","itemsperpage":""},"data":{"test":"hi"}}}'; - $params = [ - 'data' => [ - 'test' => 'hi' - ], - 'statuscode' => 400 - ]; + $params = new DataResponse(['test' => 'hi']); $out = $controller->buildResponse($params, 'json')->render(); $this->assertEquals($expected, $out); |