refactor: Port OpenLocalEditor to ORM system

apps/files/lib/BackgroundJob/TransferOwnership.php

@@ -23,27 +23,30 @@
 use Psr\Log\LoggerInterface;
 use function ltrim;
 
-class TransferOwnership extends QueuedJob {
+final class TransferOwnership extends QueuedJob {
 	public function __construct(
 		ITimeFactory $timeFactory,
-		private IUserManager $userManager,
-		private OwnershipTransferService $transferService,
-		private LoggerInterface $logger,
-		private NotificationManager $notificationManager,
-		private TransferOwnershipMapper $mapper,
-		private IRootFolder $rootFolder,
+		private readonly IUserManager $userManager,
+		private readonly OwnershipTransferService $transferService,
+		private readonly LoggerInterface $logger,
+		private readonly NotificationManager $notificationManager,
+		private readonly TransferOwnershipMapper $mapper,
+		private readonly IRootFolder $rootFolder,
 	) {
 		parent::__construct($timeFactory);
 	}
 
+	/**
+	 * @param array{id: int} $argument
+	 */
 	#[\Override]
-	protected function run($argument) {
+	protected function run($argument): void {
 		$id = $argument['id'];
 
 		$transfer = $this->mapper->getById($id);
-		$sourceUser = $transfer->getSourceUser();
-		$destinationUser = $transfer->getTargetUser();
-		$fileId = $transfer->getFileId();
+		$sourceUser = $transfer->sourceUser;
+		$destinationUser = $transfer->targetUser;
+		$fileId = $transfer->fileId;
 
 		$userFolder = $this->rootFolder->getUserFolder($sourceUser);
 		$node = $userFolder->getFirstNodeById($fileId);
@@ -53,7 +56,13 @@ protected function run($argument) {
 			$this->failedNotication($transfer);
 			return;
 		}
+
 		$path = $userFolder->getRelativePath($node->getPath());
+		if ($path === null) {
+			$this->logger->alert('Could not transfer ownership: Node not found');
+			$this->failedNotication($transfer);
+			return;
+		}
 
 		$sourceUserObject = $this->userManager->get($sourceUser);
 		$destinationUserObject = $this->userManager->get($destinationUser);
@@ -77,11 +86,11 @@ protected function run($argument) {
 				ltrim($path, '/')
 			);
 			$this->successNotification($transfer);
-		} catch (TransferOwnershipException $e) {
+		} catch (TransferOwnershipException $transferOwnershipException) {
 			$this->logger->error(
-				$e->getMessage(),
+				$transferOwnershipException->getMessage(),
 				[
-					'exception' => $e,
+					'exception' => $transferOwnershipException,
 				],
 			);
 			$this->failedNotication($transfer);
@@ -93,55 +102,55 @@ protected function run($argument) {
 	private function failedNotication(Transfer $transfer): void {
 		// Send notification to source user
 		$notification = $this->notificationManager->createNotification();
-		$notification->setUser($transfer->getSourceUser())
+		$notification->setUser($transfer->sourceUser)
 			->setApp(Application::APP_ID)
 			->setDateTime($this->time->getDateTime())
 			->setSubject('transferOwnershipFailedSource', [
-				'sourceUser' => $transfer->getSourceUser(),
-				'targetUser' => $transfer->getTargetUser(),
-				'nodeName' => $transfer->getNodeName(),
+				'sourceUser' => $transfer->sourceUser,
+				'targetUser' => $transfer->targetUser,
+				'nodeName' => $transfer->nodeName,
 			])
-			->setObject('transfer', (string)$transfer->getId());
+			->setObject('transfer', (string)$transfer->id);
 		$this->notificationManager->notify($notification);
 		// Send notification to source user
 		$notification = $this->notificationManager->createNotification();
-		$notification->setUser($transfer->getTargetUser())
+		$notification->setUser($transfer->targetUser)
 			->setApp(Application::APP_ID)
 			->setDateTime($this->time->getDateTime())
 			->setSubject('transferOwnershipFailedTarget', [
-				'sourceUser' => $transfer->getSourceUser(),
-				'targetUser' => $transfer->getTargetUser(),
-				'nodeName' => $transfer->getNodeName(),
+				'sourceUser' => $transfer->sourceUser,
+				'targetUser' => $transfer->targetUser,
+				'nodeName' => $transfer->nodeName,
 			])
-			->setObject('transfer', (string)$transfer->getId());
+			->setObject('transfer', (string)$transfer->id);
 		$this->notificationManager->notify($notification);
 	}
 
 	private function successNotification(Transfer $transfer): void {
 		// Send notification to source user
 		$notification = $this->notificationManager->createNotification();
-		$notification->setUser($transfer->getSourceUser())
+		$notification->setUser($transfer->sourceUser)
 			->setApp(Application::APP_ID)
 			->setDateTime($this->time->getDateTime())
 			->setSubject('transferOwnershipDoneSource', [
-				'sourceUser' => $transfer->getSourceUser(),
-				'targetUser' => $transfer->getTargetUser(),
-				'nodeName' => $transfer->getNodeName(),
+				'sourceUser' => $transfer->sourceUser,
+				'targetUser' => $transfer->targetUser,
+				'nodeName' => $transfer->nodeName,
 			])
-			->setObject('transfer', (string)$transfer->getId());
+			->setObject('transfer', (string)$transfer->id);
 		$this->notificationManager->notify($notification);
 
 		// Send notification to source user
 		$notification = $this->notificationManager->createNotification();
-		$notification->setUser($transfer->getTargetUser())
+		$notification->setUser($transfer->targetUser)
 			->setApp(Application::APP_ID)
 			->setDateTime($this->time->getDateTime())
 			->setSubject('transferOwnershipDoneTarget', [
-				'sourceUser' => $transfer->getSourceUser(),
-				'targetUser' => $transfer->getTargetUser(),
-				'nodeName' => $transfer->getNodeName(),
+				'sourceUser' => $transfer->sourceUser,
+				'targetUser' => $transfer->targetUser,
+				'nodeName' => $transfer->nodeName,
 			])
-			->setObject('transfer', (string)$transfer->getId());
+			->setObject('transfer', (string)$transfer->id);
 		$this->notificationManager->notify($notification);
 	}
 }

apps/files/lib/Controller/DirectEditingController.php

@@ -52,9 +52,9 @@ public function info(): DataResponse {
 	/**
 	 * Create a file for direct editing
 	 *
-	 * @param string $path Path of the file
-	 * @param string $editorId ID of the editor
-	 * @param string $creatorId ID of the creator
+	 * @param non-empty-string $path Path of the file
+	 * @param non-empty-string $editorId ID of the editor
+	 * @param non-empty-string $creatorId ID of the creator
 	 * @param ?string $templateId ID of the template
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array{url: string}, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
@@ -88,7 +88,7 @@ public function create(string $path, string $editorId, string $creatorId, ?strin
 	/**
 	 * Open a file for direct editing
 	 *
-	 * @param string $path Path of the file
+	 * @param non-empty-string $path Path of the file
 	 * @param ?string $editorId ID of the editor
 	 * @param ?int $fileId ID of the file
 	 *
@@ -123,8 +123,8 @@ public function open(string $path, ?string $editorId = null, ?int $fileId = null
 	/**
 	 * Get the templates for direct editing
 	 *
-	 * @param string $editorId ID of the editor
-	 * @param string $creatorId ID of the creator
+	 * @param non-empty-string $editorId ID of the editor
+	 * @param non-empty-string $creatorId ID of the creator
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array{templates: array<string, array{id: string, title: string, preview: ?string, extension: string, mimetype: string}>}, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
 	 *

apps/files/lib/Controller/DirectEditingViewController.php

@@ -24,7 +24,7 @@
 #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
 class DirectEditingViewController extends Controller {
 	public function __construct(
-		$appName,
+		string $appName,
 		IRequest $request,
 		private IEventDispatcher $eventDispatcher,
 		private IManager $directEditingManager,
@@ -34,8 +34,7 @@ public function __construct(
 	}
 
 	/**
-	 * @param string $token
-	 * @return Response
+	 * @param non-empty-string $token
 	 */
 	#[PublicPage]
 	#[NoCSRFRequired]

apps/files/lib/Controller/OpenLocalEditorController.php

@@ -21,57 +21,60 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\Exception;
 use OCP\IRequest;
+use OCP\IUser;
 use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 
-class OpenLocalEditorController extends OCSController {
-	public const TOKEN_LENGTH = 128;
-	public const TOKEN_DURATION = 600; // 10 Minutes
-	public const TOKEN_RETRIES = 50;
+final class OpenLocalEditorController extends OCSController {
+	public const int TOKEN_LENGTH = 128;
+
+	// 10 Minutes
+	public const int TOKEN_DURATION = 600;
+
+	public const int TOKEN_RETRIES = 50;
 
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		protected ITimeFactory $timeFactory,
-		protected OpenLocalEditorMapper $mapper,
-		protected ISecureRandom $secureRandom,
-		protected LoggerInterface $logger,
-		protected ?string $userId,
+		private readonly ITimeFactory $timeFactory,
+		private readonly OpenLocalEditorMapper $mapper,
+		private readonly ISecureRandom $secureRandom,
+		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($appName, $request);
 	}
 
 	/**
 	 * Create a local editor
 	 *
-	 * @param string $path Path of the file
+	 * @param non-empty-string $path Path of the file
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array{userId: ?string, pathHash: string, expirationTime: int, token: string}, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, list<empty>, array{}>
 	 *
 	 * 200: Local editor returned
 	 */
 	#[NoAdminRequired]
 	#[UserRateLimit(limit: 10, period: 120)]
-	public function create(string $path): DataResponse {
+	public function create(IUser $user, string $path): DataResponse {
 		$pathHash = sha1($path);
 
 		$entity = new OpenLocalEditor();
-		$entity->setUserId($this->userId);
-		$entity->setPathHash($pathHash);
-		$entity->setExpirationTime($this->timeFactory->getTime() + self::TOKEN_DURATION); // Expire in 10 minutes
+		$entity->userId = $user->getUID();
+		$entity->pathHash = $pathHash;
+		$entity->expirationTime = $this->timeFactory->getTime() + self::TOKEN_DURATION; // Expire in 10 minutes
 
-		for ($i = 1; $i <= self::TOKEN_RETRIES; $i++) {
+		for ($i = 1; $i <= self::TOKEN_RETRIES; ++$i) {
 			$token = $this->secureRandom->generate(self::TOKEN_LENGTH, ISecureRandom::CHAR_ALPHANUMERIC);
-			$entity->setToken($token);
+			$entity->token = $token;
 
 			try {
 				$this->mapper->insert($entity);
 
 				return new DataResponse([
-					'userId' => $this->userId,
+					'userId' => $user->getUID(),
 					'pathHash' => $pathHash,
-					'expirationTime' => $entity->getExpirationTime(),
-					'token' => $entity->getToken(),
+					'expirationTime' => $entity->expirationTime,
+					'token' => $entity->token,
 				]);
 			} catch (Exception $e) {
 				if ($e->getCode() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
@@ -88,8 +91,8 @@ public function create(string $path): DataResponse {
 	/**
 	 * Validate a local editor
 	 *
-	 * @param string $path Path of the file
-	 * @param string $token Token of the local editor
+	 * @param non-empty-string $path Path of the file
+	 * @param non-empty-string $token Token of the local editor
 	 *
 	 * @return DataResponse<Http::STATUS_OK, array{userId: string, pathHash: string, expirationTime: int, token: string}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, list<empty>, array{}>
 	 *
@@ -98,31 +101,30 @@ public function create(string $path): DataResponse {
 	 */
 	#[NoAdminRequired]
 	#[BruteForceProtection(action: 'openLocalEditor')]
-	public function validate(string $path, string $token): DataResponse {
+	public function validate(IUser $user, string $path, string $token): DataResponse {
 		$pathHash = sha1($path);
 
 		try {
-			$entity = $this->mapper->verifyToken($this->userId, $pathHash, $token);
-		} catch (DoesNotExistException $e) {
+			$entity = $this->mapper->verifyToken($user->getUID(), $pathHash, $token);
+		} catch (DoesNotExistException) {
 			$response = new DataResponse([], Http::STATUS_NOT_FOUND);
-			$response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]);
+			$response->throttle(['userId' => $user->getUID(), 'pathHash' => $pathHash]);
 			return $response;
 		}
 
 		$this->mapper->delete($entity);
 
-		if ($entity->getExpirationTime() <= $this->timeFactory->getTime()) {
+		if ($entity->expirationTime <= $this->timeFactory->getTime()) {
 			$response = new DataResponse([], Http::STATUS_NOT_FOUND);
-			$response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]);
+			$response->throttle(['userId' => $user->getUID(), 'pathHash' => $pathHash]);
 			return $response;
 		}
 
 		return new DataResponse([
-			'userId' => $this->userId,
+			'userId' => $user->getUID(),
 			'pathHash' => $pathHash,
-			'expirationTime' => $entity->getExpirationTime(),
-			'token' => $entity->getToken(),
+			'expirationTime' => $entity->expirationTime,
+			'token' => $entity->token,
 		]);
 	}
-
 }