Fix reshared file listed as missing share

This commit is contained in:
Ming Ming 2021-12-07 22:28:10 +08:00
parent fc779ee2e2
commit 97799f5f1b
7 changed files with 126 additions and 21 deletions

View file

@ -312,10 +312,13 @@ class ListAlbumShareOutlierBloc extends Bloc<ListAlbumShareOutlierBlocEvent,
List<Object> errors,
) async {
final shareItems = <ListAlbumShareOutlierShareItem>[];
final shares = (await ListShare(_c)(account, fileItem.file))
.where((element) => element.shareType == ShareType.user)
final shares = (await ListShare(_c)(
account,
fileItem.file,
isIncludeReshare: true,
))
.where((s) => s.shareType == ShareType.user)
.toList();
final sharees = shares.map((s) => s.shareWith!).toSet();
final albumSharees = albumShares.keys.toSet();
final managedAlbumSharees = albumShares.values
.where((s) => _isItemSharePairOfInterest(account, album, fileItem, s))
@ -324,9 +327,11 @@ class ListAlbumShareOutlierBloc extends Bloc<ListAlbumShareOutlierBlocEvent,
_log.info(
"[_processSingleFileItem] Sharees: ${albumSharees.map((s) => managedAlbumSharees.contains(s) ? "(managed)$s" : s).toReadableString()} for file: ${logFilename(fileItem.file.path)}");
// check only against sharees that are managed by us
// check all shares (including reshares) against sharees that are managed by
// us
final allSharees = shares.map((s) => s.shareWith!).toSet();
var missings = managedAlbumSharees
.difference(sharees)
.difference(allSharees)
// Can't share to ourselves or the file owner
.where((s) => s != account.username && s != fileItem.file.ownerId)
.toList();
@ -338,9 +343,13 @@ class ListAlbumShareOutlierBloc extends Bloc<ListAlbumShareOutlierBlocEvent,
ListAlbumShareOutlierMissingShareItem(as.userId, as.displayName));
}
// check against all sharees. Otherwise non-managed sharees will be listed
// and are quite confusing
final extras = sharees.difference(albumSharees);
// check owned shares against all album sharees. Use all album sharees such
// that non-managed sharees will not be listed
final ownedSharees = shares
.where((s) => s.uidOwner == account.username)
.map((s) => s.shareWith!)
.toSet();
final extras = ownedSharees.difference(albumSharees);
_log.info(
"[_processSingleFileItem] Extra shares: ${extras.toReadableString()} for file: ${logFilename(fileItem.file.path)}");
for (final e in extras) {

View file

@ -91,6 +91,7 @@ class Share with EquatableMixin {
required this.stime,
required this.uidOwner,
required this.displaynameOwner,
required this.uidFileOwner,
required String path,
required this.itemType,
required this.mimeType,
@ -108,6 +109,7 @@ class Share with EquatableMixin {
"stime: $stime, "
"uidOwner: $uidOwner, "
"displaynameOwner: $displaynameOwner, "
"uidFileOwner: $uidFileOwner, "
"path: $path, "
"itemType: $itemType, "
"mimeType: $mimeType, "
@ -125,6 +127,7 @@ class Share with EquatableMixin {
stime,
uidOwner,
displaynameOwner,
uidFileOwner,
path,
itemType,
mimeType,
@ -140,6 +143,7 @@ class Share with EquatableMixin {
final DateTime stime;
final CiString uidOwner;
final String displaynameOwner;
final CiString uidFileOwner;
final String path;
final ShareItemType itemType;
final String mimeType;
@ -157,8 +161,12 @@ class ShareRepo {
ShareRepo(this.dataSrc);
/// See [ShareDataSource.list]
Future<List<Share>> list(Account account, File file) =>
dataSrc.list(account, file);
Future<List<Share>> list(
Account account,
File file, {
bool? isIncludeReshare,
}) =>
dataSrc.list(account, file, isIncludeReshare: isIncludeReshare);
/// See [ShareDataSource.listDir]
Future<List<Share>> listDir(Account account, File dir) =>
@ -196,7 +204,11 @@ class ShareRepo {
abstract class ShareDataSource {
/// List all shares from a given file
Future<List<Share>> list(Account account, File file);
Future<List<Share>> list(
Account account,
File file, {
bool? isIncludeReshare,
});
/// List all shares from a given directory
Future<List<Share>> listDir(Account account, File dir);

View file

@ -11,10 +11,15 @@ import 'package:nc_photos/type.dart';
class ShareRemoteDataSource implements ShareDataSource {
@override
list(Account account, File file) async {
list(
Account account,
File file, {
bool? isIncludeReshare,
}) async {
_log.info("[list] ${file.path}");
final response = await Api(account).ocs().filesSharing().shares().get(
path: file.strippedPath,
reshares: isIncludeReshare,
);
return _onListResult(response);
}
@ -150,6 +155,7 @@ class _ShareParser {
stime: DateTime.fromMillisecondsSinceEpoch(json["stime"] * 1000),
uidOwner: CiString(json["uid_owner"]),
displaynameOwner: json["displayname_owner"],
uidFileOwner: CiString(json["uid_file_owner"]),
path: json["path"],
itemType: itemType,
mimeType: json["mimetype"],

View file

@ -14,7 +14,11 @@ class ListShare {
static bool require(DiContainer c) =>
DiContainer.has(c, DiType.shareRepo) && DiContainer.has(c, DiType.appDb);
Future<List<Share>> call(Account account, File file) async {
Future<List<Share>> call(
Account account,
File file, {
bool? isIncludeReshare,
}) async {
try {
if (file_util.getUserDirName(file) != account.username) {
file = await FindFile(_c.appDb)(account, file.fileId!);
@ -23,7 +27,11 @@ class ListShare {
// file not found
_log.warning("[call] File not found in db: ${logFilename(file.path)}");
}
return _c.shareRepo.list(account, file);
return _c.shareRepo.list(
account,
file,
isIncludeReshare: isIncludeReshare,
);
}
final DiContainer _c;

View file

@ -23,6 +23,8 @@ void main() {
"missing managed share added by others");
_testQuerySharedAlbumMissingUnmanagedShareOtherAdded(
"missing unmanaged share added by others");
_testQuerySharedAlbumMissingManagedShareOtherReshared(
"missing managed share reshared by others");
_testQuerySharedAlbumMissingJsonShare("missing json share");
_testQuerySharedAlbumExtraShare("extra share");
@ -232,6 +234,60 @@ void _testQuerySharedAlbumMissingManagedShareOtherAdded(String description) {
);
}
/// Query a shared album (admin -> user1, user2), with file managed by admin,
/// shared (admin -> user1) and reshared (user1 -> user2)
///
/// Expect: emit empty list
void _testQuerySharedAlbumMissingManagedShareOtherReshared(String description) {
final account = util.buildAccount();
final user1Account = util.buildAccount(username: "user1");
final files =
(util.FilesBuilder(initialFileId: 1)..addJpeg("admin/test1.jpg")).build();
final user1Files = [
files[0].copyWith(path: "remote.php/dav/files/user1/dir/test1.jpg"),
];
final album = (util.AlbumBuilder()
..addFileItem(files[0])
..addShare("user1")
..addShare("user2"))
.build();
final albumFile = album.albumFile!;
late final DiContainer c;
blocTest<ListAlbumShareOutlierBloc, ListAlbumShareOutlierBlocState>(
description,
setUp: () async {
c = DiContainer(
shareRepo: MockShareMemoryRepo([
util.buildShare(id: "0", file: albumFile, shareWith: "user1"),
util.buildShare(id: "1", file: albumFile, shareWith: "user2"),
util.buildShare(id: "2", file: files[0], shareWith: "user1"),
util.buildShare(
id: "3",
file: user1Files[0],
uidOwner: "user1",
shareWith: "user2"),
]),
shareeRepo: MockShareeMemoryRepo([
util.buildSharee(shareWith: "user1".toCi()),
util.buildSharee(shareWith: "user2".toCi()),
]),
appDb: await MockAppDb().applyFuture((obj) async {
await util.fillAppDb(obj, account, files);
await util.fillAppDb(obj, user1Account, user1Files);
}),
);
},
build: () => ListAlbumShareOutlierBloc(c),
act: (bloc) => bloc.add(ListAlbumShareOutlierBlocQuery(account, album)),
wait: const Duration(milliseconds: 500),
expect: () => [
ListAlbumShareOutlierBlocLoading(account, []),
ListAlbumShareOutlierBlocSuccess(account, []),
],
);
}
/// Query a shared album (admin -> user1, user2), with file added by user1,
/// managed by user1, not shared (user1 -> user2)
///

View file

@ -226,7 +226,11 @@ class MockShareRepo implements ShareRepo {
}
@override
Future<List<Share>> list(Account account, File file) {
Future<List<Share>> list(
Account account,
File file, {
bool? isIncludeReshare,
}) {
throw UnimplementedError();
}
@ -260,12 +264,20 @@ class MockShareMemoryRepo extends MockShareRepo {
}
@override
list(Account account, File file) async {
return shares
.where((element) =>
element.path == file.strippedPath &&
element.uidOwner == account.username)
.toList();
list(
Account account,
File file, {
bool? isIncludeReshare,
}) async {
return shares.where((s) {
if (s.itemSource != file.fileId) {
return false;
} else if (isIncludeReshare == true || s.uidOwner == account.username) {
return true;
} else {
return false;
}
}).toList();
}
@override
@ -276,6 +288,7 @@ class MockShareMemoryRepo extends MockShareRepo {
stime: DateTime.utc(2020, 1, 2, 3, 4, 5),
uidOwner: account.username,
displaynameOwner: account.username.toString(),
uidFileOwner: file.ownerId!,
path: file.strippedPath,
itemType: ShareItemType.file,
mimeType: file.contentType ?? "",

View file

@ -250,6 +250,7 @@ Share buildShare({
stime: stime ?? DateTime.utc(2020, 1, 2, 3, 4, 5),
uidOwner: uidOwner.toCi(),
displaynameOwner: displaynameOwner ?? uidOwner,
uidFileOwner: file.ownerId!,
path: file.strippedPath,
itemType: ShareItemType.file,
mimeType: file.contentType ?? "",