From 7f0ad8e2d25836d2c937017cb3918f45f62144bd Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Sun, 28 May 2023 03:53:29 +0200 Subject: [PATCH] fix(web+mobile): consistent filename handling (#2534) --- .../backup/services/backup.service.dart | 7 +-- .../asset-viewer/asset-viewer.svelte | 20 +------ .../asset-viewer/detail-panel.svelte | 3 +- web/src/lib/utils/asset-utils.spec.ts | 60 +++++++++++++++++++ web/src/lib/utils/asset-utils.ts | 13 +++- 5 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 web/src/lib/utils/asset-utils.spec.ts diff --git a/mobile/lib/modules/backup/services/backup.service.dart b/mobile/lib/modules/backup/services/backup.service.dart index 13553e718c..8984a18282 100644 --- a/mobile/lib/modules/backup/services/backup.service.dart +++ b/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"], diff --git a/web/src/lib/components/asset-viewer/asset-viewer.svelte b/web/src/lib/components/asset-viewer/asset-viewer.svelte index 8b91b7429a..4c7d6c18ed 100644 --- a/web/src/lib/components/asset-viewer/asset-viewer.svelte +++ b/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]) { diff --git a/web/src/lib/components/asset-viewer/detail-panel.svelte b/web/src/lib/components/asset-viewer/detail-panel.svelte index c776bae81b..6656fcf6b5 100644 --- a/web/src/lib/components/asset-viewer/detail-panel.svelte +++ b/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 @@

- {`${asset.originalFileName}.${asset.originalPath.split('.')[1]}` || ''} + {getAssetFilename(asset)}

{#if asset.exifInfo.exifImageHeight && asset.exifInfo.exifImageWidth} diff --git a/web/src/lib/utils/asset-utils.spec.ts b/web/src/lib/utils/asset-utils.spec.ts new file mode 100644 index 0000000000..e11f67f051 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/web/src/lib/utils/asset-utils.ts b/web/src/lib/utils/asset-utils.ts index f10521d2e7..f4420248df 100644 --- a/web/src/lib/utils/asset-utils.ts +++ b/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}`; } /**