Merge lp:~mvo/software-center/region-whitelist-lp1006570 into lp:software-center

Proposed by Michael Vogt on 2012-09-10
Status: Merged
Merged at revision: 3158
Proposed branch: lp:~mvo/software-center/region-whitelist-lp1006570
Merge into: lp:software-center
Diff against target: 217 lines (+93/-28)
4 files modified
softwarecenter/db/update.py (+29/-10)
softwarecenter/region.py (+2/-0)
tests/test_database.py (+60/-17)
tests/utils.py (+2/-1)
To merge this branch: bzr merge lp:~mvo/software-center/region-whitelist-lp1006570
Reviewer Review Type Date Requested Status
Gary Lasker (community) 2012-09-10 Approve on 2012-09-11
Review via email: mp+123504@code.launchpad.net

Commit message

Fix missing region whitelist support that is used by the software-center-agent (LP: #1006570)

Description of the change

Add support for the region-whitelist feature requested in LP: #1006570

To post a comment you must log in.
Anthony Lenton (elachuni) wrote :

Hi mvo!

Awesome, thanks for the fix. It probably makes sense to lowercase/uppercase the tags retrieved from the API, to make the feature case-insensitive. For example, in the API I see "whitelist-iso3166::US" for an app atm, it would be great if that case just works.

3156. By Michael Vogt on 2012-09-10

use .lower() on the tags and update test to simulate passing uppercase tags (like the server is doing)

3157. By Michael Vogt on 2012-09-10

drive-by-cleanup

3158. By Michael Vogt on 2012-09-10

another small drive-by-cleanup that allows to give make_software_center_agent_app_dict() a override dict

3159. By Michael Vogt on 2012-09-10

trivial pep8 fixes

Michael Vogt (mvo) wrote :

Thanks, good point, I fixed that now and updated the test.

Gary Lasker (gary-lasker) wrote :

This looks fine. Approval also based on Anthony's comment above and Michael's subsequent updates per his comment.

Many thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/db/update.py'
2--- softwarecenter/db/update.py 2012-08-23 14:37:28 +0000
3+++ softwarecenter/db/update.py 2012-09-10 09:45:22 +0000
4@@ -69,8 +69,16 @@
5
6
7 from softwarecenter.db.pkginfo import get_pkg_info
8-from softwarecenter.distro import get_current_arch, get_foreign_architectures
9-from softwarecenter.region import get_region_cached, REGION_BLACKLIST_TAG
10+from softwarecenter.distro import (
11+ get_current_arch,
12+ get_foreign_architectures,
13+ )
14+from softwarecenter.region import (
15+ get_region_cached,
16+ REGION_BLACKLIST_TAG,
17+ REGION_WHITELIST_TAG,
18+ )
19+
20
21 # weights for the different fields
22 WEIGHT_DESKTOP_NAME = 10
23@@ -380,19 +388,30 @@
24 doc.add_term("AT" + app_type.lower())
25
26 # (deb)tags (in addition to the pkgname debtags)
27- tags = self.get_value(AppInfoFields.TAGS)
28- if tags:
29- # register tags
30- for tag in tags.split(","):
31- doc.add_term("XT" + tag.strip())
32- # ENFORCE region blacklist by not registering the app at all
33+ tags_string = self.get_value(AppInfoFields.TAGS)
34+ if tags_string:
35+ # convert to list and register
36+ tags = [tag.strip().lower() for tag in tags_string.split(",")]
37+ for tag in tags:
38+ doc.add_term("XT" + tag)
39+ # ENFORCE region blacklist/whitelist by not registering
40+ # the app at all
41 region = get_region_cached()
42 if region:
43 countrycode = region["countrycode"].lower()
44+ # blacklist
45 if "%s%s" % (REGION_BLACKLIST_TAG, countrycode) in tags:
46- LOG.info("%r.make_doc: skipping region restricted app %r",
47- self.__class__.__name__, name)
48+ LOG.info("%r.make_doc: skipping region restricted app %r "
49+ "(blacklisted)", self.__class__.__name__, name)
50 return
51+ # whitelist
52+ for tag in tags:
53+ if (tag.startswith(REGION_WHITELIST_TAG) and not
54+ "%s%s" % (REGION_WHITELIST_TAG, countrycode) in tag):
55+ LOG.info("%r.make_doc: skipping region restricted "
56+ "app %r (region not whitelisted)",
57+ self.__class__.__name__, name)
58+ return
59
60 # popcon
61 # FIXME: popularity not only based on popcon but also
62
63=== modified file 'softwarecenter/region.py'
64--- softwarecenter/region.py 2012-08-30 13:03:42 +0000
65+++ softwarecenter/region.py 2012-09-10 09:45:22 +0000
66@@ -38,6 +38,8 @@
67
68 # blacklist this region
69 REGION_BLACKLIST_TAG = "blacklist-iso3166::"
70+# or whitelist it
71+REGION_WHITELIST_TAG = "whitelist-iso3166::"
72
73
74 def get_region_name(countrycode):
75
76=== modified file 'tests/test_database.py'
77--- tests/test_database.py 2012-09-05 10:21:27 +0000
78+++ tests/test_database.py 2012-09-10 09:45:22 +0000
79@@ -47,7 +47,11 @@
80 XapianValues,
81 PkgStates,
82 )
83-from softwarecenter.region import REGION_BLACKLIST_TAG, REGIONTAG
84+from softwarecenter.region import (
85+ REGION_BLACKLIST_TAG,
86+ REGION_WHITELIST_TAG,
87+ REGIONTAG,
88+ )
89
90 PKD_DIR = os.path.join(DATA_DIR, 'appdetails', 'var', 'lib', 'dpkg', 'status')
91 TEST_DB = os.path.join(DATA_DIR, "test.db")
92@@ -522,6 +526,12 @@
93 def setUp(self):
94 self.db = get_test_db()
95
96+ def _get_app_details_from_override(self, override_dict):
97+ app_dict = make_software_center_agent_app_dict()
98+ app_dict.update(override_dict)
99+ app_details = self._get_app_details_from_app_dict(app_dict)
100+ return app_details
101+
102 def _get_app_details_from_app_dict(self, app_dict):
103 item = PistonResponseObject.from_dict(app_dict)
104 parser = SCAApplicationParser(item)
105@@ -534,10 +544,8 @@
106 # we need to patch os.path.exists as "AppDetails.channelname" will
107 # check if there is a matching channel description file on disk
108 os.path.exists.return_value = True
109- # setup dict
110- app_dict = make_software_center_agent_app_dict()
111- app_dict["archive_root"] = "http://archive.canonical.com/"
112- app_details = self._get_app_details_from_app_dict(app_dict)
113+ app_details = self._get_app_details_from_override({
114+ "archive_root": "http://archive.canonical.com/"})
115 # ensure that archive.canonical.com archive roots are detected
116 # as the partner channel
117 dist = softwarecenter.distro.get_distro().get_codename()
118@@ -549,33 +557,68 @@
119 # check if there is a matching channel description file on disk
120 os.path.exists.return_value = True
121 # setup dict
122- app_dict = make_software_center_agent_app_dict()
123- app_dict["archive_root"] = "http://extras.ubuntu.com/"
124- app_details = self._get_app_details_from_app_dict(app_dict)
125+ app_details = self._get_app_details_from_override({
126+ "archive_root": "http://extras.ubuntu.com/"})
127 # ensure that archive.canonical.com archive roots are detected
128 # as the partner channel
129 self.assertEqual(app_details.channelname, "ubuntu-extras")
130
131 def test_date_no_published(self):
132- app_dict = make_software_center_agent_app_dict()
133- app_dict["date_published"] = "None"
134- app_details = self._get_app_details_from_app_dict(app_dict)
135+ app_details = self._get_app_details_from_override({
136+ "date_published": "None"})
137 # ensure that archive.canonical.com archive roots are detected
138 # as the partner channel
139 self.assertEqual(app_details.date_published, "")
140 # and again
141- app_dict["date_published"] = "2012-01-21 02:15:10.358926"
142- app_details = self._get_app_details_from_app_dict(app_dict)
143+ app_details = self._get_app_details_from_override({
144+ "date_published": "2012-01-21 02:15:10.358926"})
145 # ensure that archive.canonical.com archive roots are detected
146 # as the partner channel
147 self.assertEqual(app_details.date_published, "2012-01-21 02:15:10")
148
149 @patch("softwarecenter.db.update.get_region_cached")
150 def test_region_blacklist(self, get_region_cached_mock):
151- get_region_cached_mock.return_value = { "countrycode" : "es",
152- }
153- app_dict = make_software_center_agent_app_dict()
154- app_dict["debtags"] = ["%s%s" % (REGION_BLACKLIST_TAG, "es")]
155+ """Test that the region blacklist ignores blacklisted locations"""
156+ get_region_cached_mock.return_value = { "countrycode" : "es",
157+ }
158+ app_dict = make_software_center_agent_app_dict({
159+ "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "es")]})
160+ item = PistonResponseObject.from_dict(app_dict)
161+ parser = SCAApplicationParser(item)
162+ doc = parser.make_doc(self.db._aptcache)
163+ self.assertEqual(doc, None)
164+
165+ @patch("softwarecenter.db.update.get_region_cached")
166+ def test_region_blacklist_blacklists(self, get_region_cached_mock):
167+ """Test that the region blacklist adds non-blacklisted locations"""
168+ get_region_cached_mock.return_value = { "countrycode" : "de",
169+ }
170+ app_dict = make_software_center_agent_app_dict({
171+ "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "ES")]})
172+ item = PistonResponseObject.from_dict(app_dict)
173+ parser = SCAApplicationParser(item)
174+ doc = parser.make_doc(self.db._aptcache)
175+ self.assertNotEqual(doc, None)
176+
177+ @patch("softwarecenter.db.update.get_region_cached")
178+ def test_region_whitelist_whitelists(self, get_region_cached_mock):
179+ """Test that the whitelist adds whitelisted locations"""
180+ get_region_cached_mock.return_value = { "countrycode" : "es",
181+ }
182+ app_dict = make_software_center_agent_app_dict({
183+ "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
184+ item = PistonResponseObject.from_dict(app_dict)
185+ parser = SCAApplicationParser(item)
186+ doc = parser.make_doc(self.db._aptcache)
187+ self.assertNotEqual(doc, None)
188+
189+ @patch("softwarecenter.db.update.get_region_cached")
190+ def test_region_whitelist_blacklists(self, get_region_cached_mock):
191+ """Test that the whitelist ignores non-whitelist locations"""
192+ get_region_cached_mock.return_value = { "countrycode" : "de",
193+ }
194+ app_dict = make_software_center_agent_app_dict({
195+ "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
196 # see _get_app_details_from_app_dict
197 item = PistonResponseObject.from_dict(app_dict)
198 parser = SCAApplicationParser(item)
199
200=== modified file 'tests/utils.py'
201--- tests/utils.py 2012-09-06 00:49:18 +0000
202+++ tests/utils.py 2012-09-10 09:45:22 +0000
203@@ -267,7 +267,7 @@
204
205
206 # factory stuff for the agent
207-def make_software_center_agent_app_dict():
208+def make_software_center_agent_app_dict(override_dict={}):
209 app_dict = {
210 u'archive_root': 'http://private-ppa.launchpad.net/',
211 u'archive_id': u'commercial-ppa-uploaders/photobomb',
212@@ -289,6 +289,7 @@
213 '2011/08/64_Chainz.png',
214 u'categories': 'Game;LogicGame',
215 }
216+ app_dict.update(override_dict)
217 return app_dict
218
219

Subscribers

People subscribed via source and target branches