launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15555
[Merge] lp:~stevenk/launchpad/populate-previewdiff-merge_proposal into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/populate-previewdiff-merge_proposal into lp:launchpad with lp:~stevenk/launchpad/db-previewdiff-to-bmp as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/populate-previewdiff-merge_proposal/+merge/162913
Populate the new PreviewDiff.{merge_proposal,date_created} columns.
--
https://code.launchpad.net/~stevenk/launchpad/populate-previewdiff-merge_proposal/+merge/162913
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/populate-previewdiff-merge_proposal into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2013-03-25 23:39:42 +0000
+++ database/schema/security.cfg 2013-05-08 02:23:34 +0000
@@ -2248,6 +2248,7 @@
public.openidconsumerassociation = SELECT, DELETE
public.openidconsumernonce = SELECT, DELETE
public.person = SELECT, DELETE
+public.previewdiff = SELECT, UPDATE
public.product = SELECT, UPDATE
public.pofiletranslator = SELECT, INSERT, UPDATE, DELETE
public.potranslation = SELECT, DELETE
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2013-02-27 07:28:11 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2013-05-08 02:23:34 +0000
@@ -887,7 +887,7 @@
def setPreviewDiff(self, preview_diff_bytes):
preview_diff = PreviewDiff.create(
- preview_diff_bytes, u'a', u'b', None, u'')
+ self.bmp, preview_diff_bytes, u'a', u'b', None, u'')
removeSecurityProxy(self.bmp).preview_diff = preview_diff
return preview_diff
=== modified file 'lib/lp/code/doc/branch-merge-proposal-notifications.txt'
--- lib/lp/code/doc/branch-merge-proposal-notifications.txt 2010-12-15 22:01:34 +0000
+++ lib/lp/code/doc/branch-merge-proposal-notifications.txt 2013-05-08 02:23:34 +0000
@@ -25,7 +25,8 @@
>>> import transaction
>>> login('test@xxxxxxxxxxxxx')
>>> diff_bytes = ''.join(unified_diff('', 'c'))
- >>> preview_diff = PreviewDiff.create(diff_bytes, u'a', u'b', None, None)
+ >>> preview_diff = PreviewDiff.create(
+ ... None, diff_bytes, u'a', u'b', None, None)
>>> # Make Librarian happy.
>>> transaction.commit()
>>> target_owner = factory.makePerson(email='target_owner@xxxxxxxxxxx')
=== modified file 'lib/lp/code/interfaces/diff.py'
--- lib/lp/code/interfaces/diff.py 2012-09-27 05:24:26 +0000
+++ lib/lp/code/interfaces/diff.py 2013-05-08 02:23:34 +0000
@@ -20,6 +20,7 @@
from zope.schema import (
Bool,
Bytes,
+ Datetime,
Dict,
Int,
Text,
@@ -114,12 +115,18 @@
title=_('Has conflicts'), readonly=True,
description=_('The previewed merge produces conflicts.'))
+ merge_proposal_id = Int(
+ title=_('The branch merge proposal for this diff.'), readonly=True)
+
# The schema for the Reference gets patched in _schema_circular_imports.
branch_merge_proposal = exported(
Reference(
Interface, readonly=True,
title=_('The branch merge proposal that diff relates to.')))
+ date_created = Datetime(
+ title=_("When this diff was created."), readonly=True)
+
stale = exported(
Bool(readonly=True, description=_(
'If the preview diff is stale, it is out of date when '
=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py 2012-09-19 13:22:42 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2013-05-08 02:23:34 +0000
@@ -52,20 +52,9 @@
def setUp(self):
super(TestMergeProposalMailing, self).setUp('admin@xxxxxxxxxxxxx')
- def makeProposalWithSubscriber(self, diff_text=None,
- initial_comment=None,
- prerequisite=False,
- needs_review=True,
+ def makeProposalWithSubscriber(self, diff_text=None, initial_comment=None,
+ prerequisite=False, needs_review=True,
reviewer=None):
- if diff_text is not None:
- preview_diff = PreviewDiff.create(
- diff_text,
- unicode(self.factory.getUniqueString('revid')),
- unicode(self.factory.getUniqueString('revid')),
- None, None)
- transaction.commit()
- else:
- preview_diff = None
registrant = self.factory.makePerson(
name='bazqux', displayname='Baz Qux', email='baz.qux@xxxxxxxxxxx')
product = self.factory.makeProduct(name='super-product')
@@ -81,8 +70,14 @@
registrant=registrant, product=product,
set_state=initial_status,
prerequisite_branch=prerequisite_branch,
- preview_diff=preview_diff, initial_comment=initial_comment,
+ initial_comment=initial_comment,
reviewer=reviewer)
+ if diff_text is not None:
+ removeSecurityProxy(bmp).preview_diff = PreviewDiff.create(
+ bmp, diff_text,
+ unicode(self.factory.getUniqueString('revid')),
+ unicode(self.factory.getUniqueString('revid')), None, None)
+ transaction.commit()
subscriber = self.factory.makePerson(displayname='Baz Quxx',
email='baz.quxx@xxxxxxxxxxx')
bmp.source_branch.subscribe(subscriber,
=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py 2012-06-14 05:18:22 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py 2013-05-08 02:23:34 +0000
@@ -383,10 +383,9 @@
"""Requesting a review with a diff works."""
diff_text = ''.join(unified_diff('', 'Fake diff'))
preview_diff = PreviewDiff.create(
- diff_text,
- unicode(self.factory.getUniqueString('revid')),
- unicode(self.factory.getUniqueString('revid')),
- None, None)
+ None, diff_text,
+ unicode(self.factory.getUniqueString('revid')),
+ unicode(self.factory.getUniqueString('revid')), None, None)
# To record the diff in the librarian.
transaction.commit()
bmp = make_merge_proposal_without_reviewers(
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2013-05-02 00:40:14 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2013-05-08 02:23:34 +0000
@@ -862,7 +862,7 @@
"""See `IBranchMergeProposal`."""
# Create the PreviewDiff.
self.preview_diff = PreviewDiff.create(
- diff_content, source_revision_id, target_revision_id,
+ self, diff_content, source_revision_id, target_revision_id,
prerequisite_revision_id, conflicts)
# XXX: TimPenhey 2009-02-19 bug 324724
=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py 2012-12-01 07:41:41 +0000
+++ lib/lp/code/model/diff.py 2013-05-08 02:23:34 +0000
@@ -49,6 +49,8 @@
from lp.codehosting.bzrutils import read_locked
from lp.services.config import config
from lp.services.database.bulk import load_referencing
+from lp.services.database.constants import UTC_NOW
+from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.sqlbase import SQLBase
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.librarian.interfaces.client import (
@@ -362,6 +364,11 @@
prerequisite_revision_id = Unicode(name='dependent_revision_id')
+ merge_proposal_id = Int(name='merge_proposal')
+ merge_proposal = Reference(merge_proposal_id, 'BranchMergeProposal.id')
+
+ date_created = UtcDateTimeCol(dbName='date_created', default=UTC_NOW)
+
conflicts = Unicode()
@property
@@ -385,6 +392,9 @@
@cachedproperty
def branch_merge_proposal(self):
+ # This can turn into return self.merge_proposal when it's populated.
+ if self.merge_proposal:
+ return self.merge_proposal
return self._branch_merge_proposal
@classmethod
@@ -399,25 +409,29 @@
source_revision = source_branch.last_revision()
target_branch = bmp.target_branch.getBzrBranch()
target_revision = target_branch.last_revision()
- preview = cls()
- preview.source_revision_id = source_revision.decode('utf-8')
- preview.target_revision_id = target_revision.decode('utf-8')
if bmp.prerequisite_branch is not None:
prerequisite_branch = bmp.prerequisite_branch.getBzrBranch()
else:
prerequisite_branch = None
- preview.diff, conflicts = Diff.mergePreviewFromBranches(
+ diff, conflicts = Diff.mergePreviewFromBranches(
source_branch, source_revision, target_branch,
prerequisite_branch)
+
+ preview = cls()
+ preview.source_revision_id = source_revision.decode('utf-8')
+ preview.target_revision_id = target_revision.decode('utf-8')
+ preview.merge_proposal = bmp
+ preview.diff = diff
preview.conflicts = u''.join(
unicode(conflict) + '\n' for conflict in conflicts)
return preview
@classmethod
- def create(cls, diff_content, source_revision_id, target_revision_id,
+ def create(cls, bmp, diff_content, source_revision_id, target_revision_id,
prerequisite_revision_id, conflicts):
"""Create a PreviewDiff with specified values.
+ :param bmp: The `BranchMergeProposal` this diff references.
:param diff_content: The text of the dift, as bytes.
:param source_revision_id: The revision_id of the source branch.
:param target_revision_id: The revision_id of the target branch.
@@ -426,15 +440,18 @@
:param conflicts: The conflicts, as text.
:return: A `PreviewDiff` with specified values.
"""
+ filename = str(uuid1()) + '.txt'
+ size = len(diff_content)
+ diff = Diff.fromFile(StringIO(diff_content), size, filename)
+
preview = cls()
+ preview.merge_proposal = bmp
preview.source_revision_id = source_revision_id
preview.target_revision_id = target_revision_id
preview.prerequisite_revision_id = prerequisite_revision_id
preview.conflicts = conflicts
+ preview.diff = diff
- filename = str(uuid1()) + '.txt'
- size = len(diff_content)
- preview.diff = Diff.fromFile(StringIO(diff_content), size, filename)
return preview
@property
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py 2013-02-27 07:28:11 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py 2013-05-08 02:23:34 +0000
@@ -2055,8 +2055,9 @@
transaction.commit()
target = ws_object(launchpad, mp.target_branch)
mp = ws_object(launchpad, mp)
- self.assertEqual([mp], list(target.getMergeProposals(
- status=['Merged'], merged_revnos=[123])))
+ self.assertEqual(
+ [mp], list(target.getMergeProposals(
+ status=['Merged'], merged_revnos=[123])))
def test_getRelatedBugTasks(self):
"""Test the getRelatedBugTasks API."""
@@ -2066,8 +2067,7 @@
transaction.commit()
bmp = self.wsObject(db_bmp)
bugtask = self.wsObject(db_bug.default_bugtask)
- self.assertEqual(
- [bugtask], list(bmp.getRelatedBugTasks()))
+ self.assertEqual([bugtask], list(bmp.getRelatedBugTasks()))
def test_setStatus_invalid_transition(self):
"""Emit BadRequest when an invalid transition is requested."""
@@ -2085,6 +2085,6 @@
# A previewdiff with an empty diffstat doesn't crash when fetched.
previewdiff = self.factory.makePreviewDiff()
previewdiff.diff.diffstat = None
- user = previewdiff.branch_merge_proposal.target_branch.owner
+ user = previewdiff.merge_proposal.target_branch.owner
ws_previewdiff = self.wsObject(previewdiff, user=user)
self.assertIsNone(ws_previewdiff.diffstat)
=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2012-10-04 01:09:26 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2013-05-08 02:23:34 +0000
@@ -176,7 +176,8 @@
>>> from lp.code.model.diff import PreviewDiff
>>> login('foo.bar@xxxxxxxxxxxxx')
>>> removeSecurityProxy(proposal).preview_diff = PreviewDiff.create(
- ... 'a\n\b\n', u'source-id', u'target-id', u'prereq-id', u'conflicts')
+ ... proposal, 'a\n\b\n', u'source-id', u'target-id', u'prereq-id',
+ ... u'conflicts')
>>> logout()
>>> browser.open(url)
>>> print_tag_with_id(browser.contents, 'proposals')
=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2013-02-27 07:28:11 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2013-05-08 02:23:34 +0000
@@ -609,7 +609,8 @@
>>> from lp.code.model.diff import PreviewDiff
>>> diff_text = ''.join(
... unified_diff('', ['Fake Diff' + u'\u1010'.encode('utf-8')]))
- >>> preview_diff = PreviewDiff.create(diff_text, u'a', u'b', None, u'')
+ >>> preview_diff = PreviewDiff.create(
+ ... None, diff_text, u'a', u'b', None, u'')
>>> transaction.commit()
>>> bmp = factory.makeBranchMergeProposal()
>>> removeSecurityProxy(bmp).preview_diff = preview_diff
@@ -651,7 +652,7 @@
>>> login('admin@xxxxxxxxxxxxx')
>>> removeSecurityProxy(bmp).preview_diff = PreviewDiff.create(
- ... '', u'c', u'd', None, u'')
+ ... bmp, '', u'c', u'd', None, u'')
>>> logout()
>>> print extract_text(get_review_diff())
Preview Diff
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2013-03-21 05:00:52 +0000
+++ lib/lp/scripts/garbo.py 2013-05-08 02:23:34 +0000
@@ -58,8 +58,13 @@
MAX_SAMPLE_SIZE,
)
from lp.code.interfaces.revision import IRevisionSet
+from lp.code.model.branchmergeproposal import BranchMergeProposal
from lp.code.model.codeimportevent import CodeImportEvent
from lp.code.model.codeimportresult import CodeImportResult
+from lp.code.model.diff import (
+ Diff,
+ PreviewDiff,
+ )
from lp.code.model.revision import (
RevisionAuthor,
RevisionCache,
@@ -101,7 +106,10 @@
from lp.services.identity.model.account import Account
from lp.services.identity.model.emailaddress import EmailAddress
from lp.services.job.model.job import Job
-from lp.services.librarian.model import TimeLimitedToken
+from lp.services.librarian.model import (
+ LibraryFileAlias,
+ TimeLimitedToken,
+ )
from lp.services.log.logger import PrefixFilter
from lp.services.looptuner import TunableLoop
from lp.services.oauth.model import OAuthNonce
@@ -1335,6 +1343,43 @@
transaction.commit()
+class PopulatePreviewDiffMergeProposal(TunableLoop):
+
+ maximum_chunk_size = 5000
+
+ def __init__(self, log, abort_time=None):
+ super(PopulatePreviewDiffMergeProposal, self).__init__(log, abort_time)
+ self.start_at = 1
+ self.store = IMasterStore(BranchMergeProposal)
+
+ def findBranchMergeProposalIDs(self):
+ return self.store.find(
+ (BranchMergeProposal.id),
+ BranchMergeProposal.preview_diff_id != None,
+ BranchMergeProposal.id >= self.start_at).order_by(
+ BranchMergeProposal.id)
+
+ def isDone(self):
+ return self.findBranchMergeProposalIDs().is_empty()
+
+ def __call__(self, chunk_size):
+ bmp_ids = list(self.findBranchMergeProposalIDs()[:chunk_size])
+ columns = dict(
+ [(PreviewDiff.merge_proposal_id, BranchMergeProposal.id),
+ (PreviewDiff.date_created, LibraryFileAlias.date_created)])
+ self.store.execute(
+ BulkUpdate(
+ columns, table=PreviewDiff,
+ values=[BranchMergeProposal, Diff, LibraryFileAlias],
+ where=And(
+ BranchMergeProposal.id.is_in(bmp_ids),
+ PreviewDiff.diff_id == Diff.id,
+ Diff.diff_text == LibraryFileAlias.id,
+ PreviewDiff.id == BranchMergeProposal.preview_diff_id)))
+ self.start_at = bmp_ids[-1] + 1
+ transaction.commit()
+
+
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1589,6 +1634,7 @@
UnusedSessionPruner,
DuplicateSessionPruner,
BugHeatUpdater,
+ PopulatePreviewDiffMergeProposal,
]
experimental_tunable_loops = []
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2013-02-01 03:45:53 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2013-05-08 02:23:34 +0000
@@ -1167,6 +1167,21 @@
[InformationType.PRIVATESECURITY, InformationType.PROPRIETARY],
self.getAccessPolicyTypes(product))
+ def test_PopulatePreviewDiffMergeProposal(self):
+ switch_dbuser('testadmin')
+ diffs = [self.factory.makePreviewDiff() for i in range(5)]
+ expected_bmps = [diff.merge_proposal_id for diff in diffs]
+ expected_dates = [diff.diff.diff_text.date_created for diff in diffs]
+ for diff in diffs:
+ diff.merge_proposal.preview_diff = diff
+ diff.merge_proposal = None
+ diff.date_created = None
+ self.runHourly()
+ self.assertContentEqual(
+ expected_bmps, [diff.merge_proposal_id for diff in diffs])
+ self.assertEqual(
+ expected_dates, [diff.date_created for diff in diffs])
+
def test_PopulateLatestPersonSourcePackageReleaseCache(self):
switch_dbuser('testadmin')
# Make some same test data - we create published source package
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2013-05-04 00:37:58 +0000
+++ lib/lp/testing/factory.py 2013-05-08 02:23:34 +0000
@@ -1573,11 +1573,12 @@
if merge_proposal is None:
merge_proposal = self.makeBranchMergeProposal()
preview_diff = PreviewDiff()
- preview_diff._branch_merge_proposal = merge_proposal
+ preview_diff.merge_proposal = merge_proposal
preview_diff.conflicts = conflicts
preview_diff.diff = diff
preview_diff.source_revision_id = self.getUniqueUnicode()
preview_diff.target_revision_id = self.getUniqueUnicode()
+ removeSecurityProxy(merge_proposal).preview_diff = preview_diff
return preview_diff
def makeIncrementalDiff(self, merge_proposal=None, old_revision=None,
