Add OperationList to keep track of Operations

The code in `Contacts.MainWindow` that has dealt with operations is
problematic in several ways:

* It only kept track of the last operation, and didn't do that well
  either as it could easily be overwritten
* Due to `Contacts.DeleteOperation` being irreversible, it needed
  special (and buggy) workarounds
* It repeated code for dealing with operations several times, which lead
  to copy-paste bugs.

This commit tries to fix that by introducing a `Contacts.OperationList`
object, which acts as a container and wrapper API for
`Contacts.Operation`s. The most prominent extra API it provides, is that
of postponing execution with a timeout, and being able to cancel ongoing
operations. Both of these APIs allow us to remove any special-casing we
had to do for delete operations.

It also adds a `flush` API, which we can later use to prevent the
application from completely quitting before all operations have
finished executing.

9 files changed, 293 insertions, 91 deletions

diff --git a/src/contacts-app.vala b/src/contacts-app.vala
index 767a6b6..e7b0471 100644
--- a/src/contacts-app.vala
+++ b/src/contacts-app.vala
@@ -25,6 +25,13 @@ public class Contacts.App : Adw.Application {
 
   private unowned MainWindow window;
 
+  // The operations that have been (or are being) executed
+  public Contacts.OperationList operations {
+    get;
+    private set;
+    default = new OperationList ();
+  }
+
   private const GLib.ActionEntry[] action_entries = {
     { "quit",             quit                },
     { "help",             show_help           },
@@ -195,7 +202,7 @@ public class Contacts.App : Adw.Application {
   }
 
   private void create_window () {
-    var win = new MainWindow (this.settings, this, this.contacts_store);
+    var win = new MainWindow (this.settings, this.operations, this, this.contacts_store);
     win.show ();
     this.window = win;
 
diff --git a/src/contacts-contact-pane.vala b/src/contacts-contact-pane.vala
index 0b94bc0..8e4012d 100644
--- a/src/contacts-contact-pane.vala
+++ b/src/contacts-contact-pane.vala
@@ -46,7 +46,7 @@ public class Contacts.ContactPane : Adw.Bin {
   public bool on_edit_mode = false;
   private LinkSuggestionGrid? suggestion_grid = null;
 
-  public signal void contacts_linked (string? main_contact, string linked_contact, LinkOperation operation);
+  public signal void contacts_linked (LinkOperation operation);
 
   public ContactPane (MainWindow main_window, Store contacts_store) {
     this.store = contacts_store;
@@ -61,13 +61,11 @@ public class Contacts.ContactPane : Adw.Bin {
     parent_overlay.add_overlay (this.suggestion_grid);
 
     this.suggestion_grid.suggestion_accepted.connect (() => {
-      var linked_contact = this.individual.display_name;
       var to_link = new Gee.LinkedList<Individual> ();
       to_link.add (this.individual);
       to_link.add (i);
       var operation = new LinkOperation (this.store, to_link);
-      operation.execute.begin ();
-      this.contacts_linked (null, linked_contact, operation);
+      this.contacts_linked (operation);
       remove_suggestion_grid ();
     });
 
diff --git a/src/contacts-delete-operation.vala b/src/contacts-delete-operation.vala
index 022fff5..3ddee2d 100644
--- a/src/contacts-delete-operation.vala
+++ b/src/contacts-delete-operation.vala
@@ -17,17 +17,19 @@
 
 using Folks;
 
-public class Contacts.DeleteOperation : Object, Operation {
+/**
+ * A DeleteOperation permanently deletes contacts. Note that this is an
+ * irreversible operation, so to prevent accidents, it allows you to set a
+ * timeout period during which you can cancel the operation still.
+ */
+public class Contacts.DeleteOperation : Operation {
 
   private Gee.List<Individual> individuals;
 
-  // We don't support reversing a removal. What we do instead, is put a timeout
-  // before actually executing this operation so the user has time to change
-  // their mind.
-  public bool reversable { get { return false; } }
+  public override bool reversable { get { return false; } }
 
   private string _description;
-  public string description { owned get { return this._description; } }
+  public override string description { owned get { return this._description; } }
 
   public DeleteOperation (Gee.List<Individual> individuals) {
     this.individuals = individuals;
@@ -37,10 +39,12 @@ public class Contacts.DeleteOperation : Object, Operation {
   }
 
   /**
-   * Link individuals
+   * Delete individuals
    */
-  public async void execute () throws GLib.Error {
+  public override async void execute () throws GLib.Error {
     foreach (var indiv in this.individuals) {
+      debug ("Removing individual '%s'", indiv.display_name);
+
       foreach (var persona in indiv.personas) {
         // TODO: make sure it is actually removed
         yield persona.store.remove_persona (persona);
@@ -48,8 +52,7 @@ public class Contacts.DeleteOperation : Object, Operation {
     }
   }
 
-  // See comments near the reversable property
-  protected async void _undo () throws GLib.Error {
-    throw new GLib.IOError.NOT_SUPPORTED ("Undoing not supported");
+  protected override async void _undo () throws GLib.Error {
+    // No need to do anything, since reversable is true
   }
 }
diff --git a/src/contacts-link-operation.vala b/src/contacts-link-operation.vala
index 9e75841..257fd7b 100644
--- a/src/contacts-link-operation.vala
+++ b/src/contacts-link-operation.vala
@@ -17,7 +17,7 @@
 
 using Folks;
 
-public class Contacts.LinkOperation : Object, Operation {
+public class Contacts.LinkOperation : Operation {
 
   private weak Store store;
 
@@ -25,13 +25,11 @@ public class Contacts.LinkOperation : Object, Operation {
   private Gee.HashSet<Gee.HashSet<Persona>> personas_to_link
       = new Gee.HashSet<Gee.HashSet<Persona>> ();
 
-  private bool finished { get; set; default = false; }
-
   private bool _reversable = false;
-  public bool reversable { get { return this._reversable; } }
+  public override bool reversable { get { return this._reversable; } }
 
   private string _description;
-  public string description { owned get { return this._description; } }
+  public override string description { owned get { return this._description; } }
 
   public LinkOperation (Store store, Gee.LinkedList<Individual> individuals) {
     this.store = store;
@@ -45,7 +43,7 @@ public class Contacts.LinkOperation : Object, Operation {
   /**
    * Link individuals
    */
-  public async void execute () throws GLib.Error {
+  public override async void execute () throws GLib.Error {
     var personas_to_link = new Gee.HashSet<Persona> ();
     foreach (var i in individuals) {
       var saved_personas = new Gee.HashSet<Persona> ();
@@ -64,7 +62,7 @@ public class Contacts.LinkOperation : Object, Operation {
   /**
    * Undoing means unlinking
    */
-  public async void _undo () throws GLib.Error {
+  public override async void _undo () throws GLib.Error {
     var individual = this.personas_to_link.first_match(() => {return true;})
       .first_match(() => {return true;}).individual;
 
diff --git a/src/contacts-main-window.vala b/src/contacts-main-window.vala
index 4997654..8d460de 100644
--- a/src/contacts-main-window.vala
+++ b/src/contacts-main-window.vala
@@ -31,8 +31,8 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
     { "unlink-contact", unlink_contact },
     { "delete-contact", delete_contact },
     { "sort-on", null, "s", "'surname'", sort_on_changed },
-    { "undo-operation", undo_operation_action },
-    { "undo-delete", undo_delete_action },
+    { "undo-operation", undo_operation_action, "s" },
+    { "cancel-operation", cancel_operation_action, "s" },
   };
 
   [GtkChild]
@@ -83,8 +83,6 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
   [GtkChild]
   private unowned Gtk.ActionBar actions_bar;
 
-  private bool delete_cancelled;
-
   public UiState state { get; set; default = UiState.NORMAL; }
 
   // Window state
@@ -95,12 +93,11 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
 
   public Store store { get; construct set; }
 
+  public Contacts.OperationList operations { get; construct set; }
+
   // A separate SelectionModel for all marked contacts
   private Gtk.MultiSelection marked_contacts;
 
-  // If an unduable operation was recently performed, this will be set
-  public Operation? last_operation = null;
-
   construct {
     add_action_entries (ACTION_ENTRIES, this);
 
@@ -123,9 +120,13 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
         this.add_css_class ("devel");
   }
 
-  public MainWindow (Settings settings, App app, Store contacts_store) {
+  public MainWindow (Settings settings,
+                     OperationList operations,
+                     App app,
+                     Store contacts_store) {
     Object (
       application: app,
+      operations: operations,
       settings: settings,
       store: contacts_store
     );
@@ -274,24 +275,20 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
     this.store.selection.unselect_all ();
     this.state = UiState.NORMAL;
 
-    this.last_operation = new UnlinkOperation (this.store, selected);
-    this.last_operation.execute.begin ((obj, res) => {
+    var operation = new UnlinkOperation (this.store, selected);
+    this.operations.execute.begin (operation, null, (obj, res) => {
       try {
-        this.last_operation.execute.end (res);
+        this.operations.execute.end (res);
       } catch (GLib.Error e) {
         warning ("Error unlinking individuals: %s", e.message);
       }
     });
 
-    var toast = new Adw.Toast (this.last_operation.description);
-    toast.set_button_label (_("_Undo"));
-    toast.action_name = "win.undo-operation";
-
-    this.toast_overlay.add_toast (toast);
+    add_toast_for_operation (operation, "win.undo-operation", _("_Undo"));
   }
 
   private void delete_contact (GLib.SimpleAction action, GLib.Variant? parameter) {
-    var selection = this.store.selection.get_selection ();
+    var selection = this.store.selection.get_selection ().copy ();
     if (selection.is_empty ())
       return;
 
@@ -306,24 +303,25 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
   }
 
   private void undo_operation_action (SimpleAction action, GLib.Variant? parameter) {
-    if (this.last_operation == null) {
-      warning ("Undo action was called without anything that can be undone?");
-      return;
-    }
-
-    debug ("Undoing operation '%s'", this.last_operation.description);
-    this.last_operation.undo.begin ((obj, res) => {
+    unowned var uuid = parameter.get_string ();
+    this.operations.undo_operation.begin (uuid, (obj, res) => {
       try {
-        this.last_operation.undo.end (res);
+        this.operations.undo_operation.end (res);
       } catch (GLib.Error e) {
-        warning ("Couldn't undo operation '%s': %s", this.last_operation.description, e.message);
+        warning ("Couldn't undo operation '%s': %s", uuid, e.message);
       }
-      debug ("Finished undoing operation '%s'", this.last_operation.description);
     });
   }
 
-  private void undo_delete_action (SimpleAction action, GLib.Variant? parameter) {
-    this.delete_cancelled = true;
+  private void cancel_operation_action (SimpleAction action, GLib.Variant? parameter) {
+    unowned var uuid = parameter.get_string ();
+    this.operations.cancel_operation.begin (uuid, (obj, res) => {
+      try {
+        this.operations.cancel_operation.end (res);
+      } catch (GLib.Error e) {
+        warning ("Couldn't cancel operation '%s': %s", uuid, e.message);
+      }
+    });
   }
 
   private void stop_editing_contact (SimpleAction action, GLib.Variant? parameter) {
@@ -468,19 +466,16 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
                                       selection);
 
     // Perform the operation
-    this.last_operation = new LinkOperation (this.store, list);
-    this.last_operation.execute.begin ((obj, res) => {
+    var operation = new LinkOperation (this.store, list);
+    this.operations.execute.begin (operation, null, (obj, res) => {
       try {
-        this.last_operation.execute.end (res);
+        this.operations.execute.end (res);
       } catch (GLib.Error e) {
         warning ("Error linking individuals: %s", e.message);
       }
     });
 
-    var toast = new Adw.Toast (this.last_operation.description);
-    toast.set_button_label (_("_Undo"));
-    toast.action_name = "win.undo-operation";
-    this.toast_overlay.add_toast (toast);
+    add_toast_for_operation (operation, "win.undo-operation", _("_Undo"));
   }
 
   private void delete_marked_contacts (GLib.SimpleAction action, GLib.Variant? parameter) {
@@ -498,36 +493,49 @@ public class Contacts.MainWindow : Adw.ApplicationWindow {
 
     var individuals = bitset_to_individuals (this.store.filter_model,
                                              selection);
-    this.last_operation = new DeleteOperation (individuals);
-    var toast = new Adw.Toast (this.last_operation.description);
-    toast.set_button_label (_("_Undo"));
-    toast.action_name = "win.undo-delete";
-
-    this.delete_cancelled = false;
-    toast.dismissed.connect (() => {
-        if (this.delete_cancelled) {
-          this.contacts_list.set_contacts_visible (selection, true);
-          this.state = UiState.SHOWING;
-        } else {
-          this.last_operation.execute.begin ((obj, res) => {
-              try {
-                this.last_operation.execute.end (res);
-              } catch (Error e) {
-                debug ("Coudln't remove persona: %s", e.message);
-              }
-          });
-        }
+
+    // NOTE: we'll do this with a timeout, since the operation is not reversable
+    var op = new DeleteOperation (individuals);
+
+    var cancellable = new Cancellable ();
+    cancellable.cancelled.connect ((c) => {
+      this.contacts_list.set_contacts_visible (selection, true);
+      this.state = UiState.SHOWING;
     });
 
-    this.toast_overlay.add_toast (toast);
+    var toast = add_toast_for_operation (op, "win.cancel-operation", _("_Cancel"));
+    this.operations.execute_with_timeout.begin (op, toast.timeout, cancellable, (obj, res) => {
+      try {
+        this.operations.execute_with_timeout.end (res);
+      } catch (GLib.Error e) {
+        warning ("Error removing individuals: %s", e.message);
+      }
+    });
+  }
+
+  private void contact_pane_contacts_linked_cb (LinkOperation operation) {
+    add_toast_for_operation (operation, "win.undo-operation", _("_Undo"));
+
+    this.operations.execute.begin (operation, null, (obj, res) => {
+      try {
+        this.operations.execute.end (res);
+      } catch (GLib.Error e) {
+        warning ("Error linking individuals: %s", e.message);
+      }
+    });
   }
 
-  private void contact_pane_contacts_linked_cb (string? main_contact, string linked_contact, LinkOperation operation) {
-    this.last_operation = operation;
-    var toast = new Adw.Toast (this.last_operation.description);
-    toast.set_button_label (_("_Undo"));
-    toast.action_name = "win.undo-operation";
+  private Adw.Toast add_toast_for_operation (Operation operation,
+                                             string? action_name = null,
+                                             string? action_label = null) {
+    var toast = new Adw.Toast (operation.description);
+    if (action_name != null) {
+      toast.set_button_label (action_label);
+      toast.action_name = action_name;
+      toast.action_target = operation.uuid;
+    }
     this.toast_overlay.add_toast (toast);
+    return toast;
   }
 
   // Little helper
diff --git a/src/contacts-operation-list.vala b/src/contacts-operation-list.vala
new file mode 100644
index 0000000..c895bf1
--- /dev/null
+++ b/src/contacts-operation-list.vala
@@ -0,0 +1,181 @@
+/*
+ * Copyright (C) 2022 Niels De Graef <nielsdegraef@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/**
+ * A list of {@link Contacts.Operation}s.
+ */
+public class Contacts.OperationList : Object {
+
+  // A helper class to add extra (private) API to operations
+  private class OpEntry {
+    public Operation operation;
+    public Cancellable? cancellable;
+    public uint timeout_src;
+    public bool finished;
+
+    public OpEntry (Operation operation, Cancellable? cancellable = null) {
+      this.operation = operation;
+      this.cancellable = cancellable;
+      this.timeout_src = 0;
+      this.finished = false;
+    }
+
+    public bool is_cancelled () {
+      return (this.cancellable != null) && this.cancellable.is_cancelled ();
+    }
+  }
+
+  private GenericArray<OpEntry?> operations = new GenericArray<OpEntry?> ();
+
+  public OperationList () {
+  }
+
+  /** Asynchronously executes the given operation */
+  public async void execute (Operation operation,
+                             Cancellable? cancellable) throws GLib.Error {
+    yield execute_with_timeout (operation, 0, cancellable);
+  }
+
+  /** Asynchronously executes the given operation after a timeout */
+  public async void execute_with_timeout (Operation operation,
+                                          uint timeout,
+                                          Cancellable? cancellable) throws GLib.Error {
+    // Create a new OpEntry to keep track and add it
+    var entry = new OpEntry (operation, cancellable);
+    this.operations.add (entry);
+
+    // Schedule the callback
+    SourceFunc callback = execute_with_timeout.callback;
+    if (timeout > 0) {
+      entry.timeout_src = Timeout.add_seconds (timeout, (owned) callback);
+    } else {
+      entry.timeout_src = Idle.add ((owned) callback);
+    }
+
+    // Let the main loop take control again, our callback should be scheduled
+    // at this point.
+    yield;
+
+    yield execute_operation_now (entry);
+  }
+
+  /** Cancel the operation with the given UUID */
+  public async void cancel_operation (string uuid) throws GLib.Error {
+    debug ("Cancelling operation '%s'", uuid);
+
+    unowned var entry = find_by_uuid (uuid);
+    if (entry == null || entry.finished) { // FIXME: throw some error
+      warning ("Can't cancel operation with uuid '%s': not found", uuid);
+      return;
+    }
+
+    if (entry.finished) { // FIXME: throw some error
+      warning ("Can't cancel operation '%s': already finished",
+               entry.operation.description);
+      return;
+    }
+
+    if (entry.is_cancelled ())
+      return; // no-op
+
+    entry.cancellable.cancel ();
+  }
+
+  /**
+   * Undo the operation with the given UUID
+   */
+  public async void undo_operation (string uuid) throws GLib.Error {
+    debug ("Undoing operation '%s'", uuid);
+
+    unowned var entry = find_by_uuid (uuid);
+    if (entry == null) { // FIXME: throw some error
+      warning ("Can't undo operation with uuid '%s': not found", uuid);
+      return;
+    }
+
+    if (!entry.operation.reversable || !entry.finished || entry.is_cancelled ()) {
+      warning ("Can't undo operation with uuid '%s'", uuid);
+      return;
+    }
+
+    yield entry.operation.undo ();
+  }
+
+  /**
+   * Returns whether there are operations that are still unfinished
+   */
+  public bool has_pending_operations () {
+    return (find_next_todo () != null);
+  }
+
+  /**
+   * Flushes the current list of operaions. This will execute any operation
+   * that was still scheduled for execution.
+   */
+  public async void flush () throws GLib.Error {
+    if (!has_pending_operations ())
+      return;
+
+    debug ("Flushing %u operations", this.operations.length);
+
+    unowned var entry = find_next_todo ();
+    while (entry != null) {
+      debug ("Flushing operation '%s'", entry.operation.description);
+      yield execute_operation_now (entry);
+      entry = find_next_todo ();
+    }
+
+    this.operations.remove_range (0, this.operations.length);
+  }
+
+  private unowned OpEntry? find_next_todo () {
+    for (uint i = 0; i < operations.length; i++) {
+      unowned var entry = operations[i];
+      if (!entry.finished && !entry.is_cancelled ())
+        return entry;
+    }
+
+    return null;
+  }
+
+  private unowned OpEntry? find_by_uuid (string uuid) {
+    for (uint i = 0; i < operations.length; i++) {
+      if (operations[i].operation.uuid == uuid)
+        return operations[i];
+    }
+
+    return null;
+  }
+
+  private async void execute_operation_now (OpEntry? entry) throws GLib.Error {
+    // Clear any scheduled callbacks
+    entry.timeout_src = 0;
+
+    // Check if it might've been scheduled in the meantime
+    if (entry.is_cancelled ()) {
+      throw new IOError.CANCELLED ("Operation '%s' was cancelled",
+                                   entry.operation.description);
+    }
+
+    debug ("Starting execution of operation '%s' (%s)",
+           entry.operation.description, entry.operation.uuid);
+    yield entry.operation.execute ();
+    entry.finished = true;
+    debug ("Finished operation '%s' (%s)",
+           entry.operation.description, entry.operation.uuid);
+  }
+}
diff --git a/src/contacts-operation.vala b/src/contacts-operation.vala
index f77944b..97a430c 100644
--- a/src/contacts-operation.vala
+++ b/src/contacts-operation.vala
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2021 Niels De Graef <nielsdegraef@redhat.com>
+ * Copyright (C) 2021 Niels De Graef <nielsdegraef@gmail.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,8 +22,14 @@
  *
  * Since some operations might not be able undoable later onwards, there is a
  * property `reversable` that you should check first before calling undo().
+ *
+ * Note that you probably shouldn't be calling the execute() method directly,
+ * but use the API provided by {@link OperationQueue} instead.
  */
-public interface Contacts.Operation : Object {
+public abstract class Contacts.Operation : Object {
+
+  /** The UUID of the operation */
+  public string uuid { get; private set; default = Uuid.string_random (); }
 
   /**
    * Whether undo() can be called on this object
diff --git a/src/contacts-unlink-operation.vala b/src/contacts-unlink-operation.vala
index 7b67679..64d6a6d 100644
--- a/src/contacts-unlink-operation.vala
+++ b/src/contacts-unlink-operation.vala
@@ -17,7 +17,7 @@
 
 using Folks;
 
-public class Contacts.UnlinkOperation : Object, Operation {
+public class Contacts.UnlinkOperation : Operation {
 
   private weak Store store;
 
@@ -26,10 +26,10 @@ public class Contacts.UnlinkOperation : Object, Operation {
   private Gee.HashSet<Persona> personas = new Gee.HashSet<Persona> ();
 
   private bool _reversable = false;
-  public bool reversable { get { return this._reversable; } }
+  public override bool reversable { get { return this._reversable; } }
 
   private string _description;
-  public string description { owned get { return this._description; } }
+  public override string description { owned get { return this._description; } }
 
   public UnlinkOperation (Store store, Individual main) {
     this.store = store;
@@ -38,7 +38,7 @@ public class Contacts.UnlinkOperation : Object, Operation {
   }
 
   /* Remove a personas from individual */
-  public async void execute () throws GLib.Error {
+  public override async void execute () throws GLib.Error {
     foreach (var persona in this.individual.personas)
       this.personas.add (persona);
 
@@ -48,7 +48,7 @@ public class Contacts.UnlinkOperation : Object, Operation {
   }
 
   /* Undo the unlinking */
-  public async void _undo () throws GLib.Error {
+  public override async void _undo () throws GLib.Error {
     yield this.store.aggregator.link_personas (personas);
     this._reversable = false;
     notify_property ("reversable");
diff --git a/src/meson.build b/src/meson.build
index 107514e..1f3890e 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -13,6 +13,7 @@ libcontacts_sources = files(
   'contacts-individual-sorter.vala',
   'contacts-link-operation.vala',
   'contacts-operation.vala',
+  'contacts-operation-list.vala',
   'contacts-query-filter.vala',
   'contacts-store.vala',
   'contacts-typeset.vala',