launchpad-reviewers team mailing list archive

[Paul Hummer <paul@xxxxxxxxxxxxxxxxxxxx>]

Paul Hummer has proposed merging lp:~rockstar/launchpad/merge-queues-model into lp:launchpad with lp:~rockstar/launchpad/merge-queues-db as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch adds the model code for branch merge queues.  In my next branch, I'll need to break up the interfaces, because I'm exposing this in the API, but I thought that, instead of doing that in this pipe, it'd make more sense to do it when needed, as this branch seems to be a pretty good stopping point for the work it provides.
-- 
https://code.launchpad.net/~rockstar/launchpad/merge-queues-model/+merge/38762
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/merge-queues-model into lp:launchpad.

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/enums.py	2010-10-18 19:46:51 +0000
@@ -7,7 +7,6 @@
 __all__ = [
     'BranchLifecycleStatus',
     'BranchLifecycleStatusFilter',
-    'BranchMergeControlStatus',
     'BranchMergeProposalStatus',
     'BranchSubscriptionDiffSize',
     'BranchSubscriptionNotificationLevel',
@@ -77,44 +76,6 @@
     ABANDONED = DBItem(80, "Abandoned")
 
 
-class BranchMergeControlStatus(DBEnumeratedType):
-    """Branch Merge Control Status
-
-    Does the branch want Launchpad to manage a merge queue, and if it does,
-    how does the branch owner handle removing items from the queue.
-    """
-
-    NO_QUEUE = DBItem(1, """
-        Does not use a merge queue
-
-        The branch does not use the merge queue managed by Launchpad.  Merges
-        are tracked and managed elsewhere.  Users will not be able to queue up
-        approved branch merge proposals.
-        """)
-
-    MANUAL = DBItem(2, """
-        Manual processing of the merge queue
-
-        One or more people are responsible for manually processing the queued
-        branch merge proposals.
-        """)
-
-    ROBOT = DBItem(3, """
-        A branch merge robot is used to process the merge queue
-
-        An external application, like PQM, is used to merge in the queued
-        approved proposed merges.
-        """)
-
-    ROBOT_RESTRICTED = DBItem(4, """
-        The branch merge robot used to process the queue is in restricted mode
-
-        When the robot is in restricted mode, normal queued branches are not
-        returned for merging, only those with "Queued for Restricted
-        merging" will be.
-        """)
-
-
 class BranchType(DBEnumeratedType):
     """Branch Type
 

=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2010-09-02 14:30:47 +0000
+++ lib/lp/code/errors.py	2010-10-18 19:46:51 +0000
@@ -26,6 +26,7 @@
     'CodeImportNotInReviewedState',
     'ClaimReviewFailed',
     'InvalidBranchMergeProposal',
+    'InvalidMergeQueueConfig',
     'InvalidNamespace',
     'NoLinkedBranch',
     'NoSuchBranch',
@@ -291,3 +292,12 @@
         RecipeBuildException.__init__(
             self, recipe, distroseries,
             'A build against this distro is not allowed.')
+
+class InvalidMergeQueueConfig(Exception):
+    """The config specified is not a valid JSON string."""
+
+    webservice_error(400)
+
+    def __init__(self):
+        message = ('The configuration specified is not a valid JSON string.')
+        Exception.__init__(self, message)

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-10-18 19:46:48 +0000
+++ lib/lp/code/interfaces/branch.py	2010-10-18 19:46:51 +0000
@@ -75,13 +75,13 @@
     )
 from lp.code.enums import (
     BranchLifecycleStatus,
-    BranchMergeControlStatus,
     BranchSubscriptionDiffSize,
     BranchSubscriptionNotificationLevel,
     CodeReviewNotificationLevel,
     UICreatableBranchType,
     )
 from lp.code.interfaces.branchlookup import IBranchLookup
+from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
 from lp.code.interfaces.branchtarget import IHasBranchTarget
 from lp.code.interfaces.hasbranches import IHasMergeProposals
 from lp.code.interfaces.hasrecipes import IHasRecipes
@@ -591,9 +591,6 @@
     def getStackedBranches():
         """The branches that are stacked on this one."""
 
-    def getMergeQueue():
-        """The proposals that are QUEUED to land on this branch."""
-
     def getMainlineBranchRevisions(start_date, end_date=None,
                                    oldest_first=False):
         """Return the matching mainline branch revision objects.
@@ -993,6 +990,18 @@
             required=False, readonly=True,
             vocabulary=ControlFormat))
 
+    merge_queue = Reference(
+        title=_('Branch Merge Queue'),
+        schema=IBranchMergeQueue, required=False, readonly=True,
+        description=_(
+            "The branch merge queue that manages merges for this branch."))
+
+    merge_queue_config = TextLine(
+            title=_('Name'), required=True, constraint=branch_name_validator,
+            description=_(
+                "A JSON string of configuration values to send to a branch"
+                "merge robot."))
+
 
 class IBranchEdit(Interface):
     """IBranch attributes that require launchpad.Edit permission."""
@@ -1066,6 +1075,23 @@
         :raise: CannotDeleteBranch if the branch cannot be deleted.
         """
 
+    def addToQueue(queue):
+        """Add this branch to a specified queue.
+
+        A branch's merges can be managed by a queue.
+
+        :param queue: The branch merge queue that will manage the branch.
+        """
+
+    def setMergeQueueConfig(config):
+        """Set the merge_queue_config property.
+
+        A branch can store a JSON string of configuration data for a merge
+        robot to retrieve.
+
+        :param config: A JSON string of data.
+        """
+
 
 class IBranch(IBranchPublic, IBranchView, IBranchEdit,
               IBranchEditableAttributes, IBranchAnyone):

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-10-18 19:46:48 +0000
+++ lib/lp/code/model/branch.py	2010-10-18 19:46:51 +0000
@@ -10,6 +10,7 @@
     ]
 
 from datetime import datetime
+import simplejson
 
 from bzrlib import urlutils
 from bzrlib.revision import NULL_REVISION
@@ -31,7 +32,11 @@
     Or,
     Select,
     )
-from storm.locals import AutoReload
+from storm.locals import (
+    AutoReload,
+    Int,
+    Reference,
+    )
 from storm.store import Store
 from zope.component import getUtility
 from zope.event import notify
@@ -70,7 +75,6 @@
     )
 from lp.code.enums import (
     BranchLifecycleStatus,
-    BranchMergeControlStatus,
     BranchMergeProposalStatus,
     BranchType,
     )
@@ -82,6 +86,7 @@
     BranchTypeError,
     CannotDeleteBranch,
     InvalidBranchMergeProposal,
+    InvalidMergeQueueConfig,
     )
 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
 from lp.code.interfaces.branch import (
@@ -475,14 +480,6 @@
         store = Store.of(self)
         return store.find(Branch, Branch.stacked_on == self)
 
-    def getMergeQueue(self):
-        """See `IBranch`."""
-        return BranchMergeProposal.select("""
-            BranchMergeProposal.target_branch = %s AND
-            BranchMergeProposal.queue_status = %s
-            """ % sqlvalues(self, BranchMergeProposalStatus.QUEUED),
-            orderBy="queue_position")
-
     @property
     def code_is_browseable(self):
         """See `IBranch`."""
@@ -1141,6 +1138,23 @@
             SourcePackageRecipeData)
         return SourcePackageRecipeData.findRecipes(self)
 
+    merge_queue_id = Int(name='merge_queue', allow_none=True)
+    merge_queue = Reference(merge_queue_id, 'BranchMergeQueue.id')
+
+    merge_queue_config = StringCol(dbName='merge_queue_config')
+
+    def addToQueue(self, queue):
+        """See `IBranchEdit`."""
+        self.merge_queue = queue
+
+    def setMergeQueueConfig(self, config):
+        """See `IBranchEdit`."""
+        try:
+            simplejson.loads(config)
+            self.merge_queue_config = config
+        except ValueError: # The json string is invalid
+            raise InvalidMergeQueueConfig
+
 
 class DeletionOperation:
     """Represent an operation to perform as part of branch deletion."""

=== added file 'lib/lp/code/model/branchmergequeue.py'
--- lib/lp/code/model/branchmergequeue.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/branchmergequeue.py	2010-10-18 19:46:51 +0000
@@ -0,0 +1,79 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Implementation classes for IBranchMergeQueue, etc."""
+
+__metaclass__ = type
+__all__ = ['BranchMergeQueue']
+
+import simplejson
+
+from storm.locals import (
+    Int,
+    Reference,
+    Store,
+    Storm,
+    Unicode,
+    )
+from zope.interface import (
+    classProvides,
+    implements,
+    )
+
+from canonical.database.datetimecol import UtcDateTimeCol
+from lp.code.errors import InvalidMergeQueueConfig
+from lp.code.interfaces.branchmergequeue import (
+    IBranchMergeQueue,
+    IBranchMergeQueueSource,
+    )
+from lp.code.model.branch import Branch
+
+
+class BranchMergeQueue(Storm):
+    """See `IBranchMergeQueue`."""
+
+    __storm_table__ = 'BranchMergeQueue'
+    implements(IBranchMergeQueue)
+    classProvides(IBranchMergeQueueSource)
+
+    id = Int(primary=True)
+
+    registrant_id = Int(name='registrant', allow_none=True)
+    registrant = Reference(registrant_id, 'Person.id')
+
+    owner_id = Int(name='owner', allow_none=True)
+    owner = Reference(owner_id, 'Person.id')
+
+    name = Unicode(allow_none=False)
+    description = Unicode(allow_none=False)
+    configuration = Unicode(allow_none=False)
+
+    date_created = UtcDateTimeCol(notNull=True)
+
+    @property
+    def branches(self):
+        """See `IBranchMergeQueue`."""
+        return Store.of(self).find(
+            Branch,
+            Branch.merge_queue_id == self.id)
+
+    def setMergeQueueConfig(self, config):
+        """See `IBranchMergeQueue`."""
+        try:
+            simplejson.loads(config)
+            self.configuration = config
+        except ValueError: # The config string is not valid JSON
+            raise InvalidMergeQueueConfig
+
+    @staticmethod
+    def new(name, owner, registrant, description=None,
+            configuration=None):
+        """See `IBranchMergeQueueSource`."""
+        queue = BranchMergeQueue()
+        queue.name = name
+        queue.owner = owner
+        queue.registrant = registrant
+        queue.description = description
+        queue.configuration = configuration
+
+        return queue

=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2010-10-18 19:46:48 +0000
+++ lib/lp/code/model/branchnamespace.py	2010-10-18 19:46:51 +0000
@@ -27,7 +27,6 @@
     )
 from lp.code.enums import (
     BranchLifecycleStatus,
-    BranchMergeControlStatus,
     BranchSubscriptionDiffSize,
     BranchSubscriptionNotificationLevel,
     BranchVisibilityRule,

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-10-18 19:46:51 +0000
@@ -14,6 +14,7 @@
     datetime,
     timedelta,
     )
+import simplejson
 from unittest import TestLoader
 
 from bzrlib.bzrdir import BzrDir
@@ -66,6 +67,7 @@
     BranchTargetError,
     CannotDeleteBranch,
     InvalidBranchMergeProposal,
+    InvalidMergeQueueConfig,
     )
 from lp.code.interfaces.branch import (
     DEFAULT_BRANCH_STATUS_IN_LISTING,
@@ -2704,5 +2706,43 @@
         self.assertRaises(UnsafeUrlSeen, db_stacked.getBzrBranch)
 
 
+class TestMergeQueue(TestCaseWithFactory):
+    """Tests for branch merge queue functionality in branches."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_addToQueue(self):
+        """Test Branch.addToQueue."""
+        branch = self.factory.makeBranch()
+        queue = self.factory.makeBranchMergeQueue()
+        with person_logged_in(branch.owner):
+            branch.addToQueue(queue)
+
+        self.assertEqual(branch.merge_queue, queue)
+
+    def test_setMergeQueueConfig(self):
+        """Test Branch.setMergeQueueConfig."""
+        branch = self.factory.makeBranch()
+        config = simplejson.dumps({
+            'path': '/',
+            'test': 'make test',})
+
+        with person_logged_in(branch.owner):
+            branch.setMergeQueueConfig(config)
+
+        self.assertEqual(branch.merge_queue_config, config)
+
+    def test_setMergeQueueConfig_invalid(self):
+        """Test that invalid JSON strings aren't added to the database."""
+        branch = self.factory.makeBranch()
+        config = 'abc'
+
+        with person_logged_in(branch.owner):
+            self.assertRaises(
+                InvalidMergeQueueConfig,
+                branch.setMergeQueueConfig,
+                config)
+
+
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-10-18 19:46:48 +0000
+++ lib/lp/testing/factory.py	2010-10-18 19:46:51 +0000
@@ -36,6 +36,7 @@
 from operator import isSequenceType
 import os
 from random import randint
+import simplejson
 from StringIO import StringIO
 from textwrap import dedent
 from threading import local
@@ -146,6 +147,7 @@
     PreviewDiff,
     StaticDiff,
     )
+from lp.code.model.branchmergequeue import BranchMergeQueue
 from lp.codehosting.codeimport.worker import CodeImportSourceDetails
 from lp.hardwaredb.interfaces.hwdb import (
     HWSubmissionFormat,
@@ -1096,6 +1098,18 @@
         namespace = target.getNamespace(owner)
         return namespace.createBranch(branch_type, name, creator)
 
+    def makeBranchMergeQueue(self):
+        """Create a BranchMergeQueue."""
+        name = unicode(self.getUniqueString('queue'))
+        owner = self.makePerson()
+        description = unicode(self.getUniqueString('queue-description'))
+        configuration = unicode(simplejson.dumps({
+            self.getUniqueString('key'): self.getUniqueString('value')}))
+
+        queue = BranchMergeQueue.new(
+            name, owner, owner, description, configuration)
+        return queue
+
     def enableDefaultStackingForProduct(self, product, branch=None):
         """Give 'product' a default stacked-on branch.