launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/bzr-format-matching-refactoring into lp:launchpad/devel with lp:~thumper/launchpad/stop-factory-mirroring as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #610309 Refactory getting formats for bzr branches
  https://bugs.launchpad.net/bugs/610309


Simplify the say to get the enumerated values for bzr formats.

This will be used in the merge directive handling in the code mail handler, but can be reviewed independently.

-- 
https://code.launchpad.net/~thumper/launchpad/bzr-format-matching-refactoring/+merge/31014
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/bzr-format-matching-refactoring into lp:launchpad/devel.

=== modified file 'lib/lp/code/bzr.py'
--- lib/lp/code/bzr.py	2010-02-17 01:37:22 +0000
+++ lib/lp/code/bzr.py	2010-07-27 04:14:45 +0000
@@ -9,6 +9,7 @@
     'ControlFormat',
     'CURRENT_BRANCH_FORMATS',
     'CURRENT_REPOSITORY_FORMATS',
+    'get_branch_formats',
     'RepositoryFormat',
     ]
 
@@ -45,7 +46,20 @@
     return DBItem(num, format_string, description)
 
 
-class BranchFormat(DBEnumeratedType):
+class BazaarFormatEnum(DBEnumeratedType):
+    """Base class for the format enums."""
+
+    @classmethod
+    def get_enum(klass, format_name):
+        """Find the matching enum value for the format name specified."""
+        for value in klass.items:
+            if value.title == format_name:
+                return value
+        else:
+            return klass.UNRECOGNIZED
+
+
+class BranchFormat(BazaarFormatEnum):
     """Branch on-disk format.
 
     This indicates which (Bazaar) format is used on-disk.  The list must be
@@ -81,7 +95,7 @@
         107, "Bazaar-NG Loom branch format 7\n", "Loom branch format 7")
 
 
-class RepositoryFormat(DBEnumeratedType):
+class RepositoryFormat(BazaarFormatEnum):
     """Repository on-disk format.
 
     This indicates which (Bazaar) format is used on-disk.  The list must be
@@ -194,7 +208,7 @@
     BZR_CHK_2A = _format_enum(415, RepositoryFormat2a)
 
 
-class ControlFormat(DBEnumeratedType):
+class ControlFormat(BazaarFormatEnum):
     """Control directory (BzrDir) format.
 
     This indicates what control directory format is on disk.  Must be updated
@@ -236,3 +250,16 @@
     RepositoryFormat.BZR_CHK1,
     RepositoryFormat.BZR_CHK2,
     RepositoryFormat.BZR_CHK_2A)
+
+
+def get_branch_formats(bzr_branch):
+    """Return a tuple of format enumerations for the bazaar branch.
+
+    :returns: tuple of (ControlFormat, BranchFormat, RepositoryFormat)
+    """
+    control_string = bzr_branch.bzrdir._format.get_format_string()
+    branch_string = bzr_branch._format.get_format_string()
+    repository_string = bzr_branch.repository._format.get_format_string()
+    return (ControlFormat.get_enum(control_string),
+            BranchFormat.get_enum(branch_string),
+            RepositoryFormat.get_enum(repository_string))

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2010-06-11 03:52:02 +0000
+++ lib/lp/code/enums.py	2010-07-27 04:14:45 +0000
@@ -22,7 +22,6 @@
     'CodeImportReviewStatus',
     'CodeReviewNotificationLevel',
     'CodeReviewVote',
-    'match_enum_title',
     'RevisionControlSystems',
     'TeamBranchVisibilityRule',
     'UICreatableBranchType',
@@ -32,14 +31,6 @@
     DBEnumeratedType, DBItem, EnumeratedType, Item, use_template)
 
 
-def match_enum_title(enum, title, default):
-    for value in enum.items:
-        if value.title == title:
-            return value
-    else:
-        return default
-
-
 class BranchLifecycleStatus(DBEnumeratedType):
     """Branch Lifecycle Status
 

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2010-06-11 03:52:02 +0000
+++ lib/lp/code/model/branchjob.py	2010-07-27 04:14:45 +0000
@@ -46,8 +46,7 @@
 from canonical.database.enumcol import EnumCol
 from canonical.database.sqlbase import SQLBase
 from canonical.launchpad.webapp import canonical_url, errorlog
-from lp.code.bzr import BranchFormat, ControlFormat, RepositoryFormat
-from lp.code.enums import match_enum_title
+from lp.code.bzr import get_branch_formats
 from lp.code.model.branch import Branch
 from lp.code.model.branchmergeproposal import BranchMergeProposal
 from lp.code.model.diff import StaticDiff
@@ -369,25 +368,12 @@
             source_branch = BzrBranch.open_from_transport(
                 source_branch_transport)
 
-            control_format = match_enum_title(
-                ControlFormat,
-                source_branch.bzrdir._format.get_format_string(),
-                ControlFormat.UNRECOGNIZED)
-            branch_format = match_enum_title(
-                BranchFormat,
-                source_branch._format.get_format_string(),
-                BranchFormat.UNRECOGNIZED)
-            repository_format = match_enum_title(
-                RepositoryFormat,
-                source_branch.repository._format.get_format_string(),
-                RepositoryFormat.UNRECOGNIZED)
+            formats = get_branch_formats(source_branch)
 
             self.branch.branchChanged(
                 self.branch.stacked_on,
                 self.branch.last_scanned_id,
-                control_format,
-                branch_format,
-                repository_format)
+                *formats)
         finally:
             shutil.rmtree(upgrade_branch_path)
 

=== added file 'lib/lp/code/tests/test_bzr.py'
--- lib/lp/code/tests/test_bzr.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/tests/test_bzr.py	2010-07-27 04:14:45 +0000
@@ -0,0 +1,66 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for lp.code.bzr."""
+
+__metaclass__ = type
+
+from lp.code.bzr import (
+    BranchFormat, ControlFormat, get_branch_formats, RepositoryFormat)
+from lp.testing import TestCase
+from bzrlib.tests import TestCaseInTempDir
+
+
+class TestBazaarFormatEnum(TestCase):
+    """Tests for the bazaar formats."""
+
+    def test_branch_format_default(self):
+        # The default branch format is unrecognized.
+        format = BranchFormat.get_enum('no-idea')
+        self.assertEqual(BranchFormat.UNRECOGNIZED, format)
+
+    def test_control_format_default(self):
+        # The default control format is unrecognized.
+        format = ControlFormat.get_enum('no-idea')
+        self.assertEqual(ControlFormat.UNRECOGNIZED, format)
+
+    def test_repository_format_default(self):
+        # The default repository format is unrecognized.
+        format = RepositoryFormat.get_enum('no-idea')
+        self.assertEqual(RepositoryFormat.UNRECOGNIZED, format)
+
+
+class TestGetBranchFormats(TestCaseInTempDir):
+    """Tests for lp.code.bzr.get_branch_formats."""
+
+    def test_get_branch_format_2a(self):
+        # Test the 2a branch format.
+        branch = self.make_branch('test', '2a')
+        formats = get_branch_formats(branch)
+        self.assertEqual(ControlFormat.BZR_METADIR_1, formats[0])
+        self.assertEqual(BranchFormat.BZR_BRANCH_7, formats[1])
+        self.assertEqual(RepositoryFormat.BZR_CHK_2A, formats[2])
+
+    def test_get_branch_format_1_9(self):
+        # Test the 1.9 branch format.
+        branch = self.make_branch('test', '1.9')
+        formats = get_branch_formats(branch)
+        self.assertEqual(ControlFormat.BZR_METADIR_1, formats[0])
+        self.assertEqual(BranchFormat.BZR_BRANCH_7, formats[1])
+        self.assertEqual(RepositoryFormat.BZR_KNITPACK_6, formats[2])
+
+    def test_get_branch_format_packs(self):
+        # Test the packs branch format.
+        branch = self.make_branch('test', 'pack-0.92')
+        formats = get_branch_formats(branch)
+        self.assertEqual(ControlFormat.BZR_METADIR_1, formats[0])
+        self.assertEqual(BranchFormat.BZR_BRANCH_6, formats[1])
+        self.assertEqual(RepositoryFormat.BZR_KNITPACK_1, formats[2])
+
+    def test_get_branch_format_knits(self):
+        # Test the knits branch format.
+        branch = self.make_branch('test', 'knit')
+        formats = get_branch_formats(branch)
+        self.assertEqual(ControlFormat.BZR_METADIR_1, formats[0])
+        self.assertEqual(BranchFormat.BZR_BRANCH_5, formats[1])
+        self.assertEqual(RepositoryFormat.BZR_KNIT_1, formats[2])

=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
--- lib/lp/code/xmlrpc/codehosting.py	2010-06-11 03:52:02 +0000
+++ lib/lp/code/xmlrpc/codehosting.py	2010-07-27 04:14:45 +0000
@@ -33,7 +33,7 @@
 
 from lp.code.errors import UnknownBranchTypeError
 from lp.code.bzr import BranchFormat, ControlFormat, RepositoryFormat
-from lp.code.enums import BranchType, match_enum_title
+from lp.code.enums import BranchType
 from lp.code.interfaces.branch import BranchCreationException
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.branchnamespace import (
@@ -206,13 +206,9 @@
             if branch is None:
                 return faults.NoBranchWithID(branch_id)
 
-            control_format = match_enum_title(
-                ControlFormat, control_string, ControlFormat.UNRECOGNIZED)
-            branch_format = match_enum_title(
-                BranchFormat, branch_string, BranchFormat.UNRECOGNIZED)
-            repository_format = match_enum_title(
-                RepositoryFormat, repository_string,
-                RepositoryFormat.UNRECOGNIZED)
+            control_format = ControlFormat.get_enum(control_string)
+            branch_format = BranchFormat.get_enum(branch_string)
+            repository_format = RepositoryFormat.get_enum(repository_string)
 
             if requester == LAUNCHPAD_SERVICES:
                 branch = removeSecurityProxy(branch)