mirror of
https://github.com/samsonjs/immich.git
synced 2026-03-25 09:15:56 +00:00
feat(mobile): do not restore locally deleted assets during trash sync (Android) (#24218)
* feat(trash_sync): do not restore assets deleted locally only small fixes * feat(trash_sync): revert tag name * feat(trash_sync): resolve merge conflicts * refactor(trash_sync): consolidate local asset deletion logic * feat(mobile): Add TrashOrigin enum Replace isRestorable to sourse change related logic in repo * feat(mobile): fix format * fix(mobile): fix restoration scope * fix(mobile): Add coverage for ActionService deleteLocal paths Update LocalSyncService tests Set default value for source column * fix(mobile): db - require trash origin and update drift schema --------- Co-authored-by: Peter Ombodi <peter.ombodi@gmail.com>
This commit is contained in:
parent
7992fe85d6
commit
8ed81ac3e1
12 changed files with 230 additions and 30 deletions
BIN
mobile/drift_schemas/main/drift_schema_v15.json
generated
Normal file
BIN
mobile/drift_schemas/main/drift_schema_v15.json
generated
Normal file
Binary file not shown.
|
|
@ -4,6 +4,13 @@ import 'package:immich_mobile/infrastructure/entities/trashed_local_asset.entity
|
|||
import 'package:immich_mobile/infrastructure/utils/asset.mixin.dart';
|
||||
import 'package:immich_mobile/infrastructure/utils/drift_default.mixin.dart';
|
||||
|
||||
enum TrashOrigin {
|
||||
// do not change this order!
|
||||
localSync,
|
||||
remoteSync,
|
||||
localUser,
|
||||
}
|
||||
|
||||
@TableIndex.sql('CREATE INDEX IF NOT EXISTS idx_trashed_local_asset_checksum ON trashed_local_asset_entity (checksum)')
|
||||
@TableIndex.sql('CREATE INDEX IF NOT EXISTS idx_trashed_local_asset_album ON trashed_local_asset_entity (album_id)')
|
||||
class TrashedLocalAssetEntity extends Table with DriftDefaultsMixin, AssetEntityMixin {
|
||||
|
|
@ -19,6 +26,8 @@ class TrashedLocalAssetEntity extends Table with DriftDefaultsMixin, AssetEntity
|
|||
|
||||
IntColumn get orientation => integer().withDefault(const Constant(0))();
|
||||
|
||||
IntColumn get source => intEnum<TrashOrigin>()();
|
||||
|
||||
@override
|
||||
Set<Column> get primaryKey => {id, albumId};
|
||||
}
|
||||
|
|
|
|||
Binary file not shown.
|
|
@ -95,7 +95,7 @@ class Drift extends $Drift implements IDatabaseRepository {
|
|||
}
|
||||
|
||||
@override
|
||||
int get schemaVersion => 14;
|
||||
int get schemaVersion => 15;
|
||||
|
||||
@override
|
||||
MigrationStrategy get migration => MigrationStrategy(
|
||||
|
|
@ -190,6 +190,9 @@ class Drift extends $Drift implements IDatabaseRepository {
|
|||
await m.addColumn(v14.localAssetEntity, v14.localAssetEntity.latitude);
|
||||
await m.addColumn(v14.localAssetEntity, v14.localAssetEntity.longitude);
|
||||
},
|
||||
from14To15: (m, v15) async {
|
||||
await m.addColumn(v15.trashedLocalAssetEntity, v15.trashedLocalAssetEntity.source);
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
|
|
|
|||
Binary file not shown.
|
|
@ -48,7 +48,8 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
_db.remoteAssetEntity.checksum.equalsExp(_db.trashedLocalAssetEntity.checksum),
|
||||
),
|
||||
])..where(
|
||||
_db.trashedLocalAssetEntity.albumId.isInQuery(selectedAlbumIds) &
|
||||
_db.trashedLocalAssetEntity.source.equalsValue(TrashOrigin.remoteSync) &
|
||||
_db.trashedLocalAssetEntity.albumId.isInQuery(selectedAlbumIds) &
|
||||
_db.remoteAssetEntity.deletedAt.isNull(),
|
||||
))
|
||||
.get();
|
||||
|
|
@ -84,6 +85,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
durationInSeconds: Value(item.asset.durationInSeconds),
|
||||
isFavorite: Value(item.asset.isFavorite),
|
||||
orientation: Value(item.asset.orientation),
|
||||
source: TrashOrigin.localSync,
|
||||
);
|
||||
|
||||
batch.insert<$TrashedLocalAssetEntityTable, TrashedLocalAssetEntityData>(
|
||||
|
|
@ -124,7 +126,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
|
||||
Future<void> trashLocalAsset(Map<String, List<LocalAsset>> assetsByAlbums) async {
|
||||
if (assetsByAlbums.isEmpty) {
|
||||
return;
|
||||
return Future.value();
|
||||
}
|
||||
|
||||
final companions = <TrashedLocalAssetEntityCompanion>[];
|
||||
|
|
@ -147,6 +149,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
orientation: Value(asset.orientation),
|
||||
createdAt: Value(asset.createdAt),
|
||||
updatedAt: Value(asset.updatedAt),
|
||||
source: const Value(TrashOrigin.remoteSync),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
|
@ -165,7 +168,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
|
||||
Future<void> applyRestoredAssets(List<String> idList) async {
|
||||
if (idList.isEmpty) {
|
||||
return;
|
||||
return Future.value();
|
||||
}
|
||||
|
||||
final trashedAssets = <TrashedLocalAssetEntityData>[];
|
||||
|
|
@ -205,6 +208,58 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
|
|||
});
|
||||
}
|
||||
|
||||
Future<void> applyTrashedAssets(List<String> idList) async {
|
||||
if (idList.isEmpty) {
|
||||
return Future.value();
|
||||
}
|
||||
|
||||
final trashedAssets = <({LocalAssetEntityData asset, String albumId})>[];
|
||||
|
||||
for (final slice in idList.slices(kDriftMaxChunk)) {
|
||||
final rows = await (_db.select(_db.localAlbumAssetEntity).join([
|
||||
innerJoin(_db.localAssetEntity, _db.localAlbumAssetEntity.assetId.equalsExp(_db.localAssetEntity.id)),
|
||||
])..where(_db.localAlbumAssetEntity.assetId.isIn(slice))).get();
|
||||
|
||||
final assetsWithAlbum = rows.map(
|
||||
(row) =>
|
||||
(albumId: row.readTable(_db.localAlbumAssetEntity).albumId, asset: row.readTable(_db.localAssetEntity)),
|
||||
);
|
||||
|
||||
trashedAssets.addAll(assetsWithAlbum);
|
||||
}
|
||||
|
||||
if (trashedAssets.isEmpty) {
|
||||
return;
|
||||
}
|
||||
|
||||
final companions = trashedAssets.map((e) {
|
||||
return TrashedLocalAssetEntityCompanion.insert(
|
||||
id: e.asset.id,
|
||||
name: e.asset.name,
|
||||
type: e.asset.type,
|
||||
createdAt: Value(e.asset.createdAt),
|
||||
updatedAt: Value(e.asset.updatedAt),
|
||||
width: Value(e.asset.width),
|
||||
height: Value(e.asset.height),
|
||||
durationInSeconds: Value(e.asset.durationInSeconds),
|
||||
checksum: Value(e.asset.checksum),
|
||||
isFavorite: Value(e.asset.isFavorite),
|
||||
orientation: Value(e.asset.orientation),
|
||||
source: TrashOrigin.localUser,
|
||||
albumId: e.albumId,
|
||||
);
|
||||
});
|
||||
|
||||
await _db.transaction(() async {
|
||||
for (final companion in companions) {
|
||||
await _db.into(_db.trashedLocalAssetEntity).insertOnConflictUpdate(companion);
|
||||
}
|
||||
for (final slice in idList.slices(kDriftMaxChunk)) {
|
||||
await (_db.delete(_db.localAssetEntity)..where((t) => t.id.isIn(slice))).go();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Future<Map<String, List<LocalAsset>>> getToTrash() async {
|
||||
final result = <String, List<LocalAsset>>{};
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ final localFilesManagerRepositoryProvider = Provider(
|
|||
class LocalFilesManagerRepository {
|
||||
LocalFilesManagerRepository(this._service);
|
||||
|
||||
final Logger _logger = Logger('SyncStreamService');
|
||||
final Logger _logger = Logger('LocalFilesManagerRepo');
|
||||
final LocalFilesManagerService _service;
|
||||
|
||||
Future<bool> moveToTrash(List<String> mediaUrls) async {
|
||||
|
|
@ -38,8 +38,10 @@ class LocalFilesManagerRepository {
|
|||
for (final asset in assets) {
|
||||
_logger.info("Restoring from trash, localId: ${asset.id}, remoteId: ${asset.checksum}");
|
||||
try {
|
||||
await _service.restoreFromTrashById(asset.id, asset.type.index);
|
||||
restoredIds.add(asset.id);
|
||||
final result = await _service.restoreFromTrashById(asset.id, asset.type.index);
|
||||
if (result) {
|
||||
restoredIds.add(asset.id);
|
||||
}
|
||||
} catch (e) {
|
||||
_logger.warning("Restoring failure: $e");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,9 +5,13 @@ import 'package:flutter/material.dart';
|
|||
import 'package:hooks_riverpod/hooks_riverpod.dart';
|
||||
import 'package:immich_mobile/constants/enums.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/entities/store.entity.dart';
|
||||
import 'package:immich_mobile/extensions/platform_extensions.dart';
|
||||
import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart';
|
||||
import 'package:immich_mobile/infrastructure/repositories/remote_album.repository.dart';
|
||||
import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart';
|
||||
import 'package:immich_mobile/infrastructure/repositories/trashed_local_asset.repository.dart';
|
||||
import 'package:immich_mobile/providers/infrastructure/album.provider.dart';
|
||||
import 'package:immich_mobile/providers/infrastructure/asset.provider.dart';
|
||||
import 'package:immich_mobile/repositories/asset_api.repository.dart';
|
||||
|
|
@ -28,6 +32,7 @@ final actionServiceProvider = Provider<ActionService>(
|
|||
ref.watch(localAssetRepository),
|
||||
ref.watch(driftAlbumApiRepositoryProvider),
|
||||
ref.watch(remoteAlbumRepository),
|
||||
ref.watch(trashedLocalAssetRepository),
|
||||
ref.watch(assetMediaRepositoryProvider),
|
||||
ref.watch(downloadRepositoryProvider),
|
||||
),
|
||||
|
|
@ -39,6 +44,7 @@ class ActionService {
|
|||
final DriftLocalAssetRepository _localAssetRepository;
|
||||
final DriftAlbumApiRepository _albumApiRepository;
|
||||
final DriftRemoteAlbumRepository _remoteAlbumRepository;
|
||||
final DriftTrashedLocalAssetRepository _trashedLocalAssetRepository;
|
||||
final AssetMediaRepository _assetMediaRepository;
|
||||
final DownloadRepository _downloadRepository;
|
||||
|
||||
|
|
@ -48,6 +54,7 @@ class ActionService {
|
|||
this._localAssetRepository,
|
||||
this._albumApiRepository,
|
||||
this._remoteAlbumRepository,
|
||||
this._trashedLocalAssetRepository,
|
||||
this._assetMediaRepository,
|
||||
this._downloadRepository,
|
||||
);
|
||||
|
|
@ -82,11 +89,7 @@ class ActionService {
|
|||
|
||||
// Ask user if they want to delete local copies
|
||||
if (localIds.isNotEmpty) {
|
||||
final deletedIds = await _assetMediaRepository.deleteAll(localIds);
|
||||
|
||||
if (deletedIds.isNotEmpty) {
|
||||
await _localAssetRepository.delete(deletedIds);
|
||||
}
|
||||
await _deleteLocalAssets(localIds);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -110,11 +113,7 @@ class ActionService {
|
|||
await _remoteAssetRepository.trash(remoteIds);
|
||||
|
||||
if (localIds.isNotEmpty) {
|
||||
final deletedIds = await _assetMediaRepository.deleteAll(localIds);
|
||||
|
||||
if (deletedIds.isNotEmpty) {
|
||||
await _localAssetRepository.delete(deletedIds);
|
||||
}
|
||||
await _deleteLocalAssets(localIds);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -123,22 +122,12 @@ class ActionService {
|
|||
await _remoteAssetRepository.delete(remoteIds);
|
||||
|
||||
if (localIds.isNotEmpty) {
|
||||
final deletedIds = await _assetMediaRepository.deleteAll(localIds);
|
||||
|
||||
if (deletedIds.isNotEmpty) {
|
||||
await _localAssetRepository.delete(deletedIds);
|
||||
}
|
||||
await _deleteLocalAssets(localIds);
|
||||
}
|
||||
}
|
||||
|
||||
Future<int> deleteLocal(List<String> localIds) async {
|
||||
final deletedIds = await _assetMediaRepository.deleteAll(localIds);
|
||||
if (deletedIds.isNotEmpty) {
|
||||
await _localAssetRepository.delete(deletedIds);
|
||||
return deletedIds.length;
|
||||
}
|
||||
|
||||
return 0;
|
||||
return await _deleteLocalAssets(localIds);
|
||||
}
|
||||
|
||||
Future<bool> editLocation(List<String> remoteIds, BuildContext context) async {
|
||||
|
|
@ -242,4 +231,17 @@ class ActionService {
|
|||
Future<List<bool>> downloadAll(List<RemoteAsset> assets) {
|
||||
return _downloadRepository.downloadAllAssets(assets);
|
||||
}
|
||||
|
||||
Future<int> _deleteLocalAssets(List<String> localIds) async {
|
||||
final deletedIds = await _assetMediaRepository.deleteAll(localIds);
|
||||
if (deletedIds.isEmpty) {
|
||||
return 0;
|
||||
}
|
||||
if (CurrentPlatform.isAndroid && Store.get(StoreKey.manageLocalMediaAndroid, false)) {
|
||||
await _trashedLocalAssetRepository.applyTrashedAssets(deletedIds);
|
||||
} else {
|
||||
await _localAssetRepository.delete(deletedIds);
|
||||
}
|
||||
return deletedIds.length;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -153,7 +153,14 @@ void main() {
|
|||
'album-a': [platformAsset],
|
||||
});
|
||||
|
||||
verify(() => mockTrashedLocalAssetRepository.processTrashSnapshot(any())).called(1);
|
||||
final trashedSnapshot =
|
||||
verify(() => mockTrashedLocalAssetRepository.processTrashSnapshot(captureAny())).captured.single
|
||||
as Iterable<TrashedAsset>;
|
||||
expect(trashedSnapshot.length, 1);
|
||||
final trashedEntry = trashedSnapshot.single;
|
||||
expect(trashedEntry.albumId, 'album-a');
|
||||
expect(trashedEntry.asset.id, platformAsset.id);
|
||||
expect(trashedEntry.asset.name, platformAsset.name);
|
||||
verify(() => mockTrashedLocalAssetRepository.getToTrash()).called(1);
|
||||
|
||||
verify(() => mockLocalFilesManager.restoreAssetsFromTrash(any())).called(1);
|
||||
|
|
@ -174,6 +181,10 @@ void main() {
|
|||
|
||||
await sut.processTrashedAssets({});
|
||||
|
||||
final trashedSnapshot =
|
||||
verify(() => mockTrashedLocalAssetRepository.processTrashSnapshot(captureAny())).captured.single
|
||||
as Iterable<TrashedAsset>;
|
||||
expect(trashedSnapshot, isEmpty);
|
||||
verifyNever(() => mockLocalFilesManager.restoreAssetsFromTrash(any()));
|
||||
verifyNever(() => mockTrashedLocalAssetRepository.applyRestoredAssets(any()));
|
||||
});
|
||||
|
|
|
|||
BIN
mobile/test/drift/main/generated/schema.dart
generated
BIN
mobile/test/drift/main/generated/schema.dart
generated
Binary file not shown.
BIN
mobile/test/drift/main/generated/schema_v15.dart
generated
Normal file
BIN
mobile/test/drift/main/generated/schema_v15.dart
generated
Normal file
Binary file not shown.
118
mobile/test/services/action.service_test.dart
Normal file
118
mobile/test/services/action.service_test.dart
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
import 'package:drift/drift.dart' as drift;
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter/foundation.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/repositories/download.repository.dart';
|
||||
import 'package:immich_mobile/services/action.service.dart';
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
|
||||
import '../infrastructure/repository.mock.dart';
|
||||
import '../repository.mocks.dart';
|
||||
|
||||
class MockDownloadRepository extends Mock implements DownloadRepository {}
|
||||
|
||||
void main() {
|
||||
late ActionService sut;
|
||||
|
||||
late MockAssetApiRepository assetApiRepository;
|
||||
late MockRemoteAssetRepository remoteAssetRepository;
|
||||
late MockDriftLocalAssetRepository localAssetRepository;
|
||||
late MockDriftAlbumApiRepository albumApiRepository;
|
||||
late MockRemoteAlbumRepository remoteAlbumRepository;
|
||||
late MockTrashedLocalAssetRepository trashedLocalAssetRepository;
|
||||
late MockAssetMediaRepository assetMediaRepository;
|
||||
late MockDownloadRepository downloadRepository;
|
||||
|
||||
late Drift db;
|
||||
|
||||
setUpAll(() async {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
debugDefaultTargetPlatformOverride = TargetPlatform.android;
|
||||
|
||||
db = Drift(drift.DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true));
|
||||
await StoreService.init(storeRepository: DriftStoreRepository(db));
|
||||
});
|
||||
|
||||
tearDownAll(() async {
|
||||
debugDefaultTargetPlatformOverride = null;
|
||||
await Store.clear();
|
||||
await db.close();
|
||||
});
|
||||
|
||||
setUp(() {
|
||||
assetApiRepository = MockAssetApiRepository();
|
||||
remoteAssetRepository = MockRemoteAssetRepository();
|
||||
localAssetRepository = MockDriftLocalAssetRepository();
|
||||
albumApiRepository = MockDriftAlbumApiRepository();
|
||||
remoteAlbumRepository = MockRemoteAlbumRepository();
|
||||
trashedLocalAssetRepository = MockTrashedLocalAssetRepository();
|
||||
assetMediaRepository = MockAssetMediaRepository();
|
||||
downloadRepository = MockDownloadRepository();
|
||||
|
||||
sut = ActionService(
|
||||
assetApiRepository,
|
||||
remoteAssetRepository,
|
||||
localAssetRepository,
|
||||
albumApiRepository,
|
||||
remoteAlbumRepository,
|
||||
trashedLocalAssetRepository,
|
||||
assetMediaRepository,
|
||||
downloadRepository,
|
||||
);
|
||||
});
|
||||
|
||||
tearDown(() async {
|
||||
await Store.clear();
|
||||
});
|
||||
|
||||
group('ActionService.deleteLocal', () {
|
||||
test('routes deleted ids to trashed repository when Android trash handling is enabled', () async {
|
||||
await Store.put(StoreKey.manageLocalMediaAndroid, true);
|
||||
const ids = ['a', 'b'];
|
||||
|
||||
when(() => assetMediaRepository.deleteAll(ids)).thenAnswer((_) async => ids);
|
||||
when(() => trashedLocalAssetRepository.applyTrashedAssets(ids)).thenAnswer((_) async {});
|
||||
|
||||
final result = await sut.deleteLocal(ids);
|
||||
|
||||
expect(result, ids.length);
|
||||
verify(() => assetMediaRepository.deleteAll(ids)).called(1);
|
||||
verify(() => trashedLocalAssetRepository.applyTrashedAssets(ids)).called(1);
|
||||
verifyNever(() => localAssetRepository.delete(any()));
|
||||
});
|
||||
|
||||
test('deletes locally when Android trash handling is disabled', () async {
|
||||
await Store.put(StoreKey.manageLocalMediaAndroid, false);
|
||||
const ids = ['c'];
|
||||
|
||||
when(() => assetMediaRepository.deleteAll(ids)).thenAnswer((_) async => ids);
|
||||
when(() => localAssetRepository.delete(ids)).thenAnswer((_) async {});
|
||||
|
||||
final result = await sut.deleteLocal(ids);
|
||||
|
||||
expect(result, ids.length);
|
||||
verify(() => assetMediaRepository.deleteAll(ids)).called(1);
|
||||
verify(() => localAssetRepository.delete(ids)).called(1);
|
||||
verifyNever(() => trashedLocalAssetRepository.applyTrashedAssets(any()));
|
||||
});
|
||||
|
||||
test('short-circuits when nothing was deleted', () async {
|
||||
await Store.put(StoreKey.manageLocalMediaAndroid, true);
|
||||
const ids = ['x'];
|
||||
|
||||
when(() => assetMediaRepository.deleteAll(ids)).thenAnswer((_) async => <String>[]);
|
||||
|
||||
final result = await sut.deleteLocal(ids);
|
||||
|
||||
expect(result, 0);
|
||||
verify(() => assetMediaRepository.deleteAll(ids)).called(1);
|
||||
verifyNever(() => trashedLocalAssetRepository.applyTrashedAssets(any()));
|
||||
verifyNever(() => localAssetRepository.delete(any()));
|
||||
});
|
||||
});
|
||||
}
|
||||
Loading…
Reference in a new issue