Browse Source

fix(web+mobile): consistent filename handling (#2534)

Michel Heusschen 2 years ago
parent
commit
7f0ad8e2d2

+ 2 - 5
mobile/lib/modules/backup/services/backup.service.dart

@@ -219,8 +219,6 @@ class BackupService {
 
         if (file != null) {
           String originalFileName = await entity.titleAsync;
-          String fileNameWithoutPath =
-              originalFileName.toString().split(".")[0];
           var fileExtension = p.extension(file.path);
           var mimeType = FileHelper.getMimeType(file.path);
           var fileStream = file.openRead();
@@ -228,7 +226,7 @@ class BackupService {
             "assetData",
             fileStream,
             file.lengthSync(),
-            filename: fileNameWithoutPath,
+            filename: originalFileName,
             contentType: MediaType(
               mimeType["type"],
               mimeType["subType"],
@@ -334,14 +332,13 @@ class BackupService {
       var motionFile = File(validPath);
       var fileStream = motionFile.openRead();
       String originalFileName = await entity.titleAsync;
-      String fileNameWithoutPath = originalFileName.toString().split(".")[0];
       var mimeType = FileHelper.getMimeType(validPath);
 
       return http.MultipartFile(
         "livePhotoData",
         fileStream,
         motionFile.lengthSync(),
-        filename: fileNameWithoutPath,
+        filename: originalFileName,
         contentType: MediaType(
           mimeType["type"],
           mimeType["subType"],

+ 3 - 17
web/src/lib/components/asset-viewer/asset-viewer.svelte

@@ -24,7 +24,7 @@
 	import VideoViewer from './video-viewer.svelte';
 
 	import { assetStore } from '$lib/stores/assets.store';
-	import { addAssetsToAlbum } from '$lib/utils/asset-utils';
+	import { addAssetsToAlbum, getFilenameExtension } from '$lib/utils/asset-utils';
 	import { browser } from '$app/environment';
 
 	export let asset: AssetResponseDto;
@@ -125,24 +125,10 @@
 		downloadFile(asset.id, false, publicSharedKey);
 	};
 
-	/**
-	 * Get the filename of the asset based on the user defined template
-	 */
-	const getTemplateFilename = () => {
-		const filenameWithExtension = asset.originalPath.split('/').pop() as string;
-		const filenameWithoutExtension = filenameWithExtension.split('.')[0];
-		return {
-			filenameWithExtension,
-			filenameWithoutExtension
-		};
-	};
-
 	const downloadFile = async (assetId: string, isLivePhoto: boolean, key: string) => {
 		try {
-			const { filenameWithoutExtension } = getTemplateFilename();
-
-			const imageExtension = isLivePhoto ? 'mov' : asset.originalPath.split('.')[1];
-			const imageFileName = filenameWithoutExtension + '.' + imageExtension;
+			const imageExtension = isLivePhoto ? 'mov' : getFilenameExtension(asset.originalPath);
+			const imageFileName = asset.originalFileName + '.' + imageExtension;
 
 			// If assets is already download -> return;
 			if ($downloadAssets[imageFileName]) {

+ 2 - 1
web/src/lib/components/asset-viewer/detail-panel.svelte

@@ -12,6 +12,7 @@
 	import { AssetResponseDto, AlbumResponseDto, api, ThumbnailFormat } from '@api';
 	import { asByteUnitString } from '../../utils/byte-units';
 	import ImageThumbnail from '../assets/thumbnail/image-thumbnail.svelte';
+	import { getAssetFilename } from '$lib/utils/asset-utils';
 
 	export let asset: AssetResponseDto;
 	export let albums: AlbumResponseDto[] = [];
@@ -176,7 +177,7 @@
 
 				<div>
 					<p class="break-all">
-						{`${asset.originalFileName}.${asset.originalPath.split('.')[1]}` || ''}
+						{getAssetFilename(asset)}
 					</p>
 					<div class="flex text-sm gap-2">
 						{#if asset.exifInfo.exifImageHeight && asset.exifInfo.exifImageWidth}

+ 60 - 0
web/src/lib/utils/asset-utils.spec.ts

@@ -0,0 +1,60 @@
+import type { AssetResponseDto } from '@api';
+import { describe, expect, it } from '@jest/globals';
+import { getAssetFilename, getFilenameExtension } from './asset-utils';
+
+describe('get file extension from filename', () => {
+	it('returns the extension without including the dot', () => {
+		expect(getFilenameExtension('filename.txt')).toEqual('txt');
+	});
+
+	it('takes the last file extension and ignores the rest', () => {
+		expect(getFilenameExtension('filename.txt.pdf')).toEqual('pdf');
+		expect(getFilenameExtension('filename.txt.pdf.jpg')).toEqual('jpg');
+	});
+
+	it('returns an empty string when no file extension is found', () => {
+		expect(getFilenameExtension('filename')).toEqual('');
+		expect(getFilenameExtension('filename.')).toEqual('');
+		expect(getFilenameExtension('filename..')).toEqual('');
+		expect(getFilenameExtension('.filename')).toEqual('');
+	});
+
+	it('returns the extension from a filepath', () => {
+		expect(getFilenameExtension('/folder/file.txt')).toEqual('txt');
+		expect(getFilenameExtension('./folder/file.txt')).toEqual('txt');
+		expect(getFilenameExtension('~/folder/file.txt')).toEqual('txt');
+		expect(getFilenameExtension('./folder/.file.txt')).toEqual('txt');
+		expect(getFilenameExtension('/folder.with.dots/file.txt')).toEqual('txt');
+	});
+});
+
+describe('get asset filename', () => {
+	it('returns the filename including file extension', () => {
+		[
+			{
+				asset: {
+					originalFileName: 'filename',
+					originalPath: 'upload/library/test/2016/2016-08-30/filename.jpg'
+				},
+				result: 'filename.jpg'
+			},
+			{
+				asset: {
+					originalFileName: 'new-filename',
+					originalPath:
+						'upload/library/89d14e47-a40d-4cae-a347-a914cdef1f22/2016/2016-08-30/filename.jpg'
+				},
+				result: 'new-filename.jpg'
+			},
+			{
+				asset: {
+					originalFileName: 'new-filename.txt',
+					originalPath: 'upload/library/test/2016/2016-08-30/filename.txt.jpg'
+				},
+				result: 'new-filename.txt.jpg'
+			}
+		].forEach(({ asset, result }) => {
+			expect(getAssetFilename(asset as AssetResponseDto)).toEqual(result);
+		});
+	});
+});

+ 11 - 2
web/src/lib/utils/asset-utils.ts

@@ -108,8 +108,17 @@ export async function bulkDownload(
  * an empty string when not found.
  */
 export function getFilenameExtension(filename: string): string {
-	const lastIndex = filename.lastIndexOf('.');
-	return filename.slice(lastIndex + 1).toLowerCase();
+	const lastIndex = Math.max(0, filename.lastIndexOf('.'));
+	const startIndex = (lastIndex || Infinity) + 1;
+	return filename.slice(startIndex).toLowerCase();
+}
+
+/**
+ * Returns the filename of an asset including file extension
+ */
+export function getAssetFilename(asset: AssetResponseDto): string {
+	const fileExtension = getFilenameExtension(asset.originalPath);
+	return `${asset.originalFileName}.${fileExtension}`;
 }
 
 /**