launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21194
[Merge] lp:~cjwatson/launchpad/preload-git-landing-targets-candidates into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/preload-git-landing-targets-candidates into lp:launchpad with lp:~cjwatson/launchpad/preload-branch-landing-targets as a prerequisite.
Commit message:
Preload {GitRepository,GitRef}.{landing_candidates,landing_targets}.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/preload-git-landing-targets-candidates/+merge/310655
This is in line with similar handling in Branch.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/preload-git-landing-targets-candidates into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py 2016-11-11 15:10:55 +0000
+++ lib/lp/_schema_circular_imports.py 2016-11-11 15:10:56 +0000
@@ -566,8 +566,10 @@
IGitRef, 'createMergeProposal', 'merge_target', IGitRef)
patch_plain_parameter_type(
IGitRef, 'createMergeProposal', 'merge_prerequisite', IGitRef)
-patch_collection_property(IGitRef, 'landing_targets', IBranchMergeProposal)
-patch_collection_property(IGitRef, 'landing_candidates', IBranchMergeProposal)
+patch_collection_property(
+ IGitRef, '_api_landing_targets', IBranchMergeProposal)
+patch_collection_property(
+ IGitRef, '_api_landing_candidates', IBranchMergeProposal)
patch_collection_property(IGitRef, 'dependent_landings', IBranchMergeProposal)
patch_entry_return_type(IGitRef, 'createMergeProposal', IBranchMergeProposal)
patch_collection_return_type(
@@ -581,9 +583,9 @@
patch_entry_return_type(IGitRepository, 'getSubscription', IGitSubscription)
patch_reference_property(IGitRepository, 'code_import', ICodeImport)
patch_collection_property(
- IGitRepository, 'landing_targets', IBranchMergeProposal)
+ IGitRepository, '_api_landing_targets', IBranchMergeProposal)
patch_collection_property(
- IGitRepository, 'landing_candidates', IBranchMergeProposal)
+ IGitRepository, '_api_landing_candidates', IBranchMergeProposal)
patch_collection_property(
IGitRepository, 'dependent_landings', IBranchMergeProposal)
=== modified file 'lib/lp/code/browser/gitref.py'
--- lib/lp/code/browser/gitref.py 2016-10-14 16:16:18 +0000
+++ lib/lp/code/browser/gitref.py 2016-11-11 15:10:56 +0000
@@ -133,12 +133,14 @@
@cachedproperty
def landing_targets(self):
"""Return a filtered list of landing targets."""
- return latest_proposals_for_each_branch(self.context.landing_targets)
+ targets = self.context.getPrecachedLandingTargets(self.user)
+ return latest_proposals_for_each_branch(targets)
@cachedproperty
def landing_candidates(self):
"""Return a decorated list of landing candidates."""
- return [proposal for proposal in self.context.landing_candidates
+ candidates = self.context.getPrecachedLandingCandidates(self.user)
+ return [proposal for proposal in candidates
if check_permission("launchpad.View", proposal)]
def _getBranchCountText(self, count):
=== modified file 'lib/lp/code/browser/tests/test_gitref.py'
--- lib/lp/code/browser/tests/test_gitref.py 2016-09-07 11:12:58 +0000
+++ lib/lp/code/browser/tests/test_gitref.py 2016-11-11 15:10:56 +0000
@@ -12,6 +12,8 @@
from BeautifulSoup import BeautifulSoup
import pytz
import soupmatchers
+from storm.store import Store
+from testtools.matchers import Equals
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -23,9 +25,11 @@
from lp.testing import (
admin_logged_in,
BrowserTestCase,
+ StormStatementRecorder,
)
from lp.testing.dbuser import dbuser
from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.matchers import HasQueryCount
from lp.testing.pages import (
extract_text,
find_tags_by_class,
@@ -220,3 +224,35 @@
soupmatchers.Tag(
'all commits link', 'a', text='All commits',
attrs={'href': expected_url}))))
+
+ def test_query_count_landing_candidates(self):
+ project = self.factory.makeProduct()
+ [ref] = self.factory.makeGitRefs(target=project)
+ for i in range(10):
+ self.factory.makeBranchMergeProposalForGit(target_ref=ref)
+ [source] = self.factory.makeGitRefs(target=project)
+ [prereq] = self.factory.makeGitRefs(target=project)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=ref, prerequisite_ref=prereq)
+ Store.of(ref).flush()
+ Store.of(ref).invalidate()
+ view = create_view(ref, '+index')
+ with StormStatementRecorder() as recorder:
+ view.landing_candidates
+ self.assertThat(recorder, HasQueryCount(Equals(11)))
+
+ def test_query_count_landing_targets(self):
+ project = self.factory.makeProduct()
+ [ref] = self.factory.makeGitRefs(target=project)
+ for i in range(10):
+ self.factory.makeBranchMergeProposalForGit(source_ref=ref)
+ [target] = self.factory.makeGitRefs(target=project)
+ [prereq] = self.factory.makeGitRefs(target=project)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=ref, target_ref=target, prerequisite_ref=prereq)
+ Store.of(ref).flush()
+ Store.of(ref).invalidate()
+ view = create_view(ref, '+index')
+ with StormStatementRecorder() as recorder:
+ view.landing_targets
+ self.assertThat(recorder, HasQueryCount(Equals(11)))
=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py 2016-11-09 17:18:21 +0000
+++ lib/lp/code/interfaces/gitref.py 2016-11-11 15:10:56 +0000
@@ -221,22 +221,34 @@
and their subscriptions.
"""
- landing_targets = exported(CollectionField(
- title=_("Landing targets"),
- description=_(
- "A collection of the merge proposals where this reference is the "
- "source."),
- readonly=True,
- # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
- value_type=Reference(Interface)))
- landing_candidates = exported(CollectionField(
- title=_("Landing candidates"),
- description=_(
- "A collection of the merge proposals where this reference is the "
- "target."),
- readonly=True,
- # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
- value_type=Reference(Interface)))
+ landing_targets = Attribute(
+ "A collection of the merge proposals where this reference is "
+ "the source.")
+ _api_landing_targets = exported(
+ CollectionField(
+ title=_("Landing targets"),
+ description=_(
+ "A collection of the merge proposals where this reference is "
+ "the source."),
+ readonly=True,
+ # Really IBranchMergeProposal, patched in
+ # _schema_circular_imports.py.
+ value_type=Reference(Interface)),
+ exported_as="landing_targets")
+ landing_candidates = Attribute(
+ "A collection of the merge proposals where this reference is "
+ "the target.")
+ _api_landing_candidates = exported(
+ CollectionField(
+ title=_("Landing candidates"),
+ description=_(
+ "A collection of the merge proposals where this reference is "
+ "the target."),
+ readonly=True,
+ # Really IBranchMergeProposal, patched in
+ # _schema_circular_imports.py.
+ value_type=Reference(Interface)),
+ exported_as="landing_candidates")
dependent_landings = exported(CollectionField(
title=_("Dependent landings"),
description=_(
@@ -246,6 +258,18 @@
# Really IBranchMergeProposal, patched in _schema_circular_imports.py.
value_type=Reference(Interface)))
+ def getPrecachedLandingTargets(user):
+ """Return precached landing targets.
+
+ Target and prerequisite repositories are preloaded.
+ """
+
+ def getPrecachedLandingCandidates(user):
+ """Return precached landing candidates.
+
+ Source and prerequisite repositories are preloaded.
+ """
+
# XXX cjwatson 2015-04-16: Rename in line with landing_targets above
# once we have a better name.
def addLandingTarget(registrant, merge_target, merge_prerequisite=None,
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2016-10-14 15:08:36 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2016-11-11 15:10:56 +0000
@@ -476,22 +476,34 @@
and their subscriptions.
"""
- landing_targets = exported(CollectionField(
- title=_("Landing targets"),
- description=_(
- "A collection of the merge proposals where this repository is the "
- "source."),
- readonly=True,
- # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
- value_type=Reference(Interface)))
- landing_candidates = exported(CollectionField(
- title=_("Landing candidates"),
- description=_(
- "A collection of the merge proposals where this repository is the "
- "target."),
- readonly=True,
- # Really IBranchMergeProposal, patched in _schema_circular_imports.py.
- value_type=Reference(Interface)))
+ landing_targets = Attribute(
+ "A collection of the merge proposals where this repository is "
+ "the source.")
+ _api_landing_targets = exported(
+ CollectionField(
+ title=_("Landing targets"),
+ description=_(
+ "A collection of the merge proposals where this repository is "
+ "the source."),
+ readonly=True,
+ # Really IBranchMergeProposal, patched in
+ # _schema_circular_imports.py.
+ value_type=Reference(Interface)),
+ exported_as="landing_targets")
+ landing_candidates = Attribute(
+ "A collection of the merge proposals where this repository is "
+ "the target.")
+ _api_landing_candidates = exported(
+ CollectionField(
+ title=_("Landing candidates"),
+ description=_(
+ "A collection of the merge proposals where this repository is "
+ "the target."),
+ readonly=True,
+ # Really IBranchMergeProposal, patched in
+ # _schema_circular_imports.py.
+ value_type=Reference(Interface)),
+ exported_as="landing_candidates")
dependent_landings = exported(CollectionField(
title=_("Dependent landings"),
description=_(
@@ -501,6 +513,18 @@
# Really IBranchMergeProposal, patched in _schema_circular_imports.py.
value_type=Reference(Interface)))
+ def getPrecachedLandingTargets(user):
+ """Return precached landing targets.
+
+ Target and prerequisite repositories are preloaded.
+ """
+
+ def getPrecachedLandingCandidates(user):
+ """Return precached landing candidates.
+
+ Source and prerequisite repositories are preloaded.
+ """
+
def getMergeProposalByID(id):
"""Return this repository's merge proposal with this id, or None."""
=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py 2016-11-09 17:18:21 +0000
+++ lib/lp/code/model/gitref.py 2016-11-11 15:10:56 +0000
@@ -8,6 +8,7 @@
]
from datetime import datetime
+from functools import partial
import json
from urllib import quote_plus
from urlparse import urlsplit
@@ -51,7 +52,6 @@
BranchMergeProposalGetter,
)
from lp.services.config import config
-from lp.services.database.bulk import load_related
from lp.services.database.constants import UTC_NOW
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.enumcol import EnumCol
@@ -59,6 +59,7 @@
from lp.services.database.stormbase import StormBase
from lp.services.features import getFeatureFlag
from lp.services.memcache.interfaces import IMemcacheClient
+from lp.services.webapp.interfaces import ILaunchBag
class GitRefMixin:
@@ -194,25 +195,34 @@
BranchMergeProposal.source_git_repository == self.repository,
BranchMergeProposal.source_git_path == self.path)
+ def getPrecachedLandingTargets(self, user):
+ """See `IGitRef`."""
+ loader = partial(BranchMergeProposal.preloadDataForBMPs, user=user)
+ return DecoratedResultSet(self.landing_targets, pre_iter_hook=loader)
+
+ @property
+ def _api_landing_targets(self):
+ return self.getPrecachedLandingTargets(getUtility(ILaunchBag).user)
+
@property
def landing_candidates(self):
"""See `IGitRef`."""
- # Circular import.
- from lp.code.model.gitrepository import GitRepository
-
- result = Store.of(self).find(
+ return Store.of(self).find(
BranchMergeProposal,
BranchMergeProposal.target_git_repository == self.repository,
BranchMergeProposal.target_git_path == self.path,
Not(BranchMergeProposal.queue_status.is_in(
BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
- def eager_load(rows):
- load_related(
- GitRepository, rows,
- ["source_git_repositoryID", "prerequisite_git_repositoryID"])
+ def getPrecachedLandingCandidates(self, user):
+ """See `IGitRef`."""
+ loader = partial(BranchMergeProposal.preloadDataForBMPs, user=user)
+ return DecoratedResultSet(
+ self.landing_candidates, pre_iter_hook=loader)
- return DecoratedResultSet(result, pre_iter_hook=eager_load)
+ @property
+ def _api_landing_candidates(self):
+ return self.getPrecachedLandingCandidates(getUtility(ILaunchBag).user)
@property
def dependent_landings(self):
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2016-10-14 15:08:58 +0000
+++ lib/lp/code/model/gitrepository.py 2016-11-11 15:10:56 +0000
@@ -155,6 +155,7 @@
get_property_cache,
)
from lp.services.webapp.authorization import available_with_permission
+from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webhooks.interfaces import IWebhookSet
from lp.services.webhooks.model import WebhookTargetMixin
from lp.snappy.interfaces.snap import ISnapSet
@@ -887,6 +888,15 @@
BranchMergeProposal,
BranchMergeProposal.source_git_repository == self)
+ def getPrecachedLandingTargets(self, user):
+ """See `IGitRef`."""
+ loader = partial(BranchMergeProposal.preloadDataForBMPs, user=user)
+ return DecoratedResultSet(self.landing_targets, pre_iter_hook=loader)
+
+ @property
+ def _api_landing_targets(self):
+ return self.getPrecachedLandingTargets(getUtility(ILaunchBag).user)
+
def getActiveLandingTargets(self, paths):
"""Merge proposals not in final states where these refs are source."""
return Store.of(self).find(
@@ -905,6 +915,16 @@
Not(BranchMergeProposal.queue_status.is_in(
BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+ def getPrecachedLandingCandidates(self, user):
+ """See `IGitRef`."""
+ loader = partial(BranchMergeProposal.preloadDataForBMPs, user=user)
+ return DecoratedResultSet(
+ self.landing_candidates, pre_iter_hook=loader)
+
+ @property
+ def _api_landing_candidates(self):
+ return self.getPrecachedLandingCandidates(getUtility(ILaunchBag).user)
+
def getActiveLandingCandidates(self, paths):
"""Merge proposals not in final states where these refs are target."""
return Store.of(self).find(
=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py 2016-09-07 11:12:58 +0000
+++ lib/lp/code/model/tests/test_gitref.py 2016-11-11 15:10:56 +0000
@@ -18,6 +18,7 @@
EndsWith,
Equals,
Is,
+ LessThan,
MatchesListwise,
MatchesStructure,
)
@@ -36,6 +37,7 @@
ANONYMOUS,
api_url,
person_logged_in,
+ record_two_runs,
TestCaseWithFactory,
verifyObject,
)
@@ -43,6 +45,7 @@
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
)
+from lp.testing.matchers import HasQueryCount
from lp.testing.pages import webservice_for_person
@@ -461,6 +464,30 @@
self.assertThat(
landing_candidates["entries"][0]["self_link"], EndsWith(bmp_url))
+ def test_landing_candidates_constant_queries(self):
+ project = self.factory.makeProduct()
+ with person_logged_in(project.owner):
+ [trunk] = self.factory.makeGitRefs(target=project)
+ trunk_url = api_url(trunk)
+ webservice = webservice_for_person(
+ project.owner, permission=OAuthPermission.WRITE_PRIVATE)
+
+ def create_mp():
+ with admin_logged_in():
+ [ref] = self.factory.makeGitRefs(
+ target=project,
+ information_type=InformationType.PRIVATESECURITY)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=ref, target_ref=trunk)
+
+ def list_mps():
+ webservice.get(trunk_url + "/landing_candidates")
+
+ list_mps()
+ recorder1, recorder2 = record_two_runs(list_mps, create_mp, 2)
+ self.assertThat(recorder1, HasQueryCount(LessThan(30)))
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
def test_landing_targets(self):
bmp_db = self.factory.makeBranchMergeProposalForGit()
with person_logged_in(bmp_db.registrant):
@@ -476,6 +503,30 @@
self.assertThat(
landing_targets["entries"][0]["self_link"], EndsWith(bmp_url))
+ def test_landing_targets_constant_queries(self):
+ project = self.factory.makeProduct()
+ with person_logged_in(project.owner):
+ [source] = self.factory.makeGitRefs(target=project)
+ source_url = api_url(source)
+ webservice = webservice_for_person(
+ project.owner, permission=OAuthPermission.WRITE_PRIVATE)
+
+ def create_mp():
+ with admin_logged_in():
+ [ref] = self.factory.makeGitRefs(
+ target=project,
+ information_type=InformationType.PRIVATESECURITY)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=ref)
+
+ def list_mps():
+ webservice.get(source_url + "/landing_targets")
+
+ list_mps()
+ recorder1, recorder2 = record_two_runs(list_mps, create_mp, 2)
+ self.assertThat(recorder1, HasQueryCount(LessThan(30)))
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
def test_dependent_landings(self):
[ref] = self.factory.makeGitRefs()
bmp_db = self.factory.makeBranchMergeProposalForGit(
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2016-10-14 16:16:18 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2016-11-11 15:10:56 +0000
@@ -20,6 +20,7 @@
from storm.store import Store
from testtools.matchers import (
EndsWith,
+ LessThan,
MatchesSetwise,
MatchesStructure,
)
@@ -2800,6 +2801,31 @@
self.assertThat(
landing_candidates["entries"][0]["self_link"], EndsWith(bmp_url))
+ def test_landing_candidates_constant_queries(self):
+ project = self.factory.makeProduct()
+ with person_logged_in(project.owner):
+ repository = self.factory.makeGitRepository(target=project)
+ repository_url = api_url(repository)
+ webservice = webservice_for_person(
+ project.owner, permission=OAuthPermission.WRITE_PRIVATE)
+
+ def create_mp():
+ with admin_logged_in():
+ [target] = self.factory.makeGitRefs(repository=repository)
+ [source] = self.factory.makeGitRefs(
+ target=project,
+ information_type=InformationType.PRIVATESECURITY)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+
+ def list_mps():
+ webservice.get(repository_url + "/landing_candidates")
+
+ list_mps()
+ recorder1, recorder2 = record_two_runs(list_mps, create_mp, 2)
+ self.assertThat(recorder1, HasQueryCount(LessThan(40)))
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
def test_landing_targets(self):
bmp_db = self.factory.makeBranchMergeProposalForGit()
with person_logged_in(bmp_db.registrant):
@@ -2815,6 +2841,31 @@
self.assertThat(
landing_targets["entries"][0]["self_link"], EndsWith(bmp_url))
+ def test_landing_targets_constant_queries(self):
+ project = self.factory.makeProduct()
+ with person_logged_in(project.owner):
+ repository = self.factory.makeGitRepository(target=project)
+ repository_url = api_url(repository)
+ webservice = webservice_for_person(
+ project.owner, permission=OAuthPermission.WRITE_PRIVATE)
+
+ def create_mp():
+ with admin_logged_in():
+ [source] = self.factory.makeGitRefs(repository=repository)
+ [target] = self.factory.makeGitRefs(
+ target=project,
+ information_type=InformationType.PRIVATESECURITY)
+ self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+
+ def list_mps():
+ webservice.get(repository_url + "/landing_targets")
+
+ list_mps()
+ recorder1, recorder2 = record_two_runs(list_mps, create_mp, 2)
+ self.assertThat(recorder1, HasQueryCount(LessThan(30)))
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
def test_dependent_landings(self):
[ref] = self.factory.makeGitRefs()
bmp_db = self.factory.makeBranchMergeProposalForGit(
Follow ups
