From 478c25b5d08ea391025672d0c22fd30cce5b4854 Mon Sep 17 00:00:00 2001 From: Ming Ming Date: Tue, 12 Jul 2022 02:14:42 +0800 Subject: [PATCH 1/2] Fix user ID and display name mixed up in logic --- app/lib/account.dart | 102 ++++++++---- app/lib/api/api.dart | 2 +- app/lib/api/api_util.dart | 4 +- app/lib/bloc/bloc_util.dart | 4 +- app/lib/bloc/list_album_share_outlier.dart | 16 +- app/lib/bloc/scan_account_dir.dart | 2 +- app/lib/entity/album/data_source.dart | 2 +- app/lib/entity/album/upgrader.dart | 4 +- app/lib/entity/file.dart | 2 +- app/lib/entity/sqlite_table_converter.dart | 4 +- app/lib/entity/sqlite_table_extension.dart | 13 +- app/lib/metadata_task_manager.dart | 2 +- app/lib/pref.dart | 18 ++- app/lib/service.dart | 2 +- app/lib/touch_token_manager.dart | 4 +- app/lib/use_case/add_to_album.dart | 4 +- .../use_case/list_potential_shared_album.dart | 2 +- app/lib/use_case/list_share.dart | 2 +- app/lib/use_case/populate_album.dart | 6 +- app/lib/use_case/remove.dart | 6 +- app/lib/use_case/remove_album.dart | 4 +- app/lib/use_case/remove_from_album.dart | 4 +- app/lib/use_case/restore_trashbin.dart | 2 +- app/lib/use_case/scan_dir_offline.dart | 3 +- app/lib/widget/account_picker_dialog.dart | 4 +- app/lib/widget/album_browser.dart | 17 +- app/lib/widget/connect.dart | 14 +- app/lib/widget/dynamic_album_browser.dart | 2 +- .../add_selection_to_album_handler.dart | 2 +- app/lib/widget/home_albums.dart | 2 +- app/lib/widget/home_app_bar.dart | 2 +- app/lib/widget/share_album_dialog.dart | 2 +- app/lib/widget/shared_file_viewer.dart | 2 +- app/lib/widget/sharing_browser.dart | 6 +- app/lib/widget/sign_in.dart | 2 +- app/lib/widget/viewer_detail_pane.dart | 8 +- app/test/account_test.dart | 152 ++++++++++++++++++ .../bloc/list_album_share_outlier_test.dart | 6 +- app/test/entity/album_test.dart | 2 +- .../entity/file/file_cache_manager_test.dart | 8 +- app/test/mock_type.dart | 8 +- app/test/test_util.dart | 5 +- app/test/use_case/add_to_album_test.dart | 2 +- app/test/use_case/remove_album_test.dart | 2 +- app/test/use_case/remove_from_album_test.dart | 2 +- app/test/use_case/remove_test.dart | 2 +- app/test/use_case/scan_dir_offline_test.dart | 2 +- 47 files changed, 340 insertions(+), 128 deletions(-) create mode 100644 app/test/account_test.dart diff --git a/app/lib/account.dart b/app/lib/account.dart index a1beea6b..6e04d2be 100644 --- a/app/lib/account.dart +++ b/app/lib/account.dart @@ -2,9 +2,8 @@ import 'dart:math'; import 'package:equatable/equatable.dart'; import 'package:flutter/foundation.dart'; +import 'package:logging/logging.dart'; import 'package:nc_photos/ci_string.dart'; -import 'package:nc_photos/object_extension.dart'; -import 'package:nc_photos/or_null.dart'; import 'package:nc_photos/string_extension.dart'; import 'package:nc_photos/type.dart'; @@ -14,9 +13,9 @@ class Account with EquatableMixin { this.id, this.scheme, String address, - this.username, + this.userId, + this.username2, this.password, - this.altHomeDir, List roots, ) : address = address.trimRightAny("/"), _roots = roots.map((e) => e.trimRightAny("/")).toList() { @@ -29,18 +28,18 @@ class Account with EquatableMixin { String? id, String? scheme, String? address, - CiString? username, + CiString? userId, + String? username2, String? password, - OrNull? altHomeDir, List? roots, }) { return Account( id ?? this.id, scheme ?? this.scheme, address ?? this.address, - username ?? this.username, + userId ?? this.userId, + username2 ?? this.username2, password ?? this.password, - altHomeDir == null ? this.altHomeDir : altHomeDir.obj, roots ?? List.of(_roots), ); } @@ -57,49 +56,68 @@ class Account with EquatableMixin { "id: '$id', " "scheme: '$scheme', " "address: '${kDebugMode ? address : "***"}', " - "username: '${kDebugMode ? username : "***"}', " + "userId: '${kDebugMode ? userId : "***"}', " + "username2: '${kDebugMode ? username2 : "***"}', " "password: '${password.isNotEmpty == true ? (kDebugMode ? password : '***') : null}', " - "altHomeDir: '${altHomeDir == null || kDebugMode ? altHomeDir : "***"}', " "roots: List {'${roots.join('\', \'')}'}, " "}"; } - Account.fromJson(JsonObj json) - : id = json["id"], - scheme = json["scheme"], - address = json["address"], - username = CiString(json["username"]), - password = json["password"], - altHomeDir = (json["altHomeDir"] as String?)?.run((v) => CiString(v)), - _roots = json["roots"].cast(); + static Account? fromJson( + JsonObj json, { + required AccountUpgraderV1? upgraderV1, + }) { + final jsonVersion = json["version"] ?? 1; + JsonObj? result = json; + if (jsonVersion < 2) { + result = upgraderV1?.call(result); + if (result == null) { + _log.info("[fromJson] Version $jsonVersion not compatible"); + return null; + } + } + return Account( + result["id"], + result["scheme"], + result["address"], + CiString(result["userId"]), + result["username2"], + result["password"], + result["roots"].cast(), + ); + } JsonObj toJson() => { + "version": version, "id": id, "scheme": scheme, "address": address, - "username": username.toString(), + "userId": userId.toString(), + "username2": username2, "password": password, - "altHomeDir": altHomeDir?.toString(), "roots": _roots, }; @override - get props => [id, scheme, address, username, password, altHomeDir, _roots]; + get props => [id, scheme, address, userId, username2, password, _roots]; List get roots => _roots; - CiString get homeDir => altHomeDir ?? username; - final String id; final String scheme; final String address; - final CiString username; + // For non LDAP users, this is the username used to sign in + final CiString userId; + // Username used to sign in. For non-LDAP users, this is identical to userId + final String username2; final String password; - /// Name of the user home dir. Normally [username] is used as the home dir - /// name, but for LDAP users, their home dir might be named differently - final CiString? altHomeDir; final List _roots; + + /// versioning of this class, use to upgrade old persisted accounts + static const version = 2; + + static final _log = Logger("account.Account"); } extension AccountExtension on Account { @@ -113,6 +131,34 @@ extension AccountExtension on Account { bool compareServerIdentity(Account other) { return scheme == other.scheme && address == other.address && - username == other.username; + userId == other.userId; } } + +abstract class AccountUpgrader { + JsonObj? call(JsonObj json); +} + +class AccountUpgraderV1 implements AccountUpgrader { + const AccountUpgraderV1({ + this.logAccountId, + }); + + @override + call(JsonObj json) { + // clarify user id and display name v1 + _log.fine("[call] Upgrade v1 Account: $logAccountId"); + final result = JsonObj.of(json); + result["userId"] = json["altHomeDir"] ?? json["username"]; + result["username2"] = json["username"]; + result + ..remove("username") + ..remove("altHomeDir"); + return result; + } + + /// Account ID for logging only + final String? logAccountId; + + static final _log = Logger("account.AccountUpgraderV1"); +} diff --git a/app/lib/api/api.dart b/app/lib/api/api.dart index a644b527..419da594 100644 --- a/app/lib/api/api.dart +++ b/app/lib/api/api.dart @@ -38,7 +38,7 @@ class Api { static String getAuthorizationHeaderValue(Account account) { final auth = - base64.encode(utf8.encode("${account.username}:${account.password}")); + base64.encode(utf8.encode("${account.username2}:${account.password}")); return "Basic $auth"; } diff --git a/app/lib/api/api_util.dart b/app/lib/api/api_util.dart index 49acf5a9..dbadf434 100644 --- a/app/lib/api/api_util.dart +++ b/app/lib/api/api_util.dart @@ -65,10 +65,10 @@ String getFileUrlRelative(File file) { } String getWebdavRootUrlRelative(Account account) => - "remote.php/dav/files/${account.homeDir}"; + "remote.php/dav/files/${account.userId}"; String getTrashbinPath(Account account) => - "remote.php/dav/trashbin/${account.homeDir}/trash"; + "remote.php/dav/trashbin/${account.userId}/trash"; /// Return the face image URL. See [getFacePreviewUrlRelative] String getFacePreviewUrl( diff --git a/app/lib/bloc/bloc_util.dart b/app/lib/bloc/bloc_util.dart index bd780aa3..c1056fa2 100644 --- a/app/lib/bloc/bloc_util.dart +++ b/app/lib/bloc/bloc_util.dart @@ -1,7 +1,7 @@ import 'package:nc_photos/account.dart'; String getInstNameForAccount(String className, Account account) => - "$className(${account.scheme}://${account.username.toCaseInsensitiveString()}@${account.address})"; + "$className(${account.scheme}://${account.userId.toCaseInsensitiveString()}@${account.address})"; String getInstNameForRootAwareAccount(String className, Account account) => - "$className(${account.scheme}://${account.username.toCaseInsensitiveString()}@${account.address}?${account.roots.join('&')})"; + "$className(${account.scheme}://${account.userId.toCaseInsensitiveString()}@${account.address}?${account.roots.join('&')})"; diff --git a/app/lib/bloc/list_album_share_outlier.dart b/app/lib/bloc/list_album_share_outlier.dart index 9906a75e..6ce2afe8 100644 --- a/app/lib/bloc/list_album_share_outlier.dart +++ b/app/lib/bloc/list_album_share_outlier.dart @@ -202,9 +202,9 @@ class ListAlbumShareOutlierBloc extends Bloc s.userId != ev.account.username) + .where((s) => s.userId != ev.account.userId) .toList(); - if (ev.album.albumFile!.ownerId != ev.account.username) { + if (ev.album.albumFile!.ownerId != ev.account.userId) { // add owner if the album is not owned by this account final ownerSharee = (await ListSharee(_c.shareeRepo)(ev.account)) .firstWhere((s) => s.shareWith == ev.album.albumFile!.ownerId); @@ -241,7 +241,7 @@ class ListAlbumShareOutlierBloc extends Bloc albumShares, List errors, ) async { - if (!album.albumFile!.isOwned(account.username)) { + if (!album.albumFile!.isOwned(account.userId)) { // album file is always managed by the owner return []; } @@ -335,7 +335,7 @@ class ListAlbumShareOutlierBloc extends Bloc s != account.username && s != fileItem.file.ownerId) + .where((s) => s != account.userId && s != fileItem.file.ownerId) .toList(); _log.info( "[_processSingleFileItem] Missing shares: ${missings.toReadableString()} for file: ${logFilename(fileItem.file.path)}"); @@ -348,7 +348,7 @@ class ListAlbumShareOutlierBloc extends Bloc s.uidOwner == account.username) + .where((s) => s.uidOwner == account.userId) .map((s) => s.shareWith!) .toSet(); final extras = ownedSharees.difference(albumSharees); @@ -375,13 +375,13 @@ class ListAlbumShareOutlierBloc extends Bloc - file_util.isSupportedFormat(f) && !f.isOwned(account.username)) + file_util.isSupportedFormat(f) && !f.isOwned(account.userId)) .toList(); } catch (e, stackTrace) { yield ExceptionEvent(e, stackTrace); diff --git a/app/lib/entity/album/data_source.dart b/app/lib/entity/album/data_source.dart index d8ae1293..49c0e556 100644 --- a/app/lib/entity/album/data_source.dart +++ b/app/lib/entity/album/data_source.dart @@ -166,7 +166,7 @@ class AlbumSqliteDbDataSource implements AlbumDataSource { } else { try { final f = SqliteFileConverter.fromSql( - account.homeDir.toString(), item["file"]); + account.userId.toString(), item["file"]); yield SqliteAlbumConverter.fromSql( item["album"], f, item["shares"] ?? []); } catch (e, stackTrace) { diff --git a/app/lib/entity/album/upgrader.dart b/app/lib/entity/album/upgrader.dart index 0b46328a..d413e496 100644 --- a/app/lib/entity/album/upgrader.dart +++ b/app/lib/entity/album/upgrader.dart @@ -181,10 +181,10 @@ class AlbumUpgraderV5 implements AlbumUpgrader { final CiString addedBy; if (result.containsKey("albumFile")) { addedBy = result["albumFile"]["ownerId"] == null - ? account.username + ? account.userId : CiString(result["albumFile"]["ownerId"]); } else { - addedBy = albumFile?.ownerId ?? account.username; + addedBy = albumFile?.ownerId ?? account.userId; } item["addedBy"] = addedBy.toString(); item["addedAt"] = result["lastUpdated"]; diff --git a/app/lib/entity/file.dart b/app/lib/entity/file.dart index 99af600b..92403c0d 100644 --- a/app/lib/entity/file.dart +++ b/app/lib/entity/file.dart @@ -465,7 +465,7 @@ extension FileExtension on File { lastModified ?? DateTime.now().toUtc(); - bool isOwned(CiString username) => ownerId == null || ownerId == username; + bool isOwned(CiString userId) => ownerId == null || ownerId == userId; /// Return the path of this file with the DAV part stripped /// diff --git a/app/lib/entity/sqlite_table_converter.dart b/app/lib/entity/sqlite_table_converter.dart index c134000d..994d9715 100644 --- a/app/lib/entity/sqlite_table_converter.dart +++ b/app/lib/entity/sqlite_table_converter.dart @@ -72,7 +72,7 @@ class SqliteAlbumConverter { } class SqliteFileConverter { - static File fromSql(String homeDir, sql.CompleteFile f) { + static File fromSql(String userId, sql.CompleteFile f) { final metadata = f.image?.run((obj) => Metadata( lastUpdated: obj.lastUpdated, fileEtag: obj.fileEtag, @@ -81,7 +81,7 @@ class SqliteFileConverter { exif: obj.exifRaw?.run((e) => Exif.fromJson(jsonDecode(e))), )); return File( - path: "remote.php/dav/files/$homeDir/${f.accountFile.relativePath}", + path: "remote.php/dav/files/$userId/${f.accountFile.relativePath}", contentLength: f.file.contentLength, contentType: f.file.contentType, etag: f.file.etag, diff --git a/app/lib/entity/sqlite_table_extension.dart b/app/lib/entity/sqlite_table_extension.dart index 036c8418..dd179ba0 100644 --- a/app/lib/entity/sqlite_table_extension.dart +++ b/app/lib/entity/sqlite_table_extension.dart @@ -35,7 +35,7 @@ class CompleteFileCompanion { extension CompleteFileListExtension on List { Future> convertToAppFile(app.Account account) { return map((f) => { - "homeDir": account.homeDir.toString(), + "userId": account.userId.toString(), "completeFile": f, }).computeAll(_covertSqliteDbFile); } @@ -141,7 +141,7 @@ extension SqliteDbExtension on SqliteDb { await into(accounts).insert( AccountsCompanion.insert( server: dbServer.rowId, - userId: account.username.toCaseInsensitiveString(), + userId: account.userId.toCaseInsensitiveString(), ), mode: InsertMode.insertOrIgnore, ); @@ -153,8 +153,7 @@ extension SqliteDbExtension on SqliteDb { useColumns: false) ]) ..where(servers.address.equals(account.url)) - ..where( - accounts.userId.equals(account.username.toCaseInsensitiveString())) + ..where(accounts.userId.equals(account.userId.toCaseInsensitiveString())) ..limit(1); return query.map((r) => r.readTable(accounts)).getSingle(); } @@ -522,7 +521,7 @@ class FilesQueryBuilder { query ..where(db.servers.address.equals(_appAccount!.url)) ..where(db.accounts.userId - .equals(_appAccount!.username.toCaseInsensitiveString())); + .equals(_appAccount!.userId.toCaseInsensitiveString())); } if (_byRowId != null) { @@ -585,9 +584,9 @@ class FilesQueryBuilder { } app.File _covertSqliteDbFile(Map map) { - final homeDir = map["homeDir"] as String; + final userId = map["userId"] as String; final file = map["completeFile"] as CompleteFile; - return SqliteFileConverter.fromSql(homeDir, file); + return SqliteFileConverter.fromSql(userId, file); } CompleteFileCompanion _convertAppFile(Map map) { diff --git a/app/lib/metadata_task_manager.dart b/app/lib/metadata_task_manager.dart index 0fd46d60..3fa1a2ad 100644 --- a/app/lib/metadata_task_manager.dart +++ b/app/lib/metadata_task_manager.dart @@ -49,7 +49,7 @@ class MetadataTask { account, shareFolder, isRecursive: false, - filter: (f) => f.ownerId != account.username, + filter: (f) => f.ownerId != account.userId, )) { if (!Pref().isEnableExifOr()) { _log.info("[call] EXIF disabled, task ending immaturely"); diff --git a/app/lib/pref.dart b/app/lib/pref.dart index e338d5c7..23806c87 100644 --- a/app/lib/pref.dart +++ b/app/lib/pref.dart @@ -1,8 +1,10 @@ import 'dart:async'; import 'dart:convert'; +import 'package:collection/collection.dart'; import 'package:event_bus/event_bus.dart'; import 'package:kiwi/kiwi.dart'; +import 'package:logging/logging.dart'; import 'package:nc_photos/account.dart'; import 'package:nc_photos/event/event.dart'; import 'package:nc_photos/mobile/platform.dart' @@ -26,7 +28,19 @@ class Pref { List? getAccounts3() { final jsonObjs = provider.getStringList(PrefKey.accounts3); - return jsonObjs?.map((e) => Account.fromJson(jsonDecode(e))).toList(); + return jsonObjs + ?.map((e) => Account.fromJson( + jsonDecode(e), + upgraderV1: const AccountUpgraderV1(), + )) + .where((e) { + if (e == null) { + _log.shout("[getAccounts3] Failed upgrading account"); + } + return true; + }) + .whereNotNull() + .toList(); } List getAccounts3Or(List def) => getAccounts3() ?? def; @@ -235,6 +249,8 @@ class Pref { final PrefProvider provider; static Pref? _inst; + + static final _log = Logger("pref.Pref"); } class AccountPref { diff --git a/app/lib/service.dart b/app/lib/service.dart index 88a36225..66e9b43c 100644 --- a/app/lib/service.dart +++ b/app/lib/service.dart @@ -277,7 +277,7 @@ class _MetadataTask { account, shareFolder, isRecursive: false, - filter: (f) => f.ownerId != account.username, + filter: (f) => f.ownerId != account.userId, )) { if (ev is File) { _onFileProcessed(ev); diff --git a/app/lib/touch_token_manager.dart b/app/lib/touch_token_manager.dart index 06262096..530937b7 100644 --- a/app/lib/touch_token_manager.dart +++ b/app/lib/touch_token_manager.dart @@ -108,9 +108,9 @@ class TouchTokenManager { String _getLocalStorageName(Account account, File file) { final strippedPath = file.strippedPath; if (strippedPath == ".") { - return "touch/${account.url.replaceFirst('://', '_')}/${account.username}/token"; + return "touch/${account.url.replaceFirst('://', '_')}/${account.userId}/token"; } else { - return "touch/${account.url.replaceFirst('://', '_')}/${account.username}/${file.strippedPath}/token"; + return "touch/${account.url.replaceFirst('://', '_')}/${account.userId}/${file.strippedPath}/token"; } } diff --git a/app/lib/use_case/add_to_album.dart b/app/lib/use_case/add_to_album.dart index a239faf6..7307f417 100644 --- a/app/lib/use_case/add_to_album.dart +++ b/app/lib/use_case/add_to_album.dart @@ -72,8 +72,8 @@ class AddToAlbum { Future _shareFiles( Account account, Album album, List files) async { final albumShares = (album.shares!.map((e) => e.userId).toList() - ..add(album.albumFile!.ownerId ?? account.username)) - .where((element) => element != account.username) + ..add(album.albumFile!.ownerId ?? account.userId)) + .where((element) => element != account.userId) .toSet(); if (albumShares.isEmpty) { return; diff --git a/app/lib/use_case/list_potential_shared_album.dart b/app/lib/use_case/list_potential_shared_album.dart index 53be4a8a..939b3471 100644 --- a/app/lib/use_case/list_potential_shared_album.dart +++ b/app/lib/use_case/list_potential_shared_album.dart @@ -28,7 +28,7 @@ class ListPotentialSharedAlbum { return results; } - bool _checkOwner(Account account, File f) => !f.isOwned(account.username); + bool _checkOwner(Account account, File f) => !f.isOwned(account.userId); bool _checkFileName(File f) { try { diff --git a/app/lib/use_case/list_share.dart b/app/lib/use_case/list_share.dart index a7287438..896208ff 100644 --- a/app/lib/use_case/list_share.dart +++ b/app/lib/use_case/list_share.dart @@ -21,7 +21,7 @@ class ListShare { bool? isIncludeReshare, }) async { try { - if (file_util.getUserDirName(file) != account.homeDir) { + if (file_util.getUserDirName(file) != account.userId) { file = (await FindFile(_c)(account, [file.fileId!])).first; } } catch (_) { diff --git a/app/lib/use_case/populate_album.dart b/app/lib/use_case/populate_album.dart index 3c5dc6c9..a4a651a7 100644 --- a/app/lib/use_case/populate_album.dart +++ b/app/lib/use_case/populate_album.dart @@ -51,7 +51,7 @@ class PopulateAlbum { continue; } products.addAll((result as List).cast().map((f) => AlbumFileItem( - addedBy: account.username, + addedBy: account.userId, addedAt: DateTime.now(), file: f, ))); @@ -68,7 +68,7 @@ class PopulateAlbum { final c = KiwiContainer().resolve(); final files = await ListTaggedFile(c)(account, provider.tags); products.addAll(files.map((f) => AlbumFileItem( - addedBy: account.username, + addedBy: account.userId, addedAt: DateTime.now(), file: f, ))); @@ -87,7 +87,7 @@ class PopulateAlbum { return files .where((f) => file_util.isSupportedFormat(f)) .map((f) => AlbumFileItem( - addedBy: account.username, + addedBy: account.userId, addedAt: DateTime.now(), file: f, )) diff --git a/app/lib/use_case/remove.dart b/app/lib/use_case/remove.dart index e7cddc3c..0d6faf09 100644 --- a/app/lib/use_case/remove.dart +++ b/app/lib/use_case/remove.dart @@ -65,8 +65,8 @@ class Remove { final itemsToRemove = provider.items .whereType() .where((i) => - (i.file.isOwned(account.username) || - i.addedBy == account.username) && + (i.file.isOwned(account.userId) || + i.addedBy == account.userId) && removes.any((r) => r.compareServerIdentity(i.file))) .toList(); if (itemsToRemove.isEmpty) { @@ -76,7 +76,7 @@ class Remove { final key = FileServerIdentityComparator(i.file); final value = (a.shares?.map((s) => s.userId).toList() ?? []) ..add(a.albumFile!.ownerId!) - ..remove(account.username); + ..remove(account.userId); (unshares[key] ??= {}).addAll(value); } _log.fine( diff --git a/app/lib/use_case/remove_album.dart b/app/lib/use_case/remove_album.dart index 7cb70402..16d57c26 100644 --- a/app/lib/use_case/remove_album.dart +++ b/app/lib/use_case/remove_album.dart @@ -51,8 +51,8 @@ class RemoveAlbum { return; } final albumShares = (album.shares!.map((e) => e.userId).toList() - ..add(album.albumFile!.ownerId ?? account.username)) - .where((element) => element != account.username) + ..add(album.albumFile!.ownerId ?? account.userId)) + .where((element) => element != account.userId) .toList(); if (albumShares.isEmpty) { return; diff --git a/app/lib/use_case/remove_from_album.dart b/app/lib/use_case/remove_from_album.dart index ca0bc235..ab527889 100644 --- a/app/lib/use_case/remove_from_album.dart +++ b/app/lib/use_case/remove_from_album.dart @@ -103,8 +103,8 @@ class RemoveFromAlbum { Future _unshareFiles( Account account, Album album, List files) async { final albumShares = (album.shares!.map((e) => e.userId).toList() - ..add(album.albumFile!.ownerId ?? account.username)) - .where((element) => element != account.username) + ..add(album.albumFile!.ownerId ?? account.userId)) + .where((element) => element != account.userId) .toList(); if (albumShares.isNotEmpty) { await UnshareFileFromAlbum(_c)(account, album, files, albumShares); diff --git a/app/lib/use_case/restore_trashbin.dart b/app/lib/use_case/restore_trashbin.dart index cc7a674e..0df65280 100644 --- a/app/lib/use_case/restore_trashbin.dart +++ b/app/lib/use_case/restore_trashbin.dart @@ -17,7 +17,7 @@ class RestoreTrashbin { await Move(_c)( account, file, - "remote.php/dav/trashbin/${account.homeDir}/restore/${file.filename}", + "remote.php/dav/trashbin/${account.userId}/restore/${file.filename}", shouldOverwrite: true, ); KiwiContainer() diff --git a/app/lib/use_case/scan_dir_offline.dart b/app/lib/use_case/scan_dir_offline.dart index 6d0e129b..46c47cf1 100644 --- a/app/lib/use_case/scan_dir_offline.dart +++ b/app/lib/use_case/scan_dir_offline.dart @@ -50,8 +50,7 @@ class ScanDirOffline { .get(); }); return dbFiles - .map( - (f) => SqliteFileConverter.fromSql(account.homeDir.toString(), f)) + .map((f) => SqliteFileConverter.fromSql(account.userId.toString(), f)) .toList(); }); } diff --git a/app/lib/widget/account_picker_dialog.dart b/app/lib/widget/account_picker_dialog.dart index 2186791e..c87eb8b8 100644 --- a/app/lib/widget/account_picker_dialog.dart +++ b/app/lib/widget/account_picker_dialog.dart @@ -45,7 +45,7 @@ class _AccountPickerDialogState extends State { child: ListTile( dense: true, title: Text(label ?? a.url), - subtitle: label == null ? Text(a.username.toString()) : null, + subtitle: label == null ? Text(a.username2) : null, trailing: IconButton( icon: Icon( Icons.close, @@ -87,7 +87,7 @@ class _AccountPickerDialogState extends State { ), subtitle: accountLabel == null ? Text( - widget.account.username.toString(), + widget.account.username2, style: const TextStyle(fontWeight: FontWeight.bold), ) : null, diff --git a/app/lib/widget/album_browser.dart b/app/lib/widget/album_browser.dart index a992cdf4..a7ccd854 100644 --- a/app/lib/widget/album_browser.dart +++ b/app/lib/widget/album_browser.dart @@ -133,7 +133,7 @@ class _AlbumBrowserState extends State @override @protected - get canEdit => _album?.albumFile?.isOwned(widget.account.username) == true; + get canEdit => _album?.albumFile?.isOwned(widget.account.userId) == true; @override enterEditMode() { @@ -274,7 +274,7 @@ class _AlbumBrowserState extends State widget.account, _album!, actions: [ - if (_album!.albumFile!.isOwned(widget.account.username) && + if (_album!.albumFile!.isOwned(widget.account.userId) && Pref().isLabEnableSharedAlbumOr(false)) IconButton( onPressed: () => _onSharePressed(context), @@ -462,8 +462,8 @@ class _AlbumBrowserState extends State .takeIndex(selectedIndexes) // can only remove owned files .where((element) => - _album!.albumFile!.isOwned(widget.account.username) == true || - element.addedBy == widget.account.username) + _album!.albumFile!.isOwned(widget.account.userId) == true || + element.addedBy == widget.account.userId) .toList(); setState(() { clearSelectedItems(); @@ -668,7 +668,7 @@ class _AlbumBrowserState extends State provider: AlbumStaticProvider.of(_editAlbum!).copyWith( items: [ AlbumLabelItem( - addedBy: widget.account.username, + addedBy: widget.account.userId, addedAt: DateTime.now(), text: value, ), @@ -822,7 +822,7 @@ class _AlbumBrowserState extends State Future _setAlbum(Album album) async { assert(album.provider is AlbumStaticProvider); final items = await PreProcessAlbum(_c)(widget.account, album); - if (album.albumFile!.isOwned(widget.account.username)) { + if (album.albumFile!.isOwned(widget.account.userId)) { album = await _updateAlbumPostResync(album, items); } album = album.copyWith( @@ -858,14 +858,13 @@ class _AlbumBrowserState extends State } bool get _canRemoveSelection { - if (_album!.albumFile!.isOwned(widget.account.username) == true) { + if (_album!.albumFile!.isOwned(widget.account.userId) == true) { return true; } final selectedIndexes = selectedListItems.whereType<_ListItem>().map((e) => e.index).toList(); final selectedItemsIt = _sortedItems.takeIndex(selectedIndexes); - return selectedItemsIt - .any((item) => item.addedBy == widget.account.username); + return selectedItemsIt.any((item) => item.addedBy == widget.account.userId); } static List _getAlbumItemsOf(Album a) => diff --git a/app/lib/widget/connect.dart b/app/lib/widget/connect.dart index 130ec954..a78e3d3e 100644 --- a/app/lib/widget/connect.dart +++ b/app/lib/widget/connect.dart @@ -198,10 +198,10 @@ class _ConnectState extends State { Future _onCheckWebDavUrlFailed( BuildContext context, Account account) async { - final altHomeDir = await _askWebDavUrl(context, account); - if (altHomeDir != null) { + final userId = await _askWebDavUrl(context, account); + if (userId != null) { final newAccount = account.copyWith( - altHomeDir: OrNull(altHomeDir.toCi()), + userId: userId.toCi(), ); return _checkWebDavUrl(context, newAccount); } @@ -286,9 +286,9 @@ class _WebDavUrlDialogState extends State<_WebDavUrlDialog> { return L10n.global().homeFolderInputInvalidEmpty; }, onSaved: (value) { - _formValue.altHomeDir = value!.trimAny("/"); + _formValue.userId = value!.trimAny("/"); }, - initialValue: widget.account.homeDir.toString(), + initialValue: widget.account.userId.toString(), ), ], ), @@ -309,7 +309,7 @@ class _WebDavUrlDialogState extends State<_WebDavUrlDialog> { void _onOkPressed() { if (_formKey.currentState?.validate() == true) { _formKey.currentState!.save(); - Navigator.of(context).pop(_formValue.altHomeDir); + Navigator.of(context).pop(_formValue.userId); } } @@ -322,5 +322,5 @@ class _WebDavUrlDialogState extends State<_WebDavUrlDialog> { } class _FormValue { - late String altHomeDir; + late String userId; } diff --git a/app/lib/widget/dynamic_album_browser.dart b/app/lib/widget/dynamic_album_browser.dart index c90b5962..c30032c2 100644 --- a/app/lib/widget/dynamic_album_browser.dart +++ b/app/lib/widget/dynamic_album_browser.dart @@ -127,7 +127,7 @@ class _DynamicAlbumBrowserState extends State @override @protected - get canEdit => _album?.albumFile?.isOwned(widget.account.username) == true; + get canEdit => _album?.albumFile?.isOwned(widget.account.userId) == true; @override enterEditMode() { diff --git a/app/lib/widget/handler/add_selection_to_album_handler.dart b/app/lib/widget/handler/add_selection_to_album_handler.dart index ba7e044b..6910712b 100644 --- a/app/lib/widget/handler/add_selection_to_album_handler.dart +++ b/app/lib/widget/handler/add_selection_to_album_handler.dart @@ -34,7 +34,7 @@ class AddSelectionToAlbumHandler { assert(value.provider is AlbumStaticProvider); final selected = selectedFiles .map((f) => AlbumFileItem( - addedBy: account.username, + addedBy: account.userId, addedAt: DateTime.now(), file: f, )) diff --git a/app/lib/widget/home_albums.dart b/app/lib/widget/home_albums.dart index 37bbd05d..86e94187 100644 --- a/app/lib/widget/home_albums.dart +++ b/app/lib/widget/home_albums.dart @@ -462,7 +462,7 @@ class _HomeAlbumsState extends State final failures = []; for (final a in selected) { try { - if (a.albumFile?.isOwned(widget.account.username) == true) { + if (a.albumFile?.isOwned(widget.account.userId) == true) { // delete owned albums await RemoveAlbum(KiwiContainer().resolve())( widget.account, a); diff --git a/app/lib/widget/home_app_bar.dart b/app/lib/widget/home_app_bar.dart index 824b7aa2..cc056cf8 100644 --- a/app/lib/widget/home_app_bar.dart +++ b/app/lib/widget/home_app_bar.dart @@ -63,7 +63,7 @@ class HomeSliverAppBar extends StatelessWidget { ), if (accountLabel == null) Text( - account.username.toString(), + account.username2, style: TextStyle( fontSize: 14, color: AppTheme.getSecondaryTextColor(context), diff --git a/app/lib/widget/share_album_dialog.dart b/app/lib/widget/share_album_dialog.dart index 504cf69f..1237518e 100644 --- a/app/lib/widget/share_album_dialog.dart +++ b/app/lib/widget/share_album_dialog.dart @@ -225,7 +225,7 @@ class _ShareAlbumDialogState extends State { void _transformShareeItems(List sharees) { final candidates = sharees .where((s) => - s.shareWith != widget.account.username && + s.shareWith != widget.account.userId && // remove users already shared with !_items.any((i) => i.shareWith == s.shareWith)) .toList(); diff --git a/app/lib/widget/shared_file_viewer.dart b/app/lib/widget/shared_file_viewer.dart index 9a79dd17..cdd22d0d 100644 --- a/app/lib/widget/shared_file_viewer.dart +++ b/app/lib/widget/shared_file_viewer.dart @@ -129,7 +129,7 @@ class _SharedFileViewerState extends State { title: Text(widget.file.strippedPath), ), ), - if (widget.shares.first.uidOwner == widget.account.username) ...[ + if (widget.shares.first.uidOwner == widget.account.userId) ...[ SliverToBoxAdapter( child: Padding( padding: const EdgeInsets.all(16), diff --git a/app/lib/widget/sharing_browser.dart b/app/lib/widget/sharing_browser.dart index c3c79b27..9a7e0e1f 100644 --- a/app/lib/widget/sharing_browser.dart +++ b/app/lib/widget/sharing_browser.dart @@ -202,7 +202,7 @@ class _SharingBrowserState extends State { ), ), label: shares.first.share.filename, - description: shares.first.share.uidOwner == widget.account.username + description: shares.first.share.uidOwner == widget.account.userId ? L10n.global().fileLastSharedDescription(dateStr) : L10n.global().fileLastSharedByOthersDescription( shares.first.share.displaynameOwner, dateStr), @@ -260,7 +260,7 @@ class _SharingBrowserState extends State { ), ), label: firstItem.album.name, - description: shares.first.share.uidOwner == widget.account.username + description: shares.first.share.uidOwner == widget.account.userId ? L10n.global().fileLastSharedDescription(dateStr) : L10n.global().albumLastSharedByOthersDescription( shares.first.share.displaynameOwner, dateStr), @@ -307,7 +307,7 @@ class _SharingBrowserState extends State { // group shares of the same file final map = >{}; for (final i in items) { - final isSharedByMe = (i.share.uidOwner == widget.account.username); + final isSharedByMe = (i.share.uidOwner == widget.account.userId); final groupKey = "${i.share.path}?$isSharedByMe"; map[groupKey] ??= []; map[groupKey]!.add(i); diff --git a/app/lib/widget/sign_in.dart b/app/lib/widget/sign_in.dart index 5d8ab68d..37fb92a0 100644 --- a/app/lib/widget/sign_in.dart +++ b/app/lib/widget/sign_in.dart @@ -238,8 +238,8 @@ class _SignInState extends State { _formValue.scheme, _formValue.address, _formValue.username.toCi(), + _formValue.username, _formValue.password, - null, [""], ); _log.info("[_connect] Try connecting with account: $account"); diff --git a/app/lib/widget/viewer_detail_pane.dart b/app/lib/widget/viewer_detail_pane.dart index 29f92cee..7c6a34a4 100644 --- a/app/lib/widget/viewer_detail_pane.dart +++ b/app/lib/widget/viewer_detail_pane.dart @@ -149,7 +149,7 @@ class _ViewerDetailPaneState extends State { onPressed: () => _onRemoveFromAlbumPressed(context), ), if (widget.album != null && - widget.album!.albumFile?.isOwned(widget.account.username) == + widget.album!.albumFile?.isOwned(widget.account.userId) == true) _DetailPaneButton( icon: Icons.photo_album_outlined, @@ -195,7 +195,7 @@ class _ViewerDetailPaneState extends State { title: Text(path_lib.basenameWithoutExtension(widget.file.path)), subtitle: Text(widget.file.strippedPath), ), - if (!widget.file.isOwned(widget.account.username)) + if (!widget.file.isOwned(widget.account.userId)) ListTile( leading: ListTileCenterLeading( child: Icon( @@ -509,7 +509,7 @@ class _ViewerDetailPaneState extends State { widget.album!.provider is! AlbumStaticProvider) { return false; } - if (widget.album!.albumFile?.isOwned(widget.account.username) == true) { + if (widget.album!.albumFile?.isOwned(widget.account.userId) == true) { return true; } try { @@ -518,7 +518,7 @@ class _ViewerDetailPaneState extends State { .whereType() .firstWhere( (element) => element.file.compareServerIdentity(widget.file)); - if (thisItem.addedBy == widget.account.username) { + if (thisItem.addedBy == widget.account.userId) { return true; } } catch (_) {} diff --git a/app/test/account_test.dart b/app/test/account_test.dart new file mode 100644 index 00000000..c5a6aa00 --- /dev/null +++ b/app/test/account_test.dart @@ -0,0 +1,152 @@ +import 'package:nc_photos/account.dart'; +import 'package:nc_photos/ci_string.dart'; +import 'package:test/test.dart'; + +void main() { + group("Account", () { + group("constructor", () { + test("trim address", _constructTrimAddress); + test("invalid scheme", _constructInvalidScheme); + }); + test("fromJson", _fromJson); + }); + group("AccountUpgraderV1", () { + test("normal", _upgraderV1); + test("ldap", _upgraderV1Ldap); + }); +} + +/// Convert json obj to Account +/// +/// Expect: Account constructed +void _fromJson() { + final json = { + "version": Account.version, + "id": "123456", + "scheme": "https", + "address": "example.com", + "userId": "00000000-1111-aaaa-bbbb-223344ccddee", + "username2": "admin", + "password": "123456", + "roots": ["test1", "test2"], + }; + expect( + Account.fromJson( + json, + upgraderV1: null, + ), + Account( + "123456", + "https", + "example.com", + "00000000-1111-aaaa-bbbb-223344ccddee".toCi(), + "admin", + "123456", + ["test1", "test2"], + ), + ); +} + +/// Upgrade v1 Account json to v2 Account json +/// +/// Expect: v2.userId = v1.username; +/// v2.username2 = v1.username +void _upgraderV1() { + final json = { + "version": 1, + "id": "123456", + "scheme": "https", + "address": "example.com", + "username": "admin", + "password": "123456", + "roots": ["test1", "test2"], + }; + expect( + const AccountUpgraderV1()(json), + { + "version": 1, + "id": "123456", + "scheme": "https", + "address": "example.com", + "userId": "admin", + "username2": "admin", + "password": "123456", + "roots": ["test1", "test2"], + }, + ); +} + +/// Upgrade v1 Account json to v2 Account json for a LDAP account +/// +/// Expect: v2.userId = v1.altHomeDir; +/// v2.username2 = v1.username +void _upgraderV1Ldap() { + final json = { + "version": 1, + "id": "123456", + "scheme": "https", + "address": "example.com", + "username": "admin", + "altHomeDir": "00000000-1111-aaaa-bbbb-223344ccddee", + "password": "123456", + "roots": ["test1", "test2"], + }; + expect( + const AccountUpgraderV1()(json), + { + "version": 1, + "id": "123456", + "scheme": "https", + "address": "example.com", + "userId": "00000000-1111-aaaa-bbbb-223344ccddee", + "username2": "admin", + "password": "123456", + "roots": ["test1", "test2"], + }, + ); +} + +/// Construct a new Account, with address ending with / +/// +/// Expect: Account constructed; +/// Trailing / in address removed +void _constructTrimAddress() { + expect( + Account( + "123456", + "https", + "example.com/", + "00000000-1111-aaaa-bbbb-223344ccddee".toCi(), + "admin", + "123456", + ["test1", "test2"], + ), + Account( + "123456", + "https", + "example.com", + "00000000-1111-aaaa-bbbb-223344ccddee".toCi(), + "admin", + "123456", + ["test1", "test2"], + ), + ); +} + +/// Construct a new Account, with scheme != http/https +/// +/// Expect: FormatException +void _constructInvalidScheme() { + expect( + () => Account( + "123456", + "ssh", + "example.com/", + "00000000-1111-aaaa-bbbb-223344ccddee".toCi(), + "admin", + "123456", + ["test1", "test2"], + ), + throwsFormatException, + ); +} diff --git a/app/test/bloc/list_album_share_outlier_test.dart b/app/test/bloc/list_album_share_outlier_test.dart index 97fc379b..a82d4958 100644 --- a/app/test/bloc/list_album_share_outlier_test.dart +++ b/app/test/bloc/list_album_share_outlier_test.dart @@ -254,7 +254,7 @@ void _testQuerySharedAlbumMissingManagedShareOtherAdded(String description) { /// Expect: emit empty list void _testQuerySharedAlbumMissingManagedShareOtherReshared(String description) { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1)..addJpeg("admin/test1.jpg")).build(); final user1Files = [ @@ -450,7 +450,7 @@ void _testQuerySharedAlbumExtraShare(String description) { /// Expect: emit the file with extra share (admin -> user2) void _testQuerySharedAlbumExtraShareOtherAdded(String description) { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1) ..addJpeg("admin/test1.jpg", ownerId: "user1")) .build(); @@ -510,7 +510,7 @@ void _testQuerySharedAlbumExtraShareOtherAdded(String description) { /// Expect: emit empty list void _testQuerySharedAlbumExtraUnmanagedShare(String description) { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1) ..addJpeg("admin/test1.jpg", ownerId: "user1")) .build(); diff --git a/app/test/entity/album_test.dart b/app/test/entity/album_test.dart index dc13b723..6f268180 100644 --- a/app/test/entity/album_test.dart +++ b/app/test/entity/album_test.dart @@ -1484,7 +1484,7 @@ void main() { }); group("AlbumUpgraderV5", () { - final account = util.buildAccount(username: "user1"); + final account = util.buildAccount(userId: "user1"); test("w/ ownerId", () { final json = { diff --git a/app/test/entity/file/file_cache_manager_test.dart b/app/test/entity/file/file_cache_manager_test.dart index 3c5d7fb4..0c8ad823 100644 --- a/app/test/entity/file/file_cache_manager_test.dart +++ b/app/test/entity/file/file_cache_manager_test.dart @@ -347,7 +347,7 @@ Future _updaterUpdateFile() async { /// Expect: file added to AccountFiles table Future _updaterNewSharedFile() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder() ..addDir("admin") ..addJpeg("admin/test1.jpg") @@ -385,7 +385,7 @@ Future _updaterNewSharedFile() async { /// Expect: file added to AccountFiles table Future _updaterNewSharedDir() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder() ..addDir("admin") ..addJpeg("admin/test1.jpg", ownerId: "user1") @@ -423,7 +423,7 @@ Future _updaterNewSharedDir() async { /// file remained in Files table Future _updaterDeleteSharedFile() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder() ..addDir("admin") ..addJpeg("admin/test1.jpg") @@ -465,7 +465,7 @@ Future _updaterDeleteSharedFile() async { /// file remained in Files table Future _updaterDeleteSharedDir() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder() ..addDir("admin") ..addJpeg("admin/test1.jpg") diff --git a/app/test/mock_type.dart b/app/test/mock_type.dart index 9680383e..468b7c7a 100644 --- a/app/test/mock_type.dart +++ b/app/test/mock_type.dart @@ -328,7 +328,7 @@ class MockShareMemoryRepo extends MockShareRepo { return shares.where((s) { if (s.itemSource != file.fileId) { return false; - } else if (isIncludeReshare == true || s.uidOwner == account.username) { + } else if (isIncludeReshare == true || s.uidOwner == account.userId) { return true; } else { return false; @@ -342,8 +342,8 @@ class MockShareMemoryRepo extends MockShareRepo { id: (_id++).toString(), shareType: ShareType.user, stime: DateTime.utc(2020, 1, 2, 3, 4, 5), - uidOwner: account.username, - displaynameOwner: account.username.toString(), + uidOwner: account.userId, + displaynameOwner: account.username2, uidFileOwner: file.ownerId!, path: file.strippedPath, itemType: ShareItemType.file, @@ -383,7 +383,7 @@ class MockShareeMemoryRepo extends MockShareeRepo { @override list(Account account) async { - return sharees.where((s) => s.shareWith != account.username).toList(); + return sharees.where((s) => s.shareWith != account.userId).toList(); } final List sharees; diff --git a/app/test/test_util.dart b/app/test/test_util.dart index c808b7cc..ad5213ac 100644 --- a/app/test/test_util.dart +++ b/app/test/test_util.dart @@ -286,11 +286,12 @@ Account buildAccount({ String id = "123456-000000", String scheme = "http", String address = "example.com", - String username = "admin", + String userId = "admin", + String username2 = "admin", String password = "pass", List roots = const [""], }) => - Account(id, scheme, address, username.toCi(), password, null, roots); + Account(id, scheme, address, userId.toCi(), username2, password, roots); /// Build a mock [File] pointing to a album JSON file /// diff --git a/app/test/use_case/add_to_album_test.dart b/app/test/use_case/add_to_album_test.dart index 6dc5ac9d..fd1b8e90 100644 --- a/app/test/use_case/add_to_album_test.dart +++ b/app/test/use_case/add_to_album_test.dart @@ -187,7 +187,7 @@ Future _addExistingFile() async { /// Expect: file not added to album Future _addExistingSharedFile() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1)..addJpeg("admin/test1.jpg")).build(); final user1Files = [ diff --git a/app/test/use_case/remove_album_test.dart b/app/test/use_case/remove_album_test.dart index b14aecce..6df8a68b 100644 --- a/app/test/use_case/remove_album_test.dart +++ b/app/test/use_case/remove_album_test.dart @@ -131,7 +131,7 @@ Future _removeSharedAlbumFileInOtherAlbum() async { /// share (admin -> user1) for the file delete Future _removeSharedAlbumResyncedFile() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1)..addJpeg("admin/test1.jpg")).build(); final user1Files = [ diff --git a/app/test/use_case/remove_from_album_test.dart b/app/test/use_case/remove_from_album_test.dart index cc6edc7c..e059ec55 100644 --- a/app/test/use_case/remove_from_album_test.dart +++ b/app/test/use_case/remove_from_album_test.dart @@ -291,7 +291,7 @@ Future _removeFromSharedAlbumOwned() async { /// unchanged Future _removeFromSharedAlbumOwnedWithOtherShare() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1) ..addJpeg("user1/test1.jpg", ownerId: "user1")) .build(); diff --git a/app/test/use_case/remove_test.dart b/app/test/use_case/remove_test.dart index 9376da73..7746fb7d 100644 --- a/app/test/use_case/remove_test.dart +++ b/app/test/use_case/remove_test.dart @@ -238,7 +238,7 @@ Future _removeSharedAlbumFile() async { /// file share (admin -> user2) deleted Future _removeSharedAlbumSharedFile() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder(initialFileId: 1) ..addJpeg("admin/test1.jpg", ownerId: "user1")) .build(); diff --git a/app/test/use_case/scan_dir_offline_test.dart b/app/test/use_case/scan_dir_offline_test.dart index 54ae625c..3e03b998 100644 --- a/app/test/use_case/scan_dir_offline_test.dart +++ b/app/test/use_case/scan_dir_offline_test.dart @@ -116,7 +116,7 @@ Future _unsupportedFile() async { /// Expect: user1/test1.jpg, user1/test/test2.jpg Future _multiAccountRoot() async { final account = util.buildAccount(); - final user1Account = util.buildAccount(username: "user1"); + final user1Account = util.buildAccount(userId: "user1"); final files = (util.FilesBuilder() ..addJpeg("admin/test1.jpg") ..addJpeg("admin/test/test2.jpg")) From a65da95f75f23da7169c524372f946fb9d2930e1 Mon Sep 17 00:00:00 2001 From: Ming Ming Date: Tue, 12 Jul 2022 03:45:20 +0800 Subject: [PATCH 2/2] Owner display name should be shown instead of owner id --- app/lib/entity/file.dart | 10 ++++ app/lib/entity/file/data_source.dart | 1 + app/lib/entity/sqlite_table.dart | 1 + app/lib/entity/sqlite_table.g.dart | 59 +++++++++++++++++++--- app/lib/entity/sqlite_table_converter.dart | 2 + app/lib/entity/webdav_response_parser.dart | 8 +++ app/lib/widget/viewer_detail_pane.dart | 3 +- app/test/test_util.dart | 14 +++++ 8 files changed, 89 insertions(+), 9 deletions(-) diff --git a/app/lib/entity/file.dart b/app/lib/entity/file.dart index 92403c0d..c54e8761 100644 --- a/app/lib/entity/file.dart +++ b/app/lib/entity/file.dart @@ -231,6 +231,7 @@ class File with EquatableMixin { this.fileId, this.isFavorite, this.ownerId, + this.ownerDisplayName, this.metadata, this.isArchived, this.overrideDateTime, @@ -268,6 +269,7 @@ class File with EquatableMixin { fileId: json["fileId"], isFavorite: json_util.boolFromJson(json["isFavorite"]), ownerId: json["ownerId"] == null ? null : CiString(json["ownerId"]), + ownerDisplayName: json["ownerDisplayName"], trashbinFilename: json["trashbinFilename"], trashbinOriginalLocation: json["trashbinOriginalLocation"], trashbinDeletionTime: json["trashbinDeletionTime"] == null @@ -327,6 +329,9 @@ class File with EquatableMixin { if (ownerId != null) { product += "ownerId: '$ownerId', "; } + if (ownerDisplayName != null) { + product += "ownerDisplayName: '$ownerDisplayName', "; + } if (trashbinFilename != null) { product += "trashbinFilename: '$trashbinFilename', "; } @@ -362,6 +367,7 @@ class File with EquatableMixin { if (fileId != null) "fileId": fileId, if (isFavorite != null) "isFavorite": json_util.boolToJson(isFavorite), if (ownerId != null) "ownerId": ownerId.toString(), + if (ownerDisplayName != null) "ownerDisplayName": ownerDisplayName, if (trashbinFilename != null) "trashbinFilename": trashbinFilename, if (trashbinOriginalLocation != null) "trashbinOriginalLocation": trashbinOriginalLocation, @@ -386,6 +392,7 @@ class File with EquatableMixin { int? fileId, bool? isFavorite, CiString? ownerId, + String? ownerDisplayName, String? trashbinFilename, String? trashbinOriginalLocation, DateTime? trashbinDeletionTime, @@ -405,6 +412,7 @@ class File with EquatableMixin { fileId: fileId ?? this.fileId, isFavorite: isFavorite ?? this.isFavorite, ownerId: ownerId ?? this.ownerId, + ownerDisplayName: ownerDisplayName ?? this.ownerDisplayName, trashbinFilename: trashbinFilename ?? this.trashbinFilename, trashbinOriginalLocation: trashbinOriginalLocation ?? this.trashbinOriginalLocation, @@ -430,6 +438,7 @@ class File with EquatableMixin { fileId, isFavorite, ownerId, + ownerDisplayName, trashbinFilename, trashbinOriginalLocation, trashbinDeletionTime, @@ -449,6 +458,7 @@ class File with EquatableMixin { final int? fileId; final bool? isFavorite; final CiString? ownerId; + final String? ownerDisplayName; final String? trashbinFilename; final String? trashbinOriginalLocation; final DateTime? trashbinDeletionTime; diff --git a/app/lib/entity/file/data_source.dart b/app/lib/entity/file/data_source.dart index f5f7c567..74cb506d 100644 --- a/app/lib/entity/file/data_source.dart +++ b/app/lib/entity/file/data_source.dart @@ -45,6 +45,7 @@ class FileWebdavDataSource implements FileDataSource { fileid: 1, favorite: 1, ownerId: 1, + ownerDisplayName: 1, trashbinFilename: 1, trashbinOriginalLocation: 1, trashbinDeletionTime: 1, diff --git a/app/lib/entity/sqlite_table.dart b/app/lib/entity/sqlite_table.dart index 1719cbaf..30d4eb3b 100644 --- a/app/lib/entity/sqlite_table.dart +++ b/app/lib/entity/sqlite_table.dart @@ -36,6 +36,7 @@ class Files extends Table { IntColumn get usedBytes => integer().nullable()(); BoolColumn get hasPreview => boolean().nullable()(); TextColumn get ownerId => text().nullable()(); + TextColumn get ownerDisplayName => text().nullable()(); @override get uniqueKeys => [ diff --git a/app/lib/entity/sqlite_table.g.dart b/app/lib/entity/sqlite_table.g.dart index a16991bb..fa4d81ea 100644 --- a/app/lib/entity/sqlite_table.g.dart +++ b/app/lib/entity/sqlite_table.g.dart @@ -406,6 +406,7 @@ class File extends DataClass implements Insertable { final int? usedBytes; final bool? hasPreview; final String? ownerId; + final String? ownerDisplayName; File( {required this.rowId, required this.server, @@ -417,7 +418,8 @@ class File extends DataClass implements Insertable { this.isCollection, this.usedBytes, this.hasPreview, - this.ownerId}); + this.ownerId, + this.ownerDisplayName}); factory File.fromData(Map data, {String? prefix}) { final effectivePrefix = prefix ?? ''; return File( @@ -443,6 +445,8 @@ class File extends DataClass implements Insertable { .mapFromDatabaseResponse(data['${effectivePrefix}has_preview']), ownerId: const StringType() .mapFromDatabaseResponse(data['${effectivePrefix}owner_id']), + ownerDisplayName: const StringType().mapFromDatabaseResponse( + data['${effectivePrefix}owner_display_name']), ); } @override @@ -477,6 +481,9 @@ class File extends DataClass implements Insertable { if (!nullToAbsent || ownerId != null) { map['owner_id'] = Variable(ownerId); } + if (!nullToAbsent || ownerDisplayName != null) { + map['owner_display_name'] = Variable(ownerDisplayName); + } return map; } @@ -507,6 +514,9 @@ class File extends DataClass implements Insertable { ownerId: ownerId == null && nullToAbsent ? const Value.absent() : Value(ownerId), + ownerDisplayName: ownerDisplayName == null && nullToAbsent + ? const Value.absent() + : Value(ownerDisplayName), ); } @@ -525,6 +535,7 @@ class File extends DataClass implements Insertable { usedBytes: serializer.fromJson(json['usedBytes']), hasPreview: serializer.fromJson(json['hasPreview']), ownerId: serializer.fromJson(json['ownerId']), + ownerDisplayName: serializer.fromJson(json['ownerDisplayName']), ); } @override @@ -542,6 +553,7 @@ class File extends DataClass implements Insertable { 'usedBytes': serializer.toJson(usedBytes), 'hasPreview': serializer.toJson(hasPreview), 'ownerId': serializer.toJson(ownerId), + 'ownerDisplayName': serializer.toJson(ownerDisplayName), }; } @@ -556,7 +568,8 @@ class File extends DataClass implements Insertable { Value isCollection = const Value.absent(), Value usedBytes = const Value.absent(), Value hasPreview = const Value.absent(), - Value ownerId = const Value.absent()}) => + Value ownerId = const Value.absent(), + Value ownerDisplayName = const Value.absent()}) => File( rowId: rowId ?? this.rowId, server: server ?? this.server, @@ -572,6 +585,9 @@ class File extends DataClass implements Insertable { usedBytes: usedBytes.present ? usedBytes.value : this.usedBytes, hasPreview: hasPreview.present ? hasPreview.value : this.hasPreview, ownerId: ownerId.present ? ownerId.value : this.ownerId, + ownerDisplayName: ownerDisplayName.present + ? ownerDisplayName.value + : this.ownerDisplayName, ); @override String toString() { @@ -586,7 +602,8 @@ class File extends DataClass implements Insertable { ..write('isCollection: $isCollection, ') ..write('usedBytes: $usedBytes, ') ..write('hasPreview: $hasPreview, ') - ..write('ownerId: $ownerId') + ..write('ownerId: $ownerId, ') + ..write('ownerDisplayName: $ownerDisplayName') ..write(')')) .toString(); } @@ -603,7 +620,8 @@ class File extends DataClass implements Insertable { isCollection, usedBytes, hasPreview, - ownerId); + ownerId, + ownerDisplayName); @override bool operator ==(Object other) => identical(this, other) || @@ -618,7 +636,8 @@ class File extends DataClass implements Insertable { other.isCollection == this.isCollection && other.usedBytes == this.usedBytes && other.hasPreview == this.hasPreview && - other.ownerId == this.ownerId); + other.ownerId == this.ownerId && + other.ownerDisplayName == this.ownerDisplayName); } class FilesCompanion extends UpdateCompanion { @@ -633,6 +652,7 @@ class FilesCompanion extends UpdateCompanion { final Value usedBytes; final Value hasPreview; final Value ownerId; + final Value ownerDisplayName; const FilesCompanion({ this.rowId = const Value.absent(), this.server = const Value.absent(), @@ -645,6 +665,7 @@ class FilesCompanion extends UpdateCompanion { this.usedBytes = const Value.absent(), this.hasPreview = const Value.absent(), this.ownerId = const Value.absent(), + this.ownerDisplayName = const Value.absent(), }); FilesCompanion.insert({ this.rowId = const Value.absent(), @@ -658,6 +679,7 @@ class FilesCompanion extends UpdateCompanion { this.usedBytes = const Value.absent(), this.hasPreview = const Value.absent(), this.ownerId = const Value.absent(), + this.ownerDisplayName = const Value.absent(), }) : server = Value(server), fileId = Value(fileId); static Insertable custom({ @@ -672,6 +694,7 @@ class FilesCompanion extends UpdateCompanion { Expression? usedBytes, Expression? hasPreview, Expression? ownerId, + Expression? ownerDisplayName, }) { return RawValuesInsertable({ if (rowId != null) 'row_id': rowId, @@ -685,6 +708,7 @@ class FilesCompanion extends UpdateCompanion { if (usedBytes != null) 'used_bytes': usedBytes, if (hasPreview != null) 'has_preview': hasPreview, if (ownerId != null) 'owner_id': ownerId, + if (ownerDisplayName != null) 'owner_display_name': ownerDisplayName, }); } @@ -699,7 +723,8 @@ class FilesCompanion extends UpdateCompanion { Value? isCollection, Value? usedBytes, Value? hasPreview, - Value? ownerId}) { + Value? ownerId, + Value? ownerDisplayName}) { return FilesCompanion( rowId: rowId ?? this.rowId, server: server ?? this.server, @@ -712,6 +737,7 @@ class FilesCompanion extends UpdateCompanion { usedBytes: usedBytes ?? this.usedBytes, hasPreview: hasPreview ?? this.hasPreview, ownerId: ownerId ?? this.ownerId, + ownerDisplayName: ownerDisplayName ?? this.ownerDisplayName, ); } @@ -753,6 +779,9 @@ class FilesCompanion extends UpdateCompanion { if (ownerId.present) { map['owner_id'] = Variable(ownerId.value); } + if (ownerDisplayName.present) { + map['owner_display_name'] = Variable(ownerDisplayName.value); + } return map; } @@ -769,7 +798,8 @@ class FilesCompanion extends UpdateCompanion { ..write('isCollection: $isCollection, ') ..write('usedBytes: $usedBytes, ') ..write('hasPreview: $hasPreview, ') - ..write('ownerId: $ownerId') + ..write('ownerId: $ownerId, ') + ..write('ownerDisplayName: $ownerDisplayName') ..write(')')) .toString(); } @@ -849,6 +879,12 @@ class $FilesTable extends Files with TableInfo<$FilesTable, File> { late final GeneratedColumn ownerId = GeneratedColumn( 'owner_id', aliasedName, true, type: const StringType(), requiredDuringInsert: false); + final VerificationMeta _ownerDisplayNameMeta = + const VerificationMeta('ownerDisplayName'); + @override + late final GeneratedColumn ownerDisplayName = + GeneratedColumn('owner_display_name', aliasedName, true, + type: const StringType(), requiredDuringInsert: false); @override List get $columns => [ rowId, @@ -861,7 +897,8 @@ class $FilesTable extends Files with TableInfo<$FilesTable, File> { isCollection, usedBytes, hasPreview, - ownerId + ownerId, + ownerDisplayName ]; @override String get aliasedName => _alias ?? 'files'; @@ -925,6 +962,12 @@ class $FilesTable extends Files with TableInfo<$FilesTable, File> { context.handle(_ownerIdMeta, ownerId.isAcceptableOrUnknown(data['owner_id']!, _ownerIdMeta)); } + if (data.containsKey('owner_display_name')) { + context.handle( + _ownerDisplayNameMeta, + ownerDisplayName.isAcceptableOrUnknown( + data['owner_display_name']!, _ownerDisplayNameMeta)); + } return context; } diff --git a/app/lib/entity/sqlite_table_converter.dart b/app/lib/entity/sqlite_table_converter.dart index 994d9715..f3ec1dd1 100644 --- a/app/lib/entity/sqlite_table_converter.dart +++ b/app/lib/entity/sqlite_table_converter.dart @@ -92,6 +92,7 @@ class SqliteFileConverter { fileId: f.file.fileId, isFavorite: f.accountFile.isFavorite, ownerId: f.file.ownerId?.toCi(), + ownerDisplayName: f.file.ownerDisplayName, trashbinFilename: f.trash?.filename, trashbinOriginalLocation: f.trash?.originalLocation, trashbinDeletionTime: f.trash?.deletionTime, @@ -113,6 +114,7 @@ class SqliteFileConverter { usedBytes: Value(file.usedBytes), hasPreview: Value(file.hasPreview), ownerId: Value(file.ownerId!.toCaseInsensitiveString()), + ownerDisplayName: Value(file.ownerDisplayName), ); final dbAccountFile = sql.AccountFilesCompanion( account: account == null ? const Value.absent() : Value(account.rowId), diff --git a/app/lib/entity/webdav_response_parser.dart b/app/lib/entity/webdav_response_parser.dart index 7952ba49..ec2ef5e6 100644 --- a/app/lib/entity/webdav_response_parser.dart +++ b/app/lib/entity/webdav_response_parser.dart @@ -94,6 +94,7 @@ class WebdavResponseParser { int? fileId; bool? isFavorite; CiString? ownerId; + String? ownerDisplayName; Metadata? metadata; bool? isArchived; DateTime? overrideDateTime; @@ -131,6 +132,7 @@ class WebdavResponseParser { fileId = propParser.fileId; isFavorite = propParser.isFavorite; ownerId = propParser.ownerId; + ownerDisplayName = propParser.ownerDisplayName; metadata = propParser.metadata; isArchived = propParser.isArchived; overrideDateTime = propParser.overrideDateTime; @@ -152,6 +154,7 @@ class WebdavResponseParser { fileId: fileId, isFavorite: isFavorite, ownerId: ownerId, + ownerDisplayName: ownerDisplayName, metadata: metadata, isArchived: isArchived, overrideDateTime: overrideDateTime, @@ -338,6 +341,9 @@ class _FilePropParser { } else if (child.matchQualifiedName("owner-id", prefix: "http://owncloud.org/ns", namespaces: namespaces)) { _ownerId = child.innerText.toCi(); + } else if (child.matchQualifiedName("owner-display-name", + prefix: "http://owncloud.org/ns", namespaces: namespaces)) { + _ownerDisplayName = child.innerText; } else if (child.matchQualifiedName("trashbin-filename", prefix: "http://nextcloud.org/ns", namespaces: namespaces)) { _trashbinFilename = child.innerText; @@ -385,6 +391,7 @@ class _FilePropParser { int? get fileId => _fileId; bool? get isFavorite => _isFavorite; CiString? get ownerId => _ownerId; + String? get ownerDisplayName => _ownerDisplayName; Metadata? get metadata => _metadata; bool? get isArchived => _isArchived; DateTime? get overrideDateTime => _overrideDateTime; @@ -407,6 +414,7 @@ class _FilePropParser { int? _fileId; bool? _isFavorite; CiString? _ownerId; + String? _ownerDisplayName; Metadata? _metadata; bool? _isArchived; DateTime? _overrideDateTime; diff --git a/app/lib/widget/viewer_detail_pane.dart b/app/lib/widget/viewer_detail_pane.dart index 7c6a34a4..be6bb233 100644 --- a/app/lib/widget/viewer_detail_pane.dart +++ b/app/lib/widget/viewer_detail_pane.dart @@ -203,7 +203,8 @@ class _ViewerDetailPaneState extends State { color: AppTheme.getSecondaryTextColor(context), ), ), - title: Text(widget.file.ownerId!.toString()), + title: Text(widget.file.ownerDisplayName ?? + widget.file.ownerId!.toString()), subtitle: Text(L10n.global().fileSharedByDescription), ), if (_tags.isNotEmpty) diff --git a/app/test/test_util.dart b/app/test/test_util.dart index ad5213ac..bd9bd9c4 100644 --- a/app/test/test_util.dart +++ b/app/test/test_util.dart @@ -39,6 +39,7 @@ class FilesBuilder { bool isCollection = false, bool hasPreview = true, String ownerId = "admin", + String? ownerDisplayName, Metadata? metadata, }) { files.add(File( @@ -52,6 +53,7 @@ class FilesBuilder { hasPreview: hasPreview, fileId: fileId++, ownerId: ownerId.toCi(), + ownerDisplayName: ownerDisplayName ?? ownerId.toString(), metadata: metadata, )); } @@ -64,6 +66,7 @@ class FilesBuilder { DateTime? lastModified, bool hasPreview = true, String ownerId = "admin", + String? ownerDisplayName, }) => add( relativePath, @@ -73,6 +76,7 @@ class FilesBuilder { lastModified: lastModified, hasPreview: hasPreview, ownerId: ownerId, + ownerDisplayName: ownerDisplayName, ); void addJpeg( @@ -82,6 +86,7 @@ class FilesBuilder { DateTime? lastModified, bool hasPreview = true, String ownerId = "admin", + String? ownerDisplayName, OrNull? metadata, }) => add( @@ -92,6 +97,7 @@ class FilesBuilder { lastModified: lastModified, hasPreview: hasPreview, ownerId: ownerId, + ownerDisplayName: ownerDisplayName, metadata: metadata?.obj ?? Metadata( lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5), @@ -106,6 +112,7 @@ class FilesBuilder { String? etag, DateTime? lastModified, String ownerId = "admin", + String? ownerDisplayName, }) => add( relativePath, @@ -114,6 +121,7 @@ class FilesBuilder { isCollection: true, hasPreview: false, ownerId: ownerId, + ownerDisplayName: ownerDisplayName, ); void addAlbumJson( @@ -123,6 +131,7 @@ class FilesBuilder { String? etag, DateTime? lastModified, String ownerId = "admin", + String? ownerDisplayName, }) => add( "$homeDir/.com.nkming.nc_photos/albums/$filename.nc_album.json", @@ -132,6 +141,7 @@ class FilesBuilder { lastModified: lastModified, hasPreview: false, ownerId: ownerId, + ownerDisplayName: ownerDisplayName, ); final files = []; @@ -302,6 +312,7 @@ File buildAlbumFile({ DateTime? lastModified, required int fileId, String ownerId = "admin", + String? ownerDisplayName, }) => File( path: path, @@ -312,6 +323,7 @@ File buildAlbumFile({ hasPreview: false, fileId: fileId, ownerId: ownerId.toCi(), + ownerDisplayName: ownerDisplayName ?? ownerId.toString(), ); String buildAlbumFilePath( @@ -341,6 +353,7 @@ File buildJpegFile({ bool hasPreview = true, required int fileId, String ownerId = "admin", + String? ownerDisplayName, }) => File( path: path, @@ -351,6 +364,7 @@ File buildJpegFile({ hasPreview: hasPreview, fileId: fileId, ownerId: ownerId.toCi(), + ownerDisplayName: ownerDisplayName ?? ownerId.toString(), ); Share buildShare({