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
=== modified file 'softwarecenter/db/update.py'
--- softwarecenter/db/update.py 2012-08-23 14:37:28 +0000
+++ softwarecenter/db/update.py 2012-09-10 09:45:22 +0000
@@ -69,8 +69,16 @@
6969
7070
71from softwarecenter.db.pkginfo import get_pkg_info71from softwarecenter.db.pkginfo import get_pkg_info
72from softwarecenter.distro import get_current_arch, get_foreign_architectures72from softwarecenter.distro import (
73from softwarecenter.region import get_region_cached, REGION_BLACKLIST_TAG73 get_current_arch,
74 get_foreign_architectures,
75 )
76from softwarecenter.region import (
77 get_region_cached,
78 REGION_BLACKLIST_TAG,
79 REGION_WHITELIST_TAG,
80 )
81
7482
75# weights for the different fields83# weights for the different fields
76WEIGHT_DESKTOP_NAME = 1084WEIGHT_DESKTOP_NAME = 10
@@ -380,19 +388,30 @@
380 doc.add_term("AT" + app_type.lower())388 doc.add_term("AT" + app_type.lower())
381389
382 # (deb)tags (in addition to the pkgname debtags)390 # (deb)tags (in addition to the pkgname debtags)
383 tags = self.get_value(AppInfoFields.TAGS)391 tags_string = self.get_value(AppInfoFields.TAGS)
384 if tags:392 if tags_string:
385 # register tags393 # convert to list and register
386 for tag in tags.split(","):394 tags = [tag.strip().lower() for tag in tags_string.split(",")]
387 doc.add_term("XT" + tag.strip())395 for tag in tags:
388 # ENFORCE region blacklist by not registering the app at all396 doc.add_term("XT" + tag)
397 # ENFORCE region blacklist/whitelist by not registering
398 # the app at all
389 region = get_region_cached()399 region = get_region_cached()
390 if region:400 if region:
391 countrycode = region["countrycode"].lower()401 countrycode = region["countrycode"].lower()
402 # blacklist
392 if "%s%s" % (REGION_BLACKLIST_TAG, countrycode) in tags:403 if "%s%s" % (REGION_BLACKLIST_TAG, countrycode) in tags:
393 LOG.info("%r.make_doc: skipping region restricted app %r",404 LOG.info("%r.make_doc: skipping region restricted app %r "
394 self.__class__.__name__, name)405 "(blacklisted)", self.__class__.__name__, name)
395 return406 return
407 # whitelist
408 for tag in tags:
409 if (tag.startswith(REGION_WHITELIST_TAG) and not
410 "%s%s" % (REGION_WHITELIST_TAG, countrycode) in tag):
411 LOG.info("%r.make_doc: skipping region restricted "
412 "app %r (region not whitelisted)",
413 self.__class__.__name__, name)
414 return
396415
397 # popcon416 # popcon
398 # FIXME: popularity not only based on popcon but also417 # FIXME: popularity not only based on popcon but also
399418
=== modified file 'softwarecenter/region.py'
--- softwarecenter/region.py 2012-08-30 13:03:42 +0000
+++ softwarecenter/region.py 2012-09-10 09:45:22 +0000
@@ -38,6 +38,8 @@
3838
39# blacklist this region39# blacklist this region
40REGION_BLACKLIST_TAG = "blacklist-iso3166::"40REGION_BLACKLIST_TAG = "blacklist-iso3166::"
41# or whitelist it
42REGION_WHITELIST_TAG = "whitelist-iso3166::"
4143
4244
43def get_region_name(countrycode):45def get_region_name(countrycode):
4446
=== modified file 'tests/test_database.py'
--- tests/test_database.py 2012-09-05 10:21:27 +0000
+++ tests/test_database.py 2012-09-10 09:45:22 +0000
@@ -47,7 +47,11 @@
47 XapianValues,47 XapianValues,
48 PkgStates,48 PkgStates,
49 )49 )
50from softwarecenter.region import REGION_BLACKLIST_TAG, REGIONTAG50from softwarecenter.region import (
51 REGION_BLACKLIST_TAG,
52 REGION_WHITELIST_TAG,
53 REGIONTAG,
54 )
5155
52PKD_DIR = os.path.join(DATA_DIR, 'appdetails', 'var', 'lib', 'dpkg', 'status')56PKD_DIR = os.path.join(DATA_DIR, 'appdetails', 'var', 'lib', 'dpkg', 'status')
53TEST_DB = os.path.join(DATA_DIR, "test.db")57TEST_DB = os.path.join(DATA_DIR, "test.db")
@@ -522,6 +526,12 @@
522 def setUp(self):526 def setUp(self):
523 self.db = get_test_db()527 self.db = get_test_db()
524528
529 def _get_app_details_from_override(self, override_dict):
530 app_dict = make_software_center_agent_app_dict()
531 app_dict.update(override_dict)
532 app_details = self._get_app_details_from_app_dict(app_dict)
533 return app_details
534
525 def _get_app_details_from_app_dict(self, app_dict):535 def _get_app_details_from_app_dict(self, app_dict):
526 item = PistonResponseObject.from_dict(app_dict)536 item = PistonResponseObject.from_dict(app_dict)
527 parser = SCAApplicationParser(item)537 parser = SCAApplicationParser(item)
@@ -534,10 +544,8 @@
534 # we need to patch os.path.exists as "AppDetails.channelname" will544 # we need to patch os.path.exists as "AppDetails.channelname" will
535 # check if there is a matching channel description file on disk545 # check if there is a matching channel description file on disk
536 os.path.exists.return_value = True546 os.path.exists.return_value = True
537 # setup dict547 app_details = self._get_app_details_from_override({
538 app_dict = make_software_center_agent_app_dict()548 "archive_root": "http://archive.canonical.com/"})
539 app_dict["archive_root"] = "http://archive.canonical.com/"
540 app_details = self._get_app_details_from_app_dict(app_dict)
541 # ensure that archive.canonical.com archive roots are detected549 # ensure that archive.canonical.com archive roots are detected
542 # as the partner channel550 # as the partner channel
543 dist = softwarecenter.distro.get_distro().get_codename()551 dist = softwarecenter.distro.get_distro().get_codename()
@@ -549,33 +557,68 @@
549 # check if there is a matching channel description file on disk557 # check if there is a matching channel description file on disk
550 os.path.exists.return_value = True558 os.path.exists.return_value = True
551 # setup dict559 # setup dict
552 app_dict = make_software_center_agent_app_dict()560 app_details = self._get_app_details_from_override({
553 app_dict["archive_root"] = "http://extras.ubuntu.com/"561 "archive_root": "http://extras.ubuntu.com/"})
554 app_details = self._get_app_details_from_app_dict(app_dict)
555 # ensure that archive.canonical.com archive roots are detected562 # ensure that archive.canonical.com archive roots are detected
556 # as the partner channel563 # as the partner channel
557 self.assertEqual(app_details.channelname, "ubuntu-extras")564 self.assertEqual(app_details.channelname, "ubuntu-extras")
558565
559 def test_date_no_published(self):566 def test_date_no_published(self):
560 app_dict = make_software_center_agent_app_dict()567 app_details = self._get_app_details_from_override({
561 app_dict["date_published"] = "None"568 "date_published": "None"})
562 app_details = self._get_app_details_from_app_dict(app_dict)
563 # ensure that archive.canonical.com archive roots are detected569 # ensure that archive.canonical.com archive roots are detected
564 # as the partner channel570 # as the partner channel
565 self.assertEqual(app_details.date_published, "")571 self.assertEqual(app_details.date_published, "")
566 # and again572 # and again
567 app_dict["date_published"] = "2012-01-21 02:15:10.358926"573 app_details = self._get_app_details_from_override({
568 app_details = self._get_app_details_from_app_dict(app_dict)574 "date_published": "2012-01-21 02:15:10.358926"})
569 # ensure that archive.canonical.com archive roots are detected575 # ensure that archive.canonical.com archive roots are detected
570 # as the partner channel576 # as the partner channel
571 self.assertEqual(app_details.date_published, "2012-01-21 02:15:10")577 self.assertEqual(app_details.date_published, "2012-01-21 02:15:10")
572578
573 @patch("softwarecenter.db.update.get_region_cached")579 @patch("softwarecenter.db.update.get_region_cached")
574 def test_region_blacklist(self, get_region_cached_mock):580 def test_region_blacklist(self, get_region_cached_mock):
575 get_region_cached_mock.return_value = { "countrycode" : "es",581 """Test that the region blacklist ignores blacklisted locations"""
576 }582 get_region_cached_mock.return_value = { "countrycode" : "es",
577 app_dict = make_software_center_agent_app_dict()583 }
578 app_dict["debtags"] = ["%s%s" % (REGION_BLACKLIST_TAG, "es")]584 app_dict = make_software_center_agent_app_dict({
585 "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "es")]})
586 item = PistonResponseObject.from_dict(app_dict)
587 parser = SCAApplicationParser(item)
588 doc = parser.make_doc(self.db._aptcache)
589 self.assertEqual(doc, None)
590
591 @patch("softwarecenter.db.update.get_region_cached")
592 def test_region_blacklist_blacklists(self, get_region_cached_mock):
593 """Test that the region blacklist adds non-blacklisted locations"""
594 get_region_cached_mock.return_value = { "countrycode" : "de",
595 }
596 app_dict = make_software_center_agent_app_dict({
597 "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "ES")]})
598 item = PistonResponseObject.from_dict(app_dict)
599 parser = SCAApplicationParser(item)
600 doc = parser.make_doc(self.db._aptcache)
601 self.assertNotEqual(doc, None)
602
603 @patch("softwarecenter.db.update.get_region_cached")
604 def test_region_whitelist_whitelists(self, get_region_cached_mock):
605 """Test that the whitelist adds whitelisted locations"""
606 get_region_cached_mock.return_value = { "countrycode" : "es",
607 }
608 app_dict = make_software_center_agent_app_dict({
609 "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
610 item = PistonResponseObject.from_dict(app_dict)
611 parser = SCAApplicationParser(item)
612 doc = parser.make_doc(self.db._aptcache)
613 self.assertNotEqual(doc, None)
614
615 @patch("softwarecenter.db.update.get_region_cached")
616 def test_region_whitelist_blacklists(self, get_region_cached_mock):
617 """Test that the whitelist ignores non-whitelist locations"""
618 get_region_cached_mock.return_value = { "countrycode" : "de",
619 }
620 app_dict = make_software_center_agent_app_dict({
621 "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
579 # see _get_app_details_from_app_dict622 # see _get_app_details_from_app_dict
580 item = PistonResponseObject.from_dict(app_dict)623 item = PistonResponseObject.from_dict(app_dict)
581 parser = SCAApplicationParser(item)624 parser = SCAApplicationParser(item)
582625
=== modified file 'tests/utils.py'
--- tests/utils.py 2012-09-06 00:49:18 +0000
+++ tests/utils.py 2012-09-10 09:45:22 +0000
@@ -267,7 +267,7 @@
267267
268268
269# factory stuff for the agent269# factory stuff for the agent
270def make_software_center_agent_app_dict():270def make_software_center_agent_app_dict(override_dict={}):
271 app_dict = {271 app_dict = {
272 u'archive_root': 'http://private-ppa.launchpad.net/',272 u'archive_root': 'http://private-ppa.launchpad.net/',
273 u'archive_id': u'commercial-ppa-uploaders/photobomb',273 u'archive_id': u'commercial-ppa-uploaders/photobomb',
@@ -289,6 +289,7 @@
289 '2011/08/64_Chainz.png',289 '2011/08/64_Chainz.png',
290 u'categories': 'Game;LogicGame',290 u'categories': 'Game;LogicGame',
291 }291 }
292 app_dict.update(override_dict)
292 return app_dict293 return app_dict
293294
294295

Subscribers

People subscribed via source and target branches