launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/double-archivesub into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #971587 in Launchpad itself: "ArchiveSubscriptionError raised on PPA +archivesubscription page"
  https://bugs.launchpad.net/launchpad/+bug/971587

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/double-archivesub/+merge/124102

Rather than raising an exception which isn't caught higher up the stack in IArchive.newAuthToken(), just return the existing token if one exists.
-- 
https://code.launchpad.net/~stevenk/launchpad/double-archivesub/+merge/124102
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/double-archivesub into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-09-07 05:43:28 +0000
+++ lib/lp/soyuz/model/archive.py	2012-09-13 05:45:25 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 """Database class for table Archive."""
 
 __metaclass__ = type
@@ -153,10 +151,7 @@
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
-from lp.soyuz.interfaces.archivesubscriber import (
-    ArchiveSubscriptionError,
-    IArchiveSubscriberSet,
-    )
+from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.soyuz.interfaces.buildrecords import (
     IHasBuildRecords,
@@ -1832,11 +1827,10 @@
             raise NoTokensForTeams(
                 "Subscription tokens can be created for individuals only.")
 
-        # Ensure that the current subscription does not already have a token
-        if self.getAuthToken(person) is not None:
-            raise ArchiveSubscriptionError(
-                "%s already has a token for %s." % (
-                    person.displayname, self.displayname))
+        # If a token already exists, return it.
+        existing_token = self.getAuthToken(person)
+        if existing_token:
+            return existing_token
 
         # Now onto the actual token creation:
         if token is None:

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2012-08-21 14:15:43 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2012-09-13 05:45:25 +0000
@@ -1161,6 +1161,11 @@
         token = self.private_ppa.newAuthToken(self.joe)
         self.assertEqual(self.private_ppa.getAuthToken(self.joe), token)
 
+    def test_newAuthToken_returns_existing(self):
+        token = self.private_ppa.newAuthToken(self.joe)
+        token2 = self.private_ppa.newAuthToken(self.joe)
+        self.assertEqual(token2, token)
+
     def test_getArchiveSubscriptionURL(self):
         url = self.joe.getArchiveSubscriptionURL(self.joe, self.private_ppa)
         token = self.private_ppa.getAuthToken(self.joe)