Skip to content

Commit 5cc60c7

Browse files
Merge pull request #51747 from nextcloud/backport/51744/stable30
[stable30] fix(files_versions): Rely on server mime fallback icons
2 parents 37d13a0 + f10b4f5 commit 5cc60c7

File tree

8 files changed

+63
-19
lines changed

8 files changed

+63
-19
lines changed

apps/files_versions/lib/Controller/PreviewController.php

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
1313
use OCP\AppFramework\Http\DataResponse;
1414
use OCP\AppFramework\Http\FileDisplayResponse;
15+
use OCP\AppFramework\Http\RedirectResponse;
1516
use OCP\Files\IRootFolder;
1617
use OCP\Files\NotFoundException;
1718
use OCP\IPreview;
1819
use OCP\IRequest;
1920
use OCP\IUserSession;
21+
use OCP\Preview\IMimeIconProvider;
2022

2123
class PreviewController extends Controller {
2224

@@ -38,7 +40,8 @@ public function __construct(
3840
IRootFolder $rootFolder,
3941
IUserSession $userSession,
4042
IVersionManager $versionManager,
41-
IPreview $previewManager
43+
IPreview $previewManager,
44+
private IMimeIconProvider $mimeIconProvider,
4245
) {
4346
parent::__construct($appName, $request);
4447

@@ -55,9 +58,11 @@ public function __construct(
5558
* @param int $x Width of the preview
5659
* @param int $y Height of the preview
5760
* @param string $version Version of the file to get the preview for
58-
* @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, array<empty>, array{}>
61+
* @param bool $mimeFallback Whether to fallback to the mime icon if no preview is available
62+
* @return FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, array<empty>, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
5963
*
6064
* 200: Preview returned
65+
* 303: Redirect to the mime icon url if mimeFallback is true
6166
* 400: Getting preview is not possible
6267
* 404: Preview not found
6368
*/
@@ -67,20 +72,32 @@ public function getPreview(
6772
string $file = '',
6873
int $x = 44,
6974
int $y = 44,
70-
string $version = ''
75+
string $version = '',
76+
bool $mimeFallback = false,
7177
) {
7278
if ($file === '' || $version === '' || $x === 0 || $y === 0) {
7379
return new DataResponse([], Http::STATUS_BAD_REQUEST);
7480
}
7581

82+
$versionFile = null;
7683
try {
7784
$user = $this->userSession->getUser();
7885
$userFolder = $this->rootFolder->getUserFolder($user->getUID());
7986
$file = $userFolder->get($file);
8087
$versionFile = $this->versionManager->getVersionFile($user, $file, $version);
8188
$preview = $this->previewManager->getPreview($versionFile, $x, $y, true, IPreview::MODE_FILL, $versionFile->getMimetype());
82-
return new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
89+
$response = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
90+
$response->cacheFor(3600 * 24, false, true);
91+
return $response;
8392
} catch (NotFoundException $e) {
93+
// If we have no preview enabled, we can redirect to the mime icon if any
94+
if ($mimeFallback && $versionFile !== null) {
95+
$url = $this->mimeIconProvider->getMimeIconUrl($versionFile->getMimeType());
96+
if ($url !== null) {
97+
return new RedirectResponse($url);
98+
}
99+
}
100+
84101
return new DataResponse([], Http::STATUS_NOT_FOUND);
85102
} catch (\InvalidArgumentException $e) {
86103
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
@@ -10,7 +10,7 @@
1010
<!-- Icon -->
1111
<template #icon>
1212
<div v-if="!(loadPreview || previewLoaded)" class="version__image" />
13-
<img v-else-if="(isCurrent || version.hasPreview) && !previewErrored"
13+
<img v-else-if="version.previewUrl && !previewErrored"
1414
:src="version.previewUrl"
1515
alt=""
1616
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
@@ -274,13 +274,12 @@ export default {
274274
return
275275
}
276276
277-
// Versions previews are too small for our use case, so we override hasPreview and previewUrl
277+
// Versions previews are too small for our use case, so we override previewUrl
278278
// which makes the viewer render the original file.
279279
// We also point to the original filename if the version is the current one.
280280
const versions = this.versions.map(version => ({
281281
...version,
282282
filename: version.mtime === this.fileInfo.mtime ? path.join('files', getCurrentUser()?.uid ?? '', this.fileInfo.path, this.fileInfo.name) : version.filename,
283-
hasPreview: false,
284283
previewUrl: undefined,
285284
}))
286285
@@ -291,7 +290,7 @@ export default {
291290
},
292291
293292
compareVersion({ version }) {
294-
const versions = this.versions.map(version => ({ ...version, hasPreview: false, previewUrl: undefined }))
293+
const versions = this.versions.map(version => ({ ...version, previewUrl: undefined }))
295294
296295
OCA.Viewer.compare(this.viewerFileInfo, versions.find(v => v.source === version.source))
297296
},

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() {
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() {

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.

0 commit comments

Comments
 (0)