Merge lp:~osomon/software-center/fix_appstore_update into lp:software-center

[Olivier Tilloy]

This branch fixes updating an existing AppStore with a new one. There were two issues:

1) Before an update, the model's "active" attribute was set to False to avoid catching signals while being updated, but it was never set back to True afterwards.

2) In the update itself, the internal (package name -> index) map was incorrectly filled with integers instead of lists of integers (and it was not cleared in the first place), leaving it in an inconsisten state.

I'm not aware if there is a bug report for this already, I can file one if needed.

[Gary Lasker (gary-lasker)]

Great catch, thanks a lot, Olivier! I made two small changes when merging, please take a look to make sure you see no problem with them:

 1. Also include a clear() for the app_index_map in the appstore's update() method.
 2. Drop the self.app_view.get_model().active = True in channelpane as that is always initialized when the new appstore is created.

review: Approve

[Olivier Tilloy (osomon)]

Thanks for the fast review Gary!
Regarding your tweaks:

1) clear()ing the app_index_map is not strictly necessary, as the update of the model is going to update the necessary entries and stale entries are never going to be accessed, but it doesn't hurt either.

2) This isn't correct, because in the case of an update, the existing appstore is updated with the contents of the new appstore, but its 'active' attribute is never re-set to True (the new appstore is deleted afterwards). I think this tweak should be reverted.

[Gary Lasker (gary-lasker)]

Hey Olivier! Thanks for your very-fast review review. :D

Regarding item 1), point taken. I'll leave it for now as I need to look at this area more closely in any case so there will be more work here.

Regarding item 2), I added that back in to trunk. Thanks again!

=== modified file 'softwarecenter/view/appview.py'
--- softwarecenter/view/appview.py	2010-05-27 17:20:33 +0000
+++ softwarecenter/view/appview.py	2010-05-28 17:11:33 +0000
@@ -257,12 +257,16 @@
         """ update this appstore to match data from another """
         # Updating instead of replacing prevents a distracting white
         # flash. First, match list of apps.
+        self.pkgname_index_map.clear()
         to_update = min(len(self), len(appstore))
         for i in range(to_update):
             self.apps[i] = appstore.apps[i]
             self.row_changed(i, self.get_iter(i))
             self.app_index_map[self.apps[i]] = i
-            self.pkgname_index_map[self.apps[i].pkgname] = i
+            pkgname = self.apps[i].pkgname
+            if pkgname not in self.pkgname_index_map:
+                self.pkgname_index_map[pkgname] = []
+            self.pkgname_index_map[pkgname].append(i)
 
         to_remove = max(0, len(self) - len(appstore))
         for i in range(to_remove):

=== modified file 'softwarecenter/view/channelpane.py'
--- softwarecenter/view/channelpane.py	2010-05-21 05:31:19 +0000
+++ softwarecenter/view/channelpane.py	2010-05-28 17:11:33 +0000
@@ -121,6 +121,7 @@
             self.scroll_app_list.window.set_cursor(None)
         if seq_nr == self.refresh_seq_nr:
             self.app_view.set_model(new_model)
+            self.app_view.get_model().active = True
             self.emit("app-list-changed", len(new_model))
         else:
             logging.debug("discarding new model (%s != %s)" % (seq_nr, self.refresh_seq_nr))