From 53b29213512552345d331a36784e647db5766a6e Mon Sep 17 00:00:00 2001 From: Navid Shokri Date: Fri, 14 Jul 2023 09:13:59 +0000 Subject: [PATCH 1/4] improve logics --- apps/files_sharing/js/app.js | 7 ++--- apps/files_sharing/js/sharedfilelist.js | 1 - .../lib/Controller/RemoteOcsController.php | 16 ++++++++-- .../lib/Controller/Share20OcsController.php | 10 +++--- .../Controllers/ExternalSharesController.php | 31 ++++++++++--------- lib/private/Share20/Manager.php | 25 +++++++++------ 6 files changed, 53 insertions(+), 37 deletions(-) diff --git a/apps/files_sharing/js/app.js b/apps/files_sharing/js/app.js index c3575d2fb540..0198285c444d 100644 --- a/apps/files_sharing/js/app.js +++ b/apps/files_sharing/js/app.js @@ -214,16 +214,13 @@ OCA.Sharing.App = { if (state === OC.Share.STATE_REJECTED) { method = 'DELETE'; } - var endPoint = isRemote === true ? 'remote_shares/pending/' : 'shares/pending/'; var xhr = $.ajax({ url: OC.linkToOCS('apps/files_sharing/api/v1') + endPoint + encodeURIComponent(fileId) + '?format=json', contentType: 'application/json', dataType: 'json', type: method, - data: JSON.stringify({ - ...(!!shareType && { shareType }), - }), + data: JSON.stringify((Boolean(shareType) ? { shareType: shareType } : {})), }); xhr.fail(function(response) { var message = ''; @@ -239,7 +236,7 @@ OCA.Sharing.App = { _shareStateActionHandler: function(context, newState) { var targetFileData = context.fileList.elementToFile(context.$file); var isRemote = targetFileData.shareLocationType === 'remote'; - const { shareType } = targetFileData; + const shareType = targetFileData.shareType; function responseCallback(response, status) { if (status === 'success') { var meta = response.ocs.meta; diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index e6a288d30cc4..e9b55312cbac 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -346,7 +346,6 @@ files = _.chain(files) // convert share data to file data .map(function(share) { - const {} = share var file = { shareOwner: share.owner + '@' + share.remote.replace(/.*?:\/\//g, ""), shareState: !!parseInt(share.accepted, 10) ? OC.Share.STATE_ACCEPTED : OC.Share.STATE_PENDING, diff --git a/apps/files_sharing/lib/Controller/RemoteOcsController.php b/apps/files_sharing/lib/Controller/RemoteOcsController.php index 87068f5ca4e5..6530161fbfee 100644 --- a/apps/files_sharing/lib/Controller/RemoteOcsController.php +++ b/apps/files_sharing/lib/Controller/RemoteOcsController.php @@ -30,6 +30,7 @@ use OCP\Files\StorageNotAvailableException; use OCP\Files\StorageInvalidException; use OCP\IConfig; +use OCP\ILogger; class RemoteOcsController extends OCSController { /** @var IRequest */ @@ -46,6 +47,11 @@ class RemoteOcsController extends OCSController { */ protected $config; + /** + * @var ILogger + */ + protected $logger; + /** * RemoteOcsController constructor. * @@ -53,6 +59,7 @@ class RemoteOcsController extends OCSController { * @param IRequest $request * @param Manager $externalManager * @param IConfig config + * @param ILogger $loggar * @param string $uid */ public function __construct( @@ -60,12 +67,15 @@ public function __construct( IRequest $request, Manager $externalManager, IConfig $config, + ILogger $logger, $uid + ) { parent::__construct($appName, $request); $this->request = $request; $this->externalManager = $externalManager; $this->config = $config; + $this->logger = $logger; $this->uid = $uid; } @@ -156,9 +166,9 @@ public function getShares($includingPending = false) { try { $shares[] = $this->extendShareInfo($shareInfo); } catch (StorageNotAvailableException $e) { - //TODO: Log the exception here? There are several logs already below the stack + $this->logger->logException($e, ['app' => 'files_sharing']); } catch (StorageInvalidException $e) { - //TODO: Log the exception here? There are several logs already below the stack + $this->logger->logException($e, ['app' => 'files_sharing']); } } } @@ -181,7 +191,7 @@ function ($share) { }, \array_merge( $this->externalManager->getOpenShares(), - $groupExternalManager == null ? [] : $groupExternalManager->getOpenShares() + $groupExternalManager === null ? [] : $groupExternalManager->getOpenShares() ) ); $shares = \array_merge($shares, $openShares); diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index 14724e9358e2..e9f91b2dfa1f 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -1215,6 +1215,7 @@ private function getShareById($id, $recipient = null) { continue; } catch (ProviderException $e) { + // We should iterate all provider to find proper provider for given share continue; } } @@ -1300,10 +1301,11 @@ private function getSupportedShareTypes() { $shareTypes = []; foreach ($providersCapabilities as $capabilities) { - $shareTypes = array_merge($shareTypes, array_keys($capabilities)); - } - - $shareTypes = array_keys(array_intersect(Share::CONVERT_SHARE_TYPE_TO_STRING, $shareTypes)); + foreach ($capabilities as $key => $value) { + $shareTypes[] = $key; + } + } + $shareTypes = \array_unique($shareTypes); return $shareTypes; } diff --git a/apps/files_sharing/lib/Controllers/ExternalSharesController.php b/apps/files_sharing/lib/Controllers/ExternalSharesController.php index 668b357a9452..db68b8e755eb 100644 --- a/apps/files_sharing/lib/Controllers/ExternalSharesController.php +++ b/apps/files_sharing/lib/Controllers/ExternalSharesController.php @@ -54,6 +54,7 @@ class ExternalSharesController extends Controller { /** @var IConfig $config */ private $config; + const group_share_type = "group"; /** * ExternalSharesController constructor. * @@ -77,11 +78,6 @@ public function __construct( $this->clientService = $clientService; $this->dispatcher = $eventDispatcher; $this->config = $config; - // Allow other apps to add an external manager for user-to-group shares - $managerClass = $this->config->getSystemValue('sharing.groupExternalManager'); - if ($managerClass !== '') { - $this->groupExternalManager = \OC::$server->query($managerClass); - } } /** @@ -92,8 +88,9 @@ public function __construct( */ public function index() { $federatedGroupResult = []; - if ($this->groupExternalManager !== null) { - $federatedGroupResult = $this->groupExternalManager->getOpenShares(); + $groupExternalManager = $this->initGroupManager(); + if ($groupExternalManager !== null) { + $federatedGroupResult = $groupExternalManager->getOpenShares(); } $result = array_merge($federatedGroupResult, $this->externalManager->getOpenShares()); return new JSONResponse($result); @@ -108,11 +105,8 @@ public function index() { * @return JSONResponse */ public function create($id, $share_type) { - if ($share_type === "group" && $this->groupExternalManager !== null) { - $manager = $this->groupExternalManager; - } else { - $manager = $this->externalManager; - } + + $manager = $this->getManagerForShareType($share_type); $shareInfo = $manager->getShare($id); if ($shareInfo !== false) { @@ -163,9 +157,18 @@ public function destroy($id, $share_type) { return new JSONResponse(); } + private function initGroupManager(){ + // Allow other apps to add an external manager for user-to-group shares + $managerClass = $this->config->getSystemValue('sharing.groupExternalManager'); + if ($managerClass !== '') { + return \OC::$server->query($managerClass); + } + return null; + } private function getManagerForShareType($share_type) { - if ($share_type === "group" && $this->groupExternalManager !== null) { - $manager = $this->groupExternalManager; + $groupExternalManager = $this->initGroupManager(); + if ($share_type === self::group_share_type && $groupExternalManager !== null) { + $manager = $groupExternalManager; } else { $manager = $this->externalManager; } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 7b9ebd7a9a5a..d2f911106d8a 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1538,19 +1538,24 @@ public function getShareByToken($token) { "shared file not found by token: $token for federated user share, try to check federated group share.", ['app' => __CLASS__] ); - } - } - - if ($share === null) { - try { - $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_REMOTE_GROUP); - if ($provider !== null) { - $share = $provider->getShareByToken($token); + try { + $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_REMOTE_GROUP); + if ($provider !== null) { + $share = $provider->getShareByToken($token); + } + } catch (ShareNotFound $ex) { + $this->logger->error( + "shared file not found by token: $token for federated group share", + ['app' => __CLASS__] + ); + } catch (ProviderException $ex) { + $this->logger->logException( + $ex, + ['app' => __CLASS__] + ); } - } catch (ProviderException $ex) { } } - if (self::shareHasExpired($share)) { $this->activityManager->setAgentAuthor(IEvent::AUTOMATION_AUTHOR); try { From 36b625d99eda7a5ef9343d111451938e72089f4f Mon Sep 17 00:00:00 2001 From: Navid Shokri Date: Fri, 14 Jul 2023 09:19:06 +0000 Subject: [PATCH 2/4] adjust code format --- .../files_sharing/lib/Controller/Share20OcsController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index e9f91b2dfa1f..cf54e5ad1f4b 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -1302,11 +1302,11 @@ private function getSupportedShareTypes() { foreach ($providersCapabilities as $capabilities) { foreach ($capabilities as $key => $value) { - $shareTypes[] = $key; + $shareTypes[] = $key; } - } - $shareTypes = \array_unique($shareTypes); - + } + + $shareTypes = \array_unique($shareTypes); return $shareTypes; } } From 263ceaa2c2389d5778843042e9a6afa5b4c2536f Mon Sep 17 00:00:00 2001 From: Navid Shokri Date: Fri, 14 Jul 2023 09:21:44 +0000 Subject: [PATCH 3/4] remove extra space --- lib/private/Share20/Manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index d2f911106d8a..4a69bd7f4c84 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1547,7 +1547,7 @@ public function getShareByToken($token) { $this->logger->error( "shared file not found by token: $token for federated group share", ['app' => __CLASS__] - ); + ); } catch (ProviderException $ex) { $this->logger->logException( $ex, From 2bea3c659bb2b476788a7ee8aef497c1f7706812 Mon Sep 17 00:00:00 2001 From: Navid Shokri Date: Fri, 14 Jul 2023 09:21:44 +0000 Subject: [PATCH 4/4] remove extra space --- apps/files_sharing/lib/Controller/RemoteOcsController.php | 1 - .../lib/Controllers/ExternalSharesController.php | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/lib/Controller/RemoteOcsController.php b/apps/files_sharing/lib/Controller/RemoteOcsController.php index 6530161fbfee..8d82b28cf736 100644 --- a/apps/files_sharing/lib/Controller/RemoteOcsController.php +++ b/apps/files_sharing/lib/Controller/RemoteOcsController.php @@ -69,7 +69,6 @@ public function __construct( IConfig $config, ILogger $logger, $uid - ) { parent::__construct($appName, $request); $this->request = $request; diff --git a/apps/files_sharing/lib/Controllers/ExternalSharesController.php b/apps/files_sharing/lib/Controllers/ExternalSharesController.php index db68b8e755eb..7cc05cca6921 100644 --- a/apps/files_sharing/lib/Controllers/ExternalSharesController.php +++ b/apps/files_sharing/lib/Controllers/ExternalSharesController.php @@ -54,7 +54,7 @@ class ExternalSharesController extends Controller { /** @var IConfig $config */ private $config; - const group_share_type = "group"; + public const group_share_type = "group"; /** * ExternalSharesController constructor. * @@ -88,7 +88,7 @@ public function __construct( */ public function index() { $federatedGroupResult = []; - $groupExternalManager = $this->initGroupManager(); + $groupExternalManager = $this->initGroupManager(); if ($groupExternalManager !== null) { $federatedGroupResult = $groupExternalManager->getOpenShares(); } @@ -105,7 +105,6 @@ public function index() { * @return JSONResponse */ public function create($id, $share_type) { - $manager = $this->getManagerForShareType($share_type); $shareInfo = $manager->getShare($id); @@ -157,7 +156,7 @@ public function destroy($id, $share_type) { return new JSONResponse(); } - private function initGroupManager(){ + private function initGroupManager() { // Allow other apps to add an external manager for user-to-group shares $managerClass = $this->config->getSystemValue('sharing.groupExternalManager'); if ($managerClass !== '') { @@ -165,6 +164,7 @@ private function initGroupManager(){ } return null; } + private function getManagerForShareType($share_type) { $groupExternalManager = $this->initGroupManager(); if ($share_type === self::group_share_type && $groupExternalManager !== null) {