From b23c1286c12a53948c09a6b245eac79b54028eae Mon Sep 17 00:00:00 2001
From: Ming Ming <nkming2@gmail.com>
Date: Sat, 27 Nov 2021 23:30:33 +0800
Subject: [PATCH] Fix not removing shares correctly in RemoveAlbum

---
 lib/use_case/remove_album.dart | 88 +++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 32 deletions(-)

diff --git a/lib/use_case/remove_album.dart b/lib/use_case/remove_album.dart
index 2800d649..b43d4f55 100644
--- a/lib/use_case/remove_album.dart
+++ b/lib/use_case/remove_album.dart
@@ -5,11 +5,13 @@ import 'package:nc_photos/entity/album/item.dart';
 import 'package:nc_photos/entity/album/provider.dart';
 import 'package:nc_photos/entity/file.dart';
 import 'package:nc_photos/entity/share.dart';
+import 'package:nc_photos/or_null.dart';
 import 'package:nc_photos/pref.dart';
 import 'package:nc_photos/use_case/list_share.dart';
 import 'package:nc_photos/use_case/remove.dart';
 import 'package:nc_photos/use_case/remove_share.dart';
 import 'package:nc_photos/use_case/unshare_file_from_album.dart';
+import 'package:nc_photos/use_case/update_album.dart';
 
 class RemoveAlbum {
   const RemoveAlbum(this.fileRepo, this.albumRepo, this.shareRepo, this.pref);
@@ -18,44 +20,66 @@ class RemoveAlbum {
   Future<void> call(Account account, Album album) async {
     _log.info("[call] Remove album: $album");
     if (pref.isLabEnableSharedAlbumOr(false)) {
-      final files = <File>[];
-      if (album.provider is AlbumStaticProvider) {
-        files.addAll(AlbumStaticProvider.of(album)
-            .items
-            .whereType<AlbumFileItem>()
-            .map((e) => e.file));
-      }
-      final albumShares =
-          (await ListShare(shareRepo)(account, album.albumFile!))
-              .where((element) => element.shareType == ShareType.user)
-              .toList();
-      final albumShareWith = albumShares.map((e) => e.shareWith!).toList();
-      // remove file shares if necessary
-      if (files.isNotEmpty && albumShareWith.isNotEmpty) {
-        try {
-          await UnshareFileFromAlbum(shareRepo, fileRepo, albumRepo)(
-              account, album, files, albumShareWith);
-        } catch (e, stackTrace) {
-          _log.severe(
-              "[call] Failed while UnshareFileFromAlbum", e, stackTrace);
-        }
-      }
-      // then remove the album file
-      // Nextcloud currently will restore also the shares after restoring from
-      // trash, but we aren't handling it for the files, so
-      for (final s in albumShares) {
-        try {
-          await RemoveShare(shareRepo)(account, s);
-        } catch (e, stackTrace) {
-          _log.severe("[call] Failed while RemoveShare: $s", e, stackTrace);
-        }
-      }
+      // remove shares from the album json. This should be the first so if this
+      // fail the whole op can fail safely
+      await UpdateAlbum(albumRepo)(
+        account,
+        album.copyWith(
+          shares: OrNull(null),
+        ),
+      );
+      // remove file shares
+      await _unshareFiles(account, album);
+      // remove shares for the album json itself
+      await _unshareAlbumFile(account, album);
     }
     // you can't add an album to another album, so passing null here can save
     // a few queries
     await Remove(fileRepo, null)(account, album.albumFile!);
   }
 
+  Future<void> _unshareFiles(Account account, Album album) async {
+    if (album.shares?.isNotEmpty != true ||
+        album.provider is! AlbumStaticProvider) {
+      return;
+    }
+    final albumShares = (album.shares!.map((e) => e.userId).toList()
+          ..add(album.albumFile!.ownerId ?? account.username))
+        .where((element) => element != account.username)
+        .toList();
+    if (albumShares.isEmpty) {
+      return;
+    }
+    final files = AlbumStaticProvider.of(album)
+        .items
+        .whereType<AlbumFileItem>()
+        .map((e) => e.file)
+        .toList();
+    if (files.isEmpty) {
+      return;
+    }
+    try {
+      await UnshareFileFromAlbum(shareRepo, fileRepo, albumRepo)(
+          account, album, files, albumShares);
+    } catch (e, stackTrace) {
+      _log.shout(
+          "[_unshareFiles] Failed while UnshareFileFromAlbum", e, stackTrace);
+    }
+  }
+
+  Future<void> _unshareAlbumFile(Account account, Album album) async {
+    final shares = (await ListShare(shareRepo)(account, album.albumFile!))
+        .where((s) => s.shareType == ShareType.user);
+    for (final s in shares) {
+      try {
+        await RemoveShare(shareRepo)(account, s);
+      } catch (e, stackTrace) {
+        _log.shout(
+            "[_unshareAlbumFile] Failed while RemoveShare: $s", e, stackTrace);
+      }
+    }
+  }
+
   final FileRepo fileRepo;
   final AlbumRepo albumRepo;
   final ShareRepo shareRepo;