Merge ~cjwatson/launchpad:webservice-for-person-anonymous into launchpad:master

[Colin Watson]

Extend webservice_for_person to support anonymous login

This simplifies some tests of anonymous webservice behaviour, and allows those tests to look more like nearby tests that authenticate as a real user.

[Ioana Lasc (ilasc)]

review: Approve

diff --git a/lib/lp/app/webservice/tests/test_marshallers.py b/lib/lp/app/webservice/tests/test_marshallers.py
index e689d96..08c2068 100644
--- a/lib/lp/app/webservice/tests/test_marshallers.py
+++ b/lib/lp/app/webservice/tests/test_marshallers.py
@@ -30,16 +30,12 @@ from lp.services.job.interfaces.job import JobStatus
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.servers import WebServiceTestRequest
 from lp.testing import (
-    logout,
     person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.fixture import ZopeAdapterFixture
 from lp.testing.layers import DatabaseFunctionalLayer
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 def ws_url(bug):
@@ -90,8 +86,7 @@ class TestWebServiceObfuscation(TestCaseWithFactory):
     def test_email_address_obfuscated(self):
         # Email addresses are obfuscated for anonymous users.
         bug = self._makeBug()
-        logout()
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         result = webservice(ws_url(bug)).jsonBody()
         self.assertEqual(
             self.bug_title % self.email_address_obfuscated,
@@ -125,8 +120,7 @@ class TestWebServiceObfuscation(TestCaseWithFactory):
         # Email addresses are obfuscated in the XML representation for
         # anonymous users.
         bug = self._makeBug()
-        logout()
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         result = webservice(
             ws_url(bug), headers={'Accept': 'application/xhtml+xml'})
         self.assertNotIn(self.email_address.encode('UTF-8'), result.body)
@@ -146,8 +140,7 @@ class TestWebServiceObfuscation(TestCaseWithFactory):
         user = self.factory.makePerson()
         webservice = webservice_for_person(user)
         etag_logged_in = webservice(ws_url(bug)).getheader('etag')
-        logout()
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         etag_logged_out = webservice(ws_url(bug)).getheader('etag')
         self.assertNotEqual(etag_logged_in, etag_logged_out)
 
diff --git a/lib/lp/blueprints/tests/test_webservice.py b/lib/lp/blueprints/tests/test_webservice.py
index 7ff1ce7..39f1b92 100644
--- a/lib/lp/blueprints/tests/test_webservice.py
+++ b/lib/lp/blueprints/tests/test_webservice.py
@@ -28,15 +28,11 @@ from lp.testing import (
     admin_logged_in,
     api_url,
     login,
-    logout,
     person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class SpecificationWebserviceTests(TestCaseWithFactory):
@@ -424,9 +420,7 @@ class IHasSpecificationsTests(TestCaseWithFactory):
         self.factory.makeSpecification(product=product, name="spec1")
         self.factory.makeSpecification(product=product, name="spec2")
         product_url = api_url(product)
-        logout()
-        webservice = LaunchpadWebServiceCaller(
-            "test", "", default_api_version="devel")
+        webservice = webservice_for_person(None, default_api_version="devel")
         response = webservice.get(product_url)
         self.assertEqual(200, response.status)
         response = webservice.get(
diff --git a/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py b/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
index 4faa851..a517d97 100644
--- a/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
+++ b/lib/lp/bugs/browser/tests/test_bugattachment_file_access.py
@@ -31,14 +31,10 @@ from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     api_url,
     login_person,
-    logout,
     TestCaseWithFactory,
     )
 from lp.testing.layers import LaunchpadFunctionalLayer
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class TestAccessToBugAttachmentFiles(TestCaseWithFactory):
@@ -138,9 +134,7 @@ class TestWebserviceAccessToBugAttachmentFiles(TestCaseWithFactory):
 
     def test_anon_access_to_public_bug_attachment(self):
         # Attachments of public bugs can be accessed by anonymous users.
-        logout()
-        webservice = LaunchpadWebServiceCaller(
-            'test', '', default_api_version='devel')
+        webservice = webservice_for_person(None, default_api_version='devel')
         ws_bug = self.getWebserviceJSON(webservice, self.bug_url)
         ws_bug_attachment = self.getWebserviceJSON(
             webservice, ws_bug['attachments_collection_link'])['entries'][0]
diff --git a/lib/lp/bugs/tests/test_bug_messages_webservice.py b/lib/lp/bugs/tests/test_bug_messages_webservice.py
index 5faae0e..dbd3507 100644
--- a/lib/lp/bugs/tests/test_bug_messages_webservice.py
+++ b/lib/lp/bugs/tests/test_bug_messages_webservice.py
@@ -21,7 +21,6 @@ from lp.testing import (
     api_url,
     launchpadlib_for,
     login_celebrity,
-    logout,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -29,10 +28,7 @@ from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class TestMessageTraversal(TestCaseWithFactory):
@@ -104,9 +100,8 @@ class TestBugMessage(TestCaseWithFactory):
                 self.factory.makeBugAttachment(bug).id for i in range(3))
             bug_url = api_url(bug)
         self.assertThat(created_attachment_ids, HasLength(3))
-        logout()
 
-        webservice = LaunchpadWebServiceCaller('test', None)
+        webservice = webservice_for_person(None)
         bug_attachments = webservice.get(
             bug_url + '/attachments').jsonBody()['entries']
         bug_attachment_ids = set(
diff --git a/lib/lp/buildmaster/tests/test_processor.py b/lib/lp/buildmaster/tests/test_processor.py
index a639b7a..7bd8269 100644
--- a/lib/lp/buildmaster/tests/test_processor.py
+++ b/lib/lp/buildmaster/tests/test_processor.py
@@ -16,11 +16,10 @@ from lp.buildmaster.model.processor import Processor
 from lp.services.database.interfaces import IStore
 from lp.testing import (
     ExpectedException,
-    logout,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
-from lp.testing.pages import LaunchpadWebServiceCaller
+from lp.testing.pages import webservice_for_person
 
 
 class ProcessorSetTests(TestCaseWithFactory):
@@ -59,15 +58,11 @@ class ProcessorSetTests(TestCaseWithFactory):
 class ProcessorSetWebServiceTests(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
-    def setUp(self):
-        super(ProcessorSetWebServiceTests, self).setUp()
-        self.webservice = LaunchpadWebServiceCaller()
-
     def test_getByName(self):
         self.factory.makeProcessor(name='transmeta')
-        logout()
 
-        processor = self.webservice.named_get(
+        webservice = webservice_for_person(None)
+        processor = webservice.named_get(
             '/+processors', 'getByName', name='transmeta',
             api_version='devel').jsonBody()
         self.assertEqual('transmeta', processor['name'])
@@ -80,9 +75,8 @@ class ProcessorSetWebServiceTests(TestCaseWithFactory):
         self.factory.makeProcessor(name='i686')
         self.factory.makeProcessor(name='g4')
 
-        logout()
-
-        collection = self.webservice.get(
+        webservice = webservice_for_person(None)
+        collection = webservice.get(
             '/+processors?ws.size=10', api_version='devel').jsonBody()
         self.assertEqual(
             ['g4', 'i686', 'q1'],
diff --git a/lib/lp/code/model/tests/test_branchset.py b/lib/lp/code/model/tests/test_branchset.py
index 9de4d0a..6f742eb 100644
--- a/lib/lp/code/model/tests/test_branchset.py
+++ b/lib/lp/code/model/tests/test_branchset.py
@@ -23,13 +23,12 @@ from lp.code.model.branch import BranchSet
 from lp.services.propertycache import clear_property_cache
 from lp.testing import (
     login_person,
-    logout,
     RequestTimelineCollector,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
-from lp.testing.pages import LaunchpadWebServiceCaller
+from lp.testing.pages import webservice_for_person
 
 
 class TestBranchSet(TestCaseWithFactory):
@@ -101,14 +100,13 @@ class TestBranchSet(TestCaseWithFactory):
                 None, order_by=BranchListingSort.MOST_RECENTLY_CHANGED_FIRST)))
 
     def test_api_branches_query_count(self):
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         collector = RequestTimelineCollector()
         collector.register()
         self.addCleanup(collector.unregister)
         # Get 'all' of the 50 branches this collection is limited to - rather
         # than the default in-test-suite pagination size of 5.
         url = "/branches?ws.size=50"
-        logout()
         response = webservice.get(url,
             headers={'User-Agent': 'AnonNeedsThis'})
         self.assertEqual(response.status, 200,
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 6cec839..5790338 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -174,7 +174,6 @@ from lp.testing import (
     api_url,
     celebrity_logged_in,
     login_person,
-    logout,
     person_logged_in,
     record_two_runs,
     StormStatementRecorder,
@@ -192,10 +191,7 @@ from lp.testing.matchers import (
     DoesNotSnapshot,
     HasQueryCount,
     )
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 from lp.xmlrpc import faults
 from lp.xmlrpc.interfaces import IPrivateApplication
 
@@ -4582,8 +4578,8 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
         with person_logged_in(requester):
             repository_url = api_url(repository)
         webservice = webservice_for_person(
-            requester, permission=OAuthPermission.WRITE_PUBLIC)
-        webservice.default_api_version = "devel"
+            requester, permission=OAuthPermission.WRITE_PUBLIC,
+            default_api_version="devel")
         response = webservice.named_post(repository_url, "issueAccessToken")
         self.assertEqual(200, response.status)
         macaroon = Macaroon.deserialize(response.jsonBody())
@@ -4606,9 +4602,7 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
         repository = self.factory.makeGitRepository()
         with person_logged_in(repository.owner):
             repository_url = api_url(repository)
-        logout()
-        webservice = LaunchpadWebServiceCaller()
-        webservice.default_api_version = "devel"
+        webservice = webservice_for_person(None, default_api_version="devel")
         response = webservice.named_post(repository_url, "issueAccessToken")
         self.assertEqual(401, response.status)
         self.assertEqual(
diff --git a/lib/lp/registry/browser/tests/test_person_webservice.py b/lib/lp/registry/browser/tests/test_person_webservice.py
index d9633f9..e59c87d 100644
--- a/lib/lp/registry/browser/tests/test_person_webservice.py
+++ b/lib/lp/registry/browser/tests/test_person_webservice.py
@@ -33,7 +33,6 @@ from lp.testing import (
     api_url,
     launchpadlib_for,
     login,
-    logout,
     person_logged_in,
     record_two_runs,
     TestCaseWithFactory,
@@ -252,8 +251,7 @@ class PersonSetWebServiceTests(TestCaseWithFactory):
 
     def setUp(self):
         super(PersonSetWebServiceTests, self).setUp()
-        self.webservice = LaunchpadWebServiceCaller('test', None)
-        logout()
+        self.webservice = webservice_for_person(None)
 
     def assertReturnsPeople(self, expected_names, path):
         self.assertEqual(
diff --git a/lib/lp/services/webhooks/tests/test_webservice.py b/lib/lp/services/webhooks/tests/test_webservice.py
index 4c90b0e..42408d5 100644
--- a/lib/lp/services/webhooks/tests/test_webservice.py
+++ b/lib/lp/services/webhooks/tests/test_webservice.py
@@ -39,10 +39,7 @@ from lp.testing import (
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class TestWebhook(TestCaseWithFactory):
@@ -124,7 +121,7 @@ class TestWebhook(TestCaseWithFactory):
                     u'hg:push:0.1').encode('ASCII')))
 
     def test_anon_forbidden(self):
-        response = LaunchpadWebServiceCaller().get(
+        response = webservice_for_person(None).get(
             self.webhook_url, api_version='devel')
         self.assertEqual(401, response.status)
         self.assertIn(b'launchpad.View', response.body)
@@ -292,7 +289,7 @@ class TestWebhookTargetBase:
             [entry['delivery_url'] for entry in representation['entries']])
 
     def test_webhooks_permissions(self):
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         response = webservice.get(
             self.target_url + '/webhooks', api_version='devel')
         self.assertEqual(401, response.status)
@@ -347,7 +344,7 @@ class TestWebhookTargetBase:
 
     def test_newWebhook_permissions(self):
         self.useFixture(FeatureFixture({'webhooks.new.enabled': 'true'}))
-        webservice = LaunchpadWebServiceCaller()
+        webservice = webservice_for_person(None)
         response = webservice.named_post(
             self.target_url, 'newWebhook',
             delivery_url='http://example.com/ep',
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index f827970..be338d7 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -163,10 +163,7 @@ from lp.testing.matchers import (
     DoesNotSnapshot,
     HasQueryCount,
     )
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class TestSnapFeatureFlag(TestCaseWithFactory):
@@ -3078,10 +3075,9 @@ class TestSnapWebservice(TestCaseWithFactory):
                 self.webservice.getAbsoluteUrl(api_url(snap))
                 for snap in snaps]
         admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        logout()
 
         # Anonymous requests can only see public snaps.
-        anon_webservice = LaunchpadWebServiceCaller("test", "")
+        anon_webservice = webservice_for_person(None)
         response = anon_webservice.named_get(
             "/+snaps", "findByOwner", owner=person_urls[0],
             api_version="devel")
@@ -3145,10 +3141,9 @@ class TestSnapWebservice(TestCaseWithFactory):
                 self.webservice.getAbsoluteUrl(api_url(snap))
                 for snap in snaps]
         admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        logout()
 
         # Anonymous requests can only see public snaps.
-        anon_webservice = LaunchpadWebServiceCaller("test", "")
+        anon_webservice = webservice_for_person(None)
         response = anon_webservice.named_get(
             "/+snaps", "findByURL", url=urls[0], api_version="devel")
         self.assertEqual(200, response.status)
@@ -3222,11 +3217,10 @@ class TestSnapWebservice(TestCaseWithFactory):
                 self.webservice.getAbsoluteUrl(api_url(snap))
                 for snap in snaps]
         admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        logout()
         prefix = "https://git.example.org/foo/"
 
         # Anonymous requests can only see public snaps.
-        anon_webservice = LaunchpadWebServiceCaller("test", "")
+        anon_webservice = webservice_for_person(None)
         response = anon_webservice.named_get(
             "/+snaps", "findByURLPrefix", url_prefix=prefix,
             api_version="devel")
@@ -3305,12 +3299,11 @@ class TestSnapWebservice(TestCaseWithFactory):
                 self.webservice.getAbsoluteUrl(api_url(snap))
                 for snap in snaps]
         admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        logout()
         prefixes = [
             "https://git.example.org/foo/", "https://git.example.org/bar/"]
 
         # Anonymous requests can only see public snaps.
-        anon_webservice = LaunchpadWebServiceCaller("test", "")
+        anon_webservice = webservice_for_person(None)
         response = anon_webservice.named_get(
             "/+snaps", "findByURLPrefixes", url_prefixes=prefixes,
             api_version="devel")
@@ -3381,10 +3374,9 @@ class TestSnapWebservice(TestCaseWithFactory):
                 self.webservice.getAbsoluteUrl(api_url(snap))
                 for snap in snaps]
         admin = getUtility(ILaunchpadCelebrities).admin.teamowner
-        logout()
 
         # Anonymous requests can only see public snaps.
-        anon_webservice = LaunchpadWebServiceCaller("test", "")
+        anon_webservice = webservice_for_person(None)
         response = anon_webservice.named_get(
             "/+snaps", "findByStoreName", store_name=store_names[0],
             api_version="devel")
diff --git a/lib/lp/soyuz/browser/tests/test_archive_webservice.py b/lib/lp/soyuz/browser/tests/test_archive_webservice.py
index a0511d9..cf0e850 100644
--- a/lib/lp/soyuz/browser/tests/test_archive_webservice.py
+++ b/lib/lp/soyuz/browser/tests/test_archive_webservice.py
@@ -46,10 +46,7 @@ from lp.testing import (
 from lp.testing.gpgkeys import gpgkeysdir
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
-from lp.testing.pages import (
-    LaunchpadWebServiceCaller,
-    webservice_for_person,
-    )
+from lp.testing.pages import webservice_for_person
 
 
 class TestArchiveWebservice(TestCaseWithFactory):
@@ -727,6 +724,7 @@ class TestGetPublishedBinaries(TestCaseWithFactory):
         # getPublishedBinaries has a query count constant in the number of
         # packages returned.
         archive_url = api_url(self.archive)
+        webservice = webservice_for_person(None)
 
         def create_bpph():
             with admin_logged_in():
@@ -734,7 +732,7 @@ class TestGetPublishedBinaries(TestCaseWithFactory):
                     archive=self.archive)
 
         def get_binaries():
-            LaunchpadWebServiceCaller('consumer', '').named_get(
+            webservice.named_get(
                 archive_url, 'getPublishedBinaries').jsonBody()
 
         recorder1, recorder2 = record_two_runs(get_binaries, create_bpph, 1)
@@ -814,7 +812,7 @@ class TestArchiveSet(TestCaseWithFactory):
 
     def test_getByReference(self):
         random = self.factory.makePerson()
-        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+        body = webservice_for_person(None).named_get(
             '/archives', 'getByReference', reference='ubuntu',
             api_version='devel').jsonBody()
         self.assertEqual(body['reference'], 'ubuntu')
@@ -824,13 +822,13 @@ class TestArchiveSet(TestCaseWithFactory):
         self.assertEqual(body['reference'], 'ubuntu')
 
     def test_getByReference_ppa(self):
-        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+        body = webservice_for_person(None).named_get(
             '/archives', 'getByReference', reference='~cprov/ubuntu/ppa',
             api_version='devel').jsonBody()
         self.assertEqual(body['reference'], '~cprov/ubuntu/ppa')
 
     def test_getByReference_invalid(self):
-        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+        body = webservice_for_person(None).named_get(
             '/archives', 'getByReference', reference='~cprov/ubuntu',
             api_version='devel').jsonBody()
         self.assertIs(None, body)
@@ -841,7 +839,7 @@ class TestArchiveSet(TestCaseWithFactory):
             owner = archive.owner
             reference = archive.reference
             random = self.factory.makePerson()
-        body = LaunchpadWebServiceCaller('consumer', '').named_get(
+        body = webservice_for_person(None).named_get(
             '/archives', 'getByReference', reference=reference,
             api_version='devel').jsonBody()
         self.assertIs(None, body)
diff --git a/lib/lp/testing/pages.py b/lib/lp/testing/pages.py
index 8e54faa..2e2dc6e 100644
--- a/lib/lp/testing/pages.py
+++ b/lib/lp/testing/pages.py
@@ -750,20 +750,24 @@ def webservice_for_person(person, consumer_key=u'launchpad-library',
     Use this method to create a way to test the webservice that doesn't depend
     on sample data.
     """
-    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 = {}
+    if person is not None:
+        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
+    kwargs['default_api_version'] = default_api_version
     logout()
-    service = LaunchpadWebServiceCaller(
-        consumer_key, access_token.key, access_secret,
-        default_api_version=default_api_version)
+    service = LaunchpadWebServiceCaller(**kwargs)
     service.user = person
     return service