Skip to content

Commit cc122e3

Browse files
mejo-mejo-
authored
Merge pull request #8507 from nextcloud/jtr/fix-workspace-getFile
fix(workspace): reuse WorkspaceService file lookup in direct()
2 parents a842069 + e11c84e commit cc122e3

3 files changed

Lines changed: 146 additions & 49 deletions

File tree

‎lib/Controller/WorkspaceController.php‎

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,12 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-License-Identifier: AGPL-3.0-or-later
68
*/
79

8-
declare(strict_types=1);
9-
/**
10-
* @copyright Copyright (c) 2019 Julius Härtl <jus@bitgrid.net>
11-
*
12-
* @author Julius Härtl <jus@bitgrid.net>
13-
*
14-
* @license GNU AGPL version 3 or any later version
15-
*
16-
* This program is free software: you can redistribute it and/or modify
17-
* it under the terms of the GNU Affero General Public License as
18-
* published by the Free Software Foundation, either version 3 of the
19-
* License, or (at your option) any later version.
20-
*
21-
* This program is distributed in the hope that it will be useful,
22-
* but WITHOUT ANY WARRANTY; without even the implied warranty of
23-
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
24-
* GNU Affero General Public License for more details.
25-
*
26-
* You should have received a copy of the GNU Affero General Public License
27-
* along with this program. If not, see <http://www.gnu.org/licenses/>.
28-
*
29-
*/
30-
3110
namespace OCA\Text\Controller;
3211

3312
use Exception;
@@ -43,7 +22,6 @@
4322
use OCP\DirectEditing\IManager as IDirectEditingManager;
4423
use OCP\DirectEditing\RegisterDirectEditorEvent;
4524
use OCP\EventDispatcher\IEventDispatcher;
46-
use OCP\Files\File;
4725
use OCP\Files\Folder;
4826
use OCP\Files\IRootFolder;
4927
use OCP\Files\NotFoundException;
@@ -73,12 +51,16 @@ public function __construct(
7351
}
7452

7553
/**
76-
* Checks for available files in the current folder and returns required details to present
77-
* the rich workspace
54+
* Checks for available files in the current folder and returns required
55+
* details to present the rich workspace.
56+
*
57+
* Returns 200 with file metadata and folder permissions if a README is found,
58+
* or 404 with folder permissions if not (so the client can still offer to create one).
59+
*
60+
* @param string $path Path relative to the user's root folder
7861
*/
7962
#[NoAdminRequired]
8063
public function folder(string $path = '/'): DataResponse {
81-
/** */
8264
try {
8365
/** @psalm-suppress PossiblyNullArgument */
8466
$userFolder = $this->rootFolder->getUserFolder($this->userId);
@@ -117,9 +99,11 @@ public function folder(string $path = '/'): DataResponse {
11799
}
118100

119101
/**
120-
* Checks for available files in the current folder and returns required details to present
121-
* the rich workspace
122-
* @api
102+
* Checks for available files in a publicly shared folder and returns required
103+
* details to present the rich workspace.
104+
*
105+
* @param string $shareToken Public share token
106+
* @param string $path Path relative to the share root
123107
*/
124108
#[NoAdminRequired]
125109
#[PublicPage]
@@ -166,6 +150,14 @@ public function publicFolder(string $shareToken, string $path = '/'): DataRespon
166150
return new DataResponse(['message' => 'No workspace file found'], Http::STATUS_NOT_FOUND);
167151
}
168152

153+
/**
154+
* Returns a direct editing URL for the workspace README in the given folder.
155+
*
156+
* If a README file already exists, opens it for editing. If not, creates a new
157+
* file using the first entry of getSupportedFilenames() as the default name.
158+
*
159+
* @param string $path Path to the folder, relative to the user's root folder
160+
*/
169161
#[NoAdminRequired]
170162
public function direct(string $path): DataResponse {
171163
$this->eventDispatcher->dispatchTyped(new RegisterDirectEditorEvent($this->directEditingManager));
@@ -174,9 +166,13 @@ public function direct(string $path): DataResponse {
174166
/** @psalm-suppress PossiblyNullArgument */
175167
$folder = $this->rootFolder->getUserFolder($this->userId)->get($path);
176168
if ($folder instanceof Folder) {
177-
$file = $this->getFile($folder);
169+
$file = $this->workspaceService->getFile($folder);
178170
if ($file === null) {
179-
$token = $this->directEditingManager->create($path . '/' . $this->workspaceService->getSupportedFilenames()[0], Application::APP_NAME, TextDocumentCreator::CREATOR_ID);
171+
$token = $this->directEditingManager->create(
172+
$path . '/' . $this->workspaceService->getSupportedFilenames()[0],
173+
Application::APP_NAME,
174+
TextDocumentCreator::CREATOR_ID
175+
);
180176
} else {
181177
$token = $this->directEditingManager->open($path . '/' . $file->getName(), Application::APP_NAME);
182178
}
@@ -191,18 +187,4 @@ public function direct(string $path): DataResponse {
191187

192188
return new DataResponse(['message' => 'No workspace file found'], Http::STATUS_NOT_FOUND);
193189
}
194-
195-
private function getFile(Folder $folder): ?File {
196-
$file = null;
197-
foreach ($this->workspaceService->getSupportedFilenames() as $filename) {
198-
try {
199-
$node = $folder->get($filename);
200-
if ($node instanceof File) {
201-
$file = $node;
202-
}
203-
} catch (NotFoundException) {
204-
}
205-
}
206-
return $file;
207-
}
208190
}

‎lib/Service/WorkspaceService.php‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php
22

3+
declare(strict_types=1);
34

45
/**
56
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
@@ -8,6 +9,7 @@
89

910
namespace OCA\Text\Service;
1011

12+
use OCP\Files\Cache\ICacheEntry;
1113
use OCP\Files\File;
1214
use OCP\Files\Folder;
1315
use OCP\Files\NotFoundException;
@@ -28,16 +30,23 @@ public function __construct(IL10N $l10n) {
2830
}
2931

3032
public function getFile(Folder $folder): ?File {
33+
try {
34+
$cache = $folder->getStorage()->getCache();
35+
$internalPath = $folder->getInternalPath();
36+
} catch (StorageInvalidException) {
37+
return null;
38+
}
39+
3140
foreach ($this->getSupportedFilenames() as $filename) {
3241
try {
33-
$exists = $folder->getStorage()->getCache()->get($folder->getInternalPath() . '/' . $filename);
34-
if ($exists) {
42+
$cacheEntry = $cache->get($internalPath . '/' . $filename);
43+
if ($cacheEntry !== false && $cacheEntry->getMimeType() !== ICacheEntry::DIRECTORY_MIMETYPE) {
3544
$file = $folder->get($filename);
3645
if ($file instanceof File) {
3746
return $file;
3847
}
3948
}
40-
} catch (NotFoundException|StorageInvalidException) {
49+
} catch (NotFoundException) {
4150
continue;
4251
}
4352
}

‎tests/unit/Service/WorkspaceServiceTest.php‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Text\Tests\Service;
11+
12+
use OCA\Text\Service\WorkspaceService;
13+
use OCP\Files\Cache\ICache;
14+
use OCP\Files\Cache\ICacheEntry;
15+
use OCP\Files\File;
16+
use OCP\Files\Folder;
17+
use OCP\Files\Storage\IStorage;
18+
use OCP\Files\StorageInvalidException;
19+
use OCP\IL10N;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
use Test\TestCase;
22+
23+
class WorkspaceServiceTest extends TestCase {
24+
private IL10N&MockObject $l10n;
25+
private WorkspaceService $workspaceService;
26+
27+
protected function setUp(): void {
28+
parent::setUp();
29+
30+
$this->l10n = $this->createMock(IL10N::class);
31+
$this->l10n->method('t')->with('Readme')->willReturn('Readme');
32+
33+
$this->workspaceService = new WorkspaceService($this->l10n);
34+
}
35+
36+
public function testGetFileReturnsFirstMatchingFileInPriorityOrder(): void {
37+
$folder = $this->createMock(Folder::class);
38+
$storage = $this->createMock(IStorage::class);
39+
$cache = $this->createMock(ICache::class);
40+
$readmeFile = $this->createMock(File::class);
41+
42+
$folder->method('getStorage')->willReturn($storage);
43+
$storage->method('getCache')->willReturn($cache);
44+
$folder->method('getInternalPath')->willReturn('docs');
45+
46+
$readmeEntry = $this->createMock(ICacheEntry::class);
47+
$readmeEntry->method('getMimeType')->willReturn('text/markdown');
48+
49+
$uppercaseEntry = $this->createMock(ICacheEntry::class);
50+
$uppercaseEntry->method('getMimeType')->willReturn('text/markdown');
51+
52+
$cache->method('get')->willReturnMap([
53+
['docs/Readme.md', $readmeEntry],
54+
['docs/README.md', $uppercaseEntry],
55+
['docs/readme.md', false],
56+
]);
57+
58+
$folder->expects($this->once())
59+
->method('get')
60+
->with('Readme.md')
61+
->willReturn($readmeFile);
62+
63+
$result = $this->workspaceService->getFile($folder);
64+
65+
$this->assertSame($readmeFile, $result);
66+
}
67+
68+
public function testGetFileSkipsDirectoryCacheEntries(): void {
69+
$folder = $this->createMock(Folder::class);
70+
$storage = $this->createMock(IStorage::class);
71+
$cache = $this->createMock(ICache::class);
72+
73+
$folder->method('getStorage')->willReturn($storage);
74+
$storage->method('getCache')->willReturn($cache);
75+
$folder->method('getInternalPath')->willReturn('docs');
76+
77+
$directoryEntry = $this->createMock(ICacheEntry::class);
78+
$directoryEntry->method('getMimeType')->willReturn(ICacheEntry::DIRECTORY_MIMETYPE);
79+
80+
$cache->method('get')->willReturnMap([
81+
['docs/Readme.md', $directoryEntry],
82+
['docs/README.md', false],
83+
['docs/readme.md', false],
84+
]);
85+
86+
$folder->expects($this->never())->method('get');
87+
88+
$result = $this->workspaceService->getFile($folder);
89+
90+
$this->assertNull($result);
91+
}
92+
93+
public function testGetFileReturnsNullWhenStorageIsInvalid(): void {
94+
$folder = $this->createMock(Folder::class);
95+
96+
$folder->method('getStorage')
97+
->willThrowException(new StorageInvalidException());
98+
99+
$folder->expects($this->never())->method('getInternalPath');
100+
$folder->expects($this->never())->method('get');
101+
102+
$result = $this->workspaceService->getFile($folder);
103+
104+
$this->assertNull($result);
105+
}
106+
}

0 commit comments

Comments
 (0)