William Grant has proposed merging lp:~wgrant/launchpad/bug-1201984 into
lp:launchpad.
Commit message:
Filter out disabled PPAs in ArchiveSet.getPPAsForUser, so we don't have to
check launchpad.Append on destination archive vocabs. This gets the archive
widgets down to constant queries.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1201984 in Launchpad itself: "SourcePackageRecipe:+index and
Archive:+copy-packages time out when there are many potential target PPAs"
https://bugs.launchpad.net/launchpad/+bug/1201984
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1201984/+merge/225011
The target archive widget on +copy-packages had a launchpad.Append check added
as part of the fix for bug #367796. This was inherited by the
SourcePackageRecipe destination archive widget, so both widgets check
launchpad.Append on every archive the user might be able to upload to.
But calculating launchpad.Append on an Archive directly is expensive, and can't
be easily preloaded. This causes pages with a target archive widget to timeout
for users who can upload to lots of PPAs. So I've fixed
ArchiveSet.getPPAsForUser to directly exclude disabled archives, making it a
sufficiently close approximation of launchpad.Append that we no longer need to
filter on that permission. getPPAsForUser is only used by target widgets, so
this won't change anything else.
--
https://code.launchpad.net/~wgrant/launchpad/bug-1201984/+merge/225011
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~wgrant/launchpad/bug-1201984 into lp:launchpad.
=== modified file 'lib/lp/code/vocabularies/sourcepackagerecipe.py'
--- lib/lp/code/vocabularies/sourcepackagerecipe.py 2013-08-22 01:26:23 +0000
+++ lib/lp/code/vocabularies/sourcepackagerecipe.py 2014-06-30 14:11:01 +0000
@@ -16,7 +16,6 @@
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.interfaces.distroseries import IDistroSeriesSet
from lp.registry.model.distroseries import DistroSeries
-from lp.services.webapp.authorization import check_permission
from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webapp.sorting import sorted_dotted_numbers
from lp.services.webapp.vocabulary import (
@@ -59,6 +58,5 @@
def target_ppas_vocabulary(context):
"""Return a vocabulary of ppas that the current user can target."""
- ppas = getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user)
return make_archive_vocabulary(
- ppa for ppa in ppas if check_permission('launchpad.Append', ppa))
+ getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user))
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2014-06-16 10:01:59 +0000
+++ lib/lp/soyuz/browser/archive.py 2014-06-30 14:11:01 +0000
@@ -1433,10 +1433,7 @@
@cachedproperty
def ppas_for_user(self):
"""Return all PPAs for which the user accessing the page can copy."""
- return list(
- ppa
- for ppa in getUtility(IArchiveSet).getPPAsForUser(self.user)
- if check_permission('launchpad.Append', ppa))
+ return list(getUtility(IArchiveSet).getPPAsForUser(self.user))
@cachedproperty
def can_copy(self):
=== added file 'lib/lp/soyuz/browser/tests/test_archive.py'
--- lib/lp/soyuz/browser/tests/test_archive.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/browser/tests/test_archive.py 2014-06-30 14:11:01 +0000
@@ -0,0 +1,39 @@
+# Copyright 2014 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from testtools.matchers import Equals
+
+from lp.testing import (
+ admin_logged_in,
+ login_person,
+ record_two_runs,
+ TestCaseWithFactory,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import HasQueryCount
+from lp.testing.views import create_initialized_view
+
+
+class TestArchiveCopyPackagesView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_query_count(self):
+ person = self.factory.makePerson()
+ source = self.factory.makeArchive()
+
+ def create_targets():
+ self.factory.makeArchive(
+ owner=self.factory.makeTeam(members=[person]))
+ archive = self.factory.makeArchive()
+ with admin_logged_in():
+ archive.newComponentUploader(person, 'main')
+ nb_objects = 2
+ login_person(person)
+ recorder1, recorder2 = record_two_runs(
+ lambda: create_initialized_view(
+ source, '+copy-packages', user=person),
+ create_targets, nb_objects)
+ self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2014-04-23 14:24:15 +0000
+++ lib/lp/soyuz/model/archive.py 2014-06-30 14:11:01 +0000
@@ -2301,12 +2301,14 @@
def getPPAsForUser(self, user):
"""See `IArchiveSet`."""
+ from lp.registry.model.person import Person
# If there's no user logged in, then there's no archives.
if user is None:
return []
store = Store.of(user)
direct_membership = store.find(
Archive,
+ Archive._enabled == True,
Archive.purpose == ArchivePurpose.PPA,
TeamParticipation.team == Archive.ownerID,
TeamParticipation.person == user,
@@ -2322,7 +2324,10 @@
result = direct_membership.union(third_party_upload_acl)
result.order_by(Archive.displayname)
- return result
+ def preload_owners(rows):
+ load_related(Person, rows, ['ownerID'])
+
+ return DecoratedResultSet(result, pre_iter_hook=preload_owners)
def getPPAsPendingSigningKey(self):
"""See `IArchiveSet`."""
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
