mirror of
https://github.com/immich-app/immich.git
synced 2026-04-28 12:13:09 -07:00
fix(mobile): delete assets on trash empty, Android (#26070)
* fix(mobile): improve trash sync flow - trash local assets on remote delete events - unify remote trash handling and support assetDelete cleanup by remote asset id - update sync stream tests * fix(mobile): revert pubspec.lock * refactor(mobile): remove helper remove unused columns from results * refactor(mobile): use remoteIds in getAssetsFromBackupAlbums and remove getAssetsFromBackupAlbumsByRemoteIds refactor tests --------- Co-authored-by: Peter Ombodi <peter.ombodi@gmail.com>
This commit is contained in:
@@ -3,6 +3,7 @@
|
||||
import 'dart:async';
|
||||
import 'dart:convert';
|
||||
|
||||
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/sync_event.model.dart';
|
||||
import 'package:immich_mobile/entities/store.entity.dart';
|
||||
@@ -191,17 +192,22 @@ class SyncStreamService {
|
||||
case SyncEntityType.assetV1:
|
||||
final remoteSyncAssets = data.cast<SyncAssetV1>();
|
||||
await _syncStreamRepository.updateAssetsV1(remoteSyncAssets);
|
||||
if (CurrentPlatform.isAndroid && Store.get(StoreKey.manageLocalMediaAndroid, false)) {
|
||||
final hasPermission = await _localFilesManager.hasManageMediaPermission();
|
||||
if (hasPermission) {
|
||||
await _handleRemoteTrashed(remoteSyncAssets.where((e) => e.deletedAt != null).map((e) => e.checksum));
|
||||
await _runWithManageMediaPermission(
|
||||
logContext: "Trashed Assets",
|
||||
action: () async {
|
||||
await _handleRemoteDeleted(remoteSyncAssets.where((e) => e.deletedAt != null).map((e) => e.id));
|
||||
await _applyRemoteRestoreToLocal();
|
||||
} else {
|
||||
_logger.warning("sync Trashed Assets cannot proceed because MANAGE_MEDIA permission is missing");
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
return;
|
||||
case SyncEntityType.assetDeleteV1:
|
||||
await _runWithManageMediaPermission(
|
||||
logContext: "Deleted Assets",
|
||||
action: () async {
|
||||
final remoteSyncAssets = data.cast<SyncAssetDeleteV1>();
|
||||
await _handleRemoteDeleted(remoteSyncAssets.map((e) => e.assetId));
|
||||
},
|
||||
);
|
||||
return _syncStreamRepository.deleteAssetsV1(data.cast());
|
||||
case SyncEntityType.assetExifV1:
|
||||
return _syncStreamRepository.updateAssetsExifV1(data.cast());
|
||||
@@ -382,12 +388,20 @@ class SyncStreamService {
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _handleRemoteTrashed(Iterable<String> checksums) async {
|
||||
if (checksums.isEmpty) {
|
||||
Future<void> _handleRemoteDeleted(Iterable<String> remoteIds) async {
|
||||
if (remoteIds.isEmpty) {
|
||||
return Future.value();
|
||||
} else {
|
||||
final localAssetsToTrash = await _localAssetRepository.getAssetsFromBackupAlbums(checksums);
|
||||
final localAssetsToTrash = await _localAssetRepository.getAssetsFromBackupAlbums(remoteIds);
|
||||
if (localAssetsToTrash.isNotEmpty) {
|
||||
await _trashLocalAssets(localAssetsToTrash);
|
||||
} else {
|
||||
_logger.info("No assets found in backup-enabled albums for remote assets: $remoteIds");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _trashLocalAssets(Map<String, List<LocalAsset>> localAssetsToTrash) async {
|
||||
final mediaUrls = await Future.wait(
|
||||
localAssetsToTrash.values
|
||||
.expand((e) => e)
|
||||
@@ -398,10 +412,6 @@ class SyncStreamService {
|
||||
if (result) {
|
||||
await _trashedLocalAssetRepository.trashLocalAsset(localAssetsToTrash);
|
||||
}
|
||||
} else {
|
||||
_logger.info("No assets found in backup-enabled albums for assets: $checksums");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _applyRemoteRestoreToLocal() async {
|
||||
@@ -413,4 +423,21 @@ class SyncStreamService {
|
||||
_logger.info("No remote assets found for restoration");
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _runWithManageMediaPermission({
|
||||
required String logContext,
|
||||
required Future<void> Function() action,
|
||||
}) async {
|
||||
if (!CurrentPlatform.isAndroid || !Store.get(StoreKey.manageLocalMediaAndroid, false)) {
|
||||
return;
|
||||
}
|
||||
|
||||
final hasPermission = await _localFilesManager.hasManageMediaPermission();
|
||||
if (!hasPermission) {
|
||||
_logger.warning("sync $logContext cannot proceed because MANAGE_MEDIA permission is missing");
|
||||
return;
|
||||
}
|
||||
|
||||
await action();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,31 +109,40 @@ class DriftLocalAssetRepository extends DriftDatabaseRepository {
|
||||
return query.map((localAlbum) => localAlbum.toDto()).get();
|
||||
}
|
||||
|
||||
Future<Map<String, List<LocalAsset>>> getAssetsFromBackupAlbums(Iterable<String> checksums) async {
|
||||
if (checksums.isEmpty) {
|
||||
Future<Map<String, List<LocalAsset>>> getAssetsFromBackupAlbums(Iterable<String> remoteIds) async {
|
||||
if (remoteIds.isEmpty) {
|
||||
return {};
|
||||
}
|
||||
|
||||
final result = <String, List<LocalAsset>>{};
|
||||
|
||||
for (final slice in checksums.toSet().slices(kDriftMaxChunk)) {
|
||||
for (final slice in remoteIds.toSet().slices(kDriftMaxChunk)) {
|
||||
final rows =
|
||||
await (_db.select(_db.localAlbumAssetEntity).join([
|
||||
innerJoin(_db.localAlbumEntity, _db.localAlbumAssetEntity.albumId.equalsExp(_db.localAlbumEntity.id)),
|
||||
innerJoin(
|
||||
_db.localAlbumEntity,
|
||||
_db.localAlbumAssetEntity.albumId.equalsExp(_db.localAlbumEntity.id),
|
||||
useColumns: false,
|
||||
),
|
||||
innerJoin(_db.localAssetEntity, _db.localAlbumAssetEntity.assetId.equalsExp(_db.localAssetEntity.id)),
|
||||
innerJoin(
|
||||
_db.remoteAssetEntity,
|
||||
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum),
|
||||
useColumns: false,
|
||||
),
|
||||
])..where(
|
||||
_db.localAlbumEntity.backupSelection.equalsValue(BackupSelection.selected) &
|
||||
_db.localAssetEntity.checksum.isIn(slice),
|
||||
_db.remoteAssetEntity.id.isIn(slice),
|
||||
))
|
||||
.get();
|
||||
|
||||
for (final row in rows) {
|
||||
final albumId = row.readTable(_db.localAlbumAssetEntity).albumId;
|
||||
final assetData = row.readTable(_db.localAssetEntity);
|
||||
final asset = assetData.toDto();
|
||||
final asset = row.readTable(_db.localAssetEntity).toDto();
|
||||
(result[albumId] ??= <LocalAsset>[]).add(asset);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
@@ -419,8 +419,8 @@ void main() {
|
||||
'album-b': [mergedAsset],
|
||||
};
|
||||
when(() => mockLocalAssetRepo.getAssetsFromBackupAlbums(any())).thenAnswer((invocation) async {
|
||||
final Iterable<String> requestedChecksums = invocation.positionalArguments.first as Iterable<String>;
|
||||
expect(requestedChecksums.toSet(), equals({'checksum-local', 'checksum-merged', 'checksum-remote-only'}));
|
||||
final Iterable<String> requestedRemoteIds = invocation.positionalArguments.first as Iterable<String>;
|
||||
expect(requestedRemoteIds.toSet(), equals({'remote-1', 'remote-2', 'remote-3'}));
|
||||
return assetsByAlbum;
|
||||
});
|
||||
|
||||
@@ -482,12 +482,18 @@ void main() {
|
||||
verifyNever(() => mockTrashedLocalAssetRepo.trashLocalAsset(any()));
|
||||
});
|
||||
|
||||
test("does not request local deletions for permanent remote delete events", () async {
|
||||
test("requests local deletions lookup by remote ids for permanent remote delete events", () async {
|
||||
when(() => mockLocalAssetRepo.getAssetsFromBackupAlbums(any())).thenAnswer((invocation) async {
|
||||
final Iterable<String> requestedRemoteIds = invocation.positionalArguments.first as Iterable<String>;
|
||||
expect(requestedRemoteIds.toSet(), equals({'remote-asset'}));
|
||||
return {};
|
||||
});
|
||||
|
||||
final events = [SyncStreamStub.assetDeleteV1];
|
||||
|
||||
await simulateEvents(events);
|
||||
|
||||
verifyNever(() => mockLocalAssetRepo.getAssetsFromBackupAlbums(any()));
|
||||
verify(() => mockLocalAssetRepo.getAssetsFromBackupAlbums(any())).called(1);
|
||||
verifyNever(() => mockLocalFilesManagerRepo.moveToTrash(any()));
|
||||
verify(() => mockSyncStreamRepo.deleteAssetsV1(any())).called(1);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user