Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor federatedfilesharing app controllers #39177

Closed
wants to merge 5 commits into from
Closed

refactor federatedfilesharing app controllers #39177

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
589b83a
refactor federatedfilesharing app controllers
shdehnavi Jul 5, 2023
9aee7e0
Make adjustments based on the review
shdehnavi Jul 6, 2023
112b83a
Make adjustments based on the review
shdehnavi Jul 6, 2023
a9dbe71
Make adjustments based on the review
shdehnavi Jul 14, 2023
6b79ad7
Make adjustments based on the review
shdehnavi Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 14 additions & 46 deletions apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,6 @@
*/
class MountPublicLinkController extends Controller {

/** @var FederatedShareProvider */
private $federatedShareProvider;

/** @var AddressHandler */
private $addressHandler;

/** @var IManager */
private $shareManager;

/** @var ISession */
private $session;

/** @var IL10N */
private $l;

/** @var IUserSession */
private $userSession;

/** @var IClientService */
private $clientService;

/** @var ICloudIdManager */
private $cloudIdManager;

/**
* MountPublicLinkController constructor.
*
Expand All @@ -95,27 +71,19 @@ class MountPublicLinkController extends Controller {
* @param IClientService $clientService
* @param ICloudIdManager $cloudIdManager
*/
public function __construct($appName,
IRequest $request,
FederatedShareProvider $federatedShareProvider,
IManager $shareManager,
AddressHandler $addressHandler,
ISession $session,
IL10N $l,
IUserSession $userSession,
IClientService $clientService,
ICloudIdManager $cloudIdManager
public function __construct(
$appName,
IRequest $request,
private FederatedShareProvider $federatedShareProvider,
private IManager $shareManager,
private AddressHandler $addressHandler,
private ISession $session,
private IL10N $l,
private IUserSession $userSession,
private IClientService $clientService,
private ICloudIdManager $cloudIdManager,
) {
parent::__construct($appName, $request);

$this->federatedShareProvider = $federatedShareProvider;
$this->shareManager = $shareManager;
$this->addressHandler = $addressHandler;
$this->session = $session;
$this->l = $l;
$this->userSession = $userSession;
$this->clientService = $clientService;
$this->cloudIdManager = $cloudIdManager;
}

/**
Expand All @@ -130,7 +98,7 @@ public function __construct($appName,
* @param string $password
* @return JSONResponse
*/
public function createFederatedShare($shareWith, $token, $password = '') {
public function createFederatedShare(string $shareWith, string $token, string $password = ''): JSONResponse {
if (!$this->federatedShareProvider->isOutgoingServer2serverShareEnabled()) {
return new JSONResponse(
['message' => 'This server doesn\'t support outgoing federated shares'],
Expand Down Expand Up @@ -198,7 +166,7 @@ public function createFederatedShare($shareWith, $token, $password = '') {
* @param string $name (only for legacy reasons, can be removed with legacyMountPublicLink())
* @return JSONResponse
*/
public function askForFederatedShare($token, $remote, $password = '', $owner = '', $ownerDisplayName = '', $name = '') {
public function askForFederatedShare(string $token, string $remote, string $password = '', string $owner = '', string $ownerDisplayName = '', string $name = ''): JSONResponse {
// check if server admin allows to mount public links from other servers
if ($this->federatedShareProvider->isIncomingServer2serverShareEnabled() === false) {
return new JSONResponse(['message' => $this->l->t('Server to server sharing is not enabled on this server')], Http::STATUS_BAD_REQUEST);
Expand Down Expand Up @@ -236,7 +204,7 @@ public function askForFederatedShare($token, $remote, $password = '', $owner = '
return new JSONResponse(['message' => $this->l->t('Federated Share request sent, you will receive an invitation. Check your notifications.')]);
}

// if we doesn't get the expected response we assume that we try to add
// if we don't get the expected response we assume that we try to add
// a federated share from a Nextcloud <= 9 server
$message = $this->l->t("Couldn't establish a federated share, it looks like the server to federate with is too old (Nextcloud <= 9).");
return new JSONResponse(['message' => $message], Http::STATUS_BAD_REQUEST);
Expand Down
116 changes: 39 additions & 77 deletions apps/federatedfilesharing/lib/Controller/RequestHandlerController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCA\FederatedFileSharing\Notifications;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCSController;
use OCP\App\IAppManager;
use OCP\Constants;
use OCP\DB\Exception;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Federation\Exceptions\ProviderCouldNotAddShareException;
use OCP\Federation\Exceptions\ProviderDoesNotExistsException;
Expand All @@ -48,73 +50,31 @@
use OCP\Log\Audit\CriticalActionPerformedEvent;
use OCP\Share;
use OCP\Share\Exceptions\ShareNotFound;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Psr\Log\LoggerInterface;

class RequestHandlerController extends OCSController {

/** @var FederatedShareProvider */
private $federatedShareProvider;

/** @var IDBConnection */
private $connection;

/** @var Share\IManager */
private $shareManager;

/** @var Notifications */
private $notifications;

/** @var AddressHandler */
private $addressHandler;

/** @var IUserManager */
private $userManager;

/** @var string */
private $shareTable = 'share';

/** @var ICloudIdManager */
private $cloudIdManager;

/** @var LoggerInterface */
private $logger;

/** @var ICloudFederationFactory */
private $cloudFederationFactory;

/** @var ICloudFederationProviderManager */
private $cloudFederationProviderManager;

/** @var IEventDispatcher */
private $eventDispatcher;

public function __construct(string $appName,
IRequest $request,
FederatedShareProvider $federatedShareProvider,
IDBConnection $connection,
Share\IManager $shareManager,
Notifications $notifications,
AddressHandler $addressHandler,
IUserManager $userManager,
ICloudIdManager $cloudIdManager,
LoggerInterface $logger,
ICloudFederationFactory $cloudFederationFactory,
ICloudFederationProviderManager $cloudFederationProviderManager,
IEventDispatcher $eventDispatcher
private string $shareTable = 'share';

public function __construct(
string $appName,
IRequest $request,
private FederatedShareProvider $federatedShareProvider,
private IDBConnection $connection,
private Share\IManager $shareManager,
private Notifications $notifications,
private AddressHandler $addressHandler,
private IUserManager $userManager,
private ICloudIdManager $cloudIdManager,
private LoggerInterface $logger,
private ICloudFederationFactory $cloudFederationFactory,
private ICloudFederationProviderManager $cloudFederationProviderManager,
private IEventDispatcher $eventDispatcher,
) {
parent::__construct($appName, $request);

$this->federatedShareProvider = $federatedShareProvider;
$this->connection = $connection;
$this->shareManager = $shareManager;
$this->notifications = $notifications;
$this->addressHandler = $addressHandler;
$this->userManager = $userManager;
$this->cloudIdManager = $cloudIdManager;
$this->logger = $logger;
$this->cloudFederationFactory = $cloudFederationFactory;
$this->cloudFederationProviderManager = $cloudFederationProviderManager;
$this->eventDispatcher = $eventDispatcher;
}

/**
Expand Down Expand Up @@ -145,7 +105,7 @@ public function createShare(
?int $remoteId = null,
?string $sharedByFederatedId = null,
?string $ownerFederatedId = null,
) {
): Http\DataResponse {
if ($ownerFederatedId === null) {
$ownerFederatedId = $this->cloudIdManager->getCloudId($owner, $this->cleanupRemote($remote))->getId();
}
Expand Down Expand Up @@ -203,7 +163,7 @@ public function createShare(
* @throws OCSBadRequestException
* @throws OCSException
*/
public function reShare(int $id, ?string $token = null, ?string $shareWith = null, ?int $permission = 0, ?int $remoteId = 0) {
public function reShare(int $id, ?string $token = null, ?string $shareWith = null, ?int $permission = 0, ?int $remoteId = 0): Http\DataResponse {
if ($token === null ||
$shareWith === null ||
$permission === null ||
Expand Down Expand Up @@ -246,12 +206,10 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul
*
* @param int $id
* @param string|null $token
* @return Http\DataResponse
* @return DataResponse
* @throws OCSException
* @throws ShareNotFound
* @throws \OCP\HintException
*/
public function acceptShare(int $id, ?string $token = null) {
public function acceptShare(int $id, ?string $token = null): DataResponse {
$notification = [
'sharedSecret' => $token,
'message' => 'Recipient accept the share'
Expand Down Expand Up @@ -283,7 +241,7 @@ public function acceptShare(int $id, ?string $token = null) {
* @return Http\DataResponse
* @throws OCSException
*/
public function declineShare(int $id, ?string $token = null) {
public function declineShare(int $id, ?string $token = null): DataResponse {
$notification = [
'sharedSecret' => $token,
'message' => 'Recipient declined the share'
Expand Down Expand Up @@ -315,7 +273,7 @@ public function declineShare(int $id, ?string $token = null) {
* @return Http\DataResponse
* @throws OCSException
*/
public function unshare(int $id, ?string $token = null) {
public function unshare(int $id, ?string $token = null): DataResponse {
if (!$this->isS2SEnabled()) {
throw new OCSException('Server does not support federated cloud sharing', 503);
}
Expand All @@ -332,7 +290,7 @@ public function unshare(int $id, ?string $token = null) {
return new Http\DataResponse();
}

private function cleanupRemote($remote) {
private function cleanupRemote(?string $remote = null): string {
shdehnavi marked this conversation as resolved.
Show resolved Hide resolved
$remote = substr($remote, strpos($remote, '://') + 3);

return rtrim($remote, '/');
Expand All @@ -350,7 +308,7 @@ private function cleanupRemote($remote) {
* @return Http\DataResponse
* @throws OCSBadRequestException
*/
public function revoke(int $id, ?string $token = null) {
public function revoke(int $id, ?string $token = null): DataResponse {
try {
$provider = $this->cloudFederationProviderManager->getCloudFederationProvider('file');
$notification = ['sharedSecret' => $token];
Expand All @@ -366,8 +324,10 @@ public function revoke(int $id, ?string $token = null) {
*
* @param bool $incoming
* @return bool
* @throws ContainerExceptionInterface
* @throws NotFoundExceptionInterface
shdehnavi marked this conversation as resolved.
Show resolved Hide resolved
*/
private function isS2SEnabled($incoming = false) {
private function isS2SEnabled(bool $incoming = false): bool {
$result = \OCP\Server::get(IAppManager::class)->isEnabledForUser('files_sharing');

if ($incoming) {
Expand All @@ -391,7 +351,7 @@ private function isS2SEnabled($incoming = false) {
* @return Http\DataResponse
* @throws OCSBadRequestException
*/
public function updatePermissions(int $id, ?string $token = null, ?int $permissions = null) {
public function updatePermissions(int $id, ?string $token = null, ?int $permissions = null): DataResponse {
$ncPermissions = $permissions;

try {
Expand All @@ -411,10 +371,10 @@ public function updatePermissions(int $id, ?string $token = null, ?int $permissi
/**
* translate Nextcloud permissions to OCM Permissions
*
* @param $ncPermissions
* @param int|null $ncPermissions
* @return array
*/
protected function ncPermissions2ocmPermissions($ncPermissions) {
protected function ncPermissions2ocmPermissions(?int $ncPermissions = null): array {
shdehnavi marked this conversation as resolved.
Show resolved Hide resolved
$ocmPermissions = [];

if ($ncPermissions & Constants::PERMISSION_SHARE) {
Expand Down Expand Up @@ -443,12 +403,14 @@ protected function ncPermissions2ocmPermissions($ncPermissions) {
* @param string|null $token
* @param string|null $remote
* @param string|null $remote_id
* @return Http\DataResponse
* @return DataResponse
* @throws ContainerExceptionInterface
* @throws NotFoundExceptionInterface
shdehnavi marked this conversation as resolved.
Show resolved Hide resolved
* @throws OCSBadRequestException
* @throws OCSException
* @throws \OCP\DB\Exception
* @throws Exception
*/
public function move(int $id, ?string $token = null, ?string $remote = null, ?string $remote_id = null) {
public function move(int $id, ?string $token = null, ?string $remote = null, ?string $remote_id = null): DataResponse {
if (!$this->isS2SEnabled()) {
throw new OCSException('Server does not support federated cloud sharing', 503);
}
Expand Down