Add some more tests for ocs remote api

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/private/Remote/Api/OCS.php

@@ -43,7 +43,7 @@ class OCS extends ApiBase implements ICapabilitiesApi, IUserApi {
 	 */
 	protected function request($method, $url, array $body = [], array $query = [], array $headers = []) {
 		try {
-			$response = json_decode(parent::request($method, '/ocs/v2.php/' . $url, $body, $query, $headers), true);
+			$response = json_decode(parent::request($method, 'ocs/v2.php/' . $url, $body, $query, $headers), true);
 		} catch (ClientException $e) {
 			if ($e->getResponse()->getStatusCode() === 404) {
 				throw new NotFoundException();
@@ -69,14 +69,23 @@ class OCS extends ApiBase implements ICapabilitiesApi, IUserApi {
 		return $response['ocs']['data'];
 	}
 
-	public function getUser($userId) {
-		$result = $this->request('get', 'cloud/users/' . $userId);
-		$keys = ['id', 'email', 'displayname', 'phone', 'address', 'website', 'groups', 'language', 'quota'];
+	/**
+	 * @param array $data
+	 * @param string $type
+	 * @param string[] $keys
+	 * @throws \Exception
+	 */
+	private function checkResponseArray(array $data, $type, array $keys) {
 		foreach ($keys as $key) {
-			if (!isset($result[$key])) {
-				throw new \Exception('Invalid user response, expected field ' . $key . ' not found');
+			if (!array_key_exists($key, $data)) {
+				throw new \Exception('Invalid ' . $type . ' response, expected field ' . $key . ' not found');
 			}
 		}
+	}
+
+	public function getUser($userId) {
+		$result = $this->request('get', 'cloud/users/' . $userId);
+		$this->checkResponseArray($result, 'user', User::EXPECTED_KEYS);
 		return new User($result);
 	}
 

lib/private/Remote/User.php

@@ -25,6 +25,18 @@ namespace OC\Remote;
 use OCP\Remote\IUser;
 
 class User implements IUser {
+	const EXPECTED_KEYS = [
+		'id',
+		'email',
+		'displayname',
+		'phone',
+		'address',
+		'website',
+		'groups',
+		'language',
+		'quota'
+	];
+
 	/** @var array */
 	private $data;
 

tests/lib/Remote/Api/OCSTest.php

@@ -0,0 +1,95 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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 program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Remote\Api;
+
+use OC\Memcache\ArrayCache;
+use OC\Remote\Api\OCS;
+use OC\Remote\Credentials;
+use OC\Remote\InstanceFactory;
+use OCP\Remote\IInstanceFactory;
+use Test\TestCase;
+use Test\Traits\ClientServiceTrait;
+
+class OCSTest extends TestCase {
+	use ClientServiceTrait;
+
+	/** @var IInstanceFactory */
+	private $instanceFactory;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$this->instanceFactory = new InstanceFactory(new ArrayCache(), $this->getClientService());
+		$this->expectGetRequest('https://example.com/status.php',
+			'{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+	}
+
+	protected function getOCSClient() {
+		return new OCS(
+			$this->instanceFactory->getInstance('example.com'),
+			new Credentials('user', 'pass'),
+			$this->getClientService()
+		);
+	}
+
+	protected function getOCSUrl($url) {
+		return 'https://example.com/ocs/v2.php/' . $url;
+	}
+
+	public function testGetUser() {
+		$client = $this->getOCSClient();
+
+		$this->expectGetRequest($this->getOCSUrl('cloud/users/user'),
+			'{"ocs":{"meta":{"status":"ok","statuscode":200,"message":"OK"},
+			"data":{"id":"user","quota":{"free":5366379387,"used":2329733,"total":5368709120,"relative":0.040000000000000001,"quota":5368709120},
+			"email":null,"displayname":"test","phone":"","address":"","website":"","twitter":"","groups":["Test","Test1"],"language":"en"}}}');
+
+		$user = $client->getUser('user');
+		$this->assertEquals('user', $user->getUserId());
+	}
+
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage Invalid user response, expected field email not found
+	 */
+	public function testGetUserInvalidResponse() {
+		$client = $this->getOCSClient();
+
+		$this->expectGetRequest($this->getOCSUrl('cloud/users/user'),
+			'{"ocs":{"meta":{"status":"ok","statuscode":200,"message":"OK"},
+			"data":{"id":"user"}}}');
+
+		$client->getUser('user');
+	}
+
+	/**
+	 * @expectedException \OC\ForbiddenException
+	 */
+	public function testInvalidPassword() {
+		$client = $this->getOCSClient();
+
+		$this->expectGetRequest($this->getOCSUrl('cloud/users/user'),
+			'{"ocs":{"meta":{"status":"failure","statuscode":997,"message":"Current user is not logged in"},"data":[]}}');
+
+		$client->getUser('user');
+	}
+}

tests/lib/Remote/InstanceTest.php

@@ -24,62 +24,25 @@ namespace Test\Remote;
 
 use OC\Memcache\ArrayCache;
 use OC\Remote\Instance;
-use OCP\Http\Client\IClient;
-use OCP\Http\Client\IClientService;
-use OCP\Http\Client\IResponse;
 use OCP\ICache;
 use Test\TestCase;
+use Test\Traits\ClientServiceTrait;
 
 class InstanceTest extends TestCase {
-	/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
-	private $clientService;
-	/** @var IClient|\PHPUnit_Framework_MockObject_MockObject */
-	private $client;
+	use ClientServiceTrait;
+
 	/** @var ICache */
 	private $cache;
-	private $expectedRequests = [];
 
 	protected function setUp() {
 		parent::setUp();
 
 		$this->cache = new ArrayCache();
-
-		$this->clientService = $this->createMock(IClientService::class);
-		$this->client = $this->createMock(IClient::class);
-		$this->clientService->expects($this->any())
-			->method('newClient')
-			->willReturn($this->client);
-		$this->client->expects($this->any())
-			->method('get')
-			->willReturnCallback(function ($url) {
-				if (!isset($this->expectedRequests[$url])) {
-					throw new \Exception('unexpected request');
-				}
-				$result = $this->expectedRequests[$url];
-
-				if ($result instanceof \Exception) {
-					throw $result;
-				} else {
-					$response = $this->createMock(IResponse::class);
-					$response->expects($this->any())
-						->method('getBody')
-						->willReturn($result);
-					return $response;
-				}
-			});
-	}
-
-	/**
-	 * @param string $url
-	 * @param string|\Exception $result
-	 */
-	protected function expectRequest($url, $result) {
-		$this->expectedRequests[$url] = $result;
 	}
 
 	public function testBasicStatus() {
-		$instance = new Instance('example.com', $this->cache, $this->clientService);
-		$this->expectRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+		$instance = new Instance('example.com', $this->cache, $this->getClientService());
+		$this->expectGetRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
 
 		$this->assertEquals(true, $instance->isActive());
 		$this->assertEquals('13.0.0.5', $instance->getVersion());
@@ -88,24 +51,24 @@ class InstanceTest extends TestCase {
 	}
 
 	public function testHttpFallback() {
-		$instance = new Instance('example.com', $this->cache, $this->clientService);
-		$this->expectRequest('https://example.com/status.php', new \Exception());
-		$this->expectRequest('http://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+		$instance = new Instance('example.com', $this->cache, $this->getClientService());
+		$this->expectGetRequest('https://example.com/status.php', new \Exception());
+		$this->expectGetRequest('http://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
 
 		$this->assertEquals('http', $instance->getProtocol());
 		$this->assertEquals('http://example.com', $instance->getFullUrl());
 	}
 
 	public function testRerequestHttps() {
-		$instance = new Instance('example.com', $this->cache, $this->clientService);
-		$this->expectRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+		$instance = new Instance('example.com', $this->cache, $this->getClientService());
+		$this->expectGetRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
 
 		$this->assertEquals('https', $instance->getProtocol());
 		$this->assertEquals(true, $instance->isActive());
 
 		$this->cache->remove('remote/example.com/status');
-		$this->expectRequest('https://example.com/status.php', '{"installed":true,"maintenance":true,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
-		$instance2 = new Instance('example.com', $this->cache, $this->clientService);
+		$this->expectGetRequest('https://example.com/status.php', '{"installed":true,"maintenance":true,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+		$instance2 = new Instance('example.com', $this->cache, $this->getClientService());
 		$this->assertEquals('https', $instance2->getProtocol());
 		$this->assertEquals(false, $instance2->isActive());
 	}
@@ -115,14 +78,14 @@ class InstanceTest extends TestCase {
 	 * @expectedExceptionMessage refusing to connect to remote instance(example.com) over http that was previously accessible over https
 	 */
 	public function testPreventDowngradeAttach() {
-		$instance = new Instance('example.com', $this->cache, $this->clientService);
-		$this->expectRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
+		$instance = new Instance('example.com', $this->cache, $this->getClientService());
+		$this->expectGetRequest('https://example.com/status.php', '{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"13.0.0.5","versionstring":"13.0.0 alpha","edition":"","productname":"Nextcloud"}');
 
 		$this->assertEquals('https', $instance->getProtocol());
 
-		$this->expectRequest('https://example.com/status.php', new \Exception());
+		$this->expectGetRequest('https://example.com/status.php', new \Exception());
 		$this->cache->remove('remote/example.com/status');
-		$instance2 = new Instance('example.com', $this->cache, $this->clientService);
+		$instance2 = new Instance('example.com', $this->cache, $this->getClientService());
 		$instance2->getProtocol();
 	}
 }

tests/lib/Traits/ClientServiceTrait.php

@@ -0,0 +1,121 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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 program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Traits;
+
+
+use OCP\Http\Client\IClient;
+use OCP\Http\Client\IClientService;
+use OCP\Http\Client\IResponse;
+
+trait ClientServiceTrait {
+	/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
+	private $clientService;
+	/** @var IClient|\PHPUnit_Framework_MockObject_MockObject */
+	private $client;
+	private $expectedGetRequests = [];
+	private $expectedPostRequests = [];
+
+	/**
+	 * Wrapper to be forward compatible to phpunit 5.4+
+	 *
+	 * @param string $originalClassName
+	 * @return \PHPUnit_Framework_MockObject_MockObject
+	 */
+	abstract protected function createMock($originalClassName);
+
+	/**
+	 * Returns a matcher that matches when the method is executed
+	 * zero or more times.
+	 *
+	 * @return \PHPUnit_Framework_MockObject_Matcher_AnyInvokedCount
+	 *
+	 * @since  Method available since Release 3.0.0
+	 */
+	abstract public function any();
+
+	protected function setUpClientServiceTrait() {
+		$this->clientService = $this->createMock(IClientService::class);
+		$this->client = $this->createMock(IClient::class);
+		$this->clientService->expects($this->any())
+			->method('newClient')
+			->willReturn($this->client);
+		$this->client->expects($this->any())
+			->method('get')
+			->willReturnCallback(function ($url) {
+				if (!isset($this->expectedGetRequests[$url])) {
+					throw new \Exception('unexpected request: ' . $url);
+				}
+				$result = $this->expectedGetRequests[$url];
+
+				if ($result instanceof \Exception) {
+					throw $result;
+				} else {
+					$response = $this->createMock(IResponse::class);
+					$response->expects($this->any())
+						->method('getBody')
+						->willReturn($result);
+					return $response;
+				}
+			});
+		$this->client->expects($this->any())
+			->method('post')
+			->willReturnCallback(function ($url) {
+				if (!isset($this->expectedPostRequests[$url])) {
+					throw new \Exception('unexpected request: ' . $url);
+				}
+				$result = $this->expectedPostRequests[$url];
+
+				if ($result instanceof \Exception) {
+					throw $result;
+				} else {
+					$response = $this->createMock(IResponse::class);
+					$response->expects($this->any())
+						->method('getBody')
+						->willReturn($result);
+					return $response;
+				}
+			});
+	}
+
+	/**
+	 * @param string $url
+	 * @param string|\Exception $result
+	 */
+	protected function expectGetRequest($url, $result) {
+		$this->expectedGetRequests[$url] = $result;
+	}
+
+	/**
+	 * @param string $url
+	 * @param string|\Exception $result
+	 */
+	protected function expectPostRequest($url, $result) {
+		$this->expectedPostRequests[$url] = $result;
+	}
+
+	/**
+	 * @return IClientService|\PHPUnit_Framework_MockObject_MockObject
+	 */
+	protected function getClientService() {
+		return $this->clientService;
+	}
+}