Merge lp:~michael.nelson/rnr-server/1262627-verify-click into lp:rnr-server

[Michael Nelson]

Check a packagename using the click-package-index.

Fix for bug 1262627 - when origin-distroseries is ubuntu-click, check for a valid package using the click-package-index api.

[Michael Nelson (michael.nelson)]

self-approving so I can go ahead with getting this on staging, but happy to do a second branch with any review changes.

review: Approve

[James Tait (jamestait)]

I'm not familiar with the code, but this seems self-explanatory and looks good to me.

review: Approve

=== modified file 'src/reviewsapp/forms.py'
--- src/reviewsapp/forms.py	2012-06-20 12:19:49 +0000
+++ src/reviewsapp/forms.py	2013-12-20 14:09:49 +0000
@@ -168,10 +168,10 @@
 
         if require_repository_check:
             ws = WebServices()
-            valid_in_lp = ws.lp_verify_packagename_in_repository(
+            is_valid_package = ws.verify_packagename_in_repository(
                 package_name, origin, distroseries, arch_tag=arch_tag)
 
-            if not valid_in_lp:
+            if not is_valid_package:
                 valid_in_sca = ws.sca_verify_packagename_in_repository(
                     package_name, origin, distroseries, arch_tag=arch_tag)
                 if not valid_in_sca:

=== modified file 'src/reviewsapp/schema.py'
--- src/reviewsapp/schema.py	2012-08-29 20:01:15 +0000
+++ src/reviewsapp/schema.py	2013-12-20 14:09:49 +0000
@@ -88,6 +88,10 @@
                 'http://archive.canonical.com/': 'canonical',
             })
 
+    class clickpackageindex(schema.Section):
+        click_index_api_uri = schema.StringOption(
+            default="https://search.apps.ubuntu.com/api/v1/")
+
     # Launchpad
     class launchpad(schema.Section):
         lp_service = schema.StringOption()

=== modified file 'src/reviewsapp/tests/test_handlers.py'
--- src/reviewsapp/tests/test_handlers.py	2013-04-11 07:48:45 +0000
+++ src/reviewsapp/tests/test_handlers.py	2013-12-20 14:09:49 +0000
@@ -857,7 +857,7 @@
         is_authed_fn = ('reviewsapp.auth.SSOOAuthAuthentication.'
                         'is_authenticated')
         lp_verify_fn = ('reviewsapp.utilities.WebServices.'
-                        'lp_verify_packagename_in_repository')
+                        'verify_packagename_in_repository')
         sca_verify_fn = ('reviewsapp.utilities.WebServices.'
                          'sca_verify_packagename_in_repository')
         with patch(is_authed_fn) as mock_is_authenticated:

=== modified file 'src/reviewsapp/tests/test_rnrclient.py'
--- src/reviewsapp/tests/test_rnrclient.py	2013-06-13 08:30:07 +0000
+++ src/reviewsapp/tests/test_rnrclient.py	2013-12-20 14:09:49 +0000
@@ -294,7 +294,7 @@
 
         lp_verify_packagename_method = (
             'reviewsapp.utilities.WebServices.'
-            'lp_verify_packagename_in_repository')
+            'verify_packagename_in_repository')
 
         mock_verify_package = Mock()
         mock_verify_package.return_value = True
@@ -319,7 +319,7 @@
 
         lp_verify_packagename_method = (
             'reviewsapp.utilities.WebServices.'
-            'lp_verify_packagename_in_repository')
+            'verify_packagename_in_repository')
 
         mock_verify_package = Mock()
         mock_verify_package.return_value = True
@@ -358,7 +358,7 @@
             token.token, token.token_secret, consumer.key, consumer.secret)
         lp_verify_packagename_method = (
             'reviewsapp.utilities.WebServices.'
-            'lp_verify_packagename_in_repository')
+            'verify_packagename_in_repository')
 
         if mock_verify_package is None:
             mock_verify_package = Mock()

=== modified file 'src/reviewsapp/tests/test_utilities.py'
--- src/reviewsapp/tests/test_utilities.py	2012-06-12 17:44:57 +0000
+++ src/reviewsapp/tests/test_utilities.py	2013-12-20 14:09:49 +0000
@@ -23,8 +23,8 @@
     'FullClaimedIdTestCase',
     'IdentityProviderTestCase',
     'InvalidatePaginatedReviewsTestCase',
-    'LaunchpadTestCase',
     'SCATestCase',
+    'VerifyPackageTestCase',
 ]
 
 import os
@@ -102,9 +102,9 @@
         self.assertEquals(set(fields), set(data))
 
 
-class LaunchpadTestCase(TestCaseWithFactory):
+class VerifyPackageTestCase(TestCaseWithFactory):
     def setUp(self):
-        super(LaunchpadTestCase, self).setUp()
+        super(VerifyPackageTestCase, self).setUp()
         # Force web_services to recreate the the Launchpad service root
         WebServices._launchpad_service = None
 
@@ -130,7 +130,7 @@
             self, mock_lp_api, package_name, distro, series, arch_tag):
         with patch('reviewsapp.utilities.Launchpad') as mock_launchpad:
             mock_launchpad.login_anonymously.return_value = mock_lp_api
-            return WebServices().lp_verify_packagename_in_repository(
+            return WebServices().verify_packagename_in_repository(
                 package_name, distro, series, arch_tag)
 
     def test_package_in_ubuntu(self):
@@ -216,13 +216,13 @@
         lp_api, ubuntu, archive = self._make_mock_launchpad()
         mock_launchpad.login_anonymously.return_value = lp_api
         ws = WebServices()
-        ws.lp_verify_packagename_in_repository("foobar", 'ubuntu',
-                                               "lucid", 'i386')
+        ws.verify_packagename_in_repository("foobar", 'ubuntu',
+                                            "lucid", 'i386')
         self.assertTrue(mock_launchpad.login_anonymously.called)
         mock_launchpad.login_anonymously.reset_mock()
 
-        ws.lp_verify_packagename_in_repository("foobar", 'ubuntu',
-                                               "lucid", 'i386')
+        ws.verify_packagename_in_repository("foobar", 'ubuntu',
+                                            "lucid", 'i386')
 
         self.assertFalse(mock_launchpad.login_anonymously.called)
 
@@ -235,9 +235,73 @@
         ws = WebServices()
         # SyntaxError is raised when we fail to parse our dummy wadl
         self.assertRaises(
-            SyntaxError, ws.lp_verify_packagename_in_repository,
+            SyntaxError, ws.verify_packagename_in_repository,
             "foobar", "ubuntu", "lucid", 'i386')
 
+    def patch_httplib2_request(self):
+        patcher = patch('reviewsapp.utilities.Http.request')
+        self.addCleanup(patcher.stop)
+        return patcher.start()
+
+    def patch_click_api_uri(self, uri="http://example.com/api/"):
+        patcher = patch.multiple(settings, CLICK_INDEX_API_URI=uri)
+        self.addCleanup(patcher.stop)
+        return patcher.start()
+
+    def test_click_verify(self):
+        ws = WebServices()
+        mock_request = self.patch_httplib2_request()
+        self.patch_click_api_uri()
+        mock_request.return_value = ({
+            'content-length': '1152',
+            'status': '200',
+        }, '')
+
+        result = ws.verify_packagename_in_repository(
+            "com.ubuntu.developer.dholbach.webapp-bvg", "ubuntu", "click",
+            "i386")
+
+        self.assertTrue(result)
+        mock_request.assert_called_once_with(
+            "http://example.com/api/package/"
+            "com.ubuntu.developer.dholbach.webapp-bvg", "HEAD"
+        )
+
+    def test_click_verify_false(self):
+        ws = WebServices()
+        mock_request = self.patch_httplib2_request()
+        self.patch_click_api_uri()
+        mock_request.return_value = ({
+            'content-length': '1152',
+            'status': '404',
+        }, '')
+
+        result = ws.verify_packagename_in_repository(
+            "com.ubuntu.developer.dholbach.webapp-bvg", "ubuntu", "click",
+            "i386")
+
+        self.assertFalse(result)
+        mock_request.assert_called_once_with(
+            "http://example.com/api/package/"
+            "com.ubuntu.developer.dholbach.webapp-bvg", "HEAD"
+        )
+
+    def test_click_verify_error(self):
+        ws = WebServices()
+        mock_request = self.patch_httplib2_request()
+        self.patch_click_api_uri()
+        mock_request.side_effect = ServerNotFoundError
+
+        result = ws.verify_packagename_in_repository(
+            "com.ubuntu.developer.dholbach.webapp-bvg", "ubuntu", "click",
+            "i386")
+
+        self.assertFalse(result)
+        mock_request.assert_called_once_with(
+            "http://example.com/api/package/"
+            "com.ubuntu.developer.dholbach.webapp-bvg", "HEAD"
+        )
+
 
 class SCATestCase(TestCaseWithFactory):
 

=== modified file 'src/reviewsapp/utilities.py'
--- src/reviewsapp/utilities.py	2012-06-20 12:19:49 +0000
+++ src/reviewsapp/utilities.py	2013-12-20 14:09:49 +0000
@@ -29,7 +29,11 @@
 import logging
 import math
 import urllib
-from httplib2 import ServerNotFoundError
+from httplib2 import (
+    Http,
+    HttpLib2Error,
+    ServerNotFoundError,
+)
 
 from django.conf import settings
 from django.core.cache import cache
@@ -157,7 +161,7 @@
             result.update(api.accounts.me())
         return result
 
-    def lp_verify_packagename_in_repository(
+    def verify_packagename_in_repository(
             self, pkgname, origin, distroseries, arch_tag):
         """Verify that a given package exists in a distroseries on LP.
 
@@ -168,6 +172,16 @@
         :return: Whether the package was found or not.
         :rtype: bool
         """
+        if origin == "ubuntu" and distroseries == 'click':
+            try:
+                result = Http().request(
+                    settings.CLICK_INDEX_API_URI.rstrip('/') +
+                    "/package/{}".format(pkgname), 'HEAD')
+                return result[0]['status'] == '200'
+            except HttpLib2Error:
+                self.logger.exception("Error querying click-package-index.")
+                return False
+
         ubuntu = self.launchpad_service.distributions['ubuntu']
         if origin == 'ubuntu':
             archive = ubuntu.getArchive(name='primary')