Streamline refresh flow

src/Application.vala

@@ -17,8 +17,6 @@ public class AppCenter.App : Gtk.Application {
         { null }
     };
 
-    private const int SECONDS_AFTER_NETWORK_UP = 60;
-
     public static bool show_updates;
     public static bool silent;
     public static string? local_path;
@@ -139,7 +137,7 @@ public class AppCenter.App : Gtk.Application {
         refresh_action = new SimpleAction ("refresh", null);
         refresh_action.set_enabled (!Utils.is_running_in_guest_session ());
         refresh_action.activate.connect (() => {
-            update_manager.update_cache.begin (true);
+            update_manager.refresh.begin ();
         });
 
         repair_action = new SimpleAction ("repair", null);
@@ -189,22 +187,13 @@ public class AppCenter.App : Gtk.Application {
     }
 
     public override void activate () {
-        unowned var update_manager = AppCenterCore.UpdateManager.get_default ();
-
         if (first_activation) {
             first_activation = false;
             hold ();
         }
 
         if (silent) {
             request_background.begin ();
-
-            NetworkMonitor.get_default ().network_changed.connect ((available) => {
-                schedule_cache_update (!available);
-            });
-
-            // Don't force a cache refresh for the silent daemon, it'll run if it was >24 hours since the last one
-            update_manager.update_cache.begin (false);
             silent = false;
             return;
         }
@@ -221,7 +210,8 @@ public class AppCenter.App : Gtk.Application {
 
         if (active_window == null) {
             // Force a Flatpak cache refresh when the window is created, so we get new apps
-            update_manager.update_cache.begin (true);
+            // TODO: Think about this, maybe only refresh cache?
+            AppCenterCore.UpdateManager.get_default ().refresh.begin ();
 
             var main_window = new MainWindow (this);
             add_window (main_window);
@@ -312,27 +302,6 @@ public class AppCenter.App : Gtk.Application {
         base.dbus_unregister (connection, object_path);
     }
 
-    private uint cache_update_timeout_id = 0;
-    private void schedule_cache_update (bool cancel = false) {
-        unowned var update_manager = AppCenterCore.UpdateManager.get_default ();
-
-        if (cache_update_timeout_id > 0) {
-            Source.remove (cache_update_timeout_id);
-            cache_update_timeout_id = 0;
-        }
-
-        if (cancel) {
-            update_manager.cancel_updates (true); // Also stops timeouts.
-            return;
-        } else {
-            cache_update_timeout_id = Timeout.add_seconds (SECONDS_AFTER_NETWORK_UP, () => {
-                update_manager.update_cache.begin ();
-                cache_update_timeout_id = 0;
-                return false;
-            });
-        }
-    }
-
     private void on_operation_finished (AppCenterCore.Package package, AppCenterCore.Package.State operation, Error? error) {
         switch (operation) {
             case AppCenterCore.Package.State.INSTALLING:

src/Core/FlatpakBackend.vala

@@ -397,6 +397,7 @@ public class AppCenterCore.FlatpakBackend : Object {
 
         reload_appstream_pool ();
         get_installed_applications.begin (null);
+        get_updates.begin (null);
     }
 
     public void notify_package_changed (Package package) {

src/Core/UpdateManager.vala

@@ -22,6 +22,8 @@ public class AppCenterCore.UpdateManager : Object {
 
     private const int SECONDS_BETWEEN_REFRESHES = 60 * 60 * 24;
 
+    public bool refreshing { get; private set; default = false; }
+
     public bool can_update_all {
         get {
             unowned var fp_client = FlatpakBackend.get_default ();
@@ -38,22 +40,98 @@ public class AppCenterCore.UpdateManager : Object {
         }
     }
 
-    private GLib.Cancellable cancellable;
-    private GLib.DateTime last_cache_update = null;
-    private uint update_cache_timeout_id = 0;
-    private bool refresh_in_progress = false;
+    private GLib.DateTime last_refresh_time;
+    private bool refresh_required = false;
+    private uint refresh_timeout_id = 0;
 
     construct {
-        cancellable = new GLib.Cancellable ();
-
-        last_cache_update = new DateTime.from_unix_utc (AppCenter.App.settings.get_int64 ("last-refresh-time"));
+        last_refresh_time = new DateTime.from_unix_utc (AppCenter.App.settings.get_int64 ("last-refresh-time"));
 
         unowned var fp_client = FlatpakBackend.get_default ();
         fp_client.notify["n-updatable-packages"].connect (() => notify_property ("can-update-all"));
         fp_client.notify["n-unpaid-updatable-packages"].connect (() => notify_property ("can-update-all"));
+
+        start_refresh_timeout ();
+
+        NetworkMonitor.get_default ().network_changed.connect (on_network_changed);
+    }
+
+    private void start_refresh_timeout () {
+        if (refresh_timeout_id != 0) {
+            Source.remove (refresh_timeout_id);
+            refresh_timeout_id = 0;
+        }
+
+        var seconds_since_last_refresh = new DateTime.now_utc ().difference (last_refresh_time) / GLib.TimeSpan.SECOND;
+
+        if (seconds_since_last_refresh >= SECONDS_BETWEEN_REFRESHES) {
+            refresh.begin ();
+            return;
+        }
+
+        var seconds_to_next_refresh = SECONDS_BETWEEN_REFRESHES - (uint) seconds_since_last_refresh;
+
+        refresh_timeout_id = Timeout.add_seconds_once (seconds_to_next_refresh, () => {
+            refresh_timeout_id = 0;
+            refresh.begin ();
+        });
+    }
+
+    private void on_network_changed (bool network_available) {
+        if (network_available && refresh_required) {
+            refresh_required = false;
+            Timeout.add_seconds_once (60, () => refresh.begin ());
+        }
     }
 
-    public async void get_updates (Cancellable? cancellable = null) {
+    /**
+     * This always forces a cache refresh and an update check, optionally starting the updates
+     * if automatic updates are enabled. Since that is a very expensive operation
+     * that also has effects on the browsing experience (blocking install jobs, etc.),
+     * calls to this should be carefully considered.
+     */
+    public async void refresh () {
+        if (Utils.is_running_in_demo_mode () || Utils.is_running_in_guest_session ()) {
+            return;
+        }
+
+        if (refreshing) {
+            return;
+        }
+
+        if (!NetworkMonitor.get_default ().network_available) {
+            refresh_required = true;
+            return;
+        }
+
+        refreshing = true;
+
+        unowned var fp_client = FlatpakBackend.get_default ();
+
+        var success = false;
+        try {
+            success = yield fp_client.refresh_cache (null);
+        } catch (Error e) {
+            critical ("Refresh cache failed: %s", e.message);
+            cache_update_failed (e);
+        }
+
+        yield get_updates ();
+
+        last_refresh_time = new DateTime.now_utc ();
+
+        // If the refresh failed don't save the time so that we try again
+        // immediately after app restart or system reboot
+        if (success) {
+            AppCenter.App.settings.set_int64 ("last-refresh-time", last_refresh_time.to_unix ());
+        }
+
+        start_refresh_timeout ();
+
+        refreshing = false;
+    }
+
+    private async void get_updates () {
         unowned FlatpakBackend fp_client = FlatpakBackend.get_default ();
 
         yield fp_client.get_updates ();
@@ -118,88 +196,6 @@ public class AppCenterCore.UpdateManager : Object {
         }
     }
 
-    public void cancel_updates (bool cancel_timeout) {
-        cancellable.cancel ();
-
-        if (update_cache_timeout_id > 0 && cancel_timeout) {
-            Source.remove (update_cache_timeout_id);
-            update_cache_timeout_id = 0;
-        }
-    }
-
-    public async void update_cache (bool force = false) {
-        cancellable.reset ();
-
-        if (Utils.is_running_in_demo_mode () || Utils.is_running_in_guest_session ()) {
-            return;
-        }
-
-        debug ("update cache called %s", force.to_string ());
-        bool success = false;
-
-        /* Make sure only one update cache can run at a time */
-        if (refresh_in_progress) {
-            debug ("Update cache already in progress - returning");
-            return;
-        }
-
-        if (update_cache_timeout_id > 0) {
-            if (force) {
-                debug ("Forced update_cache called when there is an on-going timeout - cancelling timeout");
-                Source.remove (update_cache_timeout_id);
-                update_cache_timeout_id = 0;
-            } else {
-                debug ("Refresh timeout running and not forced - returning");
-                return;
-            }
-        }
-
-        /* One cache update a day, keeps the doctor away! */
-        var seconds_since_last_refresh = new DateTime.now_utc ().difference (last_cache_update) / GLib.TimeSpan.SECOND;
-        bool last_cache_update_is_old = seconds_since_last_refresh >= SECONDS_BETWEEN_REFRESHES;
-
-        if (!force && !last_cache_update_is_old) {
-            debug ("Too soon to refresh and not forced");
-            return;
-        }
-
-        var nm = NetworkMonitor.get_default ();
-        if (!nm.get_network_available ()) {
-            return;
-        }
-
-        debug ("New refresh task");
-        refresh_in_progress = true;
-        try {
-            success = yield FlatpakBackend.get_default ().refresh_cache (cancellable);
-
-            if (success) {
-                last_cache_update = new DateTime.now_utc ();
-                AppCenter.App.settings.set_int64 ("last-refresh-time", last_cache_update.to_unix ());
-            }
-
-            seconds_since_last_refresh = 0;
-        } catch (Error e) {
-            if (!(e is GLib.IOError.CANCELLED)) {
-                critical ("Update_cache: Refesh cache async failed - %s", e.message);
-                cache_update_failed (e);
-            }
-        } finally {
-            refresh_in_progress = false;
-        }
-
-        var next_refresh = SECONDS_BETWEEN_REFRESHES - (uint)seconds_since_last_refresh;
-        debug ("Setting a timeout for a refresh in %f minutes", next_refresh / 60.0f);
-        update_cache_timeout_id = GLib.Timeout.add_seconds (next_refresh, () => {
-            update_cache_timeout_id = 0;
-            update_cache.begin (true);
-
-            return GLib.Source.REMOVE;
-        });
-
-        get_updates.begin (cancellable);
-    }
-
     private static GLib.Once<UpdateManager> instance;
     public static unowned UpdateManager get_default () {
         return instance.once (() => { return new UpdateManager (); });

src/Dialogs/UpdateFailDialog.vala

@@ -37,7 +37,7 @@ public class UpdateFailDialog : Granite.MessageDialog {
 
         response.connect ((response_id) => {
             if (response_id == TRY_AGAIN_RESPONSE_ID) {
-                AppCenterCore.UpdateManager.get_default ().update_cache.begin (true);
+                AppCenterCore.UpdateManager.get_default ().refresh.begin ();
             }
             destroy ();
         });

src/MainWindow.vala

@@ -156,14 +156,10 @@ public class AppCenter.MainWindow : Gtk.ApplicationWindow {
             installed_view.clear ();
         }
 
-        // We not to wrap in Idle otherwise we crash because libportal hasn't unexported us yet.
-        ((AppCenter.App) application).request_background.begin (() => Idle.add_once (() => {
-            unowned var backend = AppCenterCore.FlatpakBackend.get_default ();
-            if (backend.working) {
-                AppCenterCore.UpdateManager.get_default ().cancel_updates (false); //Timeouts keep running
-            }
-            destroy ();
-        }));
+        // We have to wrap in Idle otherwise we crash because libportal hasn't unexported us yet.
+        ((AppCenter.App) application).request_background.begin (() =>
+            Idle.add_once (() => destroy ())
+        );
 
         return true;
     }

src/Views/AppListUpdateView.vala

@@ -224,9 +224,8 @@ namespace AppCenter.Views {
 
             automatic_updates_button.notify["active"].connect (() => {
                 if (automatic_updates_button.active) {
-                    update_manager.update_cache.begin (true);
-                } else {
-                    update_manager.cancel_updates (true);
+                    // TODO: Follow up: think about this? Only update all and handle in manager?
+                    update_manager.refresh.begin ();
                 }
             });