fix: fetch original name before upload (#21877)

* fix: fetch origin name before upload

* fix: Show correct photo name in buttom sheet and backup details page (#22978)

* add tests

---------

Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>
Co-authored-by: FawenYo <40032648+FawenYo@users.noreply.github.com>
This commit is contained in:
shenlong 2025-10-27 20:02:24 +05:30 committed by GitHub
parent 3194538817
commit 664a8fa499
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 295 additions and 66 deletions

View File

@ -11,6 +11,7 @@ import 'package:immich_mobile/extensions/translate_extensions.dart';
import 'package:immich_mobile/pages/common/large_leading_tile.dart';
import 'package:immich_mobile/presentation/widgets/images/thumbnail.widget.dart';
import 'package:immich_mobile/providers/backup/drift_backup.provider.dart';
import 'package:immich_mobile/repositories/asset_media.repository.dart';
import 'package:immich_mobile/routing/router.dart';
@RoutePage()
@ -31,55 +32,66 @@ class DriftBackupAssetDetailPage extends ConsumerWidget {
itemBuilder: (context, index) {
final asset = candidates[index];
final albumsAsyncValue = ref.watch(driftCandidateBackupAlbumInfoProvider(asset.id));
return LargeLeadingTile(
title: Text(
asset.name,
style: context.textTheme.labelLarge?.copyWith(fontWeight: FontWeight.w500, fontSize: 16),
),
subtitle: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(
asset.createdAt.toString(),
style: TextStyle(fontSize: 13.0, color: context.colorScheme.onSurfaceSecondary),
final assetMediaRepository = ref.watch(assetMediaRepositoryProvider);
return FutureBuilder<String?>(
future: assetMediaRepository.getOriginalFilename(asset.id),
builder: (context, snapshot) {
final displayName = snapshot.data ?? asset.name;
return LargeLeadingTile(
title: Text(
displayName,
style: context.textTheme.labelLarge?.copyWith(fontWeight: FontWeight.w500, fontSize: 16),
),
Text(
asset.checksum ?? "N/A",
style: TextStyle(fontSize: 13.0, color: context.colorScheme.onSurfaceSecondary),
overflow: TextOverflow.ellipsis,
),
albumsAsyncValue.when(
data: (albums) {
if (albums.isEmpty) {
return const SizedBox.shrink();
}
return Text(
albums.map((a) => a.name).join(', '),
style: context.textTheme.labelLarge?.copyWith(color: context.primaryColor),
subtitle: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(
asset.createdAt.toString(),
style: TextStyle(fontSize: 13.0, color: context.colorScheme.onSurfaceSecondary),
),
Text(
asset.checksum ?? "N/A",
style: TextStyle(fontSize: 13.0, color: context.colorScheme.onSurfaceSecondary),
overflow: TextOverflow.ellipsis,
);
},
error: (error, stackTrace) => Text(
'error_saving_image'.tr(args: [error.toString()]),
style: TextStyle(color: context.colorScheme.error),
),
loading: () => const SizedBox(height: 16, width: 16, child: CircularProgressIndicator.adaptive()),
),
albumsAsyncValue.when(
data: (albums) {
if (albums.isEmpty) {
return const SizedBox.shrink();
}
return Text(
albums.map((a) => a.name).join(', '),
style: context.textTheme.labelLarge?.copyWith(color: context.primaryColor),
overflow: TextOverflow.ellipsis,
);
},
error: (error, stackTrace) => Text(
'error_saving_image'.tr(args: [error.toString()]),
style: TextStyle(color: context.colorScheme.error),
),
loading: () =>
const SizedBox(height: 16, width: 16, child: CircularProgressIndicator.adaptive()),
),
],
),
],
),
leading: ClipRRect(
borderRadius: const BorderRadius.all(Radius.circular(12)),
child: SizedBox(
width: 64,
height: 64,
child: Thumbnail.fromAsset(asset: asset, size: const Size(64, 64), fit: BoxFit.cover),
),
),
trailing: const Padding(padding: EdgeInsets.only(right: 24, left: 8), child: Icon(Icons.image_search)),
onTap: () async {
await context.maybePop();
await context.navigateTo(const TabShellRoute(children: [MainTimelineRoute()]));
EventStream.shared.emit(ScrollToDateEvent(asset.createdAt));
leading: ClipRRect(
borderRadius: const BorderRadius.all(Radius.circular(12)),
child: SizedBox(
width: 64,
height: 64,
child: Thumbnail.fromAsset(asset: asset, size: const Size(64, 64), fit: BoxFit.cover),
),
),
trailing: const Padding(
padding: EdgeInsets.only(right: 24, left: 8),
child: Icon(Icons.image_search),
),
onTap: () async {
await context.maybePop();
await context.navigateTo(const TabShellRoute(children: [MainTimelineRoute()]));
EventStream.shared.emit(ScrollToDateEvent(asset.createdAt));
},
);
},
);
},

View File

@ -19,6 +19,7 @@ import 'package:immich_mobile/providers/infrastructure/setting.provider.dart';
import 'package:immich_mobile/providers/routes.provider.dart';
import 'package:immich_mobile/providers/server_info.provider.dart';
import 'package:immich_mobile/providers/user.provider.dart';
import 'package:immich_mobile/repositories/asset_media.repository.dart';
import 'package:immich_mobile/utils/action_button.utils.dart';
import 'package:immich_mobile/utils/bytes_units.dart';
import 'package:immich_mobile/widgets/common/immich_toast.dart';
@ -142,6 +143,47 @@ class _AssetDetailBottomSheet extends ConsumerWidget {
final cameraTitle = _getCameraInfoTitle(exifInfo);
final isOwner = ref.watch(currentUserProvider)?.id == (asset is RemoteAsset ? asset.ownerId : null);
// Build file info tile based on asset type
Widget buildFileInfoTile() {
if (asset is LocalAsset) {
final assetMediaRepository = ref.watch(assetMediaRepositoryProvider);
return FutureBuilder<String?>(
future: assetMediaRepository.getOriginalFilename(asset.id),
builder: (context, snapshot) {
final displayName = snapshot.data ?? asset.name;
return _SheetTile(
title: displayName,
titleStyle: context.textTheme.labelLarge,
leading: Icon(
asset.isImage ? Icons.image_outlined : Icons.videocam_outlined,
size: 24,
color: context.textTheme.labelLarge?.color,
),
subtitle: _getFileInfo(asset, exifInfo),
subtitleStyle: context.textTheme.bodyMedium?.copyWith(
color: context.textTheme.bodyMedium?.color?.withAlpha(155),
),
);
},
);
} else {
// For remote assets, use the name directly
return _SheetTile(
title: asset.name,
titleStyle: context.textTheme.labelLarge,
leading: Icon(
asset.isImage ? Icons.image_outlined : Icons.videocam_outlined,
size: 24,
color: context.textTheme.labelLarge?.color,
),
subtitle: _getFileInfo(asset, exifInfo),
subtitleStyle: context.textTheme.bodyMedium?.copyWith(
color: context.textTheme.bodyMedium?.color?.withAlpha(155),
),
);
}
}
return SliverList.list(
children: [
// Asset Date and Time
@ -163,19 +205,7 @@ class _AssetDetailBottomSheet extends ConsumerWidget {
),
),
// File info
_SheetTile(
title: asset.name,
titleStyle: context.textTheme.labelLarge,
leading: Icon(
asset.isImage ? Icons.image_outlined : Icons.videocam_outlined,
size: 24,
color: context.textTheme.labelLarge?.color,
),
subtitle: _getFileInfo(asset, exifInfo),
subtitleStyle: context.textTheme.bodyMedium?.copyWith(
color: context.textTheme.bodyMedium?.color?.withAlpha(155),
),
),
buildFileInfoTile(),
// Camera info
if (cameraTitle != null)
_SheetTile(

View File

@ -4,6 +4,7 @@ import 'dart:io';
import 'package:background_downloader/background_downloader.dart';
import 'package:cancellation_token_http/http.dart';
import 'package:flutter/foundation.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:immich_mobile/constants/constants.dart';
import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
@ -17,6 +18,7 @@ import 'package:immich_mobile/providers/app_settings.provider.dart';
import 'package:immich_mobile/providers/backup/drift_backup.provider.dart';
import 'package:immich_mobile/providers/infrastructure/asset.provider.dart';
import 'package:immich_mobile/providers/infrastructure/storage.provider.dart';
import 'package:immich_mobile/repositories/asset_media.repository.dart';
import 'package:immich_mobile/repositories/upload.repository.dart';
import 'package:immich_mobile/services/api.service.dart';
import 'package:immich_mobile/services/app_settings.service.dart';
@ -31,6 +33,7 @@ final uploadServiceProvider = Provider((ref) {
ref.watch(storageRepositoryProvider),
ref.watch(localAssetRepository),
ref.watch(appSettingsServiceProvider),
ref.watch(assetMediaRepositoryProvider),
);
ref.onDispose(service.dispose);
@ -44,6 +47,7 @@ class UploadService {
this._storageRepository,
this._localAssetRepository,
this._appSettingsService,
this._assetMediaRepository,
) {
_uploadRepository.onUploadStatus = _onUploadCallback;
_uploadRepository.onTaskProgress = _onTaskProgressCallback;
@ -54,6 +58,7 @@ class UploadService {
final StorageRepository _storageRepository;
final DriftLocalAssetRepository _localAssetRepository;
final AppSettingsService _appSettingsService;
final AssetMediaRepository _assetMediaRepository;
final Logger _logger = Logger('UploadService');
final StreamController<TaskStatusUpdate> _taskStatusController = StreamController<TaskStatusUpdate>.broadcast();
@ -98,7 +103,7 @@ class UploadService {
await _storageRepository.clearCache();
List<UploadTask> tasks = [];
for (final asset in localAssets) {
final task = await _getUploadTask(
final task = await getUploadTask(
asset,
group: kManualUploadGroup,
priority: 1, // High priority after upload motion photo part
@ -136,7 +141,7 @@ class UploadService {
final batch = candidates.skip(i).take(batchSize).toList();
List<UploadTask> tasks = [];
for (final asset in batch) {
final task = await _getUploadTask(asset);
final task = await getUploadTask(asset);
if (task != null) {
tasks.add(task);
}
@ -248,7 +253,7 @@ class UploadService {
return;
}
final uploadTask = await _getLivePhotoUploadTask(localAsset, response['id'] as String);
final uploadTask = await getLivePhotoUploadTask(localAsset, response['id'] as String);
if (uploadTask == null) {
return;
@ -296,7 +301,8 @@ class UploadService {
);
}
Future<UploadTask?> _getUploadTask(LocalAsset asset, {String group = kBackupGroup, int? priority}) async {
@visibleForTesting
Future<UploadTask?> getUploadTask(LocalAsset asset, {String group = kBackupGroup, int? priority}) async {
final entity = await _storageRepository.getAssetEntityForAsset(asset);
if (entity == null) {
return null;
@ -324,7 +330,8 @@ class UploadService {
return null;
}
final originalFileName = entity.isLivePhoto ? p.setExtension(asset.name, p.extension(file.path)) : asset.name;
final fileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name;
final originalFileName = entity.isLivePhoto ? p.setExtension(fileName, p.extension(file.path)) : fileName;
String metadata = UploadTaskMetadata(
localAssetId: asset.id,
@ -348,7 +355,8 @@ class UploadService {
);
}
Future<UploadTask?> _getLivePhotoUploadTask(LocalAsset asset, String livePhotoVideoId) async {
@visibleForTesting
Future<UploadTask?> getLivePhotoUploadTask(LocalAsset asset, String livePhotoVideoId) async {
final entity = await _storageRepository.getAssetEntityForAsset(asset);
if (entity == null) {
return null;
@ -362,12 +370,13 @@ class UploadService {
final fields = {'livePhotoVideoId': livePhotoVideoId};
final requiresWiFi = _shouldRequireWiFi(asset);
final originalFileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name;
return buildUploadTask(
file,
createdAt: asset.createdAt,
modifiedAt: asset.updatedAt,
originalFileName: asset.name,
originalFileName: originalFileName,
deviceAssetId: asset.id,
fields: fields,
group: kBackupLivePhotoGroup,

View File

@ -1,3 +1,4 @@
import 'package:immich_mobile/infrastructure/repositories/backup.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/device_asset.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart';
@ -10,6 +11,7 @@ import 'package:immich_mobile/infrastructure/repositories/sync_stream.repository
import 'package:immich_mobile/infrastructure/repositories/user.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/user_api.repository.dart';
import 'package:immich_mobile/repositories/drift_album_api_repository.dart';
import 'package:immich_mobile/repositories/upload.repository.dart';
import 'package:mocktail/mocktail.dart';
class MockStoreRepository extends Mock implements IsarStoreRepository {}
@ -30,8 +32,14 @@ class MockRemoteAlbumRepository extends Mock implements DriftRemoteAlbumReposito
class MockLocalAssetRepository extends Mock implements DriftLocalAssetRepository {}
class MockDriftLocalAssetRepository extends Mock implements DriftLocalAssetRepository {}
class MockStorageRepository extends Mock implements StorageRepository {}
class MockDriftBackupRepository extends Mock implements DriftBackupRepository {}
class MockUploadRepository extends Mock implements UploadRepository {}
// API Repos
class MockUserApiRepository extends Mock implements UserApiRepository {}

View File

@ -0,0 +1,170 @@
import 'dart:io';
import 'package:drift/drift.dart' hide isNull, isNotNull;
import 'package:drift/native.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/models/store.model.dart';
import 'package:immich_mobile/domain/services/store.service.dart';
import 'package:immich_mobile/entities/store.entity.dart';
import 'package:immich_mobile/infrastructure/repositories/db.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/store.repository.dart';
import 'package:immich_mobile/services/app_settings.service.dart';
import 'package:immich_mobile/services/upload.service.dart';
import 'package:mocktail/mocktail.dart';
import 'package:photo_manager/photo_manager.dart';
import '../domain/service.mock.dart';
import '../fixtures/asset.stub.dart';
import '../infrastructure/repository.mock.dart';
import '../repository.mocks.dart';
class MockAssetEntity extends Mock implements AssetEntity {}
void main() {
late UploadService sut;
late MockUploadRepository mockUploadRepository;
late MockDriftBackupRepository mockBackupRepository;
late MockStorageRepository mockStorageRepository;
late MockDriftLocalAssetRepository mockLocalAssetRepository;
late MockAppSettingsService mockAppSettingsService;
late MockAssetMediaRepository mockAssetMediaRepository;
late Drift db;
setUpAll(() async {
registerFallbackValue(AppSettingsEnum.useCellularForUploadPhotos);
TestWidgetsFlutterBinding.ensureInitialized();
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
const MethodChannel('plugins.flutter.io/path_provider'),
(MethodCall methodCall) async => 'test',
);
db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true));
await StoreService.init(storeRepository: DriftStoreRepository(db));
await Store.put(StoreKey.serverEndpoint, 'http://test-server.com');
await Store.put(StoreKey.deviceId, 'test-device-id');
});
setUp(() {
mockUploadRepository = MockUploadRepository();
mockBackupRepository = MockDriftBackupRepository();
mockStorageRepository = MockStorageRepository();
mockLocalAssetRepository = MockDriftLocalAssetRepository();
mockAppSettingsService = MockAppSettingsService();
mockAssetMediaRepository = MockAssetMediaRepository();
when(() => mockAppSettingsService.getSetting(AppSettingsEnum.useCellularForUploadVideos)).thenReturn(false);
when(() => mockAppSettingsService.getSetting(AppSettingsEnum.useCellularForUploadPhotos)).thenReturn(false);
sut = UploadService(
mockUploadRepository,
mockBackupRepository,
mockStorageRepository,
mockLocalAssetRepository,
mockAppSettingsService,
mockAssetMediaRepository,
);
mockUploadRepository.onUploadStatus = (_) {};
mockUploadRepository.onTaskProgress = (_) {};
});
tearDown(() {
sut.dispose();
});
group('getUploadTask', () {
test('should call getOriginalFilename from AssetMediaRepository 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 => 'OriginalPhoto.jpg');
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
expect(task!.fields['filename'], equals('OriginalPhoto.jpg'));
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
});
test('should call getOriginalFilename when original filename is null', () async {
final asset = LocalAssetStub.image2;
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 => null);
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
expect(task!.fields['filename'], equals(asset.name));
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
});
test('should call getOriginalFilename for live photo', () async {
final asset = LocalAssetStub.image1;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/file.mov');
when(() => mockEntity.isLivePhoto).thenReturn(true);
when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
when(() => mockStorageRepository.getMotionFileForAsset(asset)).thenAnswer((_) async => mockFile);
when(
() => mockAssetMediaRepository.getOriginalFilename(asset.id),
).thenAnswer((_) async => 'OriginalLivePhoto.HEIC');
final task = await sut.getUploadTask(asset);
expect(task, isNotNull);
// For live photos, extension should be changed to match the video file
expect(task!.fields['filename'], equals('OriginalLivePhoto.mov'));
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
});
});
group('getLivePhotoUploadTask', () {
test('should call getOriginalFilename for live photo upload task', () async {
final asset = LocalAssetStub.image1;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/livephoto.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 => 'OriginalLivePhoto.HEIC');
final task = await sut.getLivePhotoUploadTask(asset, 'video-id-123');
expect(task, isNotNull);
expect(task!.fields['filename'], equals('OriginalLivePhoto.HEIC'));
expect(task.fields['livePhotoVideoId'], equals('video-id-123'));
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
});
test('should call getOriginalFilename when original filename is null', () async {
final asset = LocalAssetStub.image2;
final mockEntity = MockAssetEntity();
final mockFile = File('/path/to/fallback.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 => null);
final task = await sut.getLivePhotoUploadTask(asset, 'video-id-456');
expect(task, isNotNull);
// Should fall back to asset.name when original filename is null
expect(task!.fields['filename'], equals(asset.name));
verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
});
});
}