Merge lp:~donadigo/appcenter/client-class-improvements into lp:~elementary-apps/appcenter/appcenter
- client-class-improvements
- Merge into appcenter
Proposed by
Adam Bieńkowski
Status: | Merged |
---|---|
Approved by: | Zisu Andrei |
Approved revision: | 328 |
Merged at revision: | 334 |
Proposed branch: | lp:~donadigo/appcenter/client-class-improvements |
Merge into: | lp:~elementary-apps/appcenter/appcenter |
Diff against target: |
465 lines (+89/-107) 3 files modified
src/Core/Client.vala (+86/-100) src/Core/Task.vala (+1/-5) src/MainWindow.vala (+2/-2) |
To merge this branch: | bzr merge lp:~donadigo/appcenter/client-class-improvements |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corentin Noël | Approve | ||
Review via email: mp+308592@code.launchpad.net |
Commit message
Client:
* Use one task accross the class
* Fix code-style issues
* Cache used members / variables
Description of the change
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.
To post a comment you must log in.
Revision history for this message
RabbitBot (rabbitbot-a) wrote : | # |
Attempt to merge into lp:appcenter failed due to conflicts:
text conflict in src/Application
text conflict in src/Core/
text conflict in src/MainWindow.vala
- 328. By Adam Bieńkowski
-
Resolve conflicts
Revision history for this message
Adam Bieńkowski (donadigo) wrote : | # |
Conflicts resolved.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/Core/Client.vala' |
2 | --- src/Core/Client.vala 2016-10-26 12:59:05 +0000 |
3 | +++ src/Core/Client.vala 2016-11-06 15:34:19 +0000 |
4 | @@ -16,18 +16,16 @@ |
5 | |
6 | public class AppCenterCore.Client : Object { |
7 | public signal void updates_available (); |
8 | - public signal void tasks_finished (); |
9 | |
10 | public bool connected { public get; private set; } |
11 | public bool updating_cache { public get; private set; } |
12 | + public uint task_count { public get; private set; default = 0; } |
13 | |
14 | public AppCenterCore.Package os_updates { public get; private set; } |
15 | |
16 | - private Gee.LinkedList<AppCenter.Task> task_list; |
17 | - private Gee.LinkedList<AppCenter.Task> task_with_agreement_list; |
18 | private Gee.HashMap<string, AppCenterCore.Package> package_list; |
19 | private AppStream.Pool appstream_pool; |
20 | - private GLib.Cancellable interface_cancellable; |
21 | + private GLib.Cancellable cancellable; |
22 | private GLib.DateTime last_cache_update; |
23 | private uint updates_number = 0U; |
24 | private uint update_cache_timeout_id = 0; |
25 | @@ -35,73 +33,57 @@ |
26 | |
27 | private const int SECONDS_BETWEEN_REFRESHES = 60*60*24; |
28 | |
29 | + private Task client; |
30 | + private SuspendControl sc; |
31 | + |
32 | private Client () { |
33 | - appstream_pool.get_components ().foreach ((comp) => { |
34 | - var package = new AppCenterCore.Package (comp); |
35 | - foreach (var pkg_name in comp.get_pkgnames ()) { |
36 | - package_list.set (pkg_name, package); |
37 | - } |
38 | - }); |
39 | } |
40 | |
41 | construct { |
42 | - task_list = new Gee.LinkedList<AppCenter.Task> (); |
43 | - task_with_agreement_list = new Gee.LinkedList<AppCenter.Task> (); |
44 | package_list = new Gee.HashMap<string, AppCenterCore.Package> (null, null); |
45 | - interface_cancellable = new GLib.Cancellable (); |
46 | + cancellable = new GLib.Cancellable (); |
47 | + |
48 | + client = new Task (); |
49 | + sc = new SuspendControl (); |
50 | + |
51 | + cancellable = new GLib.Cancellable (); |
52 | last_cache_update = null; |
53 | |
54 | appstream_pool = new AppStream.Pool (); |
55 | + |
56 | try { |
57 | appstream_pool.load (); |
58 | + appstream_pool.get_components ().foreach ((comp) => { |
59 | + var package = new AppCenterCore.Package (comp); |
60 | + foreach (var pkg_name in comp.get_pkgnames ()) { |
61 | + package_list.set (pkg_name, package); |
62 | + } |
63 | + }); |
64 | } catch (Error e) { |
65 | error (e.message); |
66 | } |
67 | |
68 | + var icon = new AppStream.Icon (); |
69 | + icon.set_name ("distributor-logo"); |
70 | + icon.set_kind (AppStream.IconKind.STOCK); |
71 | + |
72 | var os_updates_component = new AppStream.Component (); |
73 | os_updates_component.id = AppCenterCore.Package.OS_UPDATES_ID; |
74 | os_updates_component.name = _("Operating System Updates"); |
75 | os_updates_component.summary = _("Updates to system components"); |
76 | - var icon = new AppStream.Icon (); |
77 | - icon.set_name ("distributor-logo"); |
78 | - icon.set_kind (AppStream.IconKind.STOCK); |
79 | os_updates_component.add_icon (icon); |
80 | + |
81 | os_updates = new AppCenterCore.Package (os_updates_component); |
82 | } |
83 | |
84 | public bool has_tasks () { |
85 | - return !task_list.is_empty; |
86 | - } |
87 | - |
88 | - private AppCenter.Task request_task (bool requires_user_agreement = true) { |
89 | - AppCenter.Task task = new AppCenter.Task (); |
90 | - task_list.add (task); |
91 | - if (requires_user_agreement) { |
92 | - if (task_with_agreement_list.size == 0) { |
93 | - Pk.polkit_agent_open (); |
94 | - } |
95 | - task_with_agreement_list.add (task); |
96 | - } |
97 | - return task; |
98 | - } |
99 | - |
100 | - private void release_task (AppCenter.Task task) { |
101 | - task_list.remove (task); |
102 | - if (task_list.is_empty) { |
103 | - tasks_finished (); |
104 | - if (task in task_with_agreement_list) { |
105 | - task_with_agreement_list.remove (task); |
106 | - if (task_with_agreement_list.size == 0) { |
107 | - Pk.polkit_agent_close (); |
108 | - } |
109 | - } |
110 | - } |
111 | + return task_count > 0; |
112 | } |
113 | |
114 | public async Pk.Exit install_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error { |
115 | + task_count++; |
116 | + |
117 | Pk.Exit exit_status = Pk.Exit.UNKNOWN; |
118 | - AppCenter.Task install_task = request_task (); |
119 | - AppCenter.Task search_task = request_task (); |
120 | string[] packages_ids = {}; |
121 | foreach (var pkg_name in package.component.get_pkgnames ()) { |
122 | packages_ids += pkg_name; |
123 | @@ -110,7 +92,7 @@ |
124 | packages_ids += null; |
125 | |
126 | try { |
127 | - var results = yield search_task.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.NEWEST, Pk.Filter.ARCH), packages_ids, cancellable, () => {}); |
128 | + var results = yield client.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.NEWEST, Pk.Filter.ARCH), packages_ids, cancellable, () => {}); |
129 | packages_ids = {}; |
130 | |
131 | results.get_package_array ().foreach ((package) => { |
132 | @@ -119,28 +101,24 @@ |
133 | |
134 | packages_ids += null; |
135 | |
136 | - results = yield install_task.install_packages_async (packages_ids, cancellable, cb); |
137 | + results = yield client.install_packages_async (packages_ids, cancellable, cb); |
138 | exit_status = results.get_exit_code (); |
139 | if (exit_status != Pk.Exit.SUCCESS) { |
140 | - release_task (search_task); |
141 | - release_task (install_task); |
142 | throw new GLib.IOError.FAILED (Pk.Exit.enum_to_string (results.get_exit_code ())); |
143 | } |
144 | } catch (Error e) { |
145 | - release_task (search_task); |
146 | - release_task (install_task); |
147 | + task_count--; |
148 | throw e; |
149 | } |
150 | |
151 | - release_task (search_task); |
152 | - release_task (install_task); |
153 | + task_count--; |
154 | return exit_status; |
155 | } |
156 | |
157 | public async Pk.Exit update_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error { |
158 | + task_count++; |
159 | + |
160 | Pk.Exit exit_status = Pk.Exit.UNKNOWN; |
161 | - SuspendControl sc = new SuspendControl (); |
162 | - AppCenter.Task update_task = request_task (); |
163 | string[] packages_ids = {}; |
164 | foreach (var pk_package in package.change_information.changes) { |
165 | packages_ids += pk_package.get_id (); |
166 | @@ -150,13 +128,14 @@ |
167 | |
168 | try { |
169 | sc.inhibit (); |
170 | - var results = yield update_task.update_packages_async (packages_ids, cancellable, cb); |
171 | + |
172 | + var results = yield client.update_packages_async (packages_ids, cancellable, cb); |
173 | exit_status = results.get_exit_code (); |
174 | } catch (Error e) { |
175 | + task_count--; |
176 | throw e; |
177 | } finally { |
178 | sc.uninhibit (); |
179 | - release_task (update_task); |
180 | } |
181 | |
182 | if (exit_status != Pk.Exit.SUCCESS) { |
183 | @@ -165,64 +144,65 @@ |
184 | package.change_information.clear_update_info (); |
185 | } |
186 | |
187 | + task_count--; |
188 | yield refresh_updates (); |
189 | - |
190 | return exit_status; |
191 | } |
192 | |
193 | public async Pk.Exit remove_package (Package package, Pk.ProgressCallback cb, GLib.Cancellable cancellable) throws GLib.Error { |
194 | + task_count++; |
195 | + |
196 | Pk.Exit exit_status = Pk.Exit.UNKNOWN; |
197 | - AppCenter.Task remove_task = request_task (); |
198 | - AppCenter.Task search_task = request_task (); |
199 | string[] packages_ids = {}; |
200 | foreach (var pkg_name in package.component.get_pkgnames ()) { |
201 | packages_ids += pkg_name; |
202 | } |
203 | + |
204 | packages_ids += null; |
205 | |
206 | try { |
207 | - var results = yield search_task.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST), packages_ids, cancellable, () => {}); |
208 | + var results = yield client.resolve_async (Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST), packages_ids, cancellable, () => {}); |
209 | packages_ids = {}; |
210 | results.get_package_array ().foreach ((package) => { |
211 | packages_ids += package.package_id; |
212 | }); |
213 | |
214 | - results = yield remove_task.remove_packages_async (packages_ids, true, true, cancellable, cb); |
215 | + results = yield client.remove_packages_async (packages_ids, true, true, cancellable, cb); |
216 | exit_status = results.get_exit_code (); |
217 | } catch (Error e) { |
218 | - release_task (search_task); |
219 | - release_task (remove_task); |
220 | + task_count--; |
221 | throw e; |
222 | } |
223 | |
224 | + task_count--; |
225 | yield refresh_updates (); |
226 | - release_task (search_task); |
227 | - release_task (remove_task); |
228 | return exit_status; |
229 | } |
230 | |
231 | public async void get_updates () { |
232 | - AppCenter.Task update_task = request_task (false); |
233 | - AppCenter.Task details_task = request_task (false); |
234 | + task_count++; |
235 | + |
236 | try { |
237 | - Pk.Results result = yield update_task.get_updates_async (0, interface_cancellable, (t, p) => { }); |
238 | + Pk.Results results = yield client.get_updates_async (0, cancellable, (t, p) => { }); |
239 | string[] packages_array = {}; |
240 | - result.get_package_array ().foreach ((pk_package) => { |
241 | + results.get_package_array ().foreach ((pk_package) => { |
242 | packages_array += pk_package.get_id (); |
243 | }); |
244 | |
245 | // We need a null to show to PackageKit that it's then end of the array. |
246 | packages_array += null; |
247 | |
248 | - Pk.Results result2 = yield details_task.get_details_async (packages_array , interface_cancellable, (t, p) => { }); |
249 | - result2.get_details_array ().foreach ((pk_detail) => { |
250 | + results = yield client.get_details_async (packages_array , cancellable, (t, p) => { }); |
251 | + results.get_details_array ().foreach ((pk_detail) => { |
252 | var pk_package = new Pk.Package (); |
253 | try { |
254 | pk_package.set_id (pk_detail.get_package_id ()); |
255 | + |
256 | unowned string pkg_name = pk_package.get_name (); |
257 | var package = package_list.get (pkg_name); |
258 | if (package == null) { |
259 | package = os_updates; |
260 | + |
261 | var pkgnames = os_updates.component.pkgnames; |
262 | pkgnames += pkg_name; |
263 | os_updates.component.pkgnames = pkgnames; |
264 | @@ -240,17 +220,19 @@ |
265 | if (e.code != 19) { |
266 | critical (e.message); |
267 | } |
268 | + |
269 | + task_count--; |
270 | + return; |
271 | } |
272 | |
273 | - release_task (details_task); |
274 | - release_task (update_task); |
275 | + task_count--; |
276 | updates_available (); |
277 | } |
278 | |
279 | public async Gee.Collection<AppCenterCore.Package> get_installed_applications () { |
280 | var packages = new Gee.TreeSet<AppCenterCore.Package> (); |
281 | - var packages_list = yield get_installed_packages (); |
282 | - foreach (var pk_package in packages_list) { |
283 | + var installed = yield get_installed_packages (); |
284 | + foreach (var pk_package in installed) { |
285 | var package = package_list.get (pk_package.get_name ()); |
286 | if (package != null) { |
287 | package.installed_packages.add (pk_package); |
288 | @@ -300,43 +282,49 @@ |
289 | } |
290 | |
291 | public Pk.Package? get_app_package (string application, Pk.Bitfield additional_filters = 0) throws GLib.Error { |
292 | - AppCenter.Task packages_task = request_task (false); |
293 | + task_count++; |
294 | + |
295 | Pk.Package? package = null; |
296 | var filter = Pk.Bitfield.from_enums (Pk.Filter.NEWEST); |
297 | filter |= additional_filters; |
298 | try { |
299 | - var results = packages_task.search_names_sync (filter, { application, null }, interface_cancellable, () => {}); |
300 | + var results = client.search_names_sync (filter, { application, null }, cancellable, () => {}); |
301 | var array = results.get_package_array (); |
302 | if (array.length > 0) { |
303 | package = array.get (0); |
304 | } |
305 | } catch (Error e) { |
306 | - release_task (packages_task); |
307 | + task_count--; |
308 | throw e; |
309 | } |
310 | |
311 | - release_task (packages_task); |
312 | + task_count--; |
313 | return package; |
314 | } |
315 | |
316 | private async void refresh_updates () { |
317 | - var update_task = new AppCenter.Task (); |
318 | updating_cache = true; |
319 | + task_count++; |
320 | |
321 | try { |
322 | - Pk.Results result = yield update_task.get_updates_async (0, interface_cancellable, (t, p) => {}); |
323 | + Pk.Results results = yield client.get_updates_async (0, null, (t, p) => {}); |
324 | + |
325 | bool was_empty = updates_number == 0U; |
326 | - updates_number = get_package_count (result.get_package_array ()); |
327 | + updates_number = get_package_count (results.get_package_array ()); |
328 | + |
329 | + var application = Application.get_default (); |
330 | if (was_empty && updates_number != 0U) { |
331 | string title = ngettext ("Update Available", "Updates Available", updates_number); |
332 | string body = ngettext ("%u update is available for your system", "%u updates are available for your system", updates_number).printf (updates_number); |
333 | + |
334 | var notification = new Notification (title); |
335 | notification.set_body (body); |
336 | notification.set_icon (new ThemedIcon ("system-software-install")); |
337 | notification.set_default_action ("app.open-application"); |
338 | - Application.get_default ().send_notification ("updates", notification); |
339 | + |
340 | + application.send_notification ("updates", notification); |
341 | } else { |
342 | - Application.get_default ().withdraw_notification ("updates"); |
343 | + application.withdraw_notification ("updates"); |
344 | } |
345 | |
346 | #if HAVE_UNITY |
347 | @@ -349,12 +337,14 @@ |
348 | } |
349 | |
350 | updating_cache = false; |
351 | + task_count--; |
352 | refresh_in_progress = false; |
353 | } |
354 | |
355 | public uint get_package_count (GLib.GenericArray<weak Pk.Package> package_array) { |
356 | bool os_update_found = false; |
357 | var result_comp = new Gee.TreeSet<AppStream.Component> (); |
358 | + |
359 | package_array.foreach ((pk_package) => { |
360 | var comp = package_list.get (pk_package.get_name ()); |
361 | if (comp != null) { |
362 | @@ -373,7 +363,7 @@ |
363 | } |
364 | |
365 | public void cancel_updates (bool cancel_timeout) { |
366 | - interface_cancellable.cancel (); |
367 | + cancellable.cancel (); |
368 | |
369 | if (update_cache_timeout_id > 0 && cancel_timeout) { |
370 | Source.remove (update_cache_timeout_id); |
371 | @@ -381,7 +371,7 @@ |
372 | last_cache_update = null; |
373 | } |
374 | |
375 | - interface_cancellable = new GLib.Cancellable (); |
376 | + cancellable = new GLib.Cancellable (); |
377 | refresh_in_progress = false; |
378 | } |
379 | |
380 | @@ -413,21 +403,16 @@ |
381 | /* One cache update a day, keeps the doctor away! */ |
382 | if (force || last_cache_update == null || |
383 | (new DateTime.now_local ()).difference (last_cache_update) / GLib.TimeSpan.SECOND >= SECONDS_BETWEEN_REFRESHES) { |
384 | - |
385 | var nm = NetworkMonitor.get_default (); |
386 | - |
387 | - if (nm.get_network_available()) { |
388 | + if (nm.get_network_available ()) { |
389 | debug ("New refresh task"); |
390 | - var refresh_task = new AppCenter.Task (); |
391 | - |
392 | |
393 | try { |
394 | - Pk.Results result = yield refresh_task.refresh_cache_async (false, interface_cancellable, (t, p) => { }); |
395 | - success = result.get_exit_code () == Pk.Exit.SUCCESS; |
396 | + Pk.Results results = yield client.refresh_cache_async (false, cancellable, (t, p) => { }); |
397 | + success = results.get_exit_code () == Pk.Exit.SUCCESS; |
398 | last_cache_update = new DateTime.now_local (); |
399 | } catch (Error e) { |
400 | critical ("Update_cache: Refesh cache async failed - %s", e.message); |
401 | - |
402 | } |
403 | |
404 | if (success) { |
405 | @@ -454,21 +439,22 @@ |
406 | } |
407 | |
408 | public async Gee.TreeSet<Pk.Package> get_installed_packages () { |
409 | - var packages_task = request_task (); |
410 | - var filter = Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST); |
411 | + task_count++; |
412 | + |
413 | + Pk.Bitfield filter = Pk.Bitfield.from_enums (Pk.Filter.INSTALLED, Pk.Filter.NEWEST); |
414 | var installed = new Gee.TreeSet<Pk.Package> (); |
415 | + |
416 | try { |
417 | - Pk.Results result = yield packages_task.get_packages_async (filter, null, (prog, type) => {}); |
418 | - result.get_package_array ().foreach ((pk_package) => { |
419 | + Pk.Results results = yield client.get_packages_async (filter, null, (prog, type) => {}); |
420 | + results.get_package_array ().foreach ((pk_package) => { |
421 | installed.add (pk_package); |
422 | }); |
423 | |
424 | - release_task (packages_task); |
425 | } catch (Error e) { |
426 | critical (e.message); |
427 | - release_task (packages_task); |
428 | } |
429 | |
430 | + task_count--; |
431 | return installed; |
432 | } |
433 | |
434 | |
435 | === modified file 'src/Core/Task.vala' |
436 | --- src/Core/Task.vala 2016-07-18 21:16:57 +0000 |
437 | +++ src/Core/Task.vala 2016-11-06 15:34:19 +0000 |
438 | @@ -18,11 +18,7 @@ |
439 | * Authored by: Corentin Noël <corentin@elementary.io> |
440 | */ |
441 | |
442 | -public class AppCenter.Task : Pk.Task { |
443 | - public Task () { |
444 | - |
445 | - } |
446 | - |
447 | +public class AppCenterCore.Task : Pk.Task { |
448 | public override void untrusted_question (uint request, Pk.Results results) { |
449 | user_accepted (request); |
450 | } |
451 | |
452 | === modified file 'src/MainWindow.vala' |
453 | --- src/MainWindow.vala 2016-10-17 17:27:00 +0000 |
454 | +++ src/MainWindow.vala 2016-11-06 15:34:19 +0000 |
455 | @@ -171,8 +171,8 @@ |
456 | } |
457 | |
458 | hide (); |
459 | - task_finished_connection = client.tasks_finished.connect (() => { |
460 | - if (!visible) { |
461 | + task_finished_connection = client.notify["task-count"].connect (() => { |
462 | + if (!visible && client.task_count == 0) { |
463 | destroy (); |
464 | } |
465 | }); |
Works fine here, everything looks sane