Merge pull request #5823 from nextcloud/improve-error-handling

Improve error handling for accepting federated shares

3 files changed, 22 insertions, 13 deletions

diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php
index 4f64e6147e1..529d7f7db67 100644
--- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php
+++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php
@@ -194,12 +194,12 @@ class RequestHandlerController extends OCSController {
 
 				$declineAction = $notification->createAction();
 				$declineAction->setLabel('decline')
-					->setLink($urlGenerator->getAbsoluteURL($urlGenerator->linkTo('', 'ocs/v1.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'DELETE');
+					->setLink($urlGenerator->getAbsoluteURL($urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'DELETE');
 				$notification->addAction($declineAction);
 
 				$acceptAction = $notification->createAction();
 				$acceptAction->setLabel('accept')
-					->setLink($urlGenerator->getAbsoluteURL($urlGenerator->linkTo('', 'ocs/v1.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'POST');
+					->setLink($urlGenerator->getAbsoluteURL($urlGenerator->linkTo('', 'ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/' . $shareId)), 'POST');
 				$notification->addAction($acceptAction);
 
 				$notificationManager->notify($notification);
diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php
index 7c7a608ff2d..299b6fe4df7 100644
--- a/apps/files_sharing/lib/Controller/RemoteController.php
+++ b/apps/files_sharing/lib/Controller/RemoteController.php
@@ -29,6 +29,7 @@ use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\OCS\OCSNotFoundException;
 use OCP\AppFramework\OCSController;
+use OCP\ILogger;
 use OCP\IRequest;
 
 class RemoteController extends OCSController {
@@ -36,6 +37,9 @@ class RemoteController extends OCSController {
 	/** @var Manager */
 	private $externalManager;
 
+	/** @var ILogger */
+	private $logger;
+
 	/**
 	 * @NoAdminRequired
 	 *
@@ -47,10 +51,12 @@ class RemoteController extends OCSController {
 	 */
 	public function __construct($appName,
 								IRequest $request,
-								Manager $externalManager) {
+								Manager $externalManager,
+								ILogger $logger) {
 		parent::__construct($appName, $request);
 
 		$this->externalManager = $externalManager;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -78,8 +84,8 @@ class RemoteController extends OCSController {
 			return new DataResponse();
 		}
 
-		// Make sure the user has no notification for something that does not exist anymore.
-		$this->externalManager->processNotification($id);
+		$this->logger->error('Could not accept federated share with id: ' . $id,
+			['app' => 'files_sharing']);
 
 		throw new OCSNotFoundException('wrong share ID, share doesn\'t exist.');
 	}
diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php
index 9473ce86658..ab20e37a79b 100644
--- a/apps/files_sharing/lib/External/Manager.php
+++ b/apps/files_sharing/lib/External/Manager.php
@@ -197,6 +197,7 @@ class Manager {
 	public function acceptShare($id) {
 
 		$share = $this->getShare($id);
+		$result = false;
 
 		if ($share) {
 			\OC_Util::setupFS($this->uid);
@@ -211,16 +212,18 @@ class Manager {
 					`mountpoint` = ?,
 					`mountpoint_hash` = ?
 				WHERE `id` = ? AND `user` = ?');
-			$acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
-			$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept');
-
-			\OC_Hook::emit('OCP\Share', 'federated_share_added', ['server' => $share['remote']]);
-
-			$this->processNotification($id);
-			return true;
+			$updated = $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid));
+			if ($updated === true) {
+				$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept');
+				\OC_Hook::emit('OCP\Share', 'federated_share_added', ['server' => $share['remote']]);
+				$result = true;
+			}
 		}
 
-		return false;
+		// Make sure the user has no notification for something that does not exist anymore.
+		$this->processNotification($id);
+
+		return $result;
 	}
 
 	/**