← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/index-previewdiff-merge_proposal into lp:launchpad

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/index-previewdiff-merge_proposal into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/index-previewdiff-merge_proposal/+merge/162912

Add an index onto PreviewDiff.merge_propsosal and set it to not null now that it is populated.
-- 
https://code.launchpad.net/~stevenk/launchpad/index-previewdiff-merge_proposal/+merge/162912
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/index-previewdiff-merge_proposal into lp:launchpad.
=== added file 'database/schema/patch-2209-44-0.sql'
--- database/schema/patch-2209-44-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2209-44-0.sql	2013-05-08 02:23:31 +0000
@@ -0,0 +1,13 @@
+-- Copyright 2013 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+ALTER TABLE previewdiff
+    ADD COLUMN merge_proposal integer REFERENCES branchmergeproposal,
+    ADD COLUMN date_created timestamp without time zone;
+ALTER TABLE previewdiff
+    ALTER COLUMN date_created
+        SET DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC');
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 44, 0);

=== added file 'database/schema/patch-2209-44-1.sql'
--- database/schema/patch-2209-44-1.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2209-44-1.sql	2013-05-08 02:23:31 +0000
@@ -0,0 +1,10 @@
+-- Copyright 2013 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+CREATE INDEX previewdiff__merge_proposal__idx
+    ON previewdiff (merge_proposal);
+ALTER TABLE previewdiff ALTER COLUMN merge_proposal SET NOT NULL;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 44, 1);

=== 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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +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:31 +0000
@@ -101,7 +101,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

=== 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:31 +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,