[gnome-contacts/nielsdg/new-operations: 1/2] Add OperationList to keep track of Operations
- From: Niels De Graef <nielsdg src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-contacts/nielsdg/new-operations: 1/2] Add OperationList to keep track of Operations
- Date: Sun, 5 Jun 2022 13:57:26 +0000 (UTC)
commit f8dfeb5047d2127ed45663e194d189df1a294d0b
Author: Niels De Graef <nielsdegraef gmail com>
Date: Sun Jun 5 09:02:00 2022 +0200
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.
src/contacts-app.vala | 9 +-
src/contacts-contact-pane.vala | 6 +-
src/contacts-delete-operation.vala | 25 +++---
src/contacts-link-operation.vala | 12 ++-
src/contacts-main-window.vala | 130 +++++++++++++++-------------
src/contacts-operation-list.vala | 171 +++++++++++++++++++++++++++++++++++++
src/contacts-operation.vala | 10 ++-
src/contacts-unlink-operation.vala | 10 +--
src/meson.build | 1 +
9 files changed, 283 insertions(+), 91 deletions(-)
---
diff --git a/src/contacts-app.vala b/src/contacts-app.vala
index 767a6b64..e7b04714 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 258104e2..a671f871 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 022fff5e..3ddee2d8 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 9e75841d..257fd7b7 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 49976543..8d460dee 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 00000000..7be24930
--- /dev/null
+++ b/src/contacts-operation-list.vala
@@ -0,0 +1,171 @@
+/*
+ * 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 ();
+ }
+
+ /**
+ * Flushes the current list of operaions. This will execute any operation
+ * that was still scheduled for execution.
+ */
+ public async void flush () throws GLib.Error {
+ 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 f77944b4..97a430c6 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 7b67679b..64d6a6da 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 107514e7..1f3890e6 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',
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
