Merge lp:~salgado/launchpad/bug-535021 into lp:launchpad

[Guilherme Salgado]

Fix bug 535021 by making makePersonNoCommit() do not return a
non-security-proxied Person object.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/tests/person-views.txt
  lib/canonical/launchpad/browser/tests/test_password_reset.py
  lib/lp/registry/tests/test_personset.py
  lib/lp/code/browser/tests/test_branchmergeproposal.py
  lib/lp/translations/stories/standalone/xx-pofile-details.txt
  lib/lp/testing/factory.py
  lib/lp/code/stories/codeimport/xx-edit-codeimport.txt
  lib/lp/translations/stories/importqueue/xx-entry-details.txt
  lib/lp/translations/stories/standalone/custom-language-codes.txt
  lib/lp/bugs/stories/patches-view/patches-view.txt
  lib/canonical/launchpad/browser/tests/test_launchpad.py

== Pylint notices ==

lib/lp/testing/factory.py
    1708: [F0401, LaunchpadObjectFactory.makeRecipe] Unable to import 'bzrlib.plugins.builder.recipe' (No module named builder)

[Abel Deuring (adeuring)]

looks good

review: Approve (code)

=== modified file 'lib/canonical/launchpad/browser/tests/test_launchpad.py'
--- lib/canonical/launchpad/browser/tests/test_launchpad.py	2009-12-23 17:20:00 +0000
+++ lib/canonical/launchpad/browser/tests/test_launchpad.py	2010-03-12 19:17:26 +0000
@@ -152,7 +152,7 @@
         name = 'suspended-person'
         person = self.factory.makePerson(name=name)
         login_person(self.admin)
-        person.account_status = AccountStatus.SUSPENDED
+        removeSecurityProxy(person).account_status = AccountStatus.SUSPENDED
         segment = '~%s' % name
         # Admins can see the suspended user.
         traversed = self.traverse(segment, segment)
 
=== modified file 'lib/canonical/launchpad/browser/tests/test_password_reset.py'
--- lib/canonical/launchpad/browser/tests/test_password_reset.py	2009-10-29 15:09:28 +0000
+++ lib/canonical/launchpad/browser/tests/test_password_reset.py	2010-03-12 19:17:26 +0000
@@ -39,7 +39,7 @@
 
     def _create_deactivated_person(self):
         self.person = self.factory.makePerson(email=self.email)
-        removeSecurityProxy(self.person.deactivateAccount('Testing'))
+        removeSecurityProxy(self.person).deactivateAccount('Testing')
         # Get the account from the master DB to make sure it has the changes
         # we did above.
         self.account = IMasterObject(self.person.account)
 
=== modified file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
--- lib/lp/bugs/stories/patches-view/patches-view.txt	2010-02-18 22:10:21 +0000
+++ lib/lp/bugs/stories/patches-view/patches-view.txt	2010-03-12 19:17:26 +0000
@@ -371,9 +371,11 @@
 
 The patches view works for people and teams.
 
+    # Must remove the security proxy because IPerson.name is protected.
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> submitter_name = removeSecurityProxy(patch_submitter).name
     >>> anon_browser.open(
-    ...     'http://bugs.launchpad.dev/~%s/+patches' % (
-    ...         patch_submitter.name,))
+    ...     'http://bugs.launchpad.dev/~%s/+patches' % submitter_name)
 
     >>> show_patches_view(anon_browser.contents)
     Bug                  Importance  Status        Project          Patch Age
 
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2010-02-23 21:48:53 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2010-03-12 19:17:26 +0000
@@ -486,7 +486,7 @@
         """Claiming a review works for members of the requested team."""
         review = self.makeTeamReview()
         albert = self.factory.makePerson()
-        albert.join(review.reviewer)
+        removeSecurityProxy(albert).join(review.reviewer)
         login_person(albert)
         view = create_initialized_view(self.bmp, '+index')
         view.claim_action.success({'review_id': review.id})
 
=== modified file 'lib/lp/code/stories/codeimport/xx-edit-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-edit-codeimport.txt	2009-11-11 08:56:46 +0000
+++ lib/lp/code/stories/codeimport/xx-edit-codeimport.txt	2010-03-12 19:17:26 +0000
@@ -22,8 +22,10 @@
     >>> hosted_branch_location = str(canonical_url(hosted_branch))
     >>> logout()
 
-    >>> import_browser = setupBrowser(
-    ...     auth='Basic %s:test' % str(registrant.preferredemail.email))
+    # Must remove the security proxy because IEmailAddress.email is protected.
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> email = removeSecurityProxy(registrant.preferredemail).email
+    >>> import_browser = setupBrowser(auth='Basic %s:test' % str(email))
 
 
 == Import details on the branch page ==
 
=== modified file 'lib/lp/registry/browser/tests/person-views.txt'
--- lib/lp/registry/browser/tests/person-views.txt	2010-02-22 12:48:38 +0000
+++ lib/lp/registry/browser/tests/person-views.txt	2010-03-12 19:17:26 +0000
@@ -47,8 +47,9 @@
 
 New users are on probation; the homepage content is escaped HTML.
 
+    >>> from zope.security.proxy import removeSecurityProxy
     >>> new_user = factory.makePerson()
-    >>> new_user.homepage_content = homepage_content
+    >>> removeSecurityProxy(new_user).homepage_content = homepage_content
     >>> view = create_initialized_view(new_user, '+index')
     >>> view.is_probationary_or_invalid_user
     True
@@ -83,7 +84,7 @@
 
 If the user has no homepage content, the view's value is None.
 
-    >>> new_user.homepage_content = None
+    >>> removeSecurityProxy(new_user).homepage_content = None
     >>> view = create_initialized_view(new_user, '+index')
     >>> print view.homepage_content
     None
 
=== modified file 'lib/lp/registry/browser/tests/test_person_webservice.py'
--- lib/lp/registry/browser/tests/test_person_webservice.py	2009-11-09 23:10:34 +0000
+++ lib/lp/registry/browser/tests/test_person_webservice.py	2010-03-12 19:17:26 +0000
@@ -5,6 +5,8 @@
 
 import unittest
 
+from zope.security.proxy import removeSecurityProxy
+
 from canonical.launchpad.ftests import login
 from lp.testing import TestCaseWithFactory
 from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
@@ -23,8 +25,10 @@
             'launchpad-library', 'salgado-change-anything')
 
     def test_GET_xhtml_representation(self):
+        # Remove the security proxy because IPerson.name is protected.
+        person_name = removeSecurityProxy(self.person).name
         response = self.webservice.get(
-            '/~%s' % self.person.name, 'application/xhtml+xml')
+            '/~%s' % person_name, 'application/xhtml+xml')
 
         self.assertEqual(response.status, 200)
 
 
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py	2009-11-12 18:31:03 +0000
+++ lib/lp/registry/tests/test_personset.py	2010-03-12 19:17:26 +0000
@@ -136,7 +136,8 @@
         self.assertEqual(0, self.cur.rowcount)
 
     def test__mergeMailingListSubscriptions_with_subscriptions(self):
-        self.from_person.mailing_list_auto_subscribe_policy = (
+        naked_person = removeSecurityProxy(self.from_person)
+        naked_person.mailing_list_auto_subscribe_policy = (
             MailingListAutoSubscribePolicy.ALWAYS)
         self.team, self.mailing_list = self.factory.makeTeamAndMailingList(
             'test-mailinglist', 'team-owner')
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-03-08 12:35:15 +0000
+++ lib/lp/testing/factory.py	2010-03-12 19:17:26 +0000
@@ -381,8 +381,8 @@
         a commit, despite all changes now being available in the main
         store.
         """
-        person = removeSecurityProxy(person) # Need to poke person's privates
-        person._preferredemail_cached = Store.of(person).find(
+        naked_person = removeSecurityProxy(person)
+        naked_person._preferredemail_cached = Store.of(person).find(
             EmailAddress, personID=person.id,
             status=EmailAddressStatus.PREFERRED).one()
 
@@ -423,16 +423,18 @@
             email, rationale=PersonCreationRationale.UNKNOWN, name=name,
             password=password, displayname=displayname,
             hide_email_addresses=hide_email_addresses)
-        person = removeSecurityProxy(person)
-        email = removeSecurityProxy(email)
-        person._password_cleartext_cached = password
+        naked_person = removeSecurityProxy(person)
+        naked_person._password_cleartext_cached = password
 
         assert person.password is not None, (
             'Password not set. Wrong default auth Store?')
 
         if (time_zone is not None or latitude is not None or
             longitude is not None):
-            person.setLocation(latitude, longitude, time_zone, person)
+            naked_person.setLocation(latitude, longitude, time_zone, person)
+
+        # Make sure the non-security-proxied object is not returned.
+        del naked_person
 
         # To make the person someone valid in Launchpad, validate the
         # email.
@@ -442,7 +444,7 @@
                 Account, person.accountID)
             account.status = AccountStatus.ACTIVE
 
-        email.status = email_address_status
+        removeSecurityProxy(email).status = email_address_status
 
         # Ensure updated ValidPersonCache
         flush_database_updates()
@@ -1837,8 +1839,7 @@
             singular = self.getUniqueString()
         potmsgset = potemplate.createMessageSetFromText(
             singular, plural, context, sequence)
-        naked_potmsgset = removeSecurityProxy(potmsgset)
-        naked_potmsgset.sync()
+        removeSecurityProxy(potmsgset).sync()
         return potmsgset
 
     def makeTranslationMessage(self, pofile=None, potmsgset=None,
 
=== modified file 'lib/lp/translations/stories/importqueue/xx-entry-details.txt'
--- lib/lp/translations/stories/importqueue/xx-entry-details.txt	2010-02-10 13:04:10 +0000
+++ lib/lp/translations/stories/importqueue/xx-entry-details.txt	2010-03-12 19:17:26 +0000
@@ -34,7 +34,9 @@
 
     >>> product.displayname in details_text
     True
-    >>> uploader.displayname in details_text
+
+    # Must remove the security proxy because IPerson.displayname is protected.
+    >>> removeSecurityProxy(uploader).displayname in details_text
     True
 
 There's also a link to the file's contents.
 
=== modified file 'lib/lp/translations/stories/standalone/custom-language-codes.txt'
--- lib/lp/translations/stories/standalone/custom-language-codes.txt	2009-11-06 16:59:34 +0000
+++ lib/lp/translations/stories/standalone/custom-language-codes.txt	2010-03-12 19:17:26 +0000
@@ -25,7 +25,8 @@
     >>> owner = factory.makePerson(email='[e-mail]', password='test')
     >>> rosetta_admin = factory.makePerson(
     ...     email='[e-mail]', password='test')
-    >>> rosetta_admin.join(getUtility(ILaunchpadCelebrities).rosetta_experts)
+    >>> removeSecurityProxy(rosetta_admin).join(
+    ...     getUtility(ILaunchpadCelebrities).rosetta_experts)
     >>> product = factory.makeProduct(displayname="Foo", owner=owner)
     >>> trunk = product.getSeries('trunk')
     >>> removeSecurityProxy(product).official_rosetta = True
 
=== modified file 'lib/lp/translations/stories/standalone/xx-pofile-details.txt'
--- lib/lp/translations/stories/standalone/xx-pofile-details.txt	2009-12-15 14:11:39 +0000
+++ lib/lp/translations/stories/standalone/xx-pofile-details.txt	2010-03-12 19:17:26 +0000
@@ -157,7 +157,8 @@
     >>> potmsgset.setSequence(template, 2)
     >>> translation = factory.makeTranslationMessage(pofile=pofile,
     ...     translator=translator, potmsgset=potmsgset)
-    >>> merged_translator.merged = translator
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> removeSecurityProxy(merged_translator).merged = translator
     >>> logout()
 
     >>> browser.open(