Merge lp:~michael.nelson/software-center/917137-previous-purchases-empty-precise-2 into lp:software-center

Proposed by Michael Nelson on 2012-01-18
Status: Merged
Merged at revision: 2671
Proposed branch: lp:~michael.nelson/software-center/917137-previous-purchases-empty-precise-2
Merge into: lp:software-center
Prerequisite: lp:~michael.nelson/software-center/833982-purchased-app-not-available
Diff against target: 333 lines (+74/-74)
5 files modified
softwarecenter/backend/scagent.py (+5/-3)
softwarecenter/db/update.py (+43/-47)
softwarecenter/ui/gtk3/app.py (+1/-3)
test/test_reinstall_purchased.py (+11/-19)
test/test_scagent.py (+14/-2)
To merge this branch: bzr merge lp:~michael.nelson/software-center/917137-previous-purchases-empty-precise-2
Reviewer Review Type Date Requested Status
Michael Vogt 2012-01-18 Approve on 2012-01-18
Review via email: mp+88995@code.launchpad.net

Description of the change

Overview
========
This branch continues on from:

https://code.launchpad.net/~michael.nelson/software-center/833982-purchased-app-not-available/+merge/88827

and finishes up the remaining items listed there.

Details
=======
 * Check if we can update PURCHASED_NEEDS_REINSTALL_MAGIC_CHANNEL_NAME = "For Purchase" (value returned via API). How will this affect existing clients, if at all?
   * No, we can't until SC agent returns different channels for available_apps and subscriptions_for_me - updated bug 917109 with tat info.
 * remove the need for the SCASubscriptionParser to both inherit from and encapsulate the SoftwareCenterAgentParser - DONE
 * Filter results of the my_subscriptions call (only keep Complete ones), as currently it looks like this: http://people.canonical.com/~michaeln/tmp/833982-previous-purchases-working-but-dupes.png - DONE (the client wasn't passing the complete_only=True option)
 * Rename s/SoftwareCenterAgentParser/SCAApplicationParser for consistency - DONE
 * Replace the MockAvailableForMeItem/List with real PistonResponseObjects - DONE

Other tid-bits:
 * I removed what I think are unused args from: def query_available_for_me(self, oauth_token, openid_identifier)

To post a comment you must log in.
Michael Vogt (mvo) wrote :

Woah, that is pretty cool magic:

+ MAPPING = dict(
+ SCAApplicationParser.MAPPING.items() + SUBSCRIPTION_MAPPING.items())

review: Approve
Michael Vogt (mvo) wrote :

Well, I guess I should have said: a cool solution for the problem of both inherit from and encapsulate.

Thanks for the removal of the args for query_available_for_me(self, oauth_token, openid_identifier) too.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/backend/scagent.py'
2--- softwarecenter/backend/scagent.py 2012-01-06 05:43:34 +0000
3+++ softwarecenter/backend/scagent.py 2012-01-18 10:07:28 +0000
4@@ -92,7 +92,7 @@
5 def _on_query_available_data(self, spawner, piston_available):
6 self.emit("available", piston_available)
7
8- def query_available_for_me(self, oauth_token, openid_identifier):
9+ def query_available_for_me(self):
10 spawner = SpawnHelper()
11 spawner.parent_xid = self.xid
12 spawner.ignore_cache = self.ignore_cache
13@@ -100,7 +100,9 @@
14 spawner.connect("error", lambda spawner, err: self.emit("error", err))
15 spawner.needs_auth = True
16 spawner.run_generic_piston_helper(
17- "SoftwareCenterAgentAPI", "subscriptions_for_me")
18+ "SoftwareCenterAgentAPI", "subscriptions_for_me",
19+ complete_only=True)
20+
21 def _on_query_available_for_me_data(self, spawner, piston_available_for_me):
22 self.emit("available-for-me", piston_available_for_me)
23
24@@ -147,7 +149,7 @@
25 scagent.connect("exhibits", _exhibits)
26 scagent.connect("error", _error)
27 #scagent.query_available("natty", "i386")
28- #scagent.query_available_for_me("dummy_oauth", "dummy openid")
29+ #scagent.query_available_for_me()
30 scagent.query_exhibits()
31
32 from gi.repository import Gtk
33
34=== modified file 'softwarecenter/db/update.py'
35--- softwarecenter/db/update.py 2012-01-18 10:07:28 +0000
36+++ softwarecenter/db/update.py 2012-01-18 10:07:28 +0000
37@@ -132,11 +132,10 @@
38 def desktopf(self):
39 """ return the file that the AppInfo comes from """
40
41-
42-class SoftwareCenterAgentParser(AppInfoParserBase):
43+class SCAApplicationParser(AppInfoParserBase):
44 """ map the data we get from the software-center-agent """
45
46- # map from requested key to sca_entry attribute
47+ # map from requested key to sca_application attribute
48 MAPPING = { 'Name' : 'name',
49 'Price' : 'price',
50 'Package' : 'package_name',
51@@ -159,8 +158,8 @@
52 STATIC_DATA = { 'Type' : 'Application',
53 }
54
55- def __init__(self, sca_entry):
56- self.sca_entry = sca_entry
57+ def __init__(self, sca_application):
58+ self.sca_application = sca_application
59 self.origin = "software-center-agent"
60 self._apply_exceptions()
61
62@@ -168,18 +167,18 @@
63 # for items from the agent, we use the full-size screenshot for
64 # the thumbnail and scale it for display, this is done because
65 # we no longer keep thumbnail versions of screenshots on the server
66- if (hasattr(self.sca_entry, "screenshot_url") and
67- not hasattr(self.sca_entry, "thumbnail_url")):
68- self.sca_entry.thumbnail_url = self.sca_entry.screenshot_url
69- if hasattr(self.sca_entry, "description"):
70- self.sca_entry.Comment = self.sca_entry.description.split("\n")[0].strip()
71- self.sca_entry.Description = "\n".join(
72- self.sca_entry.description.split("\n")[1:]).strip()
73+ if (hasattr(self.sca_application, "screenshot_url") and
74+ not hasattr(self.sca_application, "thumbnail_url")):
75+ self.sca_application.thumbnail_url = self.sca_application.screenshot_url
76+ if hasattr(self.sca_application, "description"):
77+ self.sca_application.Comment = self.sca_application.description.split("\n")[0].strip()
78+ self.sca_application.Description = "\n".join(
79+ self.sca_application.description.split("\n")[1:]).strip()
80 # WARNING: item.name needs to be different than
81 # the item.name in the DB otherwise the DB
82 # gets confused about (appname, pkgname) duplication
83- self.sca_entry.name = utf8(_("%s (already purchased)")) % utf8(
84- self.sca_entry.name)
85+ self.sca_application.name = utf8(_("%s (already purchased)")) % utf8(
86+ self.sca_application.name)
87
88 # XXX 2012-01-16 bug=917109
89 # We can remove these work-arounds once the above bug is fixed on
90@@ -187,68 +186,65 @@
91 # to make the parser happy. Note: available_apps api call includes
92 # these already, it's just the apps with subscriptions_for_me which
93 # don't currently.
94- self.sca_entry.channel = AVAILABLE_FOR_PURCHASE_MAGIC_CHANNEL_NAME
95- if not hasattr(self.sca_entry, 'categories'):
96- self.sca_entry.categories = ""
97+ self.sca_application.channel = AVAILABLE_FOR_PURCHASE_MAGIC_CHANNEL_NAME
98+ if not hasattr(self.sca_application, 'categories'):
99+ self.sca_application.categories = ""
100
101 def get_desktop(self, key, translated=True):
102 if key in self.STATIC_DATA:
103 return self.STATIC_DATA[key]
104- return getattr(self.sca_entry, self._apply_mapping(key))
105+ return getattr(self.sca_application, self._apply_mapping(key))
106
107 def get_desktop_categories(self):
108 try:
109- return ['DEPARTMENT:' + self.sca_entry.department[-1]] + self._get_desktop_list("Categories")
110+ return ['DEPARTMENT:' + self.sca_application.department[-1]] + self._get_desktop_list("Categories")
111 except:
112 return self._get_desktop_list("Categories")
113
114 def has_option_desktop(self, key):
115 return (key in self.STATIC_DATA or
116- hasattr(self.sca_entry, self._apply_mapping(key)))
117+ hasattr(self.sca_application, self._apply_mapping(key)))
118
119 @property
120 def desktopf(self):
121 return self.origin
122
123
124-class SCAPurchasedApplicationParser(SoftwareCenterAgentParser):
125- """A subscription has its own attrs with a subset of the app attributes.
126-
127- We inherit from SoftwareCenterAgentParser so that we get other methods for
128- free, and we compose a SoftwareCenterAgentParser because we need the
129- get_desktop method with the correct MAPPING. TODO: There must be a nicer
130- way to organise this so that we don't need both inheritance and composition
131- for a DRY implementation.
132- """
133+class SCAPurchasedApplicationParser(SCAApplicationParser):
134+ """A purchased application hase some additional subscription attributes."""
135
136 def __init__(self, sca_subscription):
137 # The sca_subscription is a PistonResponseObject, whereas any child
138 # objects are normal Python dicts.
139 self.sca_subscription = sca_subscription
140- self.application_parser = SoftwareCenterAgentParser(
141- PistonResponseObject.from_dict(sca_subscription.application))
142 super(SCAPurchasedApplicationParser, self).__init__(
143 PistonResponseObject.from_dict(sca_subscription.application))
144- self.application_parser.sca_entry.channel = (
145+ self.sca_application.channel = (
146 PURCHASED_NEEDS_REINSTALL_MAGIC_CHANNEL_NAME)
147
148- MAPPING = { 'Deb-Line' : 'deb_line',
149- 'Purchased-Date' : 'purchase_date',
150- 'License-Key' : 'license_key',
151- 'License-Key-Path' : 'license_key_path',
152- }
153+ SUBSCRIPTION_MAPPING = {
154+ 'Deb-Line' : 'deb_line',
155+ 'Purchased-Date' : 'purchase_date',
156+ 'License-Key' : 'license_key',
157+ 'License-Key-Path' : 'license_key_path',
158+ }
159+
160+ MAPPING = dict(
161+ SCAApplicationParser.MAPPING.items() + SUBSCRIPTION_MAPPING.items())
162
163 def get_desktop(self, key, translated=True):
164- if self.application_parser.has_option_desktop(key):
165- return self.application_parser.get_desktop(key, translated)
166-
167- return getattr(self.sca_subscription, self._apply_mapping(key))
168-
169- def has_option_desktop(self, key):
170- subscription_has_option = hasattr(
171+ if self._subscription_has_option_desktop(key):
172+ return getattr(self.sca_subscription, self._apply_mapping(key))
173+ return super(SCAPurchasedApplicationParser, self).get_desktop(key)
174+
175+ def _subscription_has_option_desktop(self, key):
176+ return hasattr(
177 self.sca_subscription, self._apply_mapping(key))
178- application_has_option = (
179- self.application_parser.has_option_desktop(key))
180+
181+ def has_option_desktop(self, key):
182+ subscription_has_option = self._subscription_has_option_desktop(key)
183+ application_has_option = super(
184+ SCAPurchasedApplicationParser, self).has_option_desktop(key)
185 return subscription_has_option or application_has_option
186
187
188@@ -614,7 +610,7 @@
189 context.iteration()
190 try:
191 # now the normal parser
192- parser = SoftwareCenterAgentParser(entry)
193+ parser = SCAApplicationParser(entry)
194 index_app_info_from_parser(parser, db, cache)
195 except Exception as e:
196 LOG.warning("error processing: %s " % e)
197
198=== modified file 'softwarecenter/ui/gtk3/app.py'
199--- softwarecenter/ui/gtk3/app.py 2012-01-12 21:09:38 +0000
200+++ softwarecenter/ui/gtk3/app.py 2012-01-18 10:07:28 +0000
201@@ -591,9 +591,7 @@
202 # appmanager needs to know about the oauth token for the reinstall
203 # previous purchases add_license_key call
204 self.app_manager.oauth_token = oauth_result
205- # consumer key is the openid identifier
206- self.scagent.query_available_for_me(oauth_result["token"],
207- oauth_result["consumer_key"])
208+ self.scagent.query_available_for_me()
209
210 def _on_style_updated(self, widget, init_css_callback, *args):
211 init_css_callback(widget, *args)
212
213=== modified file 'test/test_reinstall_purchased.py'
214--- test/test_reinstall_purchased.py 2012-01-18 10:07:28 +0000
215+++ test/test_reinstall_purchased.py 2012-01-18 10:07:28 +0000
216@@ -19,7 +19,7 @@
217 from softwarecenter.db.update import (
218 add_from_purchased_but_needs_reinstall_data,
219 SCAPurchasedApplicationParser,
220- SoftwareCenterAgentParser,
221+ SCAApplicationParser,
222 )
223
224 # Example taken from running:
225@@ -94,30 +94,22 @@
226 ]
227 """
228
229-class MockAvailableForMeItem(object):
230- def __init__(self, entry_dict):
231- for key, value in entry_dict.iteritems():
232- setattr(self, key, value)
233- self.MimeType = ""
234- self.department = []
235-
236-class MockAvailableForMeList(list):
237-
238- def __init__(self):
239- alist = json.loads(SUBSCRIPTIONS_FOR_ME_JSON)
240- for entry_dict in alist:
241- self.append(MockAvailableForMeItem(entry_dict))
242
243 class TestPurchased(unittest.TestCase):
244 """ tests the store database """
245
246+ def _make_available_for_me_list(self):
247+ my_subscriptions = json.loads(SUBSCRIPTIONS_FOR_ME_JSON)
248+ return list(
249+ PistonResponseObject.from_dict(subs) for subs in my_subscriptions)
250+
251 def setUp(self):
252 # use fixture apt data
253 apt_pkg.config.set("APT::Architecture", "i386")
254 apt_pkg.config.set("Dir::State::status",
255 "./data/appdetails/var/lib/dpkg/status")
256 # create mocks
257- self.available_to_me = MockAvailableForMeList()
258+ self.available_to_me = self._make_available_for_me_list()
259 self.cache = apt.Cache()
260
261 def test_reinstall_purchased_mock(self):
262@@ -153,18 +145,18 @@
263 "/photobomb/ubuntu natty main")
264
265
266-class SoftwareCenterAgentParserTestCase(unittest.TestCase):
267+class SCAApplicationParserTestCase(unittest.TestCase):
268
269 def _make_application_parser(self, piston_application=None):
270 if piston_application is None:
271 piston_application = PistonResponseObject.from_dict(
272 json.loads(AVAILABLE_APPS_JSON)[0])
273- return SoftwareCenterAgentParser(piston_application)
274+ return SCAApplicationParser(piston_application)
275
276 def test_parses_application_from_available_apps(self):
277 parser = self._make_application_parser()
278 inverse_map = dict(
279- (val, key) for key, val in SoftwareCenterAgentParser.MAPPING.items())
280+ (val, key) for key, val in SCAApplicationParser.MAPPING.items())
281
282 # Delete the keys which are not yet provided via the API:
283 del(inverse_map['video_url'])
284@@ -172,7 +164,7 @@
285 for key in inverse_map:
286 self.assertTrue(parser.has_option_desktop(inverse_map[key]))
287 self.assertEqual(
288- getattr(parser.sca_entry, key),
289+ getattr(parser.sca_application, key),
290 parser.get_desktop(inverse_map[key]))
291
292 def test_keys_not_provided_by_api(self):
293
294=== modified file 'test/test_scagent.py'
295--- test/test_scagent.py 2012-01-16 14:42:49 +0000
296+++ test/test_scagent.py 2012-01-18 10:07:28 +0000
297@@ -1,6 +1,7 @@
298 #!/usr/bin/python
299
300 from gi.repository import GObject
301+from mock import patch
302 import unittest
303
304 from testutils import setup_test_env
305@@ -31,8 +32,7 @@
306 self.loop.run()
307 self.assertFalse(self.error)
308
309- # disabled for now as this is not yet on staging or production
310- def disabled_test_scagent_query_exhibits(self):
311+ def test_scagent_query_exhibits(self):
312 sca = SoftwareCenterAgent()
313 sca.connect("exhibits", self.on_query_done)
314 sca.connect("error", self.on_query_error)
315@@ -40,6 +40,18 @@
316 self.loop.run()
317 self.assertFalse(self.error)
318
319+ def test_scaagent_query_available_for_me_uses_complete_only(self):
320+ run_generic_piston_helper_fn = (
321+ 'softwarecenter.backend.spawn_helper.SpawnHelper.'
322+ 'run_generic_piston_helper')
323+ with patch(run_generic_piston_helper_fn) as mock_run_piston_helper:
324+ sca = SoftwareCenterAgent()
325+ sca.query_available_for_me()
326+
327+ mock_run_piston_helper.assert_called_with(
328+ 'SoftwareCenterAgentAPI', 'subscriptions_for_me',
329+ complete_only=True)
330+
331
332 if __name__ == "__main__":
333 import logging