Merge lp:~donadigo/appcenter/refactor-package-class into lp:~elementary-apps/appcenter/appcenter

Proposed by Adam Bieńkowski
Status: Merged
Approved by: Corentin Noël
Approved revision: 355
Merged at revision: 363
Proposed branch: lp:~donadigo/appcenter/refactor-package-class
Merge into: lp:~elementary-apps/appcenter/appcenter
Diff against target: 189 lines (+55/-35)
1 file modified
src/Core/Package.vala (+55/-35)
To merge this branch: bzr merge lp:~donadigo/appcenter/refactor-package-class
Reviewer Review Type Date Requested Status
Danielle Foré Approve
Review via email: mp+313878@code.launchpad.net

Commit message

Package.vala:
* Use GObject-style construction
* Cache properties

Description of the change

This branch refactors package class, to improve performance and reduce unneded operations.

The class now uses GObject construction. In addition, instead of always getting the property of the class like name, summary and version, it has each property saved, so there is no need for getting the property again, the packagekit package is also cached.

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

Looks good. I haven't noticed any issues with this branch :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/Core/Package.vala'
--- src/Core/Package.vala 2016-09-14 16:58:13 +0000
+++ src/Core/Package.vala 2016-12-26 21:41:27 +0000
@@ -33,7 +33,7 @@
3333
34 public const string OS_UPDATES_ID = "xxx-os-updates";34 public const string OS_UPDATES_ID = "xxx-os-updates";
3535
36 public AppStream.Component component { public get; private set; }36 public AppStream.Component component { get; construct; }
37 public ChangeInformation change_information { public get; private set; }37 public ChangeInformation change_information { public get; private set; }
38 public Gee.TreeSet<Pk.Package> installed_packages { public get; private set; }38 public Gee.TreeSet<Pk.Package> installed_packages { public get; private set; }
39 public GLib.Cancellable action_cancellable { public get; private set; }39 public GLib.Cancellable action_cancellable { public get; private set; }
@@ -72,20 +72,26 @@
72 public bool is_os_updates {72 public bool is_os_updates {
73 get {73 get {
74 return component.id == "xxx-os-updates";74 return component.id == "xxx-os-updates";
75 }75 }
76 }76 }
7777
78 public Package (AppStream.Component component) {78 private string? name = null;
79 this.component = component;79 private string? summary = null;
80 private string? version = null;
81 private Pk.Package? pk_package = null;
82
83 construct {
80 installed_packages = new Gee.TreeSet<Pk.Package> ();84 installed_packages = new Gee.TreeSet<Pk.Package> ();
81 change_information = new ChangeInformation ();85 change_information = new ChangeInformation ();
82 change_information.status_changed.connect (() => {86 change_information.status_changed.connect (() => info_changed (change_information.status));
83 info_changed (change_information.status);
84 });
8587
86 action_cancellable = new GLib.Cancellable ();88 action_cancellable = new GLib.Cancellable ();
87 }89 }
8890
91 public Package (AppStream.Component component) {
92 Object (component: component);
93 }
94
89 public void update_state () {95 public void update_state () {
90 if (installed) {96 if (installed) {
91 if (change_information.has_changes ()) {97 if (change_information.has_changes ()) {
@@ -102,6 +108,7 @@
102 if (state != State.UPDATE_AVAILABLE) {108 if (state != State.UPDATE_AVAILABLE) {
103 return false;109 return false;
104 }110 }
111
105 return yield perform_operation (State.UPDATING, State.INSTALLED, State.UPDATE_AVAILABLE);112 return yield perform_operation (State.UPDATING, State.INSTALLED, State.UPDATE_AVAILABLE);
106 }113 }
107114
@@ -109,6 +116,7 @@
109 if (state != State.NOT_INSTALLED) {116 if (state != State.NOT_INSTALLED) {
110 return false;117 return false;
111 }118 }
119
112 if (yield perform_operation (State.INSTALLING, State.INSTALLED, State.NOT_INSTALLED)) {120 if (yield perform_operation (State.INSTALLING, State.INSTALLED, State.NOT_INSTALLED)) {
113 /* TODO: Move this to a higher level */121 /* TODO: Move this to a higher level */
114 var application = (Gtk.Application)Application.get_default ();122 var application = (Gtk.Application)Application.get_default ();
@@ -121,6 +129,7 @@
121129
122 application.send_notification ("installed", notification);130 application.send_notification ("installed", notification);
123 }131 }
132
124 return true;133 return true;
125 }134 }
126135
@@ -131,6 +140,7 @@
131 if (state != State.INSTALLED) {140 if (state != State.INSTALLED) {
132 return false;141 return false;
133 }142 }
143
134 return yield perform_operation (State.REMOVING, State.NOT_INSTALLED, State.INSTALLED);144 return yield perform_operation (State.REMOVING, State.NOT_INSTALLED, State.INSTALLED);
135 }145 }
136146
@@ -144,6 +154,7 @@
144 } finally {154 } finally {
145 clean_up_package_operation (exit_status, after_success, after_fail);155 clean_up_package_operation (exit_status, after_success, after_fail);
146 }156 }
157
147 return (exit_status == Pk.Exit.SUCCESS);158 return (exit_status == Pk.Exit.SUCCESS);
148 }159 }
149160
@@ -184,31 +195,35 @@
184 }195 }
185196
186 public string? get_name () {197 public string? get_name () {
187 var _name = component.get_name ();198 if (name != null) {
188 if (_name != null) {199 return name;
189 return _name;200 }
190 }201
191202 name = component.get_name ();
192 var package = find_package ();203 if (name == null) {
193 if (package != null) {204 var package = find_package ();
194 return package.get_name ();205 if (package != null) {
195 }206 name = package.get_name ();
196207 }
197 return null;208 }
209
210 return name;
198 }211 }
199212
200 public string? get_summary () {213 public string? get_summary () {
201 var summary = component.get_summary ();
202 if (summary != null) {214 if (summary != null) {
203 return summary;215 return summary;
204 }216 }
205217
206 var package = find_package ();218 summary = component.get_summary ();
207 if (package != null) {219 if (summary == null) {
208 return package.get_summary ();220 var package = find_package ();
221 if (package != null) {
222 summary = package.get_summary ();
223 }
209 }224 }
210225
211 return null;226 return summary;
212 }227 }
213228
214 public string get_progress_description () {229 public string get_progress_description () {
@@ -265,6 +280,10 @@
265 }280 }
266281
267 public string? get_version () {282 public string? get_version () {
283 if (version != null) {
284 return version;
285 }
286
268 var package = find_package ();287 var package = find_package ();
269 if (package != null) {288 if (package != null) {
270 string returned = package.get_version ();289 string returned = package.get_version ();
@@ -275,27 +294,28 @@
275 returned = returned.split (":", 2)[1];294 returned = returned.split (":", 2)[1];
276 }295 }
277296
278 return returned;297 version = returned;
279 }298 }
280299
281 return null;300 return version;
282 }301 }
283302
284 private Pk.Package? find_package (bool installed = false) {303 private Pk.Package? find_package () {
285 if (component.id == OS_UPDATES_ID) {304 if (component.id == OS_UPDATES_ID) {
286 return null;305 return null;
287 }306 }
288307
308 if (pk_package != null) {
309 return pk_package;
310 }
311
289 try {312 try {
290 Pk.Bitfield filter = 0;313 pk_package = AppCenterCore.Client.get_default ().get_app_package (component.get_pkgnames ()[0], 0);
291 if (installed) {
292 filter = Pk.Bitfield.from_enums (Pk.Filter.INSTALLED);
293 }
294
295 return AppCenterCore.Client.get_default ().get_app_package (component.get_pkgnames ()[0], filter);
296 } catch (Error e) {314 } catch (Error e) {
297 critical (e.message);315 warning (e.message);
298 return null;316 return null;
299 }317 }
318
319 return pk_package;
300 }320 }
301}321}

Subscribers

People subscribed via source and target branches