launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/get-rid-of-launchpadlib_for_anonymous-683748 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #683748 Get rid of launchpadlib_for_anonymous
  https://bugs.launchpad.net/bugs/683748


Summary
=======

A few weeks ago a method was created to allow use of anonymous launchpadlib connections in tests. This method, launchpadlib_for_anonymous, was largely redundant, and its function is better expressed as a modification to launchpadlib_for.

Implementation
==============

* lib/lp/testing/_webservice.py: launchpadlib_for is modified to accept None as a parameter for person. If none is passed in for person, the method sets up a connection with anonymous credentials in the same way that launchpadlib_for_anonymous previously did.

Tests
=====

bin/test -t IHasSpecificationTests
bin/test -t PersonEmailSecurity

Lint
====

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/blueprints/tests/test_webservice.py
  lib/lp/registry/browser/tests/test_person_webservice.py
  lib/lp/testing/__init__.py
  lib/lp/testing/_webservice.py

./lib/lp/testing/__init__.py
     135: 'anonymous_logged_in' imported but unused
     135: 'with_anonymous_login' imported but unused
     154: 'launchpadlib_for' imported but unused
     154: 'launchpadlib_credentials_for' imported but unused
     135: 'person_logged_in' imported but unused
     154: 'oauth_access_token_for' imported but unused
     135: 'login_celebrity' imported but unused
     135: 'with_celebrity_logged_in' imported but unused
     153: 'test_tales' imported but unused
     135: 'celebrity_logged_in' imported but unused
     135: 'run_with_login' imported but unused
     135: 'is_logged_in' imported but unused
     135: 'with_person_logged_in' imported but unused
     135: 'login_team' imported but unused
     135: 'login_person' imported but unused
     135: 'login_as' imported but unused
     502: E231 missing whitespace after ':'
     707: E203 whitespace before ','
     707: E231 missing whitespace after ','
     920: E302 expected 2 blank lines, found 1
     996: E302 expected 2 blank lines, found 1
./lib/lp/testing/_webservice.py
     155: Line exceeds 78 characters.

I will clean up the lint errors; I didn't want to pollute the diff with lint removals unrelated to the change. The various imports will remain as they exist to enable "from lp.testing import login_as" &c.

-- 
https://code.launchpad.net/~jcsackett/launchpad/get-rid-of-launchpadlib_for_anonymous-683748/+merge/44115
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/get-rid-of-launchpadlib_for_anonymous-683748 into lp:launchpad.

=== modified file 'lib/lp/blueprints/tests/test_webservice.py'
--- lib/lp/blueprints/tests/test_webservice.py	2010-12-02 15:13:58 +0000
+++ lib/lp/blueprints/tests/test_webservice.py	2010-12-18 00:08:55 +0000
@@ -14,7 +14,6 @@
     )
 from lp.testing import (
     launchpadlib_for,
-    launchpadlib_for_anonymous,
     person_logged_in,
     TestCaseWithFactory,
     ws_object,
@@ -237,7 +236,7 @@
         # Need to endInteraction() because launchpadlib_for_anonymous() will
         # setup a new one.
         endInteraction()
-        lplib = launchpadlib_for_anonymous('lplib-test', version='devel')
+        lplib = launchpadlib_for('lplib-test', person=None, version='devel')
         ws_product = ws_object(lplib, product)
         self.assertNamesOfSpecificationsAre(
             ["spec1", "spec2"], ws_product.all_specifications)

=== modified file 'lib/lp/registry/browser/tests/test_person_webservice.py'
--- lib/lp/registry/browser/tests/test_person_webservice.py	2010-12-02 17:53:57 +0000
+++ lib/lp/registry/browser/tests/test_person_webservice.py	2010-12-18 00:08:55 +0000
@@ -13,7 +13,6 @@
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.testing import (
     launchpadlib_for,
-    launchpadlib_for_anonymous,
     TestCaseWithFactory,
     )
 
@@ -44,10 +43,10 @@
     def test_anonymous_cannot_access(self):
         # An anonymous launchpadlib connection cannot see email addresses.
 
-        # Need to endInteraction() because launchpadlib_for_anonymous() will
+        # Need to endInteraction() because launchpadlib_for() will
         # setup a new one.
         endInteraction()
-        lp = launchpadlib_for_anonymous('test', version='devel')
+        lp = launchpadlib_for('test', person=None, version='devel')
         person = lp.people['target']
         emails = list(person.confirmed_email_addresses)
         self.assertEqual([], emails)

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-12-14 20:53:23 +0000
+++ lib/lp/testing/__init__.py	2010-12-18 00:08:55 +0000
@@ -154,7 +154,6 @@
 from lp.testing._webservice import (
     launchpadlib_credentials_for,
     launchpadlib_for,
-    launchpadlib_for_anonymous,
     oauth_access_token_for,
     )
 from lp.testing.fixture import ZopeEventHandlerFixture

=== modified file 'lib/lp/testing/_webservice.py'
--- lib/lp/testing/_webservice.py	2010-12-01 12:00:00 +0000
+++ lib/lp/testing/_webservice.py	2010-12-18 00:08:55 +0000
@@ -8,7 +8,6 @@
 __all__ = [
     'launchpadlib_credentials_for',
     'launchpadlib_for',
-    'launchpadlib_for_anonymous',
     'oauth_access_token_for',
     ]
 
@@ -120,23 +119,8 @@
     shutil.rmtree(cache, ignore_errors=True)
 
 
-def launchpadlib_for_anonymous(
-    consumer_name, version=None, service_root="http://api.launchpad.dev/";):
-    """Create a Launchpad object for the anonymous user.
-
-    :param consumer_name: An OAuth consumer name.
-    :param version: The version of the web service to access.
-    :param service_root: The root URL of the web service to access.
-
-    :return: A launchpadlib.Launchpad object.
-    """
-    token = AnonymousAccessToken()
-    credentials = Credentials(consumer_name, access_token=token)
-    return Launchpad(credentials, service_root, version=version)
-
-
 def launchpadlib_for(
-    consumer_name, person, permission=OAuthPermission.WRITE_PRIVATE,
+    consumer_name, person=None, permission=OAuthPermission.WRITE_PRIVATE,
     context=None, version=None, service_root="http://api.launchpad.dev/";):
     """Create a Launchpad object for the given person.
 
@@ -151,8 +135,12 @@
 
     :return: A launchpadlib Launchpad object.
     """
-    credentials = launchpadlib_credentials_for(
-        consumer_name, person, permission, context)
+    if person is None:
+        token = AnonymousAccessToken()
+        credentials = Credentials(consumer_name, access_token=token)
+    else:
+        credentials = launchpadlib_credentials_for(
+            consumer_name, person, permission, context)
     transaction.commit()
     version = version or Launchpad.DEFAULT_VERSION
     cache = tempfile.mkdtemp(prefix='launchpadlib-cache-')