Merge pull request #458 from ente-io/strict_remote_to_local_match

Rewrite storeDiff logic to fix matching bugs
This commit is contained in:
Manav 2022-09-04 18:51:25 +05:30 committed by GitHub
commit 85b6d8db12
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 191 additions and 96 deletions

View file

@ -1,5 +1,6 @@
import 'dart:io';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';
import 'package:path/path.dart';
import 'package:path_provider/path_provider.dart';
@ -81,6 +82,7 @@ class FilesDB {
initializationScript: initializationScript,
migrationScripts: migrationScripts,
);
// make this a singleton class
FilesDB._privateConstructor();
@ -829,7 +831,54 @@ class FilesDB {
);
}
/*
This method should only return localIDs which are not uploaded yet
and can be mapped to incoming remote entry
*/
Future<List<File>> getUnlinkedLocalMatchesForRemoteFile(
int ownerID,
String localID,
FileType fileType, {
@required String title,
@required String deviceFolder,
}) async {
final db = await instance.database;
// on iOS, match using localID and fileType. title can either match or
// might be null based on how the file was imported
String whereClause = ''' ($columnOwnerID = ? OR $columnOwnerID IS NULL) AND
$columnLocalID = ? AND $columnFileType = ? AND
($columnTitle=? OR $columnTitle IS NULL) ''';
List<Object> whereArgs = [
ownerID,
localID,
getInt(fileType),
title,
];
if (Platform.isAndroid) {
whereClause = ''' ($columnOwnerID = ? OR $columnOwnerID IS NULL) AND
$columnLocalID = ? AND $columnFileType = ? AND $columnTitle=? AND $columnDeviceFolder= ?
''';
whereArgs = [
ownerID,
localID,
getInt(fileType),
title,
deviceFolder,
];
}
final rows = await db.query(
table,
where: whereClause,
whereArgs: whereArgs,
);
return _convertToFiles(rows);
}
Future<List<File>> getMatchingFiles(
String localID,
FileType fileType,
String title,
String deviceFolder,
) async {

View file

@ -6,6 +6,7 @@ import 'package:logging/logging.dart';
import 'package:photos/core/configuration.dart';
import 'package:photos/core/errors.dart';
import 'package:photos/core/event_bus.dart';
import 'package:photos/db/file_updation_db.dart';
import 'package:photos/db/files_db.dart';
import 'package:photos/events/collection_updated_event.dart';
import 'package:photos/events/files_updated_event.dart';
@ -364,123 +365,168 @@ class RemoteSyncService {
);
}
/* _storeDiff maps each remoteDiff file to existing
entries in files table. When match is found, it compares both file to
perform relevant actions like
[1] Clear local cache when required (Both Shared and Owned files)
[2] Retain localID of remote file based on matching logic [Owned files]
[3] Refresh UI if visibility or creationTime has changed [Owned files]
[4] Schedule file update if the local file has changed since last time
[Owned files]
[Important Note: If given uploadedFileID and collectionID is already present
in files db, the generateID should already point to existing entry.
Known Issues:
[K1] Cached entry will not be cleared when if a file was edited and
moved to different collection as Vid/Image cache key is uploadedID.
[Existing]
]
*/
Future _storeDiff(List<File> diff, int collectionID) async {
int existing = 0,
updated = 0,
remote = 0,
localButUpdatedOnRemote = 0,
localButAddedToNewCollectionOnRemote = 0;
bool hasAnyCreationTimeChanged = false;
final List<File> toBeInserted = [];
int sharedFileNew = 0,
sharedFileUpdated = 0,
localUploadedFromDevice = 0,
localButUpdatedOnDevice = 0,
remoteNewFile = 0;
final int userID = Configuration.instance.getUserID();
for (File file in diff) {
final existingFiles = file.deviceFolder == null
? null
: await _db.getMatchingFiles(file.title, file.deviceFolder);
if (existingFiles == null ||
existingFiles.isEmpty ||
userID != file.ownerID) {
// File uploaded from a different device or uploaded by different user
// Other rare possibilities : The local file is present on
// device but it's not imported in local db due to missing permission
// after reinstall (iOS selected file permissions or user revoking
// permissions, or issue/delay in importing devices files.
file.localID = null;
toBeInserted.add(file);
remote++;
} else {
// File exists in ente db with same title & device folder
// Note: The file.generatedID might be already set inside
// [DiffFetcher.getEncryptedFilesDiff]
// Try to find existing file with same localID as remote file with a fallback
// to finding any existing file with localID. This is needed to handle
// case when localID for a file changes and the file is uploaded again in
// the same collection
final fileWithLocalID = existingFiles.firstWhere(
(e) =>
file.localID != null &&
e.localID != null &&
e.localID == file.localID,
orElse: () => existingFiles.firstWhere(
(e) => e.localID != null,
orElse: () => null,
),
);
if (fileWithLocalID != null) {
// File should ideally have the same localID
if (file.localID != null && file.localID != fileWithLocalID.localID) {
_logger.severe(
"unexpected mismatch in localIDs remote: ${file.toString()} and existing: ${fileWithLocalID.toString()}",
);
}
file.localID = fileWithLocalID.localID;
} else {
file.localID = null;
bool needsGalleryReload = false;
// this is required when same file is uploaded twice in the same
// collection. Without this check, if both remote files are part of same
// diff response, then we end up inserting one entry instead of two
// as we update the generatedID for remoteDiff to local file's genID
final Set<int> alreadyClaimedLocalFilesGenID = {};
final List<File> toBeInserted = [];
for (File remoteDiff in diff) {
// existingFile will be either set to existing collectionID+localID or
// to the unclaimed aka not already linked to any uploaded file.
File existingFile;
if (remoteDiff.generatedID != null) {
// Case [1] Check and clear local cache when uploadedFile already exist
existingFile = await _db.getFile(remoteDiff.generatedID);
if (_shouldClearCache(remoteDiff, existingFile)) {
await clearCache(remoteDiff);
}
final bool wasUploadedOnAPreviousInstallation =
existingFiles.length == 1 && existingFiles[0].collectionID == null;
if (wasUploadedOnAPreviousInstallation) {
file.generatedID = existingFiles[0].generatedID;
if (file.modificationTime != existingFiles[0].modificationTime) {
// File was updated since the app was uninstalled
// mark it for re-upload
_logger.info(
"re-upload because file was updated since last installation: "
"remoteFile: ${file.toString()}, localFile: ${existingFiles[0].toString()}",
);
file.modificationTime = existingFiles[0].modificationTime;
file.updationTime = null;
updated++;
} else {
existing++;
}
toBeInserted.add(file);
}
/* If file is not owned by the user, no further processing is required
as Case [2,3,4] are only relevant to files owned by user
*/
if (userID != remoteDiff.ownerID) {
if (existingFile == null) {
sharedFileNew++;
remoteDiff.localID = null;
} else {
bool foundMatchingCollection = false;
for (final existingFile in existingFiles) {
if (file.collectionID == existingFile.collectionID &&
file.uploadedFileID == existingFile.uploadedFileID) {
// File was updated on remote
if (file.creationTime != existingFile.creationTime) {
hasAnyCreationTimeChanged = true;
}
foundMatchingCollection = true;
file.generatedID = existingFile.generatedID;
toBeInserted.add(file);
await clearCache(file);
localButUpdatedOnRemote++;
break;
}
sharedFileUpdated++;
// if user has downloaded the file on the device, avoid removing the
// localID reference.
// [Todo-fix: Excluded shared file's localIDs during syncALL]
remoteDiff.localID = existingFile.localID;
}
toBeInserted.add(remoteDiff);
// end processing for file here, move to next file now
break;
}
// If remoteDiff is not already synced (i.e. existingFile is null), check
// if the remoteFile was uploaded from this device.
// Note: DeviceFolder is ignored for iOS during matching
if (existingFile == null && remoteDiff.localID != null) {
final localFileEntries = await _db.getUnlinkedLocalMatchesForRemoteFile(
userID,
remoteDiff.localID,
remoteDiff.fileType,
title: remoteDiff.title,
deviceFolder: remoteDiff.deviceFolder,
);
if (localFileEntries.isEmpty) {
// set remote file's localID as null because corresponding local file
// does not exist [Case 2, do not retain localID of the remote file]
remoteDiff.localID = null;
} else {
// case 4: Check and schedule the file for update
final int maxModificationTime = localFileEntries
.map(
(e) => e.modificationTime ?? 0,
)
.reduce(max);
if (maxModificationTime > remoteDiff.modificationTime) {
localButUpdatedOnDevice++;
await FileUpdationDB.instance.insertMultiple(
[remoteDiff.localID],
FileUpdationDB.modificationTimeUpdated,
);
}
if (!foundMatchingCollection) {
// Added to a new collection
toBeInserted.add(file);
localButAddedToNewCollectionOnRemote++;
localFileEntries.removeWhere(
(e) =>
e.uploadedFileID != null ||
alreadyClaimedLocalFilesGenID.contains(e.generatedID),
);
if (localFileEntries.isNotEmpty) {
// file uploaded from same device, replace the local file row by
// setting the generated ID of remoteFile to localFile generatedID
existingFile = localFileEntries.first;
localUploadedFromDevice++;
alreadyClaimedLocalFilesGenID.add(existingFile.generatedID);
remoteDiff.generatedID = existingFile.generatedID;
}
}
}
if (existingFile != null &&
_shouldReloadHomeGallery(remoteDiff, existingFile)) {
needsGalleryReload = true;
} else {
remoteNewFile++;
}
toBeInserted.add(remoteDiff);
}
await _db.insertMultiple(toBeInserted);
_logger.info(
"Diff to be deduplicated was: " +
diff.length.toString() +
" out of which \n" +
existing.toString() +
localUploadedFromDevice.toString() +
" was uploaded from device, \n" +
updated.toString() +
localButUpdatedOnDevice.toString() +
" was uploaded from device, but has been updated since and should be reuploaded, \n" +
remote.toString() +
" was uploaded from remote, \n" +
localButUpdatedOnRemote.toString() +
" was uploaded from device but updated on remote, and \n" +
localButAddedToNewCollectionOnRemote.toString() +
" was uploaded from device but added to a new collection on remote.",
sharedFileNew.toString() +
" new sharedFiles, \n" +
sharedFileUpdated.toString() +
" updatedSharedFiles, and \n" +
remoteNewFile.toString() +
" remoteFiles seen first time",
);
if (hasAnyCreationTimeChanged) {
if (needsGalleryReload) {
Bus.instance.fire(ForceReloadHomeGalleryEvent());
}
}
bool _shouldClearCache(File remoteFile, File existingFile) {
if (remoteFile.hash != null && existingFile.hash != null) {
return remoteFile.hash != existingFile.hash;
}
return remoteFile.updationTime != (existingFile.updationTime ?? 0);
}
bool _shouldReloadHomeGallery(File remoteFile, File existingFile) {
int remoteCreationTime = remoteFile.creationTime;
if (remoteFile.pubMmdVersion > 0 &&
(remoteFile.pubMagicMetadata.editedTime ?? 0) != 0) {
remoteCreationTime = remoteFile.pubMagicMetadata.editedTime;
}
if (remoteCreationTime != existingFile.creationTime) {
return true;
}
if (existingFile.mMdVersion > 0 &&
remoteFile.mMdVersion != existingFile.mMdVersion &&
remoteFile.magicMetadata.visibility !=
existingFile.magicMetadata.visibility) {
return false;
}
return false;
}
// return true if the client needs to re-sync the collections from previous
// version
bool _hasReSynced() {