diff --git a/app/lib/entity/file/file_cache_manager.dart b/app/lib/entity/file/file_cache_manager.dart index 5ed68a97..e27b236e 100644 --- a/app/lib/entity/file/file_cache_manager.dart +++ b/app/lib/entity/file/file_cache_manager.dart @@ -10,6 +10,7 @@ import 'package:nc_photos/entity/sqlite_table.dart' as sql; import 'package:nc_photos/entity/sqlite_table_converter.dart'; import 'package:nc_photos/entity/sqlite_table_extension.dart' as sql; import 'package:nc_photos/exception.dart'; +import 'package:nc_photos/iterable_extension.dart'; import 'package:nc_photos/list_util.dart' as list_util; import 'package:nc_photos/object_extension.dart'; import 'package:nc_photos/remote_storage_util.dart' as remote_storage_util; @@ -249,20 +250,25 @@ class FileSqliteCacheUpdater { List sqlFiles, File? dir) async { _log.info("[_insertCache] Insert ${sqlFiles.length} files"); // check if the files exist in the db in other accounts - final query = db.queryFiles().run((q) { - q - ..setQueryMode( - sql.FilesQueryMode.expression, - expressions: [db.files.rowId, db.files.fileId], - ) - ..setAccountless() - ..byServerRowId(dbAccount.server) - ..byFileIds(sqlFiles.map((f) => f.file.fileId.value)); - return q.build(); - }); - final fileRowIdMap = Map.fromEntries(await query - .map((r) => MapEntry(r.read(db.files.fileId)!, r.read(db.files.rowId)!)) - .get()); + final entries = + await sqlFiles.map((f) => f.file.fileId.value).withPartition((sublist) { + final query = db.queryFiles().run((q) { + q + ..setQueryMode( + sql.FilesQueryMode.expression, + expressions: [db.files.rowId, db.files.fileId], + ) + ..setAccountless() + ..byServerRowId(dbAccount.server) + ..byFileIds(sublist); + return q.build(); + }); + return query + .map((r) => + MapEntry(r.read(db.files.fileId)!, r.read(db.files.rowId)!)) + .get(); + }, sql.maxByFileIdsSize); + final fileRowIdMap = Map.fromEntries(entries); await Future.wait(sqlFiles.map((f) async { var rowId = fileRowIdMap[f.file.fileId.value]; @@ -376,19 +382,22 @@ class FileSqliteCacheEmptier { Future _removeSqliteFiles( sql.SqliteDb db, sql.Account dbAccount, List fileRowIds) async { // query list of children, in case some of the files are dirs - final childQuery = db.selectOnly(db.dirFiles) - ..addColumns([db.dirFiles.child]) - ..where(db.dirFiles.dir.isIn(fileRowIds)); - final childRowIds = - await childQuery.map((r) => r.read(db.dirFiles.child)!).get(); + final childRowIds = await fileRowIds.withPartition((sublist) { + final childQuery = db.selectOnly(db.dirFiles) + ..addColumns([db.dirFiles.child]) + ..where(db.dirFiles.dir.isIn(sublist)); + return childQuery.map((r) => r.read(db.dirFiles.child)!).get(); + }, sql.maxByFileIdsSize); childRowIds.removeWhere((id) => fileRowIds.contains(id)); // remove the files in AccountFiles table. We are not removing in Files table // because a file could be associated with multiple accounts - await (db.delete(db.accountFiles) - ..where( - (t) => t.account.equals(dbAccount.rowId) & t.file.isIn(fileRowIds))) - .go(); + await fileRowIds.withPartitionNoReturn((sublist) async { + await (db.delete(db.accountFiles) + ..where( + (t) => t.account.equals(dbAccount.rowId) & t.file.isIn(sublist))) + .go(); + }, sql.maxByFileIdsSize); if (childRowIds.isNotEmpty) { // remove children recursively diff --git a/app/lib/entity/sqlite_table_extension.dart b/app/lib/entity/sqlite_table_extension.dart index 92c6608f..016d4f18 100644 --- a/app/lib/entity/sqlite_table_extension.dart +++ b/app/lib/entity/sqlite_table_extension.dart @@ -13,6 +13,8 @@ import 'package:nc_photos/mobile/platform.dart' import 'package:nc_photos/object_extension.dart'; import 'package:nc_photos/platform/k.dart' as platform_k; +const maxByFileIdsSize = 30000; + class CompleteFile { const CompleteFile(this.file, this.accountFile, this.image, this.trash); @@ -193,7 +195,9 @@ extension SqliteDbExtension on SqliteDb { final fileRowIds = await query.map((r) => r.read(files.rowId)!).get(); if (fileRowIds.isNotEmpty) { _log.info("[cleanUpDanglingFiles] Delete ${fileRowIds.length} files"); - await (delete(files)..where((t) => t.rowId.isIn(fileRowIds))).go(); + await fileRowIds.withPartitionNoReturn((sublist) async { + await (delete(files)..where((t) => t.rowId.isIn(sublist))).go(); + }, maxByFileIdsSize); } } @@ -280,29 +284,31 @@ extension SqliteDbExtension on SqliteDb { app.Account? appAccount, }) { assert((sqlAccount != null) != (appAccount != null)); - final query = queryFiles().run((q) { - q.setQueryMode(FilesQueryMode.expression, expressions: [ - accountFiles.rowId, - accountFiles.account, - accountFiles.file, - files.fileId, - ]); - if (sqlAccount != null) { - q.setSqlAccount(sqlAccount); - } else { - q.setAppAccount(appAccount!); - } - q.byFileIds(fileIds); - return q.build(); - }); - return query - .map((r) => AccountFileRowIdsWithFileId( - r.read(accountFiles.rowId)!, - r.read(accountFiles.account)!, - r.read(accountFiles.file)!, - r.read(files.fileId)!, - )) - .get(); + return fileIds.withPartition((sublist) { + final query = queryFiles().run((q) { + q.setQueryMode(FilesQueryMode.expression, expressions: [ + accountFiles.rowId, + accountFiles.account, + accountFiles.file, + files.fileId, + ]); + if (sqlAccount != null) { + q.setSqlAccount(sqlAccount); + } else { + q.setAppAccount(appAccount!); + } + q.byFileIds(sublist); + return q.build(); + }); + return query + .map((r) => AccountFileRowIdsWithFileId( + r.read(accountFiles.rowId)!, + r.read(accountFiles.account)!, + r.read(accountFiles.file)!, + r.read(files.fileId)!, + )) + .get(); + }, maxByFileIdsSize); } /// Query CompleteFile by fileId @@ -314,24 +320,26 @@ extension SqliteDbExtension on SqliteDb { app.Account? appAccount, }) { assert((sqlAccount != null) != (appAccount != null)); - final query = queryFiles().run((q) { - q.setQueryMode(FilesQueryMode.completeFile); - if (sqlAccount != null) { - q.setSqlAccount(sqlAccount); - } else { - q.setAppAccount(appAccount!); - } - q.byFileIds(fileIds); - return q.build(); - }); - return query - .map((r) => CompleteFile( - r.readTable(files), - r.readTable(accountFiles), - r.readTableOrNull(images), - r.readTableOrNull(trashes), - )) - .get(); + return fileIds.withPartition((sublist) { + final query = queryFiles().run((q) { + q.setQueryMode(FilesQueryMode.completeFile); + if (sqlAccount != null) { + q.setSqlAccount(sqlAccount); + } else { + q.setAppAccount(appAccount!); + } + q.byFileIds(sublist); + return q.build(); + }); + return query + .map((r) => CompleteFile( + r.readTable(files), + r.readTable(accountFiles), + r.readTableOrNull(images), + r.readTableOrNull(trashes), + )) + .get(); + }, maxByFileIdsSize); } Future> completeFilesByDirRowId( diff --git a/app/lib/iterable_extension.dart b/app/lib/iterable_extension.dart index 81f9cef5..174b4b74 100644 --- a/app/lib/iterable_extension.dart +++ b/app/lib/iterable_extension.dart @@ -4,6 +4,7 @@ import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:nc_photos/list_extension.dart'; import 'package:nc_photos/override_comparator.dart'; +import 'package:quiver/iterables.dart'; import 'package:tuple/tuple.dart'; extension IterableExtension on Iterable { @@ -97,6 +98,24 @@ extension IterableExtension on Iterable { } return -1; } + + Future> withPartition( + FutureOr> Function(Iterable sublist) fn, int size) async { + final products = []; + final sublists = partition(this, size); + for (final l in sublists) { + products.addAll(await fn(l)); + } + return products; + } + + Future withPartitionNoReturn( + FutureOr Function(Iterable sublist) fn, int size) async { + final sublists = partition(this, size); + for (final l in sublists) { + await fn(l); + } + } } extension IterableFlattenExtension on Iterable> { diff --git a/app/lib/use_case/cache_favorite.dart b/app/lib/use_case/cache_favorite.dart index 96d5355e..e50808e5 100644 --- a/app/lib/use_case/cache_favorite.dart +++ b/app/lib/use_case/cache_favorite.dart @@ -40,21 +40,38 @@ class CacheFavorite { if (newFileIds.isNotEmpty) { final rowIds = await db.accountFileRowIdsByFileIds(newFileIds, sqlAccount: dbAccount); - final count = await (db.update(db.accountFiles) - ..where( - (t) => t.rowId.isIn(rowIds.map((id) => id.accountFileRowId)))) - .write( - const sql.AccountFilesCompanion(isFavorite: sql.Value(true))); + final counts = + await rowIds.map((id) => id.accountFileRowId).withPartition( + (sublist) async { + return [ + await (db.update(db.accountFiles) + ..where((t) => t.rowId.isIn(sublist))) + .write(const sql.AccountFilesCompanion( + isFavorite: sql.Value(true))), + ]; + }, + sql.maxByFileIdsSize, + ); + final count = counts.sum; _log.info("[call] Updated $count row (new)"); updateCount += count; } if (removedFildIds.isNotEmpty) { - final count = await (db.update(db.accountFiles) - ..where((t) => - t.account.equals(dbAccount.rowId) & - t.file.isIn(removedFildIds.map((id) => cacheMap[id])))) - .write( - const sql.AccountFilesCompanion(isFavorite: sql.Value(false))); + final counts = + await removedFildIds.map((id) => cacheMap[id]).withPartition( + (sublist) async { + return [ + await (db.update(db.accountFiles) + ..where((t) => + t.account.equals(dbAccount.rowId) & + t.file.isIn(sublist))) + .write(const sql.AccountFilesCompanion( + isFavorite: sql.Value(false))) + ]; + }, + sql.maxByFileIdsSize, + ); + final count = counts.sum; _log.info("[call] Updated $count row (remove)"); updateCount += count; } diff --git a/app/test/entity/file/file_cache_manager_test.dart b/app/test/entity/file/file_cache_manager_test.dart index 0c8ad823..5d78c2e1 100644 --- a/app/test/entity/file/file_cache_manager_test.dart +++ b/app/test/entity/file/file_cache_manager_test.dart @@ -3,6 +3,7 @@ import 'package:nc_photos/entity/file.dart'; import 'package:nc_photos/entity/file/data_source.dart'; import 'package:nc_photos/entity/file/file_cache_manager.dart'; import 'package:nc_photos/entity/sqlite_table_extension.dart' as sql; +import 'package:nc_photos/int_extension.dart'; import 'package:nc_photos/list_extension.dart'; import 'package:nc_photos/or_null.dart'; import 'package:test/test.dart'; @@ -29,6 +30,8 @@ void main() { test("new shared dir", _updaterNewSharedDir); test("delete shared file", _updaterDeleteSharedFile); test("delete shared dir", _updaterDeleteSharedDir); + test("too many files", _updaterTooManyFiles, + timeout: const Timeout(Duration(minutes: 2))); }); test("FileSqliteCacheEmptier", _emptier); } @@ -502,6 +505,41 @@ Future _updaterDeleteSharedDir() async { ); } +/// Too many SQL variables +/// +/// Expect: no error +Future _updaterTooManyFiles() async { + final account = util.buildAccount(); + final files = (util.FilesBuilder() + ..addDir("admin") + ..addJpeg("admin/test1.jpg") + ..addDir("admin/testMany") + ..addJpeg("admin/testMany/testtest.jpg")) + .build(); + final newFilesBuilder = util.FilesBuilder(initialFileId: files.length); + // 250000 is the SQLITE_MAX_VARIABLE_NUMBER used in debian + for (final i in 0.until(250000)) { + newFilesBuilder.addJpeg("admin/testMany/test$i.jpg"); + } + final newFiles = newFilesBuilder.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[2], files.slice(3)); + }); + + final updater = FileSqliteCacheUpdater(c); + await updater(account, files[2], remote: [...files.slice(2), ...newFiles]); + // we are testing to make sure the above function won't throw, so nothing to + // expect here +} + /// Empty dir in cache /// /// Expect: dir removed from DirFiles table; diff --git a/app/test/iterable_extension_test.dart b/app/test/iterable_extension_test.dart index aef52bb8..e7279884 100644 --- a/app/test/iterable_extension_test.dart +++ b/app/test/iterable_extension_test.dart @@ -1,3 +1,4 @@ +import 'package:nc_photos/int_extension.dart'; import 'package:nc_photos/iterable_extension.dart'; import 'package:quiver/core.dart'; import 'package:test/test.dart'; @@ -90,6 +91,17 @@ void main() { expect([1, 2, 3, 4, 5].indexOf(3, 3), -1); }); }); + + test("withPartition", () async { + expect( + await 0.until(10).withPartition((sublist) => [sublist], 4), + [ + [0, 1, 2, 3], + [4, 5, 6, 7], + [8, 9], + ], + ); + }); }); }