Merge lp:~donadigo/appcenter/client-class-improvements into lp:~elementary-apps/appcenter/appcenter

[Adam Bieńkowski]

Client:
 * Use one task accross the class
 * Fix code-style issues
 * Cache used members / variables

This branch does some improvements to the Client class, mostly it replaces the task list with one task (packagekit can do only one task at a time), fixes some code-style issues.

[Corentin Noël (tintou)]

Works fine here, everything looks sane

review: Approve

[RabbitBot (rabbitbot-a)]

Attempt to merge into lp:appcenter failed due to conflicts:

text conflict in src/Application.vala
text conflict in src/Core/Client.vala
text conflict in src/MainWindow.vala

[lp:~donadigo/appcenter/client-class-improvements]

328. By Adam Bieńkowski on 2016-11-06

Resolve conflicts

[Adam Bieńkowski (donadigo)]

Conflicts resolved.

=== modified file 'src/Core/Client.vala'
--- src/Core/Client.vala	2016-10-26 12:59:05 +0000
+++ src/Core/Client.vala	2016-11-06 15:34:19 +0000
@@ -16,18 +16,16 @@
 
 public class AppCenterCore.Client : Object {
     public signal void updates_available ();
-    public signal void tasks_finished ();
 
     public bool connected { public get; private set; }
     public bool updating_cache { public get; private set; }
+    public uint task_count { public get; private set; default = 0; }
 
     public AppCenterCore.Package os_updates { public get; private set; }
 
-    private Gee.LinkedList<AppCenter.Task> task_list;
-    private Gee.LinkedList<AppCenter.Task> task_with_agreement_list;
     private Gee.HashMap<string, AppCenterCore.Package> package_list;
     private AppStream.Pool appstream_pool;
-    private GLib.Cancellable interface_cancellable;
+    private GLib.Cancellable cancellable;
     private GLib.DateTime last_cache_update;
     private uint updates_number = 0U;
     private uint update_cache_timeout_id = 0;
@@ -35,73 +33,57 @@
 
     private const int SECONDS_BETWEEN_REFRESHES = 60*60*24;
 
+    private Task client;
+    private SuspendControl sc;
+
     private Client () {
-        appstream_pool.get_components ().foreach ((comp) => {
-            var package = new AppCenterCore.Package (comp);
-            foreach (var pkg_name in comp.get_pkgnames ()) {
-                package_list.set (pkg_name, package);
-            }
-        });
     }
 
     construct {
-        task_list = new Gee.LinkedList<AppCenter.Task> ();
-        task_with_agreement_list = new Gee.LinkedList<AppCenter.Task> ();
         package_list = new Gee.HashMap<string, AppCenterCore.Package> (null, null);
-        interface_cancellable = new GLib.Cancellable ();
+        cancellable = new GLib.Cancellable ();
+
+        client = new Task ();
+        sc = new SuspendControl ();
+
+        cancellable = new GLib.Cancellable ();
         last_cache_update = null;
 
         appstream_pool = new AppStream.Pool ();
+        
         try {
             appstream_pool.load ();
+            appstream_pool.get_components ().foreach ((comp) => {
+                var package = new AppCenterCore.Package (comp);
+                foreach (var pkg_name in comp.get_pkgnames ()) {
+                    package_list.set (pkg_name, package);
+                }
+            });
         } catch (Error e) {
             error (e.message);
         }
 
+        var icon = new AppStream.Icon ();
+        icon.set_name ("distributor-logo");
+        icon.set_kind (AppStream.IconKind.STOCK);
+
         var os_updates_component = new AppStream.Component ();
         os_updates_component.id = AppCenterCore.Package.OS_UPDATES_ID;
         os_updates_component.name = _("Operating System Updates");
         os_updates_component.summary = _("Updates to system components");
-        var icon = new AppStream.Icon ();
-        icon.set_name ("distributor-logo");
-        icon.set_kind (AppStream.IconKind.STOCK);
         os_updates_component.add_icon (icon);
+
         os_updates = new AppCenterCore.Package (os_updates_component);
     }
 
     public bool has_tasks () {
-        return !task_list.is_empty;
-    }
-
-    private AppCenter.Task request_task (bool requires_user_agreement = true) {
-        AppCenter.Task task = new AppCenter.Task ();
-        task_list.add (task);
-        if (requires_user_agreement) {
-            if (task_with_agreement_list.size == 0) {
-                Pk.polkit_agent_open ();
-            }
-            task_with_agreement_list.add (task);
-        }
-        return task;
-    }
-
-    private void release_task (AppCenter.Task task) {
-        task_list.remove (task);
-        if (task_list.is_empty) {
-            tasks_finished ();
-            if (task in task_with_agreement_list) {
-                task_with_agreement_list.remove (task);
-                if (task_with_agreement_list.size == 0) {
-                    Pk.polkit_agent_close ();
-                }
-            }
-        }
+        return task_count > 0;
     }
 
     public async Pk.Exit install_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error {
+        task_count++;
+
         Pk.Exit exit_status = Pk.Exit.UNKNOWN;
-        AppCenter.Task install_task = request_task ();
-        AppCenter.Task search_task = request_task ();
         string[] packages_ids = {};
         foreach (var pkg_name in package.component.get_pkgnames ()) {
             packages_ids += pkg_name;
@@ -110,7 +92,7 @@
         packages_ids += null;
 
         try {
-            var results = yield search_task.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.NEWEST, Pk.Filter.ARCH), packages_ids, cancellable, () => {});
+            var results = yield client.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.NEWEST, Pk.Filter.ARCH), packages_ids, cancellable, () => {});
             packages_ids = {};
 
             results.get_package_array ().foreach ((package) => {
@@ -119,28 +101,24 @@
 
             packages_ids += null;
 
-            results = yield install_task.install_packages_async (packages_ids, cancellable, cb);
+            results = yield client.install_packages_async (packages_ids, cancellable, cb);
             exit_status = results.get_exit_code ();
             if (exit_status != Pk.Exit.SUCCESS) {
-                release_task (search_task);
-                release_task (install_task);
                 throw new GLib.IOError.FAILED (Pk.Exit.enum_to_string (results.get_exit_code ()));
             }
         } catch (Error e) {
-            release_task (search_task);
-            release_task (install_task);
+            task_count--;
             throw e;
         }
 
-        release_task (search_task);
-        release_task (install_task);
+        task_count--;
         return exit_status;
     }
 
     public async Pk.Exit update_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error {
+        task_count++;
+
         Pk.Exit exit_status = Pk.Exit.UNKNOWN;
-        SuspendControl sc = new SuspendControl ();
-        AppCenter.Task update_task = request_task ();
         string[] packages_ids = {};
         foreach (var pk_package in package.change_information.changes) {
             packages_ids += pk_package.get_id ();
@@ -150,13 +128,14 @@
 
         try {
             sc.inhibit ();
-            var results = yield update_task.update_packages_async (packages_ids, cancellable, cb);
+
+            var results = yield client.update_packages_async (packages_ids, cancellable, cb);
             exit_status = results.get_exit_code ();
         } catch (Error e) {
+            task_count--;
             throw e;
         } finally {
             sc.uninhibit ();
-            release_task (update_task);
         }
 
         if (exit_status != Pk.Exit.SUCCESS) {
@@ -165,64 +144,65 @@
             package.change_information.clear_update_info ();
         }
 
+        task_count--;
         yield refresh_updates ();
-
         return exit_status;
     }
 
     public async Pk.Exit remove_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error {
+        task_count++;
+
         Pk.Exit exit_status = Pk.Exit.UNKNOWN;
-        AppCenter.Task remove_task = request_task ();
-        AppCenter.Task search_task = request_task ();
         string[] packages_ids = {};
         foreach (var pkg_name in package.component.get_pkgnames ()) {
             packages_ids += pkg_name;
         }
+
         packages_ids += null;
 
         try {
-            var results = yield search_task.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST), packages_ids, cancellable, () => {});
+            var results = yield client.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST), packages_ids, cancellable, () => {});
             packages_ids = {};
             results.get_package_array ().foreach ((package) => {
                 packages_ids += package.package_id;
             });
 
-            results = yield remove_task.remove_packages_async (packages_ids, true, true, cancellable, cb);
+            results = yield client.remove_packages_async (packages_ids, true, true, cancellable, cb);
             exit_status = results.get_exit_code ();
         } catch (Error e) {
-            release_task (search_task);
-            release_task (remove_task);
+            task_count--;
             throw e;
         }
 
+        task_count--;
         yield refresh_updates ();
-        release_task (search_task);
-        release_task (remove_task);
         return exit_status;
     }
 
     public async void get_updates () {
-        AppCenter.Task update_task = request_task (false);
-        AppCenter.Task details_task = request_task (false);
+        task_count++;
+
         try {
-            Pk.Results result = yield update_task.get_updates_async (0, interface_cancellable, (t, p) => { });
+            Pk.Results results = yield client.get_updates_async (0, cancellable, (t, p) => { });
             string[] packages_array = {};
-            result.get_package_array ().foreach ((pk_package) => {
+            results.get_package_array ().foreach ((pk_package) => {
                 packages_array += pk_package.get_id ();
             });
 
             // We need a null to show to PackageKit that it's then end of the array.
             packages_array += null;
 
-            Pk.Results result2 = yield details_task.get_details_async (packages_array , interface_cancellable, (t, p) => { });
-            result2.get_details_array ().foreach ((pk_detail) => {
+            results = yield client.get_details_async (packages_array , cancellable, (t, p) => { });
+            results.get_details_array ().foreach ((pk_detail) => {
                 var pk_package = new Pk.Package ();
                 try {
                     pk_package.set_id (pk_detail.get_package_id ());
+
                     unowned string pkg_name = pk_package.get_name ();
                     var package = package_list.get (pkg_name);
                     if (package == null) {
                         package = os_updates;
+
                         var pkgnames = os_updates.component.pkgnames;
                         pkgnames += pkg_name;
                         os_updates.component.pkgnames = pkgnames;
@@ -240,17 +220,19 @@
             if (e.code != 19) {
                 critical (e.message);
             }
+
+            task_count--;
+            return;
         }
 
-        release_task (details_task);
-        release_task (update_task);
+        task_count--;
         updates_available ();
     }
 
     public async Gee.Collection<AppCenterCore.Package> get_installed_applications () {
         var packages = new Gee.TreeSet<AppCenterCore.Package> ();
-        var packages_list = yield get_installed_packages ();
-        foreach (var pk_package in packages_list) {
+        var installed = yield get_installed_packages ();
+        foreach (var pk_package in installed) {
             var package = package_list.get (pk_package.get_name ());
             if (package != null) {
                 package.installed_packages.add (pk_package);
@@ -300,43 +282,49 @@
     }
 
     public Pk.Package? get_app_package (string application, Pk.Bitfield additional_filters = 0) throws GLib.Error {
-        AppCenter.Task packages_task = request_task (false);
+        task_count++;
+
         Pk.Package? package = null;
         var filter = Pk.Bitfield.from_enums (Pk.Filter.NEWEST);
         filter |= additional_filters;
         try {
-            var results = packages_task.search_names_sync (filter, { application, null }, interface_cancellable, () => {});
+            var results = client.search_names_sync (filter, { application, null }, cancellable, () => {});
             var array = results.get_package_array ();
             if (array.length > 0) {
                 package = array.get (0);
             }
         } catch (Error e) {
-            release_task (packages_task);
+            task_count--;
             throw e;
         }
 
-        release_task (packages_task);
+        task_count--;
         return package;
     }
 
     private async void refresh_updates () {
-        var update_task = new AppCenter.Task ();
         updating_cache = true;
+        task_count++;
 
         try {
-            Pk.Results result = yield update_task.get_updates_async (0, interface_cancellable, (t, p) => {});
+            Pk.Results results = yield client.get_updates_async (0, null, (t, p) => {});
+
             bool was_empty = updates_number == 0U;
-            updates_number = get_package_count (result.get_package_array ());
+            updates_number = get_package_count (results.get_package_array ());
+
+            var application = Application.get_default ();
             if (was_empty && updates_number != 0U) {
                 string title = ngettext ("Update Available", "Updates Available", updates_number);
                 string body = ngettext ("%u update is available for your system", "%u updates are available for your system", updates_number).printf (updates_number);
+
                 var notification = new Notification (title);
                 notification.set_body (body);
                 notification.set_icon (new ThemedIcon ("system-software-install"));
                 notification.set_default_action ("app.open-application");
-                Application.get_default ().send_notification ("updates", notification);
+
+                application.send_notification ("updates", notification);
             } else {
-                Application.get_default ().withdraw_notification ("updates");
+                application.withdraw_notification ("updates");
             }
 
 #if HAVE_UNITY
@@ -349,12 +337,14 @@
         }
 
         updating_cache = false;
+        task_count--;
         refresh_in_progress = false;
     }
 
     public uint get_package_count (GLib.GenericArray<weak Pk.Package> package_array) {
         bool os_update_found = false;
         var result_comp = new Gee.TreeSet<AppStream.Component> ();
+
         package_array.foreach ((pk_package) => {
             var comp = package_list.get (pk_package.get_name ());
             if (comp != null) {
@@ -373,7 +363,7 @@
     }
 
     public void cancel_updates (bool cancel_timeout) {
-        interface_cancellable.cancel ();
+        cancellable.cancel ();
 
         if (update_cache_timeout_id > 0 && cancel_timeout) {
             Source.remove (update_cache_timeout_id);
@@ -381,7 +371,7 @@
             last_cache_update = null;
         }
 
-        interface_cancellable = new GLib.Cancellable ();
+        cancellable = new GLib.Cancellable ();
         refresh_in_progress = false;
     }
 
@@ -413,21 +403,16 @@
         /* One cache update a day, keeps the doctor away! */
         if (force || last_cache_update == null ||
             (new DateTime.now_local ()).difference (last_cache_update) / GLib.TimeSpan.SECOND >= SECONDS_BETWEEN_REFRESHES) {
-
             var nm = NetworkMonitor.get_default ();
-
-            if (nm.get_network_available()) {
+            if (nm.get_network_available ()) {
                 debug ("New refresh task");
-                var refresh_task = new AppCenter.Task ();
-
 
                 try {
-                    Pk.Results result = yield refresh_task.refresh_cache_async (false, interface_cancellable, (t, p) => { });
-                    success = result.get_exit_code () == Pk.Exit.SUCCESS;
+                    Pk.Results results = yield client.refresh_cache_async (false, cancellable, (t, p) => { });
+                    success = results.get_exit_code () == Pk.Exit.SUCCESS;
                     last_cache_update = new DateTime.now_local ();
                 } catch (Error e) {
                     critical ("Update_cache: Refesh cache async failed - %s", e.message);
-
                 }
 
                 if (success) {
@@ -454,21 +439,22 @@
     }
 
     public async Gee.TreeSet<Pk.Package> get_installed_packages () {
-        var packages_task = request_task ();
-        var filter = Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST);
+        task_count++;
+
+        Pk.Bitfield filter = Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST);
         var installed = new Gee.TreeSet<Pk.Package> ();
+
         try {
-            Pk.Results result = yield packages_task.get_packages_async (filter, null, (prog, type) => {});
-            result.get_package_array ().foreach ((pk_package) => {
+            Pk.Results results = yield client.get_packages_async (filter, null, (prog, type) => {});
+            results.get_package_array ().foreach ((pk_package) => {
                 installed.add (pk_package);
             });
 
-            release_task (packages_task);
         } catch (Error e) {
             critical (e.message);
-            release_task (packages_task);
         }
 
+        task_count--;
         return installed;
     }
 
 
=== modified file 'src/Core/Task.vala'
--- src/Core/Task.vala	2016-07-18 21:16:57 +0000
+++ src/Core/Task.vala	2016-11-06 15:34:19 +0000
@@ -18,11 +18,7 @@
  * Authored by: Corentin Noël <[e-mail]>
  */
 
-public class AppCenter.Task : Pk.Task {
-    public Task () {
-        
-    }
-
+public class AppCenterCore.Task : Pk.Task {
     public override void untrusted_question (uint request, Pk.Results results) {
         user_accepted (request);
     }
 
=== modified file 'src/MainWindow.vala'
--- src/MainWindow.vala	2016-10-17 17:27:00 +0000
+++ src/MainWindow.vala	2016-11-06 15:34:19 +0000
@@ -171,8 +171,8 @@
             }
 
             hide ();
-            task_finished_connection = client.tasks_finished.connect (() => {
-                if (!visible) {
+            task_finished_connection = client.notify["task-count"].connect (() => {
+                if (!visible && client.task_count == 0) {
                     destroy ();
                 }
             });