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

[Michael Vogt]

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

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

[Anthony Lenton (elachuni)]

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.

[lp:~mvo/software-center/region-whitelist-lp1006570]

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)]

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

[Gary Lasker (gary-lasker)]

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

Many thanks!

review: Approve

=== 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 @@
 
 
 from softwarecenter.db.pkginfo import get_pkg_info
-from softwarecenter.distro import get_current_arch, get_foreign_architectures
-from softwarecenter.region import get_region_cached, REGION_BLACKLIST_TAG
+from softwarecenter.distro import (
+    get_current_arch,
+    get_foreign_architectures,
+    )
+from softwarecenter.region import (
+    get_region_cached,
+    REGION_BLACKLIST_TAG,
+    REGION_WHITELIST_TAG,
+    )
+
 
 # weights for the different fields
 WEIGHT_DESKTOP_NAME = 10
@@ -380,19 +388,30 @@
             doc.add_term("AT" + app_type.lower())
 
         # (deb)tags (in addition to the pkgname debtags)
-        tags = self.get_value(AppInfoFields.TAGS)
-        if tags:
-            # register tags
-            for tag in tags.split(","):
-                doc.add_term("XT" + tag.strip())
-            # ENFORCE region blacklist by not registering the app at all
+        tags_string = self.get_value(AppInfoFields.TAGS)
+        if tags_string:
+            # convert to list and register
+            tags = [tag.strip().lower() for tag in tags_string.split(",")]
+            for tag in tags:
+                doc.add_term("XT" + tag)
+            # ENFORCE region blacklist/whitelist by not registering
+            #          the app at all
             region = get_region_cached()
             if region:
                 countrycode = region["countrycode"].lower()
+                # blacklist
                 if "%s%s" % (REGION_BLACKLIST_TAG, countrycode) in tags:
-                    LOG.info("%r.make_doc: skipping region restricted app %r",
-                             self.__class__.__name__, name)
+                    LOG.info("%r.make_doc: skipping region restricted app %r "
+                             "(blacklisted)", self.__class__.__name__, name)
                     return
+                # whitelist
+                for tag in tags:
+                    if (tag.startswith(REGION_WHITELIST_TAG) and not
+                        "%s%s" % (REGION_WHITELIST_TAG, countrycode) in tag):
+                        LOG.info("%r.make_doc: skipping region restricted "
+                                 "app %r (region not whitelisted)",
+                                 self.__class__.__name__, name)
+                        return
 
         # popcon
         # FIXME: popularity not only based on popcon but also

=== 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 @@
 
 # blacklist this region
 REGION_BLACKLIST_TAG = "blacklist-iso3166::"
+# or whitelist it
+REGION_WHITELIST_TAG = "whitelist-iso3166::"
 
 
 def get_region_name(countrycode):

=== 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 @@
     XapianValues,
     PkgStates,
     )
-from softwarecenter.region import REGION_BLACKLIST_TAG, REGIONTAG
+from softwarecenter.region import (
+    REGION_BLACKLIST_TAG,
+    REGION_WHITELIST_TAG,
+    REGIONTAG,
+    )
 
 PKD_DIR = os.path.join(DATA_DIR, 'appdetails', 'var', 'lib', 'dpkg', 'status')
 TEST_DB = os.path.join(DATA_DIR, "test.db")
@@ -522,6 +526,12 @@
     def setUp(self):
         self.db = get_test_db()
 
+    def _get_app_details_from_override(self, override_dict):
+        app_dict = make_software_center_agent_app_dict()
+        app_dict.update(override_dict)
+        app_details =  self._get_app_details_from_app_dict(app_dict)
+        return app_details
+
     def _get_app_details_from_app_dict(self, app_dict):
         item = PistonResponseObject.from_dict(app_dict)
         parser = SCAApplicationParser(item)
@@ -534,10 +544,8 @@
         # we need to patch os.path.exists as "AppDetails.channelname" will
         # check if there is a matching channel description file on disk
         os.path.exists.return_value = True
-        # setup dict
-        app_dict = make_software_center_agent_app_dict()
-        app_dict["archive_root"] = "http://archive.canonical.com/"
-        app_details = self._get_app_details_from_app_dict(app_dict)
+        app_details = self._get_app_details_from_override({
+                "archive_root": "http://archive.canonical.com/"})
         # ensure that archive.canonical.com archive roots are detected
         # as the partner channel
         dist = softwarecenter.distro.get_distro().get_codename()
@@ -549,33 +557,68 @@
         # check if there is a matching channel description file on disk
         os.path.exists.return_value = True
         # setup dict
-        app_dict = make_software_center_agent_app_dict()
-        app_dict["archive_root"] = "http://extras.ubuntu.com/"
-        app_details = self._get_app_details_from_app_dict(app_dict)
+        app_details = self._get_app_details_from_override({
+                "archive_root": "http://extras.ubuntu.com/"})
         # ensure that archive.canonical.com archive roots are detected
         # as the partner channel
         self.assertEqual(app_details.channelname, "ubuntu-extras")
 
     def test_date_no_published(self):
-        app_dict = make_software_center_agent_app_dict()
-        app_dict["date_published"] = "None"
-        app_details = self._get_app_details_from_app_dict(app_dict)
+        app_details = self._get_app_details_from_override({
+                "date_published": "None"})
         # ensure that archive.canonical.com archive roots are detected
         # as the partner channel
         self.assertEqual(app_details.date_published, "")
         # and again
-        app_dict["date_published"] = "2012-01-21 02:15:10.358926"
-        app_details = self._get_app_details_from_app_dict(app_dict)
+        app_details = self._get_app_details_from_override({
+                "date_published": "2012-01-21 02:15:10.358926"})
         # ensure that archive.canonical.com archive roots are detected
         # as the partner channel
         self.assertEqual(app_details.date_published, "2012-01-21 02:15:10")
 
     @patch("softwarecenter.db.update.get_region_cached")
     def test_region_blacklist(self, get_region_cached_mock):
-        get_region_cached_mock.return_value = { "countrycode" : "es",
-                                              }
-        app_dict = make_software_center_agent_app_dict()
-        app_dict["debtags"] = ["%s%s" % (REGION_BLACKLIST_TAG, "es")]
+        """Test that the region blacklist ignores blacklisted locations"""
+        get_region_cached_mock.return_value = { "countrycode" : "es",
+                                              }
+        app_dict = make_software_center_agent_app_dict({
+                "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "es")]})
+        item = PistonResponseObject.from_dict(app_dict)
+        parser = SCAApplicationParser(item)
+        doc = parser.make_doc(self.db._aptcache)
+        self.assertEqual(doc, None)
+
+    @patch("softwarecenter.db.update.get_region_cached")
+    def test_region_blacklist_blacklists(self, get_region_cached_mock):
+        """Test that the region blacklist adds non-blacklisted locations"""
+        get_region_cached_mock.return_value = { "countrycode" : "de",
+                                              }
+        app_dict = make_software_center_agent_app_dict({
+                "debtags": ["%s%s" % (REGION_BLACKLIST_TAG, "ES")]})
+        item = PistonResponseObject.from_dict(app_dict)
+        parser = SCAApplicationParser(item)
+        doc = parser.make_doc(self.db._aptcache)
+        self.assertNotEqual(doc, None)
+
+    @patch("softwarecenter.db.update.get_region_cached")
+    def test_region_whitelist_whitelists(self, get_region_cached_mock):
+        """Test that the whitelist adds whitelisted locations"""
+        get_region_cached_mock.return_value = { "countrycode" : "es",
+                                              }
+        app_dict = make_software_center_agent_app_dict({
+                "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
+        item = PistonResponseObject.from_dict(app_dict)
+        parser = SCAApplicationParser(item)
+        doc = parser.make_doc(self.db._aptcache)
+        self.assertNotEqual(doc, None)
+
+    @patch("softwarecenter.db.update.get_region_cached")
+    def test_region_whitelist_blacklists(self, get_region_cached_mock):
+        """Test that the whitelist ignores non-whitelist locations"""
+        get_region_cached_mock.return_value = { "countrycode" : "de",
+                                              }
+        app_dict = make_software_center_agent_app_dict({
+                "debtags": ["%s%s" % (REGION_WHITELIST_TAG, "ES")]})
         # see _get_app_details_from_app_dict
         item = PistonResponseObject.from_dict(app_dict)
         parser = SCAApplicationParser(item)

=== 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 @@
 
 
 # factory stuff for the agent
-def make_software_center_agent_app_dict():
+def make_software_center_agent_app_dict(override_dict={}):
     app_dict = {
         u'archive_root': 'http://private-ppa.launchpad.net/',
         u'archive_id': u'commercial-ppa-uploaders/photobomb',
@@ -289,6 +289,7 @@
                       '2011/08/64_Chainz.png',
         u'categories': 'Game;LogicGame',
         }
+    app_dict.update(override_dict)
     return app_dict