launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bmp-export-scheduleDiffUpdates into lp:launchpad.

Commit message:
Add and export BranchMergeProposal.scheduleDiffUpdates.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bmp-export-scheduleDiffUpdates/+merge/334256

There are various situations where we might reasonably want to ask Launchpad to update an MP's preview diff, and there's no good way to do that right now, short of rescanning the branch or the repository - quite an invasive operation, particularly in the Git case.  This is a bit more targeted.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bmp-export-scheduleDiffUpdates into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py	2017-07-26 11:21:31 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py	2017-11-24 17:51:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The interface for branch merge proposals."""
@@ -510,6 +510,19 @@
         :param conflicts: Text describing the conflicts if any.
         """
 
+    @call_with(return_jobs=False)
+    @export_write_operation()
+    @operation_for_version('devel')
+    def scheduleDiffUpdates(return_jobs=True):
+        """Schedule updates of the diffs for this proposal.
+
+        This can be used if the previous attempt to generate diffs crashed,
+        or if Launchpad failed to notice that the current diffs are outdated
+        for some reason.
+
+        :param return_jobs: If True, return the created jobs.
+        """
+
     @call_with(user=REQUEST_USER)
     @rename_parameters_as(revision_id='revid')
     @operation_parameters(

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2017-11-06 09:32:45 +0000
+++ lib/lp/code/model/branch.py	2017-11-24 17:51:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -653,18 +653,9 @@
 
     def scheduleDiffUpdates(self):
         """See `IBranch`."""
-        from lp.code.model.branchmergeproposaljob import (
-                GenerateIncrementalDiffJob,
-                UpdatePreviewDiffJob,
-            )
         jobs = []
         for merge_proposal in self.active_landing_targets:
-            if merge_proposal.target_branch.last_scanned_id is None:
-                continue
-            jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
-            for old, new in merge_proposal.getMissingIncrementalDiffs():
-                GenerateIncrementalDiffJob.create(
-                    merge_proposal, old.revision_id, new.revision_id)
+            jobs.extend(merge_proposal.scheduleDiffUpdates())
         return jobs
 
     def markRecipesStale(self):

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2017-05-23 08:24:55 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2017-11-24 17:51:54 +0000
@@ -1179,6 +1179,23 @@
             for diff in diffs)
         return [diff_dict.get(revisions) for revisions in revision_list]
 
+    def scheduleDiffUpdates(self, return_jobs=True):
+        """See `IBranchMergeProposal`."""
+        from lp.code.model.branchmergeproposaljob import (
+            GenerateIncrementalDiffJob,
+            UpdatePreviewDiffJob,
+            )
+        jobs = []
+        if (self.target_branch is None or
+                self.target_branch.last_scanned_id is not None):
+            jobs.append(UpdatePreviewDiffJob.create(self))
+            if self.target_branch is not None:
+                for old, new in self.getMissingIncrementalDiffs():
+                    jobs.append(GenerateIncrementalDiffJob.create(
+                        self, old.revision_id, new.revision_id))
+        if return_jobs:
+            return jobs
+
     @property
     def revision_end_date(self):
         """The cutoff date for showing revisions.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2017-11-06 09:32:45 +0000
+++ lib/lp/code/model/gitrepository.py	2017-11-24 17:51:54 +0000
@@ -1011,10 +1011,9 @@
 
     def updateLandingTargets(self, paths):
         """See `IGitRepository`."""
-        from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
         jobs = []
         for merge_proposal in self.getActiveLandingTargets(paths):
-            jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
+            jobs.extend(merge_proposal.scheduleDiffUpdates())
         return jobs
 
     def _getRecipes(self, paths=None):

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2017-10-04 01:53:48 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2017-11-24 17:51:54 +0000
@@ -60,6 +60,7 @@
     BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG,
     IBranchMergeProposal,
     IBranchMergeProposalGetter,
+    IBranchMergeProposalJobSource,
     notify_modified,
     )
 from lp.code.model.branchmergeproposal import (
@@ -82,6 +83,7 @@
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.webapp import canonical_url
 from lp.services.xref.interfaces import IXRefSet
 from lp.testing import (
@@ -2082,7 +2084,7 @@
 
 
 class TestUpdatePreviewDiff(TestCaseWithFactory):
-    """Test the updateMergeDiff method of BranchMergeProposal."""
+    """Test the updatePreviewDiff method of BranchMergeProposal."""
 
     layer = LaunchpadFunctionalLayer
 
@@ -2137,6 +2139,39 @@
             diff_id, removeSecurityProxy(merge_proposal.preview_diff).diff_id)
 
 
+class TestScheduleDiffUpdates(TestCaseWithFactory):
+    """Test scheduling of diff updates."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestScheduleDiffUpdates, self).setUp()
+        self.job_source = removeSecurityProxy(
+            getUtility(IBranchMergeProposalJobSource))
+
+    def test_scheduleDiffUpdates_bzr(self):
+        bmp = self.factory.makeBranchMergeProposal()
+        self.factory.makeRevisionsForBranch(bmp.source_branch)
+        self.factory.makeRevisionsForBranch(bmp.target_branch)
+        [job] = self.job_source.iterReady()
+        removeSecurityProxy(job).job._status = JobStatus.COMPLETED
+        self.assertEqual([], list(self.job_source.iterReady()))
+        with person_logged_in(bmp.merge_target.owner):
+            bmp.scheduleDiffUpdates()
+        [job] = self.job_source.iterReady()
+        self.assertIsInstance(job, UpdatePreviewDiffJob)
+
+    def test_scheduleDiffUpdates_git(self):
+        bmp = self.factory.makeBranchMergeProposalForGit()
+        [job] = self.job_source.iterReady()
+        removeSecurityProxy(job).job._status = JobStatus.COMPLETED
+        self.assertEqual([], list(self.job_source.iterReady()))
+        with person_logged_in(bmp.merge_target.owner):
+            bmp.scheduleDiffUpdates()
+        [job] = self.job_source.iterReady()
+        self.assertIsInstance(job, UpdatePreviewDiffJob)
+
+
 class TestNextPreviewDiffJob(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer