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

Proposed by Michael Vogt
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) Approve
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.
Revision history for this message
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

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

3157. By Michael Vogt

drive-by-cleanup

3158. By Michael Vogt

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

3159. By Michael Vogt

trivial pep8 fixes

Revision history for this message
Michael Vogt (mvo) wrote :

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

Revision history for this message
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