feat(directediting): Allow opening by file id

Signed-off-by: Julius Härtl <jus@bitgrid.net>
nextcloud · Feb 20, 2023 · 614981a · 614981a
1 parent 6bb0985
commit 614981a
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 9 deletions.
diff --git a/apps/files/lib/Controller/DirectEditingController.php b/apps/files/lib/Controller/DirectEditingController.php
@@ -35,7 +35,6 @@
 use OCP\IURLGenerator;
 
 class DirectEditingController extends OCSController {
-
 	/** @var IEventDispatcher */
 	private $eventDispatcher;
 
@@ -94,14 +93,14 @@ public function create(string $path, string $editorId, string $creatorId, string
 	/**
 	 * @NoAdminRequired
 	 */
-	public function open(string $path, string $editorId = null): DataResponse {
+	public function open(string $path, string $editorId = null, ?int $fileId = null): DataResponse {
 		if (!$this->directEditingManager->isEnabled()) {
 			return new DataResponse(['message' => 'Direct editing is not enabled'], Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));
 
 		try {
-			$token = $this->directEditingManager->open($path, $editorId);
+			$token = $this->directEditingManager->open($path, $editorId, $fileId);
 			return new DataResponse([
 				'url' => $this->urlGenerator->linkToRouteAbsolute('files.DirectEditingView.edit', ['token' => $token])
 			]);

diff --git a/apps/files/lib/DirectEditingCapabilities.php b/apps/files/lib/DirectEditingCapabilities.php
@@ -1,4 +1,5 @@
 <?php
+
 declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2022 Julius Härtl <jus@bitgrid.net>
@@ -29,7 +30,6 @@
 use OCP\IURLGenerator;
 
 class DirectEditingCapabilities implements ICapability, IInitialStateExcludedCapability {
-
 	protected DirectEditingService $directEditingService;
 	protected IURLGenerator $urlGenerator;
 
@@ -43,7 +43,8 @@ public function getCapabilities() {
 			'files' => [
 				'directEditing' => [
 					'url' => $this->urlGenerator->linkToOCSRouteAbsolute('files.DirectEditing.info'),
-					'etag' => $this->directEditingService->getDirectEditingETag()
+					'etag' => $this->directEditingService->getDirectEditingETag(),
+					'supportsFileId' => true,
 				]
 			],
 		];

diff --git a/lib/private/DirectEditing/Manager.php b/lib/private/DirectEditing/Manager.php
@@ -26,10 +26,11 @@
 namespace OC\DirectEditing;
 
 use Doctrine\DBAL\FetchMode;
-use OC\Files\Node\Folder;
+use \OCP\Files\Folder;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\Constants;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\DirectEditing\ACreateFromTemplate;
 use OCP\DirectEditing\IEditor;
@@ -152,9 +153,25 @@ public function create(string $path, string $editorId, string $creatorId, $templ
 		throw new \RuntimeException('No creator found');
 	}
 
-	public function open(string $filePath, string $editorId = null): string {
-		/** @var File $file */
-		$file = $this->rootFolder->getUserFolder($this->userId)->get($filePath);
+	public function open(string $filePath, string $editorId = null, ?int $fileId = null): string {
+		$userFolder = $this->rootFolder->getUserFolder($this->userId);
+		$file = $userFolder->get($filePath);
+		if ($fileId !== null && $file instanceof Folder) {
+			$files = $file->getById($fileId);
+
+			// Workaround to always open files with edit permissions if multiple occurences of
+			// the same file id are in the user home, ideally we should also track the path of the file when opening
+			usort($files, function (Node $a, Node $b) {
+				return ($b->getPermissions() & Constants::PERMISSION_UPDATE) <=> ($a->getPermissions() & Constants::PERMISSION_UPDATE);
+			});
+			$file = array_shift($files);
+		}
+
+		if (!$file instanceof File) {
+			throw new NotFoundException();
+		}
+
+		$filePath = $userFolder->getRelativePath($file->getPath());
 
 		if ($editorId === null) {
 			$editorId = $this->findEditorForFile($file);

diff --git a/tests/lib/DirectEditing/ManagerTest.php b/tests/lib/DirectEditing/ManagerTest.php
@@ -211,6 +211,86 @@ public function testCreateTokenAccess() {
 		$this->assertInstanceOf(NotFoundResponse::class, $secondResult);
 	}
 
+	public function testOpenByPath() {
+		$expectedToken = 'TOKEN' . time();
+		$file = $this->createMock(File::class);
+		$file->expects($this->any())
+			->method('getId')
+			->willReturn(123);
+		$file->expects($this->any())
+			->method('getPath')
+			->willReturn('/admin/files/File.txt');
+		$this->random->expects($this->once())
+			->method('generate')
+			->willReturn($expectedToken);
+		$folder = $this->createMock(Folder::class);
+		$this->userFolder
+			->method('nodeExists')
+			->withConsecutive(['/File.txt'], ['/'])
+			->willReturnOnConsecutiveCalls(false, true);
+		$this->userFolder
+			->method('get')
+			->with('/File.txt')
+			->willReturn($file);
+		$this->userFolder
+			->method('getRelativePath')
+			->willReturn('/File.txt');
+		$this->manager->open('/File.txt', 'testeditor');
+		$firstResult = $this->manager->edit($expectedToken);
+		$secondResult = $this->manager->edit($expectedToken);
+		$this->assertInstanceOf(DataResponse::class, $firstResult);
+		$this->assertInstanceOf(NotFoundResponse::class, $secondResult);
+	}
+
+	public function testOpenById() {
+		$expectedToken = 'TOKEN' . time();
+		$fileRead = $this->createMock(File::class);
+		$fileRead->method('getPermissions')
+			->willReturn(1);
+		$fileRead->expects($this->any())
+			->method('getId')
+			->willReturn(123);
+		$fileRead->expects($this->any())
+			->method('getPath')
+			->willReturn('/admin/files/shared_file.txt');
+		$file = $this->createMock(File::class);
+		$file->method('getPermissions')
+			->willReturn(1);
+		$file->expects($this->any())
+			->method('getId')
+			->willReturn(123);
+		$file->expects($this->any())
+			->method('getPath')
+			->willReturn('/admin/files/File.txt');
+		$this->random->expects($this->once())
+			->method('generate')
+			->willReturn($expectedToken);
+		$folder = $this->createMock(Folder::class);
+		$folder->expects($this->any())
+			->method('getById')
+			->willReturn([
+				$fileRead,
+				$file
+			]);
+		$this->userFolder
+			->method('nodeExists')
+			->withConsecutive(['/File.txt'], ['/'])
+			->willReturnOnConsecutiveCalls(false, true);
+		$this->userFolder
+			->method('get')
+			->with('/')
+			->willReturn($folder);
+		$this->userFolder
+			->method('getRelativePath')
+			->willReturn('/File.txt');
+
+		$this->manager->open('/', 'testeditor', 123);
+		$firstResult = $this->manager->edit($expectedToken);
+		$secondResult = $this->manager->edit($expectedToken);
+		$this->assertInstanceOf(DataResponse::class, $firstResult);
+		$this->assertInstanceOf(NotFoundResponse::class, $secondResult);
+	}
+
 	public function testCreateFileAlreadyExists() {
 		$this->expectException(\RuntimeException::class);
 		$this->userFolder