From 457da17c5d02f8f8d2cd4c6c50ace402ffc4873b Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Wed, 22 Jul 2026 14:59:54 +0200 Subject: [PATCH] fix: always set extension from provided file (#29839) * fix: always set extension from provided file The livephoto case can also happen with other files; for example, we receive a DNG file as JPEG but then still call it .dng Fixes #29805, partially addresses #28602 * chore: test cases for file extension naming --- .../services/background_upload.service.dart | 11 ++- .../services/foreground_upload.service.dart | 15 ++-- .../background_upload.service_test.dart | 45 ++++++++++++ .../foreground_upload.service_test.dart | 72 +++++++++++++++++++ 4 files changed, 125 insertions(+), 18 deletions(-) diff --git a/mobile/lib/services/background_upload.service.dart b/mobile/lib/services/background_upload.service.dart index fa0a41f17c..ec731c6f68 100644 --- a/mobile/lib/services/background_upload.service.dart +++ b/mobile/lib/services/background_upload.service.dart @@ -290,13 +290,10 @@ class BackgroundUploadService { return null; } - String fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; - final hasExtension = p.extension(fileName).isNotEmpty; - if (!hasExtension) { - fileName = p.setExtension(fileName, p.extension(asset.name)); - } - - final originalFileName = entity.isLivePhoto ? p.setExtension(fileName, p.extension(file.path)) : fileName; + final fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; + // Some apps (e.g. DJI/Fusion) return names without an extension; fall back to the asset name for those. + final extension = p.extension(file.path).isNotEmpty ? p.extension(file.path) : p.extension(asset.name); + final originalFileName = p.setExtension(fileName, extension); String metadata = UploadTaskMetadata( localAssetId: asset.id, diff --git a/mobile/lib/services/foreground_upload.service.dart b/mobile/lib/services/foreground_upload.service.dart index 06915ad09f..cce1241473 100644 --- a/mobile/lib/services/foreground_upload.service.dart +++ b/mobile/lib/services/foreground_upload.service.dart @@ -309,17 +309,10 @@ class ForegroundUploadService { return; } - String fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; - - /// Handle special file name from DJI or Fusion app - /// If the file name has no extension, likely due to special renaming template by specific apps - /// we append the original extension from the asset name - final hasExtension = p.extension(fileName).isNotEmpty; - if (!hasExtension) { - fileName = p.setExtension(fileName, p.extension(asset.name)); - } - - final originalFileName = entity.isLivePhoto ? p.setExtension(fileName, p.extension(file.path)) : fileName; + final fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; + // Some apps (e.g. DJI/Fusion) return names without an extension; fall back to the asset name for those. + final extension = p.extension(file.path).isNotEmpty ? p.extension(file.path) : p.extension(asset.name); + final originalFileName = p.setExtension(fileName, extension); final deviceId = Store.get(StoreKey.deviceId); final fields = { diff --git a/mobile/test/services/background_upload.service_test.dart b/mobile/test/services/background_upload.service_test.dart index 527da1bf1b..65f17501a0 100644 --- a/mobile/test/services/background_upload.service_test.dart +++ b/mobile/test/services/background_upload.service_test.dart @@ -136,6 +136,51 @@ void main() { expect(task, isNotNull); expect(task!.fields.containsKey('visibility'), isFalse); }); + + test('corrects the extension when iOS returns a rendered file for a .dng asset', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final mockFile = File('/path/to/IMG_6499.jpg'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'IMG_6499.dng'); + + final task = await sut.getUploadTask(asset); + expect(task, isNotNull); + expect(task!.fields['filename'], equals('IMG_6499.jpg')); + }); + + test('keeps the .dng extension for a genuine RAW original', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final mockFile = File('/path/to/IMG_5210.dng'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'IMG_5210.dng'); + + final task = await sut.getUploadTask(asset); + expect(task, isNotNull); + expect(task!.fields['filename'], equals('IMG_5210.dng')); + }); + + test('borrows the extension from the asset name for an extensionless name (DJI/Fusion)', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final mockFile = File('/path/to/DJI_0001'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'DJI_0001'); + + final task = await sut.getUploadTask(asset); + expect(task, isNotNull); + expect(task!.fields['filename'], equals('DJI_0001.jpg')); + }); }); group('getLivePhotoUploadTask', () { diff --git a/mobile/test/services/foreground_upload.service_test.dart b/mobile/test/services/foreground_upload.service_test.dart index ed48270b87..46e9a82141 100644 --- a/mobile/test/services/foreground_upload.service_test.dart +++ b/mobile/test/services/foreground_upload.service_test.dart @@ -81,6 +81,24 @@ void main() { return captured; } + List captureOriginalFileNames() { + final captured = []; + when( + () => mockUploadRepository.uploadFile( + file: any(named: 'file'), + originalFileName: any(named: 'originalFileName'), + fields: any(named: 'fields'), + cancelToken: any(named: 'cancelToken'), + onProgress: any(named: 'onProgress'), + logContext: any(named: 'logContext'), + ), + ).thenAnswer((invocation) async { + captured.add(invocation.namedArguments[#originalFileName] as String); + return UploadResult.success(remoteAssetId: 'remote-${captured.length}'); + }); + return captured; + } + group('uploadSingleAsset', () { test('should upload the motion part hidden and keep the still image visible', () async { final asset = LocalAssetStub.image1; @@ -124,5 +142,59 @@ void main() { expect(captured, hasLength(1)); expect(captured[0].containsKey('visibility'), isFalse); }); + + test('corrects the extension when iOS returns a rendered file for a .dng asset', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final stillFile = File('/path/to/IMG_6499.jpg'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'IMG_6499.dng'); + + final names = captureOriginalFileNames(); + + await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks()); + + expect(names, equals(['IMG_6499.jpg'])); + }); + + test('keeps the .dng extension for a genuine RAW original', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final stillFile = File('/path/to/IMG_5210.dng'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'IMG_5210.dng'); + + final names = captureOriginalFileNames(); + + await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks()); + + expect(names, equals(['IMG_5210.dng'])); + }); + + test('borrows the extension from the asset name for an extensionless name (DJI/Fusion)', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final stillFile = File('/path/to/DJI_0001'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'DJI_0001'); + + final names = captureOriginalFileNames(); + + await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks()); + + expect(names, equals(['DJI_0001.jpg'])); + }); }); }