launchpad-reviewers team mailing list archive

[Colin Watson <mp+414197@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:testing-webservice-access-tokens into launchpad:master with ~cjwatson/launchpad:non-log-revision-status-artifacts as a prerequisite.

Commit message:
Simplify webservice testing with access tokens

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/414197

Having to explicitly construct the `Authorization` header was cumbersome.  Teach `LaunchpadWebServiceCaller` and `webservice_for_person` to help with this instead, when given an `access_token_secret` argument.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:testing-webservice-access-tokens into launchpad:master.

diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 15c3bac..f78bf5d 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -4324,18 +4324,17 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
     def test_newRevisionStatusReport_featureFlagDisabled(self):
         repository = self.factory.makeGitRepository()
         requester = repository.owner
-        webservice = webservice_for_person(None, default_api_version="devel")
         with person_logged_in(requester):
             repository_url = api_url(repository)
-
             secret, _ = self.factory.makeAccessToken(
                 owner=requester, target=repository,
                 scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
-            header = {'Authorization': 'Token %s' % secret}
+        webservice = webservice_for_person(
+            requester, default_api_version="devel", access_token_secret=secret)
 
         response = webservice.named_post(
             repository_url, "newStatusReport",
-            headers=header, title="CI",
+            title="CI",
             commit_sha1=hashlib.sha1(
                 self.factory.getUniqueBytes()).hexdigest(),
             url='https://launchpad.net/',
@@ -4350,21 +4349,20 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
     def test_newRevisionStatusReport(self):
         repository = self.factory.makeGitRepository()
         requester = repository.owner
-        webservice = webservice_for_person(None, default_api_version="devel")
         with person_logged_in(requester):
             repository_url = api_url(repository)
-
             secret, _ = self.factory.makeAccessToken(
                 owner=requester, target=repository,
                 scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
-            header = {'Authorization': 'Token %s' % secret}
+        webservice = webservice_for_person(
+            requester, default_api_version="devel", access_token_secret=secret)
 
         self.useFixture(FeatureFixture(
             {REVISION_STATUS_REPORT_ALLOW_CREATE: "on"}))
 
         response = webservice.named_post(
             repository_url, "newStatusReport",
-            headers=header, title="CI",
+            title="CI",
             commit_sha1=hashlib.sha1(
                 self.factory.getUniqueBytes()).hexdigest(),
             url='https://launchpad.net/',
@@ -4417,21 +4415,19 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
             result_summary=result_summary2,
             result=RevisionStatusResult.FAILED)
 
-        webservice = webservice_for_person(None, default_api_version="devel")
         with person_logged_in(requester):
             repository_url = api_url(repository)
-
             secret, _ = self.factory.makeAccessToken(
                 owner=requester, target=repository,
                 scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
-            header = {'Authorization': 'Token %s' % secret}
+        webservice = webservice_for_person(
+            requester, default_api_version="devel", access_token_secret=secret)
 
         self.useFixture(FeatureFixture(
             {REVISION_STATUS_REPORT_ALLOW_CREATE: "on"}))
 
         response = webservice.named_get(
             repository_url, "getStatusReports",
-            headers=header,
             commit_sha1=commit_sha1s[0])
         self.assertEqual(200, response.status)
         with person_logged_in(requester):
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index b7d9998..202c73a 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -46,22 +46,20 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
             result_summary=self.result_summary,
             result=RevisionStatusResult.FAILED)
 
-        self.webservice = webservice_for_person(
-            None, default_api_version="devel")
         with person_logged_in(self.requester):
             self.report_url = api_url(self.report)
 
             secret, _ = self.factory.makeAccessToken(
                 owner=self.requester, target=self.repository,
                 scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
-            self.header = {'Authorization': 'Token %s' % secret}
+        self.webservice = webservice_for_person(
+            self.requester, default_api_version="devel",
+            access_token_secret=secret)
 
     def test_setLog(self):
         content = b'log_content_data'
         response = self.webservice.named_post(
-            self.report_url, "setLog",
-            headers=self.header,
-            log_data=io.BytesIO(content))
+            self.report_url, "setLog", log_data=io.BytesIO(content))
         self.assertEqual(200, response.status)
 
         # A report may have multiple artifacts.
@@ -86,8 +84,8 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
         contents = [b"artifact 1", b"artifact 2"]
         for filename, content in zip(filenames, contents):
             response = self.webservice.named_post(
-                self.report_url, "attach", headers=self.header,
-                name=filename, data=io.BytesIO(content))
+                self.report_url, "attach", name=filename,
+                data=io.BytesIO(content))
             self.assertEqual(200, response.status)
 
         with person_logged_in(self.requester):
@@ -106,9 +104,7 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
 
     def test_update(self):
         response = self.webservice.named_post(
-            self.report_url, "update",
-            headers=self.header,
-            title='updated-report-title')
+            self.report_url, "update", title="updated-report-title")
         self.assertEqual(200, response.status)
         with person_logged_in(self.requester):
             self.assertEqual('updated-report-title', self.report.title)
@@ -117,9 +113,7 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
             self.assertEqual(RevisionStatusResult.FAILED, self.report.result)
             date_finished_before_update = self.report.date_finished
         response = self.webservice.named_post(
-            self.report_url, "update",
-            headers=self.header,
-            result='Succeeded')
+            self.report_url, "update", result="Succeeded")
         self.assertEqual(200, response.status)
         with person_logged_in(self.requester):
             self.assertEqual('updated-report-title', self.report.title)
diff --git a/lib/lp/testing/pages.py b/lib/lp/testing/pages.py
index f28cf7a..0772e0c 100644
--- a/lib/lp/testing/pages.py
+++ b/lib/lp/testing/pages.py
@@ -126,18 +126,23 @@ class LaunchpadWebServiceCaller(WebServiceCaller):
     """A class for making calls to Launchpad web services."""
 
     def __init__(self, oauth_consumer_key=None, oauth_access_key=None,
-                 oauth_access_secret=None, handle_errors=True,
+                 oauth_access_secret=None, access_token_secret=None,
+                 handle_errors=True,
                  domain='api.launchpad.test', protocol='http',
                  default_api_version=None):
         """Create a LaunchpadWebServiceCaller.
         :param oauth_consumer_key: The OAuth consumer key to use.
         :param oauth_access_key: The OAuth access key to use for the request.
+        :param access_token_secret: The `AccessToken` secret to use for the
+            request (mutually exclusive with OAuth).
         :param handle_errors: Should errors raise exception or be handled by
             the publisher. Default is to let the publisher handle them.
 
         Other parameters are passed to the WebServiceCaller used to make the
         calls.
         """
+        self.oauth_client = None
+        self.access_token_secret = None
         if oauth_consumer_key is not None and oauth_access_key is not None:
             if oauth_access_secret is None:
                 oauth_access_secret = SAMPLEDATA_ACCESS_SECRETS.get(
@@ -148,8 +153,8 @@ class LaunchpadWebServiceCaller(WebServiceCaller):
                 resource_owner_secret=oauth_access_secret,
                 signature_method=oauth1.SIGNATURE_PLAINTEXT)
             logout()
-        else:
-            self.oauth_client = None
+        elif access_token_secret is not None:
+            self.access_token_secret = access_token_secret
         self.handle_errors = handle_errors
         if default_api_version is not None:
             self.default_api_version = default_api_version
@@ -164,6 +169,9 @@ class LaunchpadWebServiceCaller(WebServiceCaller):
             full_headers.update({
                 wsgi_native_string(key): wsgi_native_string(value)
                 for key, value in oauth_headers.items()})
+        elif self.access_token_secret is not None:
+            full_headers['Authorization'] = (
+                'Token %s' % self.access_token_secret)
         if not self.handle_errors:
             full_headers['X_Zope_handle_errors'] = 'False'
 
@@ -739,7 +747,8 @@ def safe_canonical_url(*args, **kwargs):
 
 def webservice_for_person(person, consumer_key='launchpad-library',
                           permission=OAuthPermission.READ_PUBLIC,
-                          context=None, default_api_version=None):
+                          context=None, default_api_version=None,
+                          access_token_secret=None):
     """Return a valid LaunchpadWebServiceCaller for the person.
 
     Use this method to create a way to test the webservice that doesn't depend
@@ -750,16 +759,19 @@ def webservice_for_person(person, consumer_key='launchpad-library',
         if person.is_team:
             raise AssertionError('This cannot be used with teams.')
         login(ANONYMOUS)
-        oacs = getUtility(IOAuthConsumerSet)
-        consumer = oacs.getByKey(consumer_key)
-        if consumer is None:
-            consumer = oacs.new(consumer_key)
-        request_token, _ = consumer.newRequestToken()
-        request_token.review(person, permission, context)
-        access_token, access_secret = request_token.createAccessToken()
-        kwargs['oauth_consumer_key'] = consumer_key
-        kwargs['oauth_access_key'] = access_token.key
-        kwargs['oauth_access_secret'] = access_secret
+        if access_token_secret is None:
+            oacs = getUtility(IOAuthConsumerSet)
+            consumer = oacs.getByKey(consumer_key)
+            if consumer is None:
+                consumer = oacs.new(consumer_key)
+            request_token, _ = consumer.newRequestToken()
+            request_token.review(person, permission, context)
+            access_token, access_secret = request_token.createAccessToken()
+            kwargs['oauth_consumer_key'] = consumer_key
+            kwargs['oauth_access_key'] = access_token.key
+            kwargs['oauth_access_secret'] = access_secret
+        else:
+            kwargs['access_token_secret'] = access_token_secret
     kwargs['default_api_version'] = default_api_version
     logout()
     service = LaunchpadWebServiceCaller(**kwargs)