ID

eaabfa8

date

2023-05-23 06:40:47+00:00

author

Alex Chan <alex@alexwlchan.net>

parent

0d3fa13

message

Optimise the state toggling logic

Previously we were issuing three change requests for every review action,
e.g. running `toggle-approved` would:

1.  look at whether a photo was in the 'approved' album, and add/remove
    as appropriate
2.  remove a photo from the 'rejected' album
3.  remove a photo from the 'needs action' album

But in practice actions (2) and (3) are usually redundant; most of what
I'm reviewing is unreviewed and isn't in any of the three albums.
Since we're already getting a complete list of albums that contain this
photo, we can check which albums need to change and only issue the
appropriate change requests.

I haven't profiled extensively, but in informal testing this does seem
to make the action maybe ~25% faster?

changed files

1 file, 46 additions, 9 deletions

• actions/run_action.swift

Changed files

actions/run_action.swift (4989) → actions/run_action.swift (6346)

diff --git a/actions/run_action.swift b/actions/run_action.swift
index 9f335b8..49c5cfe 100644
--- a/actions/run_action.swift
+++ b/actions/run_action.swift
@@ -69,6 +69,18 @@ func getPhoto(withLocalIdentifier localIdentifier: String) -> PHAsset {
 }
 
 extension PHAsset {
+  func albums() -> [PHAssetCollection] {
+    var result: [PHAssetCollection] = []
+
+    PHAssetCollection
+      .fetchAssetCollectionsContaining(self, with: .album, options: nil)
+      .enumerateObjects({ (collection, index, stop) in
+        result.append(collection)
+      })
+
+    return result
+  }
+
   /// Returns true if an asset is in the given album, false otherwise.
   func isInAlbum(_ album: PHAssetCollection) -> Bool {
     var result = false
@@ -95,6 +107,17 @@ extension PHAsset {
     changeAlbum.removeAssets([self] as NSFastEnumeration)
   }
 
+  /// Add a photo to an album.
+  ///
+  /// This expects to be run inside a performChangesAndWait change block;
+  /// see https://developer.apple.com/documentation/photokit/phphotolibrary/1620747-performchangesandwait.
+  func add(toAlbum album: PHAssetCollection) -> Void {
+    let changeAlbum =
+      PHAssetCollectionChangeRequest(for: album)!
+
+    changeAlbum.addAssets([self] as NSFastEnumeration)
+  }
+
   /// Toggle a photo's inclusion in an album.
   ///
   /// If the photo is already in the album, remove it.  If the photo isn't
@@ -138,22 +161,36 @@ try PHPhotoLibrary.shared().performChangesAndWait {
       let rejected = getAlbum(withName: "Rejected")
       let needsAction = getAlbum(withName: "Needs Action")
 
-      if action == "toggle-approved" {
-        photo.toggle(inAlbum: approved)
-      } else {
+      let albums = photo.albums()
+
+      let isApproved = albums.contains(approved)
+      let isRejected = albums.contains(rejected)
+      let isNeedsAction = albums.contains(needsAction)
+
+      // Strictly speaking, the first condition is a combination of two:
+      //
+      //   1. The action is `toggle-approved` and the photo is approved,
+      //      in which case toggling means un-approving it.
+      //   2. The action is anything else and the photo is approved, in
+      //      which case setting the new status means removing approved.
+      //
+      // Similar logic applies for all three conditions.
+      if isApproved {
         photo.remove(fromAlbum: approved)
+      } else if action == "toggle-approved" {
+        photo.add(toAlbum: approved)
       }
 
-      if action == "toggle-rejected" {
-        photo.toggle(inAlbum: rejected)
-      } else {
+      if isRejected {
         photo.remove(fromAlbum: rejected)
+      } else if action == "toggle-rejected" {
+        photo.add(toAlbum: rejected)
       }
 
-      if action == "toggle-needs-action" {
-        photo.toggle(inAlbum: needsAction)
-      } else {
+      if isNeedsAction {
         photo.remove(fromAlbum: needsAction)
+      } else if action == "toggle-needs-action" {
+        photo.add(toAlbum: needsAction)
       }
 
     case "toggle-cross-stitch":