Ubuntu Software Center

Merge lp:~mvo/software-center/lp971776 into lp:software-center/5.2

  1. lp971776
  2. Merge into 5.2
Proposed by Michael Vogt
Status: Merged
Merged at revision: 3045
Proposed branch: lp:~mvo/software-center/lp971776
Merge into: lp:software-center/5.2
Diff against target: 100 lines (+33/-21)
2 files modified
softwarecenter/ui/gtk3/models/appstore2.py (+17/-12)
test/gtk3/test_appstore2.py (+16/-9)
To merge this branch: bzr merge lp:~mvo/software-center/lp971776
Reviewer Review Type Date Requested Status
Gary Lasker (community) Approve
Review via email: mp+108689@code.launchpad.net

This proposal supersedes a proposal from 2012-06-05.

Description of the change

Fix crash when a image downloaded is invalid for whatever reason (network issues, proxy issues, pay-wall in between). This is the top #5 bug on the software-center errors.ubuntu.com subpage.

To post a comment you must log in.
Revision history for this message
Gary Lasker (gary-lasker) wrote :

Looks great and should do the trick nicely! Thanks for adding the unit test for this error case. I also tested by clearing the local icon cache, everything works like a charm.

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/ui/gtk3/models/appstore2.py'
2--- softwarecenter/ui/gtk3/models/appstore2.py 2012-05-22 14:36:03 +0000
3+++ softwarecenter/ui/gtk3/models/appstore2.py 2012-06-05 07:25:29 +0000
4@@ -121,27 +121,32 @@
5 else:
6 self.icon_cache = {}
7
8+ def _on_image_download_complete(self, downloader, image_file_path, pkgname):
9+ LOG.debug("download for '%s' complete" % image_file_path)
10+ try:
11+ pb = GdkPixbuf.Pixbuf.new_from_file_at_size(image_file_path,
12+ self.icon_size,
13+ self.icon_size)
14+ except GObject.GError as e:
15+ LOG.warn("Failed to get image file for '%s' (%s)",
16+ image_file_path, e)
17+ return
18+ # replace the icon in the icon_cache now that we've got the real
19+ # one
20+ icon_file = split_icon_ext(os.path.basename(image_file_path))
21+ self.icon_cache[icon_file] = pb
22+ self.emit("needs-refresh", pkgname)
23+
24 def _download_icon_and_show_when_ready(self, url, pkgname, icon_file_name):
25 LOG.debug("did not find the icon locally, must download %s" %
26 icon_file_name)
27
28- def on_image_download_complete(downloader, image_file_path, pkgname):
29- LOG.debug("download for '%s' complete" % image_file_path)
30- pb = GdkPixbuf.Pixbuf.new_from_file_at_size(icon_file_path,
31- self.icon_size,
32- self.icon_size)
33- # replace the icon in the icon_cache now that we've got the real
34- # one
35- icon_file = split_icon_ext(os.path.basename(image_file_path))
36- self.icon_cache[icon_file] = pb
37- self.emit("needs-refresh", pkgname)
38-
39 if url is not None:
40 icon_file_path = os.path.join(SOFTWARE_CENTER_ICON_CACHE_DIR,
41 icon_file_name)
42 image_downloader = SimpleFileDownloader()
43 image_downloader.connect('file-download-complete',
44- on_image_download_complete, pkgname)
45+ self._on_image_download_complete, pkgname)
46 image_downloader.download_file(url, icon_file_path)
47
48 @property
49
50=== modified file 'test/gtk3/test_appstore2.py'
51--- test/gtk3/test_appstore2.py 2012-05-18 05:47:10 +0000
52+++ test/gtk3/test_appstore2.py 2012-06-05 07:25:29 +0000
53@@ -4,7 +4,7 @@
54 import xapian
55
56 from gi.repository import Gtk
57-from mock import patch
58+from mock import Mock, patch
59
60 from testutils import setup_test_env
61 setup_test_env()
62@@ -16,13 +16,14 @@
63 get_test_gtk3_icon_cache,
64 )
65
66-class TestAppstore(unittest.TestCase):
67+class AppStoreTestCase(unittest.TestCase):
68 """ test the appstore """
69
70- def setUp(self):
71- self.cache = get_test_pkg_info()
72- self.icons = get_test_gtk3_icon_cache()
73- self.db = get_test_db()
74+ @classmethod
75+ def setUpClass(cls):
76+ cls.cache = get_test_pkg_info()
77+ cls.icons = get_test_gtk3_icon_cache()
78+ cls.db = get_test_db()
79
80 def test_lp872760(self):
81 def monkey_(s):
82@@ -68,9 +69,15 @@
83 # ensure clear works
84 model.clear()
85 self.assertEqual(model.current_matches, None)
86-
87+
88+ def test_lp971776(self):
89+ """ ensure that refresh is not called for invalid image files """
90+ model = AppListStore(self.db, self.cache, self.icons)
91+ model.emit = Mock()
92+ model._on_image_download_complete(None, "xxx", "software-center")
93+ self.assertFalse(model.emit.called)
94
95 if __name__ == "__main__":
96- import logging
97- logging.basicConfig(level=logging.DEBUG)
98+ #import logging
99+ #logging.basicConfig(level=logging.DEBUG)
100 unittest.main()

Subscribers

People subscribed via source and target branches