Compare commits

..

1 Commits

Author SHA1 Message Date
shenlong-tanwen
6733c14f76 fix(mobile): handle missing extension during uploads 2025-12-21 06:37:48 +05:30
4 changed files with 234 additions and 24 deletions

View File

@@ -1,5 +1,5 @@
[tools] [tools]
terragrunt = "0.96.0" terragrunt = "0.93.10"
opentofu = "1.10.7" opentofu = "1.10.7"
[tasks."tg:fmt"] [tasks."tg:fmt"]

View File

@@ -4,7 +4,7 @@ experimental_monorepo_root = true
node = "24.11.1" node = "24.11.1"
flutter = "3.35.7" flutter = "3.35.7"
pnpm = "10.24.0" pnpm = "10.24.0"
terragrunt = "0.96.0" terragrunt = "0.93.10"
opentofu = "1.10.7" opentofu = "1.10.7"
java = "25.0.1" java = "25.0.1"

View File

@@ -271,12 +271,12 @@ class UploadService {
return null; return null;
} }
final file = await _storageRepository.getFileForAsset(asset.id); final uploadFileResult = await prepareUploadFile(asset, isLivePhoto: entity.isLivePhoto);
if (file == null) { if (uploadFileResult == null) {
return null; return null;
} }
final originalFileName = entity.isLivePhoto ? p.setExtension(asset.name, p.extension(file.path)) : asset.name; final (:file, :originalFilename) = uploadFileResult;
String metadata = UploadTaskMetadata( String metadata = UploadTaskMetadata(
localAssetId: asset.id, localAssetId: asset.id,
@@ -290,7 +290,7 @@ class UploadService {
file, file,
createdAt: asset.createdAt, createdAt: asset.createdAt,
modifiedAt: asset.updatedAt, modifiedAt: asset.updatedAt,
originalFileName: originalFileName, originalFileName: originalFilename,
deviceAssetId: asset.id, deviceAssetId: asset.id,
metadata: metadata, metadata: metadata,
group: "group", group: "group",
@@ -308,8 +308,6 @@ class UploadService {
return null; return null;
} }
File? file;
/// iOS LivePhoto has two files: a photo and a video. /// iOS LivePhoto has two files: a photo and a video.
/// They are uploaded separately, with video file being upload first, then returned with the assetId /// They are uploaded separately, with video file being upload first, then returned with the assetId
/// The assetId is then used as a metadata for the photo file upload task. /// The assetId is then used as a metadata for the photo file upload task.
@@ -320,18 +318,12 @@ class UploadService {
/// The cancel operation will only cancel the video group (normal group), the photo group will not /// The cancel operation will only cancel the video group (normal group), the photo group will not
/// be touched, as the video file is already uploaded. /// be touched, as the video file is already uploaded.
if (entity.isLivePhoto) { final uploadFileResult = await prepareUploadFile(asset, isLivePhoto: entity.isLivePhoto);
file = await _storageRepository.getMotionFileForAsset(asset); if (uploadFileResult == null) {
} else {
file = await _storageRepository.getFileForAsset(asset.id);
}
if (file == null) {
return null; return null;
} }
final fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; final (:file, :originalFilename) = uploadFileResult;
final originalFileName = entity.isLivePhoto ? p.setExtension(fileName, p.extension(file.path)) : fileName;
String metadata = UploadTaskMetadata( String metadata = UploadTaskMetadata(
localAssetId: asset.id, localAssetId: asset.id,
@@ -345,7 +337,7 @@ class UploadService {
file, file,
createdAt: asset.createdAt, createdAt: asset.createdAt,
modifiedAt: asset.updatedAt, modifiedAt: asset.updatedAt,
originalFileName: originalFileName, originalFileName: originalFilename,
deviceAssetId: asset.id, deviceAssetId: asset.id,
metadata: metadata, metadata: metadata,
group: group, group: group,
@@ -362,21 +354,20 @@ class UploadService {
return null; return null;
} }
final file = await _storageRepository.getFileForAsset(asset.id); final result = await prepareUploadFile(asset);
if (file == null) { if (result == null) {
return null; return null;
} }
final fields = {'livePhotoVideoId': livePhotoVideoId}; final fields = {'livePhotoVideoId': livePhotoVideoId};
final requiresWiFi = _shouldRequireWiFi(asset); final requiresWiFi = _shouldRequireWiFi(asset);
final originalFileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name;
return buildUploadTask( return buildUploadTask(
file, result.file,
createdAt: asset.createdAt, createdAt: asset.createdAt,
modifiedAt: asset.updatedAt, modifiedAt: asset.updatedAt,
originalFileName: originalFileName, originalFileName: result.originalFilename,
deviceAssetId: asset.id, deviceAssetId: asset.id,
fields: fields, fields: fields,
group: kBackupLivePhotoGroup, group: kBackupLivePhotoGroup,
@@ -398,6 +389,54 @@ class UploadService {
return requiresWiFi; return requiresWiFi;
} }
@visibleForTesting
Future<({File file, String originalFilename})?> prepareUploadFile(
LocalAsset asset, {
bool isLivePhoto = false,
}) async {
final file = isLivePhoto
? await _storageRepository.getMotionFileForAsset(asset)
: await _storageRepository.getFileForAsset(asset.id);
if (file == null) {
return null;
}
final originalFilename = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name;
if (isLivePhoto) {
final livePhotoFilename = p.setExtension(originalFilename, p.extension(file.path));
return (file: file, originalFilename: livePhotoFilename);
}
final filenameExt = p.extension(originalFilename);
if (filenameExt.isNotEmpty) {
return (file: file, originalFilename: originalFilename);
}
final assetNameExt = p.extension(asset.name);
if (assetNameExt.isNotEmpty) {
final correctedFilename = p.setExtension(originalFilename, assetNameExt);
_logger.fine(
"Corrected filename $originalFilename to $correctedFilename using asset.name extension $assetNameExt",
);
return (file: file, originalFilename: correctedFilename);
}
final filePathExt = p.extension(file.path);
if (filePathExt.isEmpty) {
_logger.warning(
"Asset ${asset.id} has no file extension in any source, using original filename - $originalFilename",
);
return (file: file, originalFilename: originalFilename);
}
final correctedFilename = p.setExtension(originalFilename, filePathExt);
_logger.fine("Corrected filename $originalFilename to $correctedFilename using file path extension $filePathExt");
return (file: file, originalFilename: correctedFilename);
}
Future<UploadTask> buildUploadTask( Future<UploadTask> buildUploadTask(
File file, { File file, {
required String group, required String group,

View File

@@ -4,6 +4,7 @@ import 'package:drift/drift.dart' hide isNull, isNotNull;
import 'package:drift/native.dart'; import 'package:drift/native.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
import 'package:immich_mobile/domain/models/store.model.dart'; import 'package:immich_mobile/domain/models/store.model.dart';
import 'package:immich_mobile/domain/services/store.service.dart'; import 'package:immich_mobile/domain/services/store.service.dart';
import 'package:immich_mobile/entities/store.entity.dart'; import 'package:immich_mobile/entities/store.entity.dart';
@@ -16,8 +17,8 @@ import 'package:mocktail/mocktail.dart';
import '../domain/service.mock.dart'; import '../domain/service.mock.dart';
import '../fixtures/asset.stub.dart'; import '../fixtures/asset.stub.dart';
import '../infrastructure/repository.mock.dart'; import '../infrastructure/repository.mock.dart';
import '../repository.mocks.dart';
import '../mocks/asset_entity.mock.dart'; import '../mocks/asset_entity.mock.dart';
import '../repository.mocks.dart';
void main() { void main() {
late UploadService sut; late UploadService sut;
@@ -165,4 +166,174 @@ void main() {
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1); verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
}); });
}); });
group('prepareUploadFile', () {
test('should keep filename with existing extension unchanged', () async {
final asset = LocalAssetStub.image1;
final mockFile = File('/tmp/123.jpg');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'photo.jpg');
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.file.path, equals('/tmp/123.jpg'));
expect(result.originalFilename, equals('photo.jpg'));
});
test('should use asset.name extension when original filename lacks one', () async {
final asset = LocalAssetStub.image1;
final mockFile = File('/tmp/cache/123.mov');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => '2024-10-23_17-00-30');
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.originalFilename, equals('2024-10-23_17-00-30.jpg'));
});
test('should use file path extension as final fallback', () async {
final asset = LocalAssetStub.image1.copyWith(name: 'document');
final mockFile = File('/tmp/cache/123.mov');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'document');
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.originalFilename, equals('document.mov'));
});
test('should handle file without extension anywhere', () async {
final asset = LocalAssetStub.image1.copyWith(name: 'document');
final mockFile = File('/tmp/temp');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'document');
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.originalFilename, equals('document'));
});
test('should preserve existing extension even if asset.name has different one', () async {
final asset = LocalAssetStub.image1;
final mockFile = File('/tmp/123.mov');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'photo.HEIC');
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.originalFilename, equals('photo.HEIC'));
});
test('should fall back to asset.name when getOriginalFilename returns null', () async {
final asset = LocalAssetStub.image1.copyWith(name: 'VID_1234.mp4');
final mockFile = File('/tmp/video.mov');
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => null);
final result = await sut.prepareUploadFile(asset);
expect(result, isNotNull);
expect(result!.originalFilename, equals('VID_1234.mp4')); // Uses asset.name directly
});
test('should return null when file is not found', () async {
final asset = LocalAssetStub.image1;
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => null);
final result = await sut.prepareUploadFile(asset);
expect(result, isNull);
});
});
group('getUploadTask with missing extensions', () {
test('should add extension for regular photo without extension', () async {
final asset = LocalAssetStub.image1;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/file.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 => '2024-10-23_17-00-30');
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
expect(task!.fields['filename'], equals('2024-10-23_17-00-30.jpg'));
});
test('should preserve existing extension for regular photo', () async {
final asset = LocalAssetStub.image1;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/file.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 => 'MyPhoto.HEIC');
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
expect(task!.fields['filename'], equals('MyPhoto.HEIC'));
});
test('should add extension for video without extension', () async {
// Create a video asset using copyWith since image2 is a video type
final asset = LocalAssetStub.image1.copyWith(id: 'video1', name: 'VID_20241023_170030', type: AssetType.video);
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/video.mov');
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 => 'VID_20241023_170030');
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
expect(task!.fields['filename'], equals('VID_20241023_170030.mov'));
});
});
group('getLivePhotoUploadTask with missing extensions', () {
test('should add extension when live photo filename lacks one', () async {
final asset = LocalAssetStub.image1.copyWith(name: 'IMG_1234.heic');
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/photo.heic');
when(() => mockEntity.isLivePhoto).thenReturn(true);
when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'IMG_1234');
final task = await sut.getLivePhotoUploadTask(asset, 'video-id-123');
expect(task, isNotNull);
expect(task!.fields['filename'], equals('IMG_1234.heic'));
expect(task.fields['livePhotoVideoId'], equals('video-id-123'));
});
test('should preserve extension when live photo filename has one', () async {
final asset = LocalAssetStub.image1;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/photo.heic');
when(() => mockEntity.isLivePhoto).thenReturn(true);
when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'MyLivePhoto.HEIC');
final task = await sut.getLivePhotoUploadTask(asset, 'video-id-456');
expect(task, isNotNull);
expect(task!.fields['filename'], equals('MyLivePhoto.HEIC'));
});
});
} }