fix(mobile): resolve owned assets when partner owns identical asset (#30137)

* fix(mobile): resolve owned assets when partner owns identical asset

* Add limit to auth user expressions

* Change debug method semantics
This commit is contained in:
Adam Gastineau 2026-07-31 07:37:12 -07:00 committed by GitHub
parent dd8d068d44
commit 483b375c26
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 359 additions and 22 deletions

View file

@ -36,8 +36,8 @@ class AssetService {
return _localRepository.getByChecksum(checksum);
}
Future<RemoteAsset?> getRemoteAssetByChecksum(String checksum) {
return _remoteRepository.getByChecksum(checksum);
Future<List<RemoteAsset>> getAllRemoteAssetDebugByChecksum(String checksum) {
return _remoteRepository.getAllDebugForChecksum(checksum);
}
Future<RemoteAsset?> getRemoteAsset(String id) {

View file

@ -24,13 +24,21 @@ class DriftLocalAssetRepository extends DriftDatabaseRepository {
const DriftLocalAssetRepository(this._db) : super(_db);
SingleOrNullSelectable<LocalAsset?> _assetSelectable(String id) {
final query = _db.localAssetEntity.select().addColumns([_db.remoteAssetEntity.id]).join([
leftOuterJoin(
_db.remoteAssetEntity,
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum),
useColumns: false,
),
])..where(_db.localAssetEntity.id.equals(id));
final query =
_db.localAssetEntity.select().addColumns([_db.remoteAssetEntity.id]).join([
leftOuterJoin(
_db.remoteAssetEntity,
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum) &
_db.remoteAssetEntity.ownerId.isInQuery(
_db.selectOnly(_db.authUserEntity)
..addColumns([_db.authUserEntity.id])
..limit(1),
),
useColumns: false,
),
])
..where(_db.localAssetEntity.id.equals(id))
..limit(1);
return query.map((row) {
final asset = row.readTable(_db.localAssetEntity).toDto();

View file

@ -59,10 +59,10 @@ class RemoteAssetRepository extends DriftDatabaseRepository {
return _assetSelectable(id).getSingleOrNull();
}
Future<RemoteAsset?> getByChecksum(String checksum) {
Future<List<RemoteAsset>> getAllDebugForChecksum(String checksum) {
final query = _db.remoteAssetEntity.select()..where((row) => row.checksum.equals(checksum));
return query.map((row) => row.toDto()).getSingleOrNull();
return query.map((row) => row.toDto()).get();
}
Future<List<RemoteAsset>> getStackChildren(RemoteAsset asset) {

View file

@ -139,11 +139,6 @@ class DriftTimelineRepository extends DriftDatabaseRepository {
_db.localAlbumAssetEntity.assetId.equalsExp(_db.localAssetEntity.id),
useColumns: false,
),
leftOuterJoin(
_db.remoteAssetEntity,
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum),
useColumns: false,
),
])
..addColumns([assetCountExp, dateExp])
..where(_db.localAlbumAssetEntity.albumId.equals(albumId))
@ -167,7 +162,12 @@ class DriftTimelineRepository extends DriftDatabaseRepository {
),
leftOuterJoin(
_db.remoteAssetEntity,
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum),
_db.localAssetEntity.checksum.equalsExp(_db.remoteAssetEntity.checksum) &
_db.remoteAssetEntity.ownerId.isInQuery(
_db.selectOnly(_db.authUserEntity)
..addColumns([_db.authUserEntity.id])
..limit(1),
),
useColumns: false,
),
])

View file

@ -16,6 +16,11 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
const DriftTrashedLocalAssetRepository(this._db) : super(_db);
/// Matches remote_asset_entity rows owned by the current user. The asset is unique over (owner, checksum),
/// so partners can have a duplicate checksum
Expression<bool> get _ownedByCurrentUser =>
_db.remoteAssetEntity.ownerId.isInQuery(_db.selectOnly(_db.authUserEntity)..addColumns([_db.authUserEntity.id]));
Future<void> updateHashes(Map<String, String> hashes) {
if (hashes.isEmpty) {
return Future.value();
@ -45,7 +50,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
await (_db.select(_db.trashedLocalAssetEntity).join([
innerJoin(
_db.remoteAssetEntity,
_db.remoteAssetEntity.checksum.equalsExp(_db.trashedLocalAssetEntity.checksum),
_db.remoteAssetEntity.checksum.equalsExp(_db.trashedLocalAssetEntity.checksum) & _ownedByCurrentUser,
),
])..where(
_db.trashedLocalAssetEntity.source.equalsValue(TrashOrigin.remoteSync) &
@ -273,7 +278,7 @@ class DriftTrashedLocalAssetRepository extends DriftDatabaseRepository {
innerJoin(_db.localAssetEntity, _db.localAlbumAssetEntity.assetId.equalsExp(_db.localAssetEntity.id)),
leftOuterJoin(
_db.remoteAssetEntity,
_db.remoteAssetEntity.checksum.equalsExp(_db.localAssetEntity.checksum),
_db.remoteAssetEntity.checksum.equalsExp(_db.localAssetEntity.checksum) & _ownedByCurrentUser,
),
])..where(
_db.localAlbumEntity.backupSelection.equalsValue(BackupSelection.selected) &

View file

@ -280,8 +280,8 @@ class _RemoteAssetSection extends ConsumerWidget {
return const SizedBox.shrink();
}
return FutureBuilder<RemoteAsset?>(
future: assetService.getRemoteAssetByChecksum(asset.checksum!),
return FutureBuilder<List<RemoteAsset>>(
future: assetService.getAllRemoteAssetDebugByChecksum(asset.checksum!),
builder: (context, snapshot) {
if (snapshot.connectionState == ConnectionState.waiting) {
return const _PropertySectionCard(
@ -297,7 +297,7 @@ class _RemoteAssetSection extends ConsumerWidget {
);
}
final remoteAsset = snapshot.data;
final remoteAsset = snapshot.data?.firstOrNull;
if (remoteAsset == null) {
return _PropertySectionCard(

View file

@ -19,6 +19,73 @@ void main() {
await ctx.dispose();
});
group('get', () {
late String userId;
setUp(() async {
final user = await ctx.newUser();
userId = user.id;
// Owner-scoped queries resolve the current user via authUserEntity.
await ctx.newAuthUser(id: userId);
});
test('allows the same checksum to exist for multiple owners (#29973)', () async {
const checksum = 'some-shared-checksum';
final mine = await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final partner = await ctx.newUser();
await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
final local = await ctx.newLocalAsset(checksum: checksum);
final result = await sut.get(local.id);
expect(result, isNotNull);
expect(result!.id, local.id);
// We must explicitly get OUR asset, not the partner's
expect(result.remoteId, mine.id);
});
test('reports local-only when only a partner has a remote copy (#29973)', () async {
// The current user has NOT uploaded this file; only a partner owns an identical-checksum remote asset
const checksum = 'partner-only';
final partner = await ctx.newUser();
await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
final local = await ctx.newLocalAsset(checksum: checksum);
final result = await sut.get(local.id);
expect(result, isNotNull);
expect(result!.remoteId, isNull);
expect(result.storage, AssetState.local);
});
test('allows the current user to have access to multiple remote rows for one checksum (#29973)', () async {
// A single user can have their own remote asset, a partner's remote asset, and a local asset all with the same checksum
const checksum = 'multi-library';
final partner = await ctx.newUser();
await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final local = await ctx.newLocalAsset(checksum: checksum);
final result = await sut.get(local.id);
expect(result, isNotNull);
expect(result!.id, local.id);
expect(result.remoteId, isNotNull);
});
test('attaches remoteId to local asset automatically in simple scenarios', () async {
const checksum = 'simple';
final remote = await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final local = await ctx.newLocalAsset(checksum: checksum);
final result = await sut.get(local.id);
expect(result, isNotNull);
expect(result!.remoteId, remote.id);
expect(result.storage, AssetState.merged);
});
});
group('getRemovalCandidates', () {
final cutoffDate = DateTime(2024, 1, 1);
final beforeCutoff = DateTime(2023, 12, 31);

View file

@ -0,0 +1,67 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart';
import '../repository_context.dart';
void main() {
late MediumRepositoryContext ctx;
late RemoteAssetRepository sut;
setUp(() {
ctx = MediumRepositoryContext();
sut = RemoteAssetRepository(ctx.db);
});
tearDown(() async {
await ctx.dispose();
});
group('getByChecksum', () {
late String userId;
setUp(() async {
final user = await ctx.newUser();
userId = user.id;
await ctx.newAuthUser(id: userId);
});
test('returns all assets when a partner shares the checksum', () async {
const checksum = 'shared-partner-checksum';
final mine = await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final partner = await ctx.newUser();
final theirs = await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
final result = await sut.getAllDebugForChecksum(checksum);
final mineResult = result.firstWhere((asset) => asset.id == mine.id);
final theirResult = result.firstWhere((asset) => asset.id == theirs.id);
expect(result, isNotEmpty);
expect(mineResult.id, mine.id);
expect(mineResult.ownerId, userId);
expect(theirResult.id, theirs.id);
expect(theirResult.ownerId, partner.id);
});
test('returns partner asset only if there is no matching user asset', () async {
const checksum = 'partner-only';
final partner = await ctx.newUser();
final theirs = await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
final result = await sut.getAllDebugForChecksum(checksum);
expect(result.length, 1);
expect(result[0].id, theirs.id);
});
test('returns the current user\'s asset', () async {
const checksum = 'simple';
final remote = await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final result = await sut.getAllDebugForChecksum(checksum);
expect(result.length, 1);
expect(result[0].id, remote.id);
});
});
}

View file

@ -102,4 +102,48 @@ void main() {
expect(remote.localId, local.id);
});
});
group('localAlbum assets', () {
late String userId;
late String otherUserId;
setUp(() async {
final user = await ctx.newUser();
userId = user.id;
await ctx.newAuthUser(id: userId);
final other = await ctx.newUser();
otherUserId = other.id;
});
test('does not duplicate assets when a partner shares the checksum', () async {
const checksum = 'shared-partner-checksum';
final album = await ctx.newLocalAlbum();
final local = await ctx.newLocalAsset(checksum: checksum);
await ctx.newLocalAlbumAsset(albumId: album.id, assetId: local.id);
final myRemote = await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
await ctx.newRemoteAsset(ownerId: otherUserId, checksum: checksum);
final assets = await sut.localAlbum(album.id, .day).assetSource(0, 10);
expect(assets, hasLength(1));
final asset = assets.single as LocalAsset;
expect(asset.id, local.id);
// Must resolve the current user's remote id
expect(asset.remoteId, myRemote.id);
});
test('bucket count ignores a partner sharing the checksum', () async {
const checksum = 'shared-partner-checksum';
final album = await ctx.newLocalAlbum();
final local = await ctx.newLocalAsset(checksum: checksum);
await ctx.newLocalAlbumAsset(albumId: album.id, assetId: local.id);
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
await ctx.newRemoteAsset(ownerId: otherUserId, checksum: checksum);
final buckets = await sut.localAlbum(album.id, .day).bucketSource().first;
expect(buckets, hasLength(1));
expect(buckets.single.assetCount, 1);
});
});
}

View file

@ -0,0 +1,100 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/models/album/local_album.model.dart';
import 'package:immich_mobile/infrastructure/repositories/trashed_local_asset.repository.dart';
import '../repository_context.dart';
void main() {
late MediumRepositoryContext ctx;
late DriftTrashedLocalAssetRepository sut;
setUp(() {
ctx = MediumRepositoryContext();
sut = DriftTrashedLocalAssetRepository(ctx.db);
});
tearDown(() async {
await ctx.dispose();
});
group('getToRestore', () {
late String userId;
setUp(() async {
final user = await ctx.newUser();
userId = user.id;
await ctx.newAuthUser(id: userId);
});
test('does not restore based on a partner\'s live remote copy', () async {
const checksum = 'shared-partner-checksum';
final album = await ctx.newLocalAlbum(backupSelection: BackupSelection.selected);
final trashed = await ctx.newTrashedLocalAsset(albumId: album.id, checksum: checksum);
// Current user's own remote copy is deleted; only a partner has an active copy.
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum, deletedAt: DateTime(2020, 1, 1));
final partner = await ctx.newUser();
await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum);
final result = await sut.getToRestore();
final ids = result.map((a) => a.id);
expect(ids, isNot(contains(trashed.id)));
});
test('restores when the current user\'s own remote copy is live', () async {
const checksum = 'my-live-copy';
final album = await ctx.newLocalAlbum(backupSelection: BackupSelection.selected);
final trashed = await ctx.newTrashedLocalAsset(albumId: album.id, checksum: checksum);
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final result = await sut.getToRestore();
final ids = result.map((a) => a.id);
expect(ids, contains(trashed.id));
});
});
group('getToTrash', () {
late String userId;
setUp(() async {
final user = await ctx.newUser();
userId = user.id;
await ctx.newAuthUser(id: userId);
});
Future<String> addLocalAssetToBackupAlbum(String checksum) async {
final album = await ctx.newLocalAlbum(backupSelection: BackupSelection.selected);
final local = await ctx.newLocalAsset(checksum: checksum);
await ctx.newLocalAlbumAsset(albumId: album.id, assetId: local.id);
return local.id;
}
test('does not trash when only a partner\'s remote copy is deleted', () async {
const checksum = 'shared-partner-checksum';
final localId = await addLocalAssetToBackupAlbum(checksum);
// Current user's own remote copy is live but a partner deleted theirs
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum);
final partner = await ctx.newUser();
await ctx.newRemoteAsset(ownerId: partner.id, checksum: checksum, deletedAt: DateTime(2020, 1, 1));
final result = await sut.getToTrash();
final ids = result.values.expand((assets) => assets).map((a) => a.id);
expect(ids, isNot(contains(localId)));
});
test('trashes when the current user\'s own remote copy is deleted', () async {
const checksum = 'my-deleted-copy';
final localId = await addLocalAssetToBackupAlbum(checksum);
await ctx.newRemoteAsset(ownerId: userId, checksum: checksum, deletedAt: DateTime(2020, 1, 1));
final result = await sut.getToTrash();
final ids = result.values.expand((assets) => assets).map((a) => a.id);
expect(ids, contains(localId));
});
});
}

View file

@ -6,6 +6,7 @@ import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
import 'package:immich_mobile/domain/models/memory.model.dart';
import 'package:immich_mobile/domain/models/user.model.dart';
import 'package:immich_mobile/infrastructure/entities/asset_face.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/auth_user.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/local_album.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/local_album_asset.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/local_asset.entity.drift.dart';
@ -18,6 +19,8 @@ import 'package:immich_mobile/infrastructure/entities/remote_album_asset.entity.
import 'package:immich_mobile/infrastructure/entities/remote_album_user.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/remote_asset.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/remote_asset_cloud_id.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/trashed_local_asset.entity.dart';
import 'package:immich_mobile/infrastructure/entities/trashed_local_asset.entity.drift.dart';
import 'package:immich_mobile/infrastructure/entities/user.entity.drift.dart';
import 'package:immich_mobile/infrastructure/repositories/db.repository.dart';
import 'package:immich_mobile/utils/option.dart';
@ -72,6 +75,22 @@ class MediumRepositoryContext {
);
}
/// Seeds a user into `authUserEntity` as the currently authenticated user
Future<AuthUserEntityData> newAuthUser({String? id, String? email, bool? isAdmin, AvatarColor? avatarColor}) async {
id ??= TestUtils.uuid();
return db
.into(db.authUserEntity)
.insertReturning(
AuthUserEntityCompanion(
id: .new(id),
email: .new(email ?? '$id@test.com'),
name: .new('user_$id'),
isAdmin: .new(isAdmin ?? false),
avatarColor: .new(avatarColor ?? TestUtils.randElement(AvatarColor.values)),
),
);
}
Future<void> newPartner({required String sharedById, required String sharedWithId, bool? inTimeline}) {
return db
.into(db.partnerEntity)
@ -286,6 +305,33 @@ class MediumRepositoryContext {
);
}
/// Seeds a trashed local asset into `trashedLocalAssetEntity`
Future<TrashedLocalAssetEntityData> newTrashedLocalAsset({
String? id,
required String albumId,
String? checksum,
TrashOrigin? source,
AssetType? type,
DateTime? createdAt,
bool? isFavorite,
}) async {
id ??= TestUtils.uuid();
return db
.into(db.trashedLocalAssetEntity)
.insertReturning(
TrashedLocalAssetEntityCompanion(
id: .new(id),
albumId: .new(albumId),
name: .new('trashed_$id.jpg'),
type: .new(type ?? .image),
checksum: .new(checksum),
source: .new(source ?? TrashOrigin.remoteSync),
isFavorite: .new(isFavorite ?? false),
createdAt: .new(TestUtils.date(createdAt)),
),
);
}
Future<LocalAlbumEntityData> newLocalAlbum({
String? id,
String? name,