Merge pull request #1046 from nextcloud/dep_getHTTPHelper

Do not use deprecated HTTPHelper
nextcloud · Aug 25, 2016 · 00c767f · 00c767f
2 parents 15bda3c + cae87d0
commit 00c767f
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 32 deletions.
diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php
@@ -242,7 +242,7 @@ private function legacyMountPublicLink($token, $remote, $password, $name, $owner
 			\OC::$server->getDatabaseConnection(),
 			\OC\Files\Filesystem::getMountManager(),
 			\OC\Files\Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC::$server->getUserSession()->getUser()->getUID()

diff --git a/apps/federatedfilesharing/lib/RequestHandler.php b/apps/federatedfilesharing/lib/RequestHandler.php
@@ -147,7 +147,7 @@ public function createShare($params) {
 					\OC::$server->getDatabaseConnection(),
 					\OC\Files\Filesystem::getMountManager(),
 					\OC\Files\Filesystem::getLoader(),
-					\OC::$server->getHTTPHelper(),
+					\OC::$server->getHTTPClientService(),
 					\OC::$server->getNotificationManager(),
 					$discoveryManager,
 					$shareWith

diff --git a/apps/federatedfilesharing/tests/RequestHandlerTest.php b/apps/federatedfilesharing/tests/RequestHandlerTest.php
@@ -250,7 +250,7 @@ function testDeleteUser($toDelete, $expected, $remainingUsers) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			$toDelete

diff --git a/apps/files_sharing/lib/API/Remote.php b/apps/files_sharing/lib/API/Remote.php
@@ -45,7 +45,7 @@ public static function getOpenShares($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()
@@ -69,7 +69,7 @@ public static function acceptShare($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()
@@ -100,7 +100,7 @@ public static function declineShare($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()
@@ -148,7 +148,7 @@ public static function getShares($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()
@@ -176,7 +176,7 @@ public static function getShare($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()
@@ -207,7 +207,7 @@ public static function unshare($params) {
 			\OC::$server->getDatabaseConnection(),
 			Filesystem::getMountManager(),
 			Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			\OC_User::getUser()

diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php
@@ -105,7 +105,7 @@ public function __construct(array $urlParams = array()) {
 				$server->getDatabaseConnection(),
 				\OC\Files\Filesystem::getMountManager(),
 				\OC\Files\Filesystem::getLoader(),
-				$server->getHTTPHelper(),
+				$server->getHTTPClientService(),
 				$server->getNotificationManager(),
 				$discoveryManager,
 				$uid

diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php
@@ -32,6 +32,7 @@
 use OC\Files\Filesystem;
 use OCA\FederatedFileSharing\DiscoveryManager;
 use OCP\Files;
+use OCP\Http\Client\IClientService;
 use OCP\Notification\IManager;
 
 class Manager {
@@ -58,9 +59,9 @@ class Manager {
 	private $storageLoader;
 
 	/**
-	 * @var \OC\HTTPHelper
+	 * @var IClientService
 	 */
-	private $httpHelper;
+	private $clientService;
 
 	/**
 	 * @var IManager
@@ -73,22 +74,22 @@ class Manager {
 	 * @param \OCP\IDBConnection $connection
 	 * @param \OC\Files\Mount\Manager $mountManager
 	 * @param \OCP\Files\Storage\IStorageFactory $storageLoader
-	 * @param \OC\HTTPHelper $httpHelper
+	 * @param IClientService $clientService
 	 * @param IManager $notificationManager
 	 * @param DiscoveryManager $discoveryManager
 	 * @param string $uid
 	 */
 	public function __construct(\OCP\IDBConnection $connection,
 								\OC\Files\Mount\Manager $mountManager,
 								\OCP\Files\Storage\IStorageFactory $storageLoader,
-								\OC\HTTPHelper $httpHelper,
+								IClientService $clientService,
 								IManager $notificationManager,
 								DiscoveryManager $discoveryManager,
 								$uid) {
 		$this->connection = $connection;
 		$this->mountManager = $mountManager;
 		$this->storageLoader = $storageLoader;
-		$this->httpHelper = $httpHelper;
+		$this->clientService = $clientService;
 		$this->uid = $uid;
 		$this->notificationManager = $notificationManager;
 		$this->discoveryManager = $discoveryManager;
@@ -262,10 +263,23 @@ private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) {
 		$url = rtrim($remote, '/') . $this->discoveryManager->getShareEndpoint($remote) . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT;
 		$fields = array('token' => $token);
 
-		$result = $this->httpHelper->post($url, $fields);
-		$status = json_decode($result['result'], true);
+		$client = $this->clientService->newClient();
+
+		try {
+			$response = $client->post(
+				$url,
+				[
+					'body' => $fields,
+					'connect_timeout' => 10,
+				]
+			);
+		} catch (\Exception $e) {
+			return false;
+		}
+
+		$status = json_decode($response->getBody(), true);
 
-		return ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200));
+		return ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200);
 	}
 
 	/**

diff --git a/apps/files_sharing/lib/Hooks.php b/apps/files_sharing/lib/Hooks.php
@@ -40,7 +40,7 @@ public static function deleteUser($params) {
 			\OC::$server->getDatabaseConnection(),
 			\OC\Files\Filesystem::getMountManager(),
 			\OC\Files\Filesystem::getLoader(),
-			\OC::$server->getHTTPHelper(),
+			\OC::$server->getHTTPClientService(),
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			$params['uid']);

diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php
@@ -30,6 +30,7 @@
 use OCA\Files_Sharing\External\Manager;
 use OCA\Files_Sharing\External\MountProvider;
 use OCA\Files_Sharing\Tests\TestCase;
+use OCP\Http\Client\IClientService;
 use Test\Traits\UserTrait;
 
 /**
@@ -48,8 +49,8 @@ class ManagerTest extends TestCase {
 	/** @var \OC\Files\Mount\Manager */
 	private $mountManager;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
-	private $httpHelper;
+	/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
+	private $clientService;
 
 	private $uid;
 
@@ -66,17 +67,17 @@ protected function setUp() {
 		$this->createUser($this->uid, '');
 		$this->user = \OC::$server->getUserManager()->get($this->uid);
 		$this->mountManager = new \OC\Files\Mount\Manager();
-		$this->httpHelper = $httpHelper = $this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock();
+		$this->clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')
+			->disableOriginalConstructor()->getMock();
 		$discoveryManager = new DiscoveryManager(
 			\OC::$server->getMemCacheFactory(),
 			\OC::$server->getHTTPClientService()
 		);
-		/** @var \OC\HTTPHelper $httpHelper */
 		$this->manager = new Manager(
 			\OC::$server->getDatabaseConnection(),
 			$this->mountManager,
 			new StorageFactory(),
-			$httpHelper,
+			$this->clientService,
 			\OC::$server->getNotificationManager(),
 			$discoveryManager,
 			$this->uid
@@ -132,9 +133,17 @@ public function testAddShare() {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$this->httpHelper->expects($this->at(0))
+		$client = $this->getMockBuilder('OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService->expects($this->at(0))
+			->method('newClient')
+			->willReturn($client);
+		$response = $this->getMockBuilder('OCP\Http\Client\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->once())
 			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything());
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything())
+			->willReturn($response);
 
 		// Accept the first share
 		$this->manager->acceptShare($openShares[0]['id']);
@@ -167,9 +176,17 @@ public function testAddShare() {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$this->httpHelper->expects($this->at(0))
+		$client = $this->getMockBuilder('OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService->expects($this->at(0))
+			->method('newClient')
+			->willReturn($client);
+		$response = $this->getMockBuilder('OCP\Http\Client\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client->expects($this->once())
 			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything());
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything())
+			->willReturn($response);
 
 		// Decline the third share
 		$this->manager->declineShare($openShares[1]['id']);
@@ -194,12 +211,26 @@ public function testAddShare() {
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}');
 		$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
 
-		$this->httpHelper->expects($this->at(0))
+		$client1 = $this->getMockBuilder('OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$client2 = $this->getMockBuilder('OCP\Http\Client\IClient')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService->expects($this->at(0))
+			->method('newClient')
+			->willReturn($client1);
+		$this->clientService->expects($this->at(1))
+			->method('newClient')
+			->willReturn($client2);
+		$response = $this->getMockBuilder('OCP\Http\Client\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client1->expects($this->once())
 			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything());
-		$this->httpHelper->expects($this->at(1))
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything())
+			->willReturn($response);
+		$client2->expects($this->once())
 			->method('post')
-			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything());
+			->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything())
+			->willReturn($response);
 
 		$this->manager->removeUserShares($this->uid);
 		$this->assertEmpty(self::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted');