Merge lp:~mwhudson/launchpad/no-hosted-area-fix-directbranchcommit into lp:launchpad

[Michael Hudson-Doyle]

Hi Tim,

This branch simplifies DirectBranchCommit and makes it work with in the new world.

It changes the behaviour of the useBzrBranches() test helper some, which will probably break lots of tests that I'll fix in the next few pipes...

Cheers,
mwh

[Tim Penhey (thumper)]

Looks like a great change.

review: Approve

=== modified file 'lib/lp/code/model/directbranchcommit.py'
--- lib/lp/code/model/directbranchcommit.py	2010-04-27 02:23:05 +0000
+++ lib/lp/code/model/directbranchcommit.py	2010-04-27 02:23:25 +0000
@@ -12,13 +12,11 @@
 
 import os.path
 
-from bzrlib.branch import Branch
 from bzrlib.generate_ids import gen_file_id
 from bzrlib.revision import NULL_REVISION
 from bzrlib.transform import TransformPreview, ROOT_PARENT
 
 from canonical.launchpad.interfaces import IMasterObject
-from lp.codehosting.vfs import make_branch_mirrorer
 
 
 class ConcurrentUpdateError(Exception):
@@ -49,7 +47,7 @@
     is_locked = False
     commit_builder = None
 
-    def __init__(self, db_branch, committer=None, to_mirror=False):
+    def __init__(self, db_branch, committer=None):
         """Create context for direct commit to branch.
 
         Before constructing a `DirectBranchCommit`, set up a server that
@@ -68,11 +66,8 @@
 
         :param db_branch: a Launchpad `Branch` object.
         :param committer: the `Person` writing to the branch.
-        :param to_mirror: If True, write to the mirrored copy of the branch
-            instead of the hosted copy.  (Mainly useful for tests)
         """
         self.db_branch = db_branch
-        self.to_mirror = to_mirror
 
         self.last_scanned_id = self.db_branch.last_scanned_id
 
@@ -83,15 +78,7 @@
         # Directories we create on the branch, and their ids.
         self.path_ids = {}
 
-        if to_mirror:
-            self.bzrbranch = Branch.open(self.db_branch.warehouse_url)
-        else:
-            # Have the opening done through a branch mirrorer.  It will
-            # pick the right policy.  In case we're writing to a hosted
-            # branch stacked on a mirrored branch, the mirrorer knows
-            # how to do the right thing.
-            mirrorer = make_branch_mirrorer(self.db_branch.branch_type)
-            self.bzrbranch = mirrorer.open(self.db_branch.getPullURL())
+        self.bzrbranch = self.db_branch.getBzrBranch()
 
         self.bzrbranch.lock_write()
         self.is_locked = True
@@ -167,10 +154,6 @@
 
         If it does, raise `ConcurrentUpdateError`.
         """
-        # A different last_scanned_id does not indicate a race for mirrored
-        # branches -- last_scanned_id is a proxy for the mirrored branch.
-        if self.to_mirror:
-            return
         assert self.is_locked, "Getting revision on un-locked branch."
         last_revision = self.bzrbranch.last_revision()
         if last_revision != self.last_scanned_id:
 
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py	2010-04-27 02:23:05 +0000
+++ lib/lp/code/model/tests/test_diff.py	2010-04-27 02:23:25 +0000
@@ -44,7 +44,7 @@
 
         This will create or modify the file, as needed.
         """
-        committer = DirectBranchCommit(branch, to_mirror=True)
+        committer = DirectBranchCommit(branch)
         committer.writeFile(path, contents)
         try:
             return committer.commit('committing')
 
=== modified file 'lib/lp/code/tests/test_directbranchcommit.py'
--- lib/lp/code/tests/test_directbranchcommit.py	2009-11-18 14:34:45 +0000
+++ lib/lp/code/tests/test_directbranchcommit.py	2010-04-27 02:23:25 +0000
@@ -20,10 +20,10 @@
 
     def setUp(self):
         super(DirectBranchCommitTestCase, self).setUp()
-        self.useBzrBranches()
+        self.useBzrBranches(direct_database=True)
 
         self.series = self.factory.makeProductSeries()
-        self.db_branch, self.tree = self.create_branch_and_tree(hosted=True)
+        self.db_branch, self.tree = self.create_branch_and_tree()
 
         self.series.translations_branch = self.db_branch
 
@@ -46,7 +46,7 @@
 
     def _getContents(self):
         """Return branch contents as dict mapping filenames to contents."""
-        return map_branch_contents(self.committer.db_branch.getPullURL())
+        return map_branch_contents(self.committer.db_branch.getBzrBranch())
 
 
 class TestDirectBranchCommit(DirectBranchCommitTestCase):
@@ -229,23 +229,5 @@
         self.assertEqual(dir_id, self.committer._getDir('foo/bar'))
 
 
-class TestDirectBranchCommitMirror(TestCaseWithFactory):
-
-    layer = ZopelessDatabaseLayer
-
-    def test_direct_branch_commit_respects_to_mirror(self):
-        # The "to_mirror" argument causes the commit to apply to the mirrored
-        # copy of the branch.
-        self.useBzrBranches()
-        branch = self.factory.makeBranch()
-        bzr_branch = self.createBzrBranch(branch)
-        dbc = DirectBranchCommit(branch, to_mirror=True)
-        try:
-            dbc.writeFile('path', 'contents')
-            dbc.commit('making commit to mirrored area.')
-        finally:
-            dbc.unlock()
-
-
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-04-27 02:23:05 +0000
+++ lib/lp/testing/__init__.py	2010-04-27 02:23:25 +0000
@@ -411,7 +411,7 @@
         self.addCleanup(logout)
         from lp.testing.factory import LaunchpadObjectFactory
         self.factory = LaunchpadObjectFactory()
-        self.real_bzr_server = False
+        self.direct_database_server = False
 
     def getUserBrowser(self, url=None, user=None, password='test'):
         """Return a Browser logged in as a fresh user, maybe opened at `url`.
@@ -443,11 +443,6 @@
         """
         if format is not None and isinstance(format, basestring):
             format = format_registry.get(format)()
-        transport = get_transport(branch_url)
-        if not self.real_bzr_server:
-            # for real bzr servers, the prefix always exists.
-            transport.create_prefix()
-        self.addCleanup(transport.delete_tree, '.')
         return BzrDir.create_branch_convenience(
             branch_url, format=format)
 
@@ -468,7 +463,7 @@
             else:
                 db_branch = self.factory.makeProductBranch(product, **kwargs)
         branch_url = 'lp-internal:///' + db_branch.unique_name
-        if self.real_bzr_server:
+        if not self.direct_database_server:
             transaction.commit()
         bzr_branch = self.createBranchAtURL(branch_url, format=format)
         if tree_location is None:
@@ -533,34 +528,20 @@
         os.environ['BZR_HOME'] = os.getcwd()
         self.addCleanup(restore_bzr_home)
 
-    def useBzrBranches(self, real_server=False, direct_database=False):
+    def useBzrBranches(self, direct_database=False):
         """Prepare for using bzr branches.
 
-        This sets up support for lp-hosted and lp-mirrored URLs,
-        changes to a temp directory, and overrides the bzr home directory.
+        This sets up support for lp-internal URLs, changes to a temp
+        directory, and overrides the bzr home directory.
 
-        :param real_server: If true, use the "real" code hosting server,
-            using an xmlrpc server, etc.
+        :param direct_database: If true, translate branch locations by
+            directly querying the database, not the internal XML-RPC server.
         """
-        from lp.codehosting.scanner.tests.test_bzrsync import (
-            FakeTransportServer)
         self.useTempBzrHome()
-        self.real_bzr_server = real_server
-        if real_server:
-            server = get_rw_server(
-                direct_database=direct_database)
-            server.start_server()
-            self.addCleanup(server.destroy)
-        else:
-            os.mkdir('lp-mirrored')
-            mirror_server = FakeTransportServer(get_transport('lp-mirrored'))
-            mirror_server.start_server()
-            self.addCleanup(mirror_server.stop_server)
-            os.mkdir('lp-hosted')
-            hosted_server = FakeTransportServer(
-                get_transport('lp-hosted'), url_prefix='lp-hosted:///')
-            hosted_server.start_server()
-            self.addCleanup(hosted_server.stop_server)
+        self.direct_database_server = direct_database
+        server = get_rw_server(direct_database=direct_database)
+        server.start_server()
+        self.addCleanup(server.destroy)
 
 
 class BrowserTestCase(TestCaseWithFactory):
@@ -855,7 +836,7 @@
 
 # XXX: This doesn't seem to be a generically useful testing function. Perhaps
 # it should go into a sub-module? -- jml
-def map_branch_contents(branch_url):
+def map_branch_contents(branch):
     """Return all files in branch at `branch_url`.
 
     :param branch_url: the URL for an accessible branch.
@@ -863,7 +844,6 @@
         files are included.
     """
     contents = {}
-    branch = BzrBranch.open(branch_url)
     tree = branch.basis_tree()
     tree.lock_read()
     try:
 
=== modified file 'lib/lp/translations/scripts/tests/test_translations_to_branch.py'
--- lib/lp/translations/scripts/tests/test_translations_to_branch.py	2010-04-27 02:23:05 +0000
+++ lib/lp/translations/scripts/tests/test_translations_to_branch.py	2010-04-27 02:23:25 +0000
@@ -40,7 +40,7 @@
         # End-to-end test of the script doing its work.
 
         # Set up a server for hosted branches.
-        self.useBzrBranches(real_server=True)
+        self.useBzrBranches(direct_database=False)
 
         # Set up a product and translatable series.
         product = self.factory.makeProduct(name='committobranch')
@@ -48,8 +48,7 @@
         series = product.getSeries('trunk')
 
         # Set up a translations_branch for the series.
-        db_branch, tree = self.create_branch_and_tree(
-            hosted=True, product=product)
+        db_branch, tree = self.create_branch_and_tree(product=product)
         removeSecurityProxy(db_branch).last_scanned_id = 'null:'
         product.official_rosetta = True
         series.translations_branch = db_branch
@@ -87,7 +86,7 @@
 
         # The branch now contains a snapshot of the translation.  (Only
         # one file: the Dutch translation we set up earlier).
-        branch_contents = map_branch_contents(db_branch.getPullURL())
+        branch_contents = map_branch_contents(db_branch.getBzrBranch())
         expected_contents = {
             'po/nl.po': """
                 # Dutch translation for .*
@@ -163,11 +162,11 @@
         self.useBzrBranches()
 
         base_branch, base_tree = self.create_branch_and_tree(
-            'base', name='base', hosted=True)
+            'base', name='base')
         self._setUpBranch(base_branch, base_tree, "Base branch.")
 
         stacked_branch, stacked_tree = self.create_branch_and_tree(
-            'stacked', name='stacked', hosted=True)
+            'stacked', name='stacked')
         stacked_tree.branch.set_stacked_on_url('/' + base_branch.unique_name)
         stacked_branch.stacked_on = base_branch
         self._setUpBranch(stacked_branch, stacked_tree, "Stacked branch.")
 
=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py	2010-03-08 19:46:02 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py	2010-04-27 02:23:25 +0000
@@ -17,7 +17,7 @@
 from storm.expr import Join, SQL
 
 from canonical.launchpad.helpers import shortlist
-from lp.codehosting.vfs import get_multi_server
+from lp.codehosting.vfs import get_rw_server
 from lp.translations.interfaces.potemplate import IPOTemplateSet
 from canonical.launchpad.webapp.interfaces import (
     IStoreSelector, MAIN_STORE, SLAVE_FLAVOR)
@@ -270,7 +270,7 @@
         # testing.
         productseries = productseries.order_by(ProductSeries.id)
 
-        bzrserver = get_multi_server(write_hosted=True)
+        bzrserver = get_rw_server()
         bzrserver.start_server()
         try:
             self._exportToBranches(productseries)
 
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-04-16 13:31:48 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-04-27 02:23:25 +0000
@@ -219,7 +219,7 @@
         # If the feature is enabled, a TipChanged event for a branch that
         # generates templates will schedule a templates build.
         branch = self._makeTranslationBranch()
-        commit = DirectBranchCommit(branch, to_mirror=True)
+        commit = DirectBranchCommit(branch)
         commit.writeFile('POTFILES.in', 'foo')
         commit.commit('message')
         notify(events.TipChanged(branch, None, False))