From 5e06136b8a7fd8bf00cc6514b823778ac7b991a4 Mon Sep 17 00:00:00 2001 From: Ming Ming Date: Wed, 14 Dec 2022 00:41:46 +0800 Subject: [PATCH] Fix moved files are removed from DB in some case --- app/lib/entity/file/file_cache_manager.dart | 45 ++++++--- .../entity/file/file_cache_manager_test.dart | 97 +++++++++++++++++++ 2 files changed, 131 insertions(+), 11 deletions(-) diff --git a/app/lib/entity/file/file_cache_manager.dart b/app/lib/entity/file/file_cache_manager.dart index 44a829fb..c9871bc3 100644 --- a/app/lib/entity/file/file_cache_manager.dart +++ b/app/lib/entity/file/file_cache_manager.dart @@ -129,14 +129,12 @@ class FileSqliteCacheUpdater { throw StateError("Row ID for dir is null"); } - final dirChildRowIdQuery = db.selectOnly(db.dirFiles) - ..addColumns([db.dirFiles.child]) - ..where(db.dirFiles.dir.equals(_dirRowId)) - ..orderBy([sql.OrderingTerm.asc(db.dirFiles.rowId)]); - final dirChildRowIds = - await dirChildRowIdQuery.map((r) => r.read(db.dirFiles.child)!).get(); - final diff = list_util.diff( - dirChildRowIds, _childRowIds.sorted(Comparable.compare)); + final dirFileQuery = db.select(db.dirFiles) + ..where((t) => t.dir.equals(_dirRowId)) + ..orderBy([(t) => sql.OrderingTerm.asc(t.rowId)]); + final dirFiles = await dirFileQuery.get(); + final diff = list_util.diff(dirFiles.map((e) => e.child), + _childRowIds.sorted(Comparable.compare)); if (diff.item1.isNotEmpty) { await db.batch((batch) { // insert new children @@ -145,9 +143,34 @@ class FileSqliteCacheUpdater { }); } if (diff.item2.isNotEmpty) { - // delete obsolete children - await _removeSqliteFiles(db, dbAccount, diff.item2); - await db.cleanUpDanglingFiles(); + // remove entries from the DirFiles table first + await diff.item2.withPartitionNoReturn((sublist) async { + final deleteQuery = db.delete(db.dirFiles) + ..where((t) => t.child.isIn(sublist)) + ..where((t) => + t.dir.equals(_dirRowId) | t.dir.equalsExp(db.dirFiles.child)); + await deleteQuery.go(); + }, sql.maxByFileIdsSize); + + // select files having another dir parent under this account (i.e., + // moved files) + final moved = await diff.item2.withPartition((sublist) async { + final query = db.selectOnly(db.dirFiles).join([ + sql.innerJoin(db.accountFiles, + db.accountFiles.file.equalsExp(db.dirFiles.dir)), + ]); + query + ..addColumns([db.dirFiles.child]) + ..where(db.accountFiles.account.equals(dbAccount.rowId)) + ..where(db.dirFiles.child.isIn(sublist)); + return query.map((r) => r.read(db.dirFiles.child)!).get(); + }, sql.maxByFileIdsSize); + final removed = diff.item2.where((e) => !moved.contains(e)).toList(); + if (removed.isNotEmpty) { + // delete obsolete children + await _removeSqliteFiles(db, dbAccount, removed); + await db.cleanUpDanglingFiles(); + } } }); } diff --git a/app/test/entity/file/file_cache_manager_test.dart b/app/test/entity/file/file_cache_manager_test.dart index 01e18aef..a85801dc 100644 --- a/app/test/entity/file/file_cache_manager_test.dart +++ b/app/test/entity/file/file_cache_manager_test.dart @@ -33,6 +33,9 @@ void main() { test("too many files", _updaterTooManyFiles, timeout: const Timeout(Duration(minutes: 2)), skip: "too slow on gitlab"); + test("moved file (to dir in front of the from dir)", + _updaterMovedFileToFront); + test("moved file (to dir behind the from dir)", _updaterMovedFileToBehind); }); test("FileSqliteCacheEmptier", _emptier); } @@ -541,6 +544,100 @@ Future _updaterTooManyFiles() async { // expect here } +/// Moved a file from test2 to test1, where test2 is sorted behind test1 +/// +/// Expect: file moved +Future _updaterMovedFileToFront() async { + final account = util.buildAccount(); + final files = (util.FilesBuilder() + ..addDir("admin") + ..addDir("admin/test1") + ..addDir("admin/test2") + ..addJpeg("admin/test2/test1.jpg")) + .build(); + final c = DiContainer( + sqliteDb: util.buildTestDb(), + ); + addTearDown(() => c.sqliteDb.close()); + await c.sqliteDb.transaction(() async { + await c.sqliteDb.insertAccountOf(account); + await util.insertFiles(c.sqliteDb, account, files); + await util.insertDirRelation( + c.sqliteDb, account, files[0], files.slice(1, 3)); + await util.insertDirRelation(c.sqliteDb, account, files[1], []); + await util.insertDirRelation(c.sqliteDb, account, files[2], [files[3]]); + }); + + final movedFile = files[3].copyWith( + path: "remote.php/dav/files/admin/test1/test1.jpg", + ); + await FileSqliteCacheUpdater(c)( + account, + files[1], + remote: [files[1], movedFile], + ); + await FileSqliteCacheUpdater(c)( + account, + files[2], + remote: [files[2]], + ); + expect( + await util.listSqliteDbFiles(c.sqliteDb), + {...files.slice(0, 3), movedFile}, + ); + final dirResult = await util.listSqliteDbDirs(c.sqliteDb); + expect(dirResult[files[0]], {...files.slice(0, 3)}); + expect(dirResult[files[1]], {files[1], movedFile}); + expect(dirResult[files[2]], {files[2]}); +} + +/// Moved a file from test1 to test2, where test1 is sorted in front of test2 +/// +/// Expect: file moved +Future _updaterMovedFileToBehind() async { + final account = util.buildAccount(); + final files = (util.FilesBuilder() + ..addDir("admin") + ..addDir("admin/test1") + ..addDir("admin/test2") + ..addJpeg("admin/test1/test1.jpg")) + .build(); + final c = DiContainer( + sqliteDb: util.buildTestDb(), + ); + addTearDown(() => c.sqliteDb.close()); + await c.sqliteDb.transaction(() async { + await c.sqliteDb.insertAccountOf(account); + await util.insertFiles(c.sqliteDb, account, files); + await util.insertDirRelation( + c.sqliteDb, account, files[0], files.slice(1, 3)); + await util.insertDirRelation(c.sqliteDb, account, files[1], [files[3]]); + await util.insertDirRelation(c.sqliteDb, account, files[2], []); + }); + + final movedFile = files[3].copyWith( + path: "remote.php/dav/files/admin/test2/test1.jpg", + ); + await FileSqliteCacheUpdater(c)( + account, + files[1], + remote: [files[1]], + ); + await FileSqliteCacheUpdater(c)( + account, + files[2], + remote: [files[2], movedFile], + ); + expect( + await util.listSqliteDbFiles(c.sqliteDb), + {...files.slice(0, 3), movedFile}, + ); + final dirResult = await util.listSqliteDbDirs(c.sqliteDb); + expect(dirResult[files[0]], {...files.slice(0, 3)}); + expect(dirResult[files[1]], {files[1]}); + expect(dirResult[files[2]], {files[2], movedFile}); +} + /// Empty dir in cache /// /// Expect: dir removed from DirFiles table;