Skip to content

Commit 196b2f1

Browse files
skjnldsvskjnldsv
authored
Merge pull request #51744 from nextcloud/fix/files_versions/previews
fix(files_versions): Rely on server mime fallback icons
2 parents f7b96ba + 0f47e8c commit 196b2f1

File tree

9 files changed

+84
-17
lines changed

9 files changed

+84
-17
lines changed

‎apps/files_versions/lib/Controller/PreviewController.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313
use OCP\AppFramework\Http\Attribute\OpenAPI;
1414
use OCP\AppFramework\Http\DataResponse;
1515
use OCP\AppFramework\Http\FileDisplayResponse;
16+
use OCP\AppFramework\Http\RedirectResponse;
1617
use OCP\Files\IRootFolder;
1718
use OCP\Files\NotFoundException;
1819
use OCP\IPreview;
1920
use OCP\IRequest;
2021
use OCP\IUserSession;
22+
use OCP\Preview\IMimeIconProvider;
2123

2224
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
2325
class PreviewController extends Controller {
@@ -29,6 +31,7 @@ public function __construct(
2931
private IUserSession $userSession,
3032
private IVersionManager $versionManager,
3133
private IPreview $previewManager,
34+
private IMimeIconProvider $mimeIconProvider,
3235
) {
3336
parent::__construct($appName, $request);
3437
}
@@ -40,9 +43,11 @@ public function __construct(
4043
* @param int $x Width of the preview
4144
* @param int $y Height of the preview
4245
* @param string $version Version of the file to get the preview for
43-
* @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, list<empty>, array{}>
46+
* @param bool $mimeFallback Whether to fallback to the mime icon if no preview is available
47+
* @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, list<empty>, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
4448
*
4549
* 200: Preview returned
50+
* 303: Redirect to the mime icon url if mimeFallback is true
4651
* 400: Getting preview is not possible
4752
* 404: Preview not found
4853
*/
@@ -53,19 +58,31 @@ public function getPreview(
5358
int $x = 44,
5459
int $y = 44,
5560
string $version = '',
61+
bool $mimeFallback = false,
5662
) {
5763
if ($file === '' || $version === '' || $x === 0 || $y === 0) {
5864
return new DataResponse([], Http::STATUS_BAD_REQUEST);
5965
}
6066

67+
$versionFile = null;
6168
try {
6269
$user = $this->userSession->getUser();
6370
$userFolder = $this->rootFolder->getUserFolder($user->getUID());
6471
$file = $userFolder->get($file);
6572
$versionFile = $this->versionManager->getVersionFile($user, $file, $version);
6673
$preview = $this->previewManager->getPreview($versionFile, $x, $y, true, IPreview::MODE_FILL, $versionFile->getMimetype());
67-
return new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
74+
$response = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
75+
$response->cacheFor(3600 * 24, false, true);
76+
return $response;
6877
} catch (NotFoundException $e) {
78+
// If we have no preview enabled, we can redirect to the mime icon if any
79+
if ($mimeFallback && $versionFile !== null) {
80+
$url = $this->mimeIconProvider->getMimeIconUrl($versionFile->getMimeType());
81+
if ($url !== null) {
82+
return new RedirectResponse($url);
83+
}
84+
}
85+
6986
return new DataResponse([], Http::STATUS_NOT_FOUND);
7087
} catch (\InvalidArgumentException $e) {
7188
return new DataResponse([], Http::STATUS_BAD_REQUEST);

‎apps/files_versions/openapi.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,19 @@
103103
"type": "string",
104104
"default": ""
105105
}
106+
},
107+
{
108+
"name": "mimeFallback",
109+
"in": "query",
110+
"description": "Whether to fallback to the mime icon if no preview is available",
111+
"schema": {
112+
"type": "integer",
113+
"default": 0,
114+
"enum": [
115+
0,
116+
1
117+
]
118+
}
106119
}
107120
],
108121
"responses": {
@@ -132,6 +145,16 @@
132145
"schema": {}
133146
}
134147
}
148+
},
149+
"303": {
150+
"description": "Redirect to the mime icon url if mimeFallback is true",
151+
"headers": {
152+
"Location": {
153+
"schema": {
154+
"type": "string"
155+
}
156+
}
157+
}
135158
}
136159
}
137160
}

‎apps/files_versions/src/components/Version.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<!-- Icon -->
1212
<template #icon>
1313
<div v-if="!(loadPreview || previewLoaded)" class="version__image" />
14-
<img v-else-if="(isCurrent || version.hasPreview) && !previewErrored"
14+
<img v-else-if="version.previewUrl && !previewErrored"
1515
:src="version.previewUrl"
1616
alt=""
1717
decoding="async"

‎apps/files_versions/src/utils/versions.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export interface Version {
2828
type: string, // 'file'
2929
mtime: number, // Version creation date as a timestamp
3030
permissions: string, // Only readable: 'R'
31-
hasPreview: boolean, // Whether the version has a preview
3231
previewUrl: string, // Preview URL of the version
3332
url: string, // Download URL of the version
3433
source: string, // The WebDAV endpoint of the ressource
@@ -78,12 +77,12 @@ function formatVersion(version: any, fileInfo: any): Version {
7877
let previewUrl = ''
7978

8079
if (mtime === fileInfo.mtime) { // Version is the current one
81-
previewUrl = generateUrl('/core/preview?fileId={fileId}&c={fileEtag}&x=250&y=250&forceIcon=0&a=0', {
80+
previewUrl = generateUrl('/core/preview?fileId={fileId}&c={fileEtag}&x=250&y=250&forceIcon=0&a=0&forceIcon=1&mimeFallback=1', {
8281
fileId: fileInfo.id,
8382
fileEtag: fileInfo.etag,
8483
})
8584
} else {
86-
previewUrl = generateUrl('/apps/files_versions/preview?file={file}&version={fileVersion}', {
85+
previewUrl = generateUrl('/apps/files_versions/preview?file={file}&version={fileVersion}&mimeFallback=1', {
8786
file: joinPaths(fileInfo.path, fileInfo.name),
8887
fileVersion: version.basename,
8988
})
@@ -102,7 +101,6 @@ function formatVersion(version: any, fileInfo: any): Version {
102101
type: version.type,
103102
mtime,
104103
permissions: 'R',
105-
hasPreview: version.props['has-preview'] === 1,
106104
previewUrl,
107105
url: joinPaths('/remote.php/dav', version.filename),
108106
source: generateRemoteUrl('dav') + encodePath(version.filename),

‎apps/files_versions/src/views/VersionTab.vue‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,13 +277,12 @@ export default {
277277
return
278278
}
279279
280-
// Versions previews are too small for our use case, so we override hasPreview and previewUrl
280+
// Versions previews are too small for our use case, so we override previewUrl
281281
// which makes the viewer render the original file.
282282
// We also point to the original filename if the version is the current one.
283283
const versions = this.versions.map(version => ({
284284
...version,
285285
filename: version.mtime === this.fileInfo.mtime ? path.join('files', getCurrentUser()?.uid ?? '', this.fileInfo.path, this.fileInfo.name) : version.filename,
286-
hasPreview: false,
287286
previewUrl: undefined,
288287
}))
289288
@@ -294,7 +293,7 @@ export default {
294293
},
295294
296295
compareVersion({ version }) {
297-
const versions = this.versions.map(version => ({ ...version, hasPreview: false, previewUrl: undefined }))
296+
const versions = this.versions.map(version => ({ ...version, previewUrl: undefined }))
298297
299298
OCA.Viewer.compare(this.viewerFileInfo, versions.find(v => v.source === version.source))
300299
},

‎apps/files_versions/tests/Controller/PreviewControllerTest.php‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
44
* SPDX-License-Identifier: AGPL-3.0-or-later
55
*/
6+
67
namespace OCA\Files_Versions\Tests\Controller;
78

89
use OCA\Files_Versions\Controller\PreviewController;
910
use OCA\Files_Versions\Versions\IVersionManager;
1011
use OCP\AppFramework\Http;
1112
use OCP\AppFramework\Http\DataResponse;
12-
use OCP\AppFramework\Http\FileDisplayResponse;
1313
use OCP\Files\File;
1414
use OCP\Files\Folder;
1515
use OCP\Files\IMimeTypeDetector;
@@ -20,6 +20,8 @@
2020
use OCP\IRequest;
2121
use OCP\IUser;
2222
use OCP\IUserSession;
23+
use OCP\Preview\IMimeIconProvider;
24+
use PHPUnit\Framework\MockObject\MockObject;
2325
use Test\TestCase;
2426

2527
class PreviewControllerTest extends TestCase {
@@ -45,6 +47,8 @@ class PreviewControllerTest extends TestCase {
4547
/** @var IVersionManager|\PHPUnit\Framework\MockObject\MockObject */
4648
private $versionManager;
4749

50+
private IMimeIconProvider&MockObject $mimeIconProvider;
51+
4852
protected function setUp(): void {
4953
parent::setUp();
5054

@@ -60,14 +64,16 @@ protected function setUp(): void {
6064
->method('getUser')
6165
->willReturn($user);
6266
$this->versionManager = $this->createMock(IVersionManager::class);
67+
$this->mimeIconProvider = $this->createMock(IMimeIconProvider::class);
6368

6469
$this->controller = new PreviewController(
6570
'files_versions',
6671
$this->createMock(IRequest::class),
6772
$this->rootFolder,
6873
$this->userSession,
6974
$this->versionManager,
70-
$this->previewManager
75+
$this->previewManager,
76+
$this->mimeIconProvider,
7177
);
7278
}
7379

@@ -131,9 +137,10 @@ public function testValidPreview(): void {
131137
->willReturn('previewMime');
132138

133139
$res = $this->controller->getPreview('file', 10, 10, '42');
134-
$expected = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => 'previewMime']);
135140

136-
$this->assertEquals($expected, $res);
141+
$this->assertEquals('previewMime', $res->getHeaders()['Content-Type']);
142+
$this->assertEquals(Http::STATUS_OK, $res->getStatus());
143+
$this->assertEquals($preview, $this->invokePrivate($res, 'file'));
137144
}
138145

139146
public function testVersionNotFound(): void {

‎dist/files_versions-files_versions.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/files_versions-files_versions.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎openapi.json‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22048,6 +22048,19 @@
2204822048
"type": "string",
2204922049
"default": ""
2205022050
}
22051+
},
22052+
{
22053+
"name": "mimeFallback",
22054+
"in": "query",
22055+
"description": "Whether to fallback to the mime icon if no preview is available",
22056+
"schema": {
22057+
"type": "integer",
22058+
"default": 0,
22059+
"enum": [
22060+
0,
22061+
1
22062+
]
22063+
}
2205122064
}
2205222065
],
2205322066
"responses": {
@@ -22077,6 +22090,16 @@
2207722090
"schema": []
2207822091
}
2207922092
}
22093+
},
22094+
"303": {
22095+
"description": "Redirect to the mime icon url if mimeFallback is true",
22096+
"headers": {
22097+
"Location": {
22098+
"schema": {
22099+
"type": "string"
22100+
}
22101+
}
22102+
}
2208022103
}
2208122104
}
2208222105
}

0 commit comments

Comments
 (0)