Merge lp:~mwhudson/launchpad/no-hosted-area-server-catchup into lp:launchpad/db-devel

[Michael Hudson-Doyle]

Hi Tim,

This branch is mostly a cleanup, it removes all uses of get_puller_server or get_scanner_server and replaces them with get_rw_server or get_ro_server as appropriate.

There are also some clean ups and test fixes that really belong in a lower pipe -- I hope you'll forgive me for those.

Cheers,
mwh

[Tim Penhey (thumper)]

This is all good.

review: Approve

=== modified file 'cronscripts/rosetta-branches.py'
--- cronscripts/rosetta-branches.py	2010-02-16 15:25:52 +0000
+++ cronscripts/rosetta-branches.py	2010-04-27 02:24:09 +0000
@@ -17,7 +17,7 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from lp.codehosting.vfs.branchfs import get_scanner_server
+from lp.codehosting.vfs.branchfs import get_ro_server
 from lp.services.job.runner import JobRunner
 from lp.code.interfaces.branchjob import IRosettaUploadJobSource
 from lp.services.scripts.base import LaunchpadCronScript
@@ -31,7 +31,7 @@
         globalErrorUtility.configure('rosettabranches')
         runner = JobRunner.fromReady(
             getUtility(IRosettaUploadJobSource), self.logger)
-        server = get_scanner_server()
+        server = get_ro_server()
         server.start_server()
         try:
             runner.runAll()
 
=== modified file 'cronscripts/sendbranchmail.py'
--- cronscripts/sendbranchmail.py	2010-02-16 15:25:52 +0000
+++ cronscripts/sendbranchmail.py	2010-04-27 02:24:09 +0000
@@ -16,7 +16,7 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from lp.codehosting.vfs import get_scanner_server
+from lp.codehosting.vfs import get_ro_server
 from lp.services.job.runner import JobRunner
 from lp.code.interfaces.branchjob import (
     IRevisionMailJobSource, IRevisionsAddedJobSource)
@@ -32,7 +32,7 @@
         jobs = list(getUtility(IRevisionMailJobSource).iterReady())
         jobs.extend(getUtility(IRevisionsAddedJobSource).iterReady())
         runner = JobRunner(jobs, self.logger)
-        server = get_scanner_server()
+        server = get_ro_server()
         server.start_server()
         try:
             runner.runAll()
 
=== modified file 'lib/lp/code/model/directbranchcommit.py'
--- lib/lp/code/model/directbranchcommit.py	2010-04-27 02:24:00 +0000
+++ lib/lp/code/model/directbranchcommit.py	2010-04-27 02:24:09 +0000
@@ -51,9 +51,9 @@
         """Create context for direct commit to branch.
 
         Before constructing a `DirectBranchCommit`, set up a server that
-        allows write access to lp-hosted:/// URLs:
+        allows write access to lp-internal:/// URLs:
 
-        bzrserver = get_multi_server(write_hosted=True)
+        bzrserver = get_rw_server()
         bzrserver.start_server()
         try:
             branchcommit = DirectBranchCommit(branch)
 
=== modified file 'lib/lp/code/scripts/tests/test_sendbranchmail.py'
--- lib/lp/code/scripts/tests/test_sendbranchmail.py	2010-04-01 04:37:30 +0000
+++ lib/lp/code/scripts/tests/test_sendbranchmail.py	2010-04-27 02:24:09 +0000
@@ -23,7 +23,7 @@
     layer = ZopelessAppServerLayer
 
     def createBranch(self):
-        branch, tree = self.createMirroredBranchAndTree()
+        branch, tree = self.create_branch_and_tree()
         branch.subscribe(branch.registrant,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.WHOLEDIFF,
@@ -36,7 +36,7 @@
 
     def test_sendbranchmail(self):
         """Ensure sendbranchmail runs and sends email."""
-        self.useTempBzrHome()
+        self.useBzrBranches()
         branch, tree = self.createBranch()
         RevisionMailJob.create(
             branch, 1, '[e-mail]', 'body', True, 'foo')
@@ -68,7 +68,7 @@
 
     def test_revision_added_job(self):
         """RevisionsAddedJobs are run by sendbranchmail."""
-        self.useTempBzrHome()
+        self.useBzrBranches()
         branch, tree = self.createBranch()
         tree.bzrdir.root_transport.put_bytes('foo', 'baz')
         tree.commit('Added foo.', rev_id='rev2')
 
=== modified file 'lib/lp/codehosting/puller/tests/test_acceptance.py'
--- lib/lp/codehosting/puller/tests/test_acceptance.py	2010-04-27 02:24:00 +0000
+++ lib/lp/codehosting/puller/tests/test_acceptance.py	2010-04-27 02:24:09 +0000
@@ -7,22 +7,18 @@
 __all__ = []
 
 
-import datetime
 import os
 from subprocess import PIPE, Popen
 import unittest
 
-import pytz
 
 import transaction
 
 from bzrlib.branch import Branch
 from bzrlib.bzrdir import BzrDir
 from bzrlib import errors
-from bzrlib.transport import get_transport
 from bzrlib.upgrade import upgrade
-from bzrlib.urlutils import (
-    join as urljoin, local_path_from_url, local_path_to_url)
+from bzrlib.urlutils import join as urljoin, local_path_from_url
 from bzrlib.workingtree import WorkingTree
 
 from zope.component import getUtility
@@ -30,7 +26,7 @@
 
 from lp.code.enums import BranchType
 from lp.code.interfaces.branchtarget import IBranchTarget
-from lp.codehosting.vfs import branch_id_to_path, get_lp_server
+from lp.codehosting.vfs import get_lp_server
 from lp.codehosting.puller.tests import PullerBranchTestCase
 from lp.codehosting.tests.helpers import LoomTestMixin
 from canonical.config import config
@@ -56,8 +52,7 @@
         self.makeCleanDirectory(
             local_path_from_url(config.launchpad.bzr_imports_root_url))
 
-    def assertMirrored(self, db_branch, source_branch,
-                       accessing_user=None):
+    def assertMirrored(self, db_branch, source_branch):
         """Assert that 'db_branch' was mirrored succesfully.
 
         This method checks that the fields on db_branch show that the branch
@@ -73,8 +68,7 @@
             supplied create a fresh user for this -- but this won't work for a
             private branch.
         """
-        if accessing_user is None:
-            accessing_user = self.factory.makePerson()
+        accessing_user = self.factory.makePerson()
         transaction.commit()
         self.assertEqual(None, db_branch.mirror_status_message)
         self.assertEqual(
@@ -219,22 +213,21 @@
             (defaults to not).
         :return: `IBranch`.
         """
-        # Make the branch.
+        # Make the branch in the database.
         product = self.factory.makeProduct()
         default_branch = self.factory.makeProductBranch(
             product=product, private=private)
-        default_branch.last_mirrored = datetime.datetime.now(pytz.UTC)
-        # Make it the default stacked-on branch.
+        transaction.commit()
+        # Create the underlying bzr branch.
+        lp_server = self.getLPServerForUser(default_branch.owner)
+        BzrDir.create_branch_convenience(
+            lp_server.get_url() + default_branch.unique_name)
+        transaction.commit()
+        # Make it the default stacked-on branch for the product.
         series = removeSecurityProxy(product.development_focus)
         series.branch = default_branch
         self.assertEqual(
             default_branch, IBranchTarget(product).default_stacked_on_branch)
-        branch_location = urljoin(
-            local_path_to_url(config.codehosting.mirrored_branches_root),
-            branch_id_to_path(default_branch.id))
-        get_transport(branch_location).create_prefix()
-        BzrDir.create_branch_convenience(branch_location)
-        transaction.commit()
         return default_branch
 
     def test_stack_mirrored_branch(self):
 
=== modified file 'lib/lp/codehosting/puller/tests/test_scheduler.py'
--- lib/lp/codehosting/puller/tests/test_scheduler.py	2010-04-27 02:24:00 +0000
+++ lib/lp/codehosting/puller/tests/test_scheduler.py	2010-04-27 02:24:09 +0000
@@ -28,6 +28,7 @@
 from canonical.launchpad.webapp import errorlog
 from canonical.testing import (
     reset_logging, TwistedLayer, TwistedAppServerLayer)
+from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
 from lp.codehosting.puller import get_lock_id_for_branch_id, scheduler
 from lp.codehosting.puller.tests import PullerBranchTestCase
 from lp.codehosting.puller.worker import (
@@ -454,9 +455,9 @@
 
         def checkMirrorCompleted(ignored):
             self.assertEqual(
-                [('branchChanged', self.arbitrary_branch_id, stacked_on_url,
-                  revid_after, control_string, branch_string,
-                  repository_string)],
+                [('branchChanged', LAUNCHPAD_SERVICES,
+                  self.arbitrary_branch_id, stacked_on_url, revid_after,
+                  control_string, branch_string, repository_string)],
                 self.status_client.calls)
         return deferred.addCallback(checkMirrorCompleted)
 
@@ -680,8 +681,9 @@
             repository_string = \
                 default_format.repository_format.get_format_string()
             self.assertEqual(
-                [('branchChanged', self.db_branch.id, '', revision_id,
-                  control_string, branch_string, repository_string)],
+                [('branchChanged', LAUNCHPAD_SERVICES, self.db_branch.id, '',
+                  revision_id, control_string, branch_string,
+                  repository_string)],
                 self.client.calls)
             return ignored
         deferred.addCallback(check_authserver_called)
 
=== modified file 'lib/lp/codehosting/vfs/__init__.py'
--- lib/lp/codehosting/vfs/__init__.py	2010-04-27 02:24:00 +0000
+++ lib/lp/codehosting/vfs/__init__.py	2010-04-27 02:24:09 +0000
@@ -9,17 +9,14 @@
     'branch_id_to_path',
     'BranchFileSystemClient',
     'get_lp_server',
-    'get_multi_server',
     'get_ro_server',
     'get_rw_server',
-    'get_scanner_server',
     'LaunchpadServer',
     'make_branch_mirrorer',
     ]
 
 from lp.codehosting.vfs.branchfs import (
-    AsyncLaunchpadTransport, branch_id_to_path, get_lp_server,
-    get_multi_server, get_ro_server, get_rw_server, get_scanner_server,
-    LaunchpadServer, make_branch_mirrorer)
+    AsyncLaunchpadTransport, LaunchpadServer, branch_id_to_path,
+    get_lp_server, get_ro_server, get_rw_server, make_branch_mirrorer)
 from lp.codehosting.vfs.branchfsclient import (
     BlockingProxy,BranchFileSystemClient)
 
=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
--- lib/lp/codehosting/vfs/branchfs.py	2010-04-27 02:24:00 +0000
+++ lib/lp/codehosting/vfs/branchfs.py	2010-04-27 02:24:09 +0000
@@ -54,10 +54,8 @@
     'BranchPolicy',
     'DirectDatabaseLaunchpadServer',
     'get_lp_server',
-    'get_multi_server',
     'get_ro_server',
     'get_rw_server',
-    'get_scanner_server',
     'make_branch_mirrorer',
     'LaunchpadInternalServer',
     'LaunchpadServer',
@@ -88,8 +86,8 @@
 from lp.codehosting.vfs.branchfsclient import (
     BlockingProxy, BranchFileSystemClient)
 from lp.codehosting.vfs.transport import (
-    AsyncVirtualServer, AsyncVirtualTransport, _MultiServer,
-    get_chrooted_transport, get_readonly_transport, TranslationError)
+    AsyncVirtualServer, AsyncVirtualTransport, TranslationError,
+    get_chrooted_transport, get_readonly_transport)
 from canonical.config import config
 from canonical.launchpad.xmlrpc import faults
 from lp.code.enums import BranchType
@@ -177,7 +175,6 @@
     return LaunchpadInternalServer(
         'lp-internal:///', codehosting_endpoint, branch_transport)
 
-get_scanner_server = get_ro_server
 
 def get_rw_server(direct_database=False):
     """Get a server that can write to the Launchpad branch vfs.
@@ -200,48 +197,6 @@
             'lp-internal:///', codehosting_endpoint, transport)
 
 
-def get_multi_server(write_hosted=False, write_mirrored=False,
-                     direct_database=False):
-    """Get a server with access to both mirrored and hosted areas.
-
-    The server wraps up two `LaunchpadInternalServer`s or
-    `DirectDatabaseLaunchpadServer`s. One server points to the hosted branch
-    area and the other points to the mirrored area.
-
-    Write permision defaults to False, but can be overridden.
-
-    :param write_hosted: if True, lp-hosted URLs are writeable.  Otherwise,
-        they are read-only.
-    :param write_mirrored: if True, lp-mirrored URLs are writeable.
-        Otherwise, they are read-only.
-
-    :param direct_database: if True, use a server implementation that talks
-        directly to the database.  If False, the default, use a server
-        implementation that talks to the internal XML-RPC server.
-    """
-    # XXX 2010-04-20, MichaelHudson: this function will disappear in a later
-    # pipe.
-    hosted_transport = get_chrooted_transport(
-        config.codehosting.mirrored_branches_root, mkdir=True)
-    if not write_hosted:
-        hosted_transport = get_readonly_transport(hosted_transport)
-    mirrored_transport = get_chrooted_transport(
-        config.codehosting.mirrored_branches_root, mkdir=True)
-    if not write_mirrored:
-        mirrored_transport = get_readonly_transport(mirrored_transport)
-    if direct_database:
-        make_server = DirectDatabaseLaunchpadServer
-    else:
-        proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
-        codehosting_endpoint = BlockingProxy(proxy)
-        def make_server(scheme, transport):
-            return LaunchpadInternalServer(
-                scheme, codehosting_endpoint, transport)
-    hosted_server = make_server('lp-hosted:///', hosted_transport)
-    mirrored_server = make_server('lp-mirrored:///', mirrored_transport)
-    return _MultiServer(hosted_server, mirrored_server)
-
-
 class ITransportDispatch(Interface):
     """Turns descriptions of transports into transports."""
 
 
=== modified file 'lib/lp/codehosting/vfs/tests/test_branchfs.py'
--- lib/lp/codehosting/vfs/tests/test_branchfs.py	2010-04-27 02:24:00 +0000
+++ lib/lp/codehosting/vfs/tests/test_branchfs.py	2010-04-27 02:24:09 +0000
@@ -826,11 +826,12 @@
         self.backing_transport = MemoryTransport()
         self.disable_directory_isolation()
 
-    def _replacement_branchChanged(self, branch_id, stacked_on_url,
+    def _replacement_branchChanged(self, user_id, branch_id, stacked_on_url,
                                    last_revision, *format_strings):
         self._branch_changed_log.append(dict(
-            branch_id=branch_id, stacked_on_url=stacked_on_url,
-            last_revision=last_revision, format_strings=format_strings))
+            user_id=user_id, branch_id=branch_id,
+            stacked_on_url=stacked_on_url, last_revision=last_revision,
+            format_strings=format_strings))
 
     def get_server(self):
         if self._server is None:
@@ -862,6 +863,19 @@
         branch.unlock()
         self.assertEqual(1, len(self._branch_changed_log))
 
+    def test_branch_unlock_reports_users_id(self):
+        # Unlocking a branch calls branchChanged on the branch filesystem
+        # endpoint with the logged in user's id.
+        db_branch = self.factory.makeAnyBranch(
+            branch_type=BranchType.HOSTED, owner=self.requester)
+        branch = self.make_branch(db_branch.unique_name)
+        del self._branch_changed_log[:]
+        branch.lock_write()
+        branch.unlock()
+        self.assertEqual(1, len(self._branch_changed_log))
+        self.assertEqual(
+            self.requester.id, self._branch_changed_log[0]['user_id'])
+
     def test_branch_unlock_reports_stacked_on_url(self):
         # Unlocking a branch reports the stacked on URL to the branch
         # filesystem endpoint.
 
=== modified file 'lib/lp/codehosting/vfs/transport.py'
--- lib/lp/codehosting/vfs/transport.py	2010-03-19 10:43:51 +0000
+++ lib/lp/codehosting/vfs/transport.py	2010-04-27 02:24:09 +0000
@@ -16,7 +16,6 @@
     'AsyncVirtualTransport',
     'get_chrooted_transport',
     'get_readonly_transport',
-    '_MultiServer',
     'SynchronousAdapter',
     'TranslationError',
     ]
@@ -69,25 +68,6 @@
     return get_transport('readonly+' + transport.base)
 
 
-class _MultiServer(Server):
-    """Server that wraps around multiple servers."""
-
-    def __init__(self, *servers):
-        self._servers = servers
-
-    def start_server(self):
-        for server in self._servers:
-            server.start_server()
-
-    def destroy(self):
-        for server in reversed(self._servers):
-            server.destroy()
-
-    def stop_server(self):
-        for server in reversed(self._servers):
-            server.stop_server()
-
-
 class AsyncVirtualTransport(Transport):
     """A transport for a virtual file system.
 
 
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-04-27 02:24:00 +0000
+++ lib/lp/testing/__init__.py	2010-04-27 02:24:09 +0000
@@ -499,25 +499,6 @@
         get_transport(base).create_prefix()
         return os.path.join(base, branch_id_to_path(branch.id))
 
-    def createMirroredBranchAndTree(self):
-        """Create a database branch, bzr branch and bzr checkout.
-
-        This always uses the configured mirrored area, ignoring whatever
-        server might be providing lp-mirrored: urls.
-
-        Unlike normal codehosting operation, the working tree is stored in the
-        branch directory.
-
-        The branch and tree files are automatically deleted at the end of the
-        test.
-
-        :return: a `Branch` and a workingtree.
-        """
-        db_branch = self.factory.makeAnyBranch()
-        bzr_branch = self.createBranchAtURL(self.getBranchPath(
-                db_branch, config.codehosting.internal_branch_by_id_root))
-        return db_branch, bzr_branch.bzrdir.open_workingtree()
-
     def useTempBzrHome(self):
         self.useTempDir()
         # Avoid leaking local user configuration into tests.
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-04-27 02:24:00 +0000
+++ lib/lp/testing/factory.py	2010-04-27 02:24:09 +0000
@@ -1058,7 +1058,8 @@
             parent = revision
             parent_ids = [parent.revision_id]
         branch.startMirroring()
-        branch.mirrorComplete(parent.revision_id)
+        removeSecurityProxy(branch).branchChanged(
+            '', parent.revision_id, None, None, None)
         branch.updateScannedDetails(parent, sequence)
 
     def makeBranchRevision(self, branch, revision_id, sequence=None):
 
=== modified file 'lib/lp/translations/tests/test_rosetta_branches_script.py'
--- lib/lp/translations/tests/test_rosetta_branches_script.py	2009-09-03 20:29:25 +0000
+++ lib/lp/translations/tests/test_rosetta_branches_script.py	2010-04-27 02:24:09 +0000
@@ -38,9 +38,9 @@
             queue.remove(entry)
 
     def _setup_series_branch(self, pot_path):
-        self.useTempBzrHome()
+        self.useBzrBranches()
         pot_content = self.factory.getUniqueString()
-        branch, tree = self.createMirroredBranchAndTree()
+        branch, tree = self.create_branch_and_tree()
         tree.bzrdir.root_transport.put_bytes(pot_path, pot_content)
         tree.add(pot_path)
         revision_id = tree.commit("first commit")
 
=== modified file 'scripts/update-stacked-on.py'
--- scripts/update-stacked-on.py	2010-04-27 02:24:00 +0000
+++ scripts/update-stacked-on.py	2010-04-27 02:24:09 +0000
@@ -22,50 +22,16 @@
 
 import _pythonpath
 import sys
-import xmlrpclib
 
 from bzrlib.bzrdir import BzrDir
 from bzrlib.config import TransportConfig
 from bzrlib import errors
 
-from lp.codehosting.vfs.branchfs import LaunchpadInternalServer
-from lp.codehosting.vfs import BlockingProxy
-from lp.codehosting.vfs.transport import (
-    get_chrooted_transport, get_readonly_transport, _MultiServer)
+from lp.codehosting.vfs import get_rw_server, get_ro_server
 from lp.codehosting.bzrutils import get_branch_stacked_on_url
-from canonical.config import config
 from lp.services.scripts.base import LaunchpadScript
 
 
-def get_server(read_only):
-    """Get a server that can write to both hosted and mirrored areas."""
-    proxy = xmlrpclib.ServerProxy(config.codehosting.codehosting_endpoint)
-    authserver = BlockingProxy(proxy)
-    hosted_transport = get_chrooted_transport(
-        config.codehosting.hosted_branches_root)
-    if read_only:
-        hosted_transport = get_readonly_transport(hosted_transport)
-    mirrored_transport = get_chrooted_transport(
-        config.codehosting.mirrored_branches_root)
-    if read_only:
-        mirrored_transport = get_readonly_transport(mirrored_transport)
-    hosted_server = LaunchpadInternalServer(
-        'lp-hosted:///', authserver, hosted_transport)
-    mirrored_server = LaunchpadInternalServer(
-        'lp-mirrored:///', authserver, mirrored_transport)
-    return _MultiServer(hosted_server, mirrored_server)
-
-
-def get_hosted_url(unique_name):
-    """Return the hosted URL for the branch with 'unique_name'."""
-    return 'lp-hosted:///%s' % unique_name
-
-
-def get_mirrored_url(unique_name):
-    """Return the mirrored URL for the branch with 'unique_name'."""
-    return 'lp-mirrored:///%s' % unique_name
-
-
 def set_branch_stacked_on_url(bzrdir, stacked_on_url):
     """Set the stacked_on_location for the branch at 'bzrdir'.
 
@@ -93,7 +59,10 @@
                   "motions."))
 
     def main(self):
-        server = get_server(self.options.dry_run)
+        if self.options.dry_run:
+            server = get_ro_server()
+        else:
+            server = get_rw_server()
         server.start_server()
         if self.options.dry_run:
             self.logger.debug('Running read-only')
@@ -109,8 +78,7 @@
 
         :param branch_id: The database ID of the branch. This is only used for
             logging.
-        :param bzr_branch_url: The URL of the Bazaar branch. Normally this is
-            of the form lp-mirrored:/// or lp-hosted:///.
+        :param bzr_branch_url: The lp-internal:/// URL of the Bazaar branch.
         :param stacked_on_location: The location to store in the branch's
             stacked_on_location configuration variable.
         """
@@ -165,12 +133,9 @@
             (branch_id, branch_type, unique_name,
              stacked_on_name) = branch_info
             stacked_on_location = '/' + stacked_on_name
-            if branch_type == 'HOSTED':
-                self.updateStackedOn(
-                    branch_id, get_hosted_url(unique_name),
-                    stacked_on_location)
             self.updateStackedOn(
-                branch_id, get_mirrored_url(unique_name), stacked_on_location)
+                branch_id, 'lp-internal:///' + unique_name,
+                stacked_on_location)
 
 
 if __name__ == '__main__':