[MERGE] [hpss part 2] Use the Command pattern for smart server request handling

Andrew Bennetts andrew at canonical.com
Tue Apr 10 15:35:40 BST 2007


This is the next part of merging the hpss branch into bzr.dev.

This bundle splits server processing of smart requests into individual command
objects (using a standard Registry), rather than dispatching to do_* methods on
SmartServerRequestHandler.  A nice benefit of this is that requests can be
grouped into modules, rather than all living on one extremely large class.

This bundle doesn't add any new requests (like the previous hpss bundle, this
is basically just a restructuring), but later ones will.  It also includes a
simple facility to disable the "VFS" smart requests by use of an environment
variable, which is intended as a development aid rather than a production
feature.

-Andrew.

-------------- next part --------------
# Bazaar revision bundle v0.9
#
# message:
#   Use the Command pattern for handling smart server commands.
#   
#   This improves readability and testability.
#   
#   This change has been extracted from the hpss branch.
#   
# committer: Andrew Bennetts <andrew.bennetts at canonical.com>
# date: Wed 2007-04-11 00:12:35.012000084 +1000

=== added file bzrlib/smart/vfs.py // file-id:vfs.py-20061108095550-gunadhxmzkd
... jfeek-2
--- /dev/null
+++ bzrlib/smart/vfs.py
@@ -0,0 +1,198 @@
+# Copyright (C) 2006 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""VFS operations for the smart server.
+
+This module defines the smart server methods that are low-level file operations
+-- i.e. methods that operate directly on files and directories, rather than
+higher-level concepts like branches and revisions.
+
+These methods, plus 'hello' and 'get_bundle', are version 1 of the smart server
+protocol, as implemented in bzr 0.11 and later.
+"""
+
+import os
+
+from bzrlib import errors
+from bzrlib.smart import request
+
+
+def _deserialise_optional_mode(mode):
+    # XXX: FIXME this should be on the protocol object.  Later protocol versions
+    # might serialise modes differently.
+    if mode == '':
+        return None
+    else:
+        return int(mode)
+
+
+def vfs_enabled():
+    """Is the VFS enabled ?
+
+    the VFS is disabled when the NO_SMART_VFS environment variable is set.
+
+    :return: True if it is enabled.
+    """
+    return not 'NO_SMART_VFS' in os.environ
+
+
+class VfsRequest(request.SmartServerRequest):
+    """Base class for VFS requests.
+    
+    VFS requests are disabled if vfs_enabled() returns False.
+    """
+
+    def _check_enabled(self):
+        if not vfs_enabled():
+            raise errors.DisabledMethod(self.__class__.__name__)
+
+
+class HasRequest(VfsRequest):
+
+    def do(self, relpath):
+        r = self._backing_transport.has(relpath) and 'yes' or 'no'
+        return request.SmartServerResponse((r,))
+
+
+class GetRequest(VfsRequest):
+
+    def do(self, relpath):
+        backing_bytes = self._backing_transport.get_bytes(relpath)
+        return request.SmartServerResponse(('ok',), backing_bytes)
+
+
+class AppendRequest(VfsRequest):
+
+    def do(self, relpath, mode):
+        self._relpath = relpath
+        self._mode = _deserialise_optional_mode(mode)
+    
+    def do_body(self, body_bytes):
+        old_length = self._backing_transport.append_bytes(
+            self._relpath, body_bytes, self._mode)
+        return request.SmartServerResponse(('appended', '%d' % old_length))
+
+
+class DeleteRequest(VfsRequest):
+
+    def do(self, relpath):
+        self._backing_transport.delete(relpath)
+        return request.SmartServerResponse(('ok', ))
+
+
+class IterFilesRecursiveRequest(VfsRequest):
+
+    def do(self, relpath):
+        transport = self._backing_transport.clone(relpath)
+        filenames = transport.iter_files_recursive()
+        return request.SmartServerResponse(('names',) + tuple(filenames))
+
+
+class ListDirRequest(VfsRequest):
+
+    def do(self, relpath):
+        filenames = self._backing_transport.list_dir(relpath)
+        return request.SmartServerResponse(('names',) + tuple(filenames))
+
+
+class MkdirRequest(VfsRequest):
+
+    def do(self, relpath, mode):
+        self._backing_transport.mkdir(relpath,
+                                      _deserialise_optional_mode(mode))
+        return request.SmartServerResponse(('ok',))
+
+
+class MoveRequest(VfsRequest):
+
+    def do(self, rel_from, rel_to):
+        self._backing_transport.move(rel_from, rel_to)
+        return request.SmartServerResponse(('ok',))
+
+
+class PutRequest(VfsRequest):
+
+    def do(self, relpath, mode):
+        self._relpath = relpath
+        self._mode = _deserialise_optional_mode(mode)
+
+    def do_body(self, body_bytes):
+        self._backing_transport.put_bytes(self._relpath, body_bytes, self._mode)
+        return request.SmartServerResponse(('ok',))
+
+
+class PutNonAtomicRequest(VfsRequest):
+
+    def do(self, relpath, mode, create_parent, dir_mode):
+        self._relpath = relpath
+        self._dir_mode = _deserialise_optional_mode(dir_mode)
+        self._mode = _deserialise_optional_mode(mode)
+        # a boolean would be nicer XXX
+        self._create_parent = (create_parent == 'T')
+
+    def do_body(self, body_bytes):
+        self._backing_transport.put_bytes_non_atomic(self._relpath,
+                body_bytes,
+                mode=self._mode,
+                create_parent_dir=self._create_parent,
+                dir_mode=self._dir_mode)
+        return request.SmartServerResponse(('ok',))
+
+
+class ReadvRequest(VfsRequest):
+
+    def do(self, relpath):
+        self._relpath = relpath
+
+    def do_body(self, body_bytes):
+        """accept offsets for a readv request."""
+        offsets = self._deserialise_offsets(body_bytes)
+        backing_bytes = ''.join(bytes for offset, bytes in
+            self._backing_transport.readv(self._relpath, offsets))
+        return request.SmartServerResponse(('readv',), backing_bytes)
+
+    def _deserialise_offsets(self, text):
+        # XXX: FIXME this should be on the protocol object.
+        offsets = []
+        for line in text.split('\n'):
+            if not line:
+                continue
+            start, length = line.split(',')
+            offsets.append((int(start), int(length)))
+        return offsets
+
+
+class RenameRequest(VfsRequest):
+
+    def do(self, rel_from, rel_to):
+        self._backing_transport.rename(rel_from, rel_to)
+        return request.SmartServerResponse(('ok', ))
+
+
+class RmdirRequest(VfsRequest):
+
+    def do(self, relpath):
+        self._backing_transport.rmdir(relpath)
+        return request.SmartServerResponse(('ok', ))
+
+
+class StatRequest(VfsRequest):
+
+    def do(self, relpath):
+        stat = self._backing_transport.stat(relpath)
+        return request.SmartServerResponse(
+            ('stat', str(stat.st_size), oct(stat.st_mode)))
+

=== modified file bzrlib/errors.py
--- bzrlib/errors.py
+++ bzrlib/errors.py
@@ -175,6 +175,17 @@
         self.message = message
 
 
+class DisabledMethod(BzrError):
+
+    _fmt = "The smart server method '%(class_name)s' is disabled."
+
+    internal_error = True
+
+    def __init__(self, class_name):
+        BzrError.__init__(self)
+        self.class_name = class_name
+
+
 class InvalidEntryName(BzrError):
     
     _fmt = "Invalid entry name: %(name)s"

=== modified file bzrlib/smart/protocol.py
--- bzrlib/smart/protocol.py
+++ bzrlib/smart/protocol.py
@@ -90,7 +90,7 @@
                 first_line += '\n'
                 req_args = _decode_tuple(first_line)
                 self.request = request.SmartServerRequestHandler(
-                    self._backing_transport)
+                    self._backing_transport, commands=request.request_handlers)
                 self.request.dispatch_command(req_args[0], req_args[1:])
                 if self.request.finished_reading:
                     # trivial request

=== modified file bzrlib/smart/request.py
--- bzrlib/smart/request.py
+++ bzrlib/smart/request.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2007 Canonical Ltd
+# Copyright (C) 2006 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -19,14 +19,54 @@
 
 import tempfile
 
-from bzrlib import (
-    bzrdir,
-    errors,
-    revision
-    )
+from bzrlib import bzrdir, errors, registry, revision
 from bzrlib.bundle.serializer import write_bundle
 
 
+class SmartServerRequest(object):
+    """Base class for request handlers.
+    """
+
+    def __init__(self, backing_transport):
+        self._backing_transport = backing_transport
+
+    def _check_enabled(self):
+        """Raises DisabledMethod if this method is disabled."""
+        pass
+
+    def do(self, *args):
+        """Mandatory extension point for SmartServerRequest subclasses.
+        
+        Subclasses must implement this.
+        
+        This should return a SmartServerResponse if this command expects to
+        receive no body.
+        """
+        raise NotImplementedError(self.do)
+
+    def execute(self, *args):
+        """Public entry point to execute this request.
+
+        It will return a SmartServerResponse if the command does not expect a
+        body.
+
+        :param *args: the arguments of the request.
+        """
+        self._check_enabled()
+        return self.do(*args)
+
+    def do_body(self, body_bytes):
+        """Called if the client sends a body with the request.
+        
+        Must return a SmartServerResponse.
+        """
+        # TODO: if a client erroneously sends a request that shouldn't have a
+        # body, what to do?  Probably SmartServerRequestHandler should catch
+        # this NotImplementedError and translate it into a 'bad request' error
+        # to send to the client.
+        raise NotImplementedError(self.do_body)
+
+
 class SmartServerResponse(object):
     """Response generated by SmartServerRequestHandler."""
 
@@ -34,10 +74,14 @@
         self.args = args
         self.body = body
 
-# XXX: TODO: Create a SmartServerRequestHandler which will take the responsibility
-# for delivering the data for a request. This could be done with as the
-# StreamServer, though that would create conflation between request and response
-# which may be undesirable.
+    def __eq__(self, other):
+        if other is None:
+            return False
+        return other.args == self.args and other.body == self.body
+
+    def __repr__(self):
+        return "<SmartServerResponse args=%r body=%r>" % (self.args, self.body)
+
 
 class SmartServerRequestHandler(object):
     """Protocol logic for smart server.
@@ -55,163 +99,46 @@
 
     # TODO: Better way of representing the body for commands that take it,
     # and allow it to be streamed into the server.
-    
-    def __init__(self, backing_transport):
+
+    def __init__(self, backing_transport, commands):
+        """Constructor.
+
+        :param backing_transport: a Transport to handle requests for.
+        :param commands: a registry mapping command names to SmartServerRequest
+            subclasses. e.g. bzrlib.transport.smart.vfs.vfs_commands.
+        """
         self._backing_transport = backing_transport
-        self._converted_command = False
-        self.finished_reading = False
+        self._commands = commands
         self._body_bytes = ''
         self.response = None
+        self.finished_reading = False
+        self._command = None
 
     def accept_body(self, bytes):
-        """Accept body data.
-
-        This should be overriden for each command that desired body data to
-        handle the right format of that data. I.e. plain bytes, a bundle etc.
-
-        The deserialisation into that format should be done in the Protocol
-        object. Set self.desired_body_format to the format your method will
-        handle.
-        """
+        """Accept body data."""
+
+        # TODO: This should be overriden for each command that desired body data
+        # to handle the right format of that data, i.e. plain bytes, a bundle,
+        # etc.  The deserialisation into that format should be done in the
+        # Protocol object.
+
         # default fallback is to accumulate bytes.
         self._body_bytes += bytes
         
-    def _end_of_body_handler(self):
-        """An unimplemented end of body handler."""
-        raise NotImplementedError(self._end_of_body_handler)
-        
-    def do_hello(self):
-        """Answer a version request with my version."""
-        return SmartServerResponse(('ok', '1'))
-
-    def do_has(self, relpath):
-        r = self._backing_transport.has(relpath) and 'yes' or 'no'
-        return SmartServerResponse((r,))
-
-    def do_get(self, relpath):
-        backing_bytes = self._backing_transport.get_bytes(relpath)
-        return SmartServerResponse(('ok',), backing_bytes)
-
-    def _deserialise_optional_mode(self, mode):
-        # XXX: FIXME this should be on the protocol object.
-        if mode == '':
-            return None
-        else:
-            return int(mode)
-
-    def do_append(self, relpath, mode):
-        self._converted_command = True
-        self._relpath = relpath
-        self._mode = self._deserialise_optional_mode(mode)
-        self._end_of_body_handler = self._handle_do_append_end
-    
-    def _handle_do_append_end(self):
-        old_length = self._backing_transport.append_bytes(
-            self._relpath, self._body_bytes, self._mode)
-        self.response = SmartServerResponse(('appended', '%d' % old_length))
-
-    def do_delete(self, relpath):
-        self._backing_transport.delete(relpath)
-
-    def do_iter_files_recursive(self, relpath):
-        transport = self._backing_transport.clone(relpath)
-        filenames = transport.iter_files_recursive()
-        return SmartServerResponse(('names',) + tuple(filenames))
-
-    def do_list_dir(self, relpath):
-        filenames = self._backing_transport.list_dir(relpath)
-        return SmartServerResponse(('names',) + tuple(filenames))
-
-    def do_mkdir(self, relpath, mode):
-        self._backing_transport.mkdir(relpath,
-                                      self._deserialise_optional_mode(mode))
-
-    def do_move(self, rel_from, rel_to):
-        self._backing_transport.move(rel_from, rel_to)
-
-    def do_put(self, relpath, mode):
-        self._converted_command = True
-        self._relpath = relpath
-        self._mode = self._deserialise_optional_mode(mode)
-        self._end_of_body_handler = self._handle_do_put
-
-    def _handle_do_put(self):
-        self._backing_transport.put_bytes(self._relpath,
-                self._body_bytes, self._mode)
-        self.response = SmartServerResponse(('ok',))
-
-    def _deserialise_offsets(self, text):
-        # XXX: FIXME this should be on the protocol object.
-        offsets = []
-        for line in text.split('\n'):
-            if not line:
-                continue
-            start, length = line.split(',')
-            offsets.append((int(start), int(length)))
-        return offsets
-
-    def do_put_non_atomic(self, relpath, mode, create_parent, dir_mode):
-        self._converted_command = True
-        self._end_of_body_handler = self._handle_put_non_atomic
-        self._relpath = relpath
-        self._dir_mode = self._deserialise_optional_mode(dir_mode)
-        self._mode = self._deserialise_optional_mode(mode)
-        # a boolean would be nicer XXX
-        self._create_parent = (create_parent == 'T')
-
-    def _handle_put_non_atomic(self):
-        self._backing_transport.put_bytes_non_atomic(self._relpath,
-                self._body_bytes,
-                mode=self._mode,
-                create_parent_dir=self._create_parent,
-                dir_mode=self._dir_mode)
-        self.response = SmartServerResponse(('ok',))
-
-    def do_readv(self, relpath):
-        self._converted_command = True
-        self._end_of_body_handler = self._handle_readv_offsets
-        self._relpath = relpath
-
     def end_of_body(self):
         """No more body data will be received."""
-        self._run_handler_code(self._end_of_body_handler, (), {})
+        self._run_handler_code(self._command.do_body, (self._body_bytes,), {})
         # cannot read after this.
         self.finished_reading = True
 
-    def _handle_readv_offsets(self):
-        """accept offsets for a readv request."""
-        offsets = self._deserialise_offsets(self._body_bytes)
-        backing_bytes = ''.join(bytes for offset, bytes in
-            self._backing_transport.readv(self._relpath, offsets))
-        self.response = SmartServerResponse(('readv',), backing_bytes)
-        
-    def do_rename(self, rel_from, rel_to):
-        self._backing_transport.rename(rel_from, rel_to)
-
-    def do_rmdir(self, relpath):
-        self._backing_transport.rmdir(relpath)
-
-    def do_stat(self, relpath):
-        stat = self._backing_transport.stat(relpath)
-        return SmartServerResponse(('stat', str(stat.st_size), oct(stat.st_mode)))
-        
-    def do_get_bundle(self, path, revision_id):
-        # open transport relative to our base
-        t = self._backing_transport.clone(path)
-        control, extra_path = bzrdir.BzrDir.open_containing_from_transport(t)
-        repo = control.open_repository()
-        tmpf = tempfile.TemporaryFile()
-        base_revision = revision.NULL_REVISION
-        write_bundle(repo, revision_id, base_revision, tmpf)
-        tmpf.seek(0)
-        return SmartServerResponse((), tmpf.read())
-
     def dispatch_command(self, cmd, args):
         """Deprecated compatibility method.""" # XXX XXX
-        func = getattr(self, 'do_' + cmd, None)
-        if func is None:
+        try:
+            command = self._commands.get(cmd)
+        except LookupError:
             raise errors.SmartProtocolError("bad request %r" % (cmd,))
-        self._run_handler_code(func, args, {})
+        self._command = command(self._backing_transport)
+        self._run_handler_code(self._command.execute, args, {})
 
     def _run_handler_code(self, callable, args, kwargs):
         """Run some handler specific code 'callable'.
@@ -223,17 +150,15 @@
         from them.
         """
         result = self._call_converting_errors(callable, args, kwargs)
+
         if result is not None:
             self.response = result
             self.finished_reading = True
-        # handle unconverted commands
-        if not self._converted_command:
-            self.finished_reading = True
-            if result is None:
-                self.response = SmartServerResponse(('ok',))
 
     def _call_converting_errors(self, callable, args, kwargs):
         """Call callable converting errors to Response objects."""
+        # XXX: most of this error conversion is VFS-related, and thus ought to
+        # be in SmartServerVFSRequestHandler somewhere.
         try:
             return callable(*args, **kwargs)
         except errors.NoSuchFile, e:
@@ -265,3 +190,87 @@
                 raise
 
 
+class HelloRequest(SmartServerRequest):
+    """Answer a version request with my version."""
+
+    def do(self):
+        return SmartServerResponse(('ok', '1'))
+
+
+class GetBundleRequest(SmartServerRequest):
+
+    def do(self, path, revision_id):
+        # open transport relative to our base
+        t = self._backing_transport.clone(path)
+        control, extra_path = bzrdir.BzrDir.open_containing_from_transport(t)
+        repo = control.open_repository()
+        tmpf = tempfile.TemporaryFile()
+        base_revision = revision.NULL_REVISION
+        write_bundle(repo, revision_id, base_revision, tmpf)
+        tmpf.seek(0)
+        return SmartServerResponse((), tmpf.read())
+
+
+# This exists solely to help RemoteObjectHacking.  It should be removed
+# eventually.  It should not be considered part of the real smart server
+# protocol!
+class ProbeDontUseRequest(SmartServerRequest):
+
+    def do(self, path):
+        from bzrlib.bzrdir import BzrDirFormat
+        t = self._backing_transport.clone(path)
+        default_format = BzrDirFormat.get_default_format()
+        real_bzrdir = default_format.open(t, _found=True)
+        try:
+            real_bzrdir._format.probe_transport(t)
+        except (errors.NotBranchError, errors.UnknownFormatError):
+            answer = 'no'
+        else:
+            answer = 'yes'
+        return SmartServerResponse((answer,))
+
+
+class SmartServerIsReadonly(SmartServerRequest):
+    # XXX: this request method belongs somewhere else.
+
+    def do(self):
+        if self._backing_transport.is_readonly():
+            answer = 'yes'
+        else:
+            answer = 'no'
+        return SmartServerResponse((answer,))
+
+
+request_handlers = registry.Registry()
+request_handlers.register_lazy(
+    'append', 'bzrlib.smart.vfs', 'AppendRequest')
+request_handlers.register_lazy(
+    'delete', 'bzrlib.smart.vfs', 'DeleteRequest')
+request_handlers.register_lazy(
+    'get', 'bzrlib.smart.vfs', 'GetRequest')
+request_handlers.register_lazy(
+    'get_bundle', 'bzrlib.smart.request', 'GetBundleRequest')
+request_handlers.register_lazy(
+    'has', 'bzrlib.smart.vfs', 'HasRequest')
+request_handlers.register_lazy(
+    'hello', 'bzrlib.smart.request', 'HelloRequest')
+request_handlers.register_lazy(
+    'iter_files_recursive', 'bzrlib.smart.vfs', 'IterFilesRecursiveRequest')
+request_handlers.register_lazy(
+    'list_dir', 'bzrlib.smart.vfs', 'ListDirRequest')
+request_handlers.register_lazy(
+    'mkdir', 'bzrlib.smart.vfs', 'MkdirRequest')
+request_handlers.register_lazy(
+    'move', 'bzrlib.smart.vfs', 'MoveRequest')
+request_handlers.register_lazy(
+    'put', 'bzrlib.smart.vfs', 'PutRequest')
+request_handlers.register_lazy(
+    'put_non_atomic', 'bzrlib.smart.vfs', 'PutNonAtomicRequest')
+request_handlers.register_lazy(
+    'readv', 'bzrlib.smart.vfs', 'ReadvRequest')
+request_handlers.register_lazy(
+    'rename', 'bzrlib.smart.vfs', 'RenameRequest')
+request_handlers.register_lazy(
+    'rmdir', 'bzrlib.smart.vfs', 'RmdirRequest')
+request_handlers.register_lazy(
+    'stat', 'bzrlib.smart.vfs', 'StatRequest')

=== modified file bzrlib/tests/test_errors.py
--- bzrlib/tests/test_errors.py
+++ bzrlib/tests/test_errors.py
@@ -30,6 +30,11 @@
 
 class TestErrors(TestCaseWithTransport):
 
+    def test_disabled_method(self):
+        error = errors.DisabledMethod("class name")
+        self.assertEqualDiff(
+            "The smart server method 'class name' is disabled.", str(error))
+
     def test_duplicate_file_id(self):
         error = errors.DuplicateFileId('a_file_id', 'foo')
         self.assertEqualDiff('File id {a_file_id} already exists in inventory'

=== modified file bzrlib/tests/test_smart_transport.py
--- bzrlib/tests/test_smart_transport.py
+++ bzrlib/tests/test_smart_transport.py
@@ -35,6 +35,7 @@
         protocol,
         request,
         server,
+        vfs,
 )
 from bzrlib.tests.HTTPTestUtil import (
         HTTPServerWithSmarts,
@@ -574,6 +575,10 @@
 
 class TestSmartServerStreamMedium(tests.TestCase):
 
+    def setUp(self):
+        super(TestSmartServerStreamMedium, self).setUp()
+        self._captureVar('NO_SMART_VFS', None)
+
     def portable_socket_pair(self):
         """Return a pair of TCP sockets connected to each other.
         
@@ -780,6 +785,7 @@
 
     def test_get_error_unexpected(self):
         """Error reported by server with no specific representation"""
+        self._captureVar('NO_SMART_VFS', None)
         class FlakyTransport(object):
             base = 'a_url'
             def get_bytes(self, path):
@@ -865,12 +871,14 @@
 
     def test_smart_transport_has(self):
         """Checking for file existence over smart."""
+        self._captureVar('NO_SMART_VFS', None)
         self.backing_transport.put_bytes("foo", "contents of foo\n")
         self.assertTrue(self.transport.has("foo"))
         self.assertFalse(self.transport.has("non-foo"))
 
     def test_smart_transport_get(self):
         """Read back a file over smart."""
+        self._captureVar('NO_SMART_VFS', None)
         self.backing_transport.put_bytes("foo", "contents\nof\nfoo\n")
         fp = self.transport.get("foo")
         self.assertEqual('contents\nof\nfoo\n', fp.read())
@@ -880,6 +888,7 @@
         # The path in a raised NoSuchFile exception should be the precise path
         # asked for by the client. This gives meaningful and unsurprising errors
         # for users.
+        self._captureVar('NO_SMART_VFS', None)
         try:
             self.transport.get('not%20a%20file')
         except errors.NoSuchFile, e:
@@ -906,6 +915,7 @@
 
     def test_open_dir(self):
         """Test changing directory"""
+        self._captureVar('NO_SMART_VFS', None)
         transport = self.transport
         self.backing_transport.mkdir('toffee')
         self.backing_transport.mkdir('toffee/apple')
@@ -933,6 +943,7 @@
 
     def test_mkdir_error_readonly(self):
         """TransportNotPossible should be preserved from the backing transport."""
+        self._captureVar('NO_SMART_VFS', None)
         self.setUpServer(readonly=True)
         self.assertRaises(errors.TransportNotPossible, self.transport.mkdir,
             'foo')
@@ -977,19 +988,16 @@
 # server-stopped hook.
 
 
-class SmartServerRequestHandlerTests(tests.TestCaseWithTransport):
-    """Test that call directly into the handler logic, bypassing the network."""
-
-    def test_construct_request_handler(self):
-        """Constructing a request handler should be easy and set defaults."""
-        handler = request.SmartServerRequestHandler(None)
-        self.assertFalse(handler.finished_reading)
-
+class SmartServerCommandTests(tests.TestCaseWithTransport):
+    """Tests that call directly into the command objects, bypassing the network
+    and the request dispatching.
+    """
+        
     def test_hello(self):
-        handler = request.SmartServerRequestHandler(None)
-        handler.dispatch_command('hello', ())
-        self.assertEqual(('ok', '1'), handler.response.args)
-        self.assertEqual(None, handler.response.body)
+        cmd = request.HelloRequest(None)
+        response = cmd.execute()
+        self.assertEqual(('ok', '1'), response.args)
+        self.assertEqual(None, response.body)
         
     def test_get_bundle(self):
         from bzrlib.bundle import serializer
@@ -998,14 +1006,49 @@
         wt.add('hello')
         rev_id = wt.commit('add hello')
         
-        handler = request.SmartServerRequestHandler(self.get_transport())
-        handler.dispatch_command('get_bundle', ('.', rev_id))
-        bundle = serializer.read_bundle(StringIO(handler.response.body))
-        self.assertEqual((), handler.response.args)
+        cmd = request.GetBundleRequest(self.get_transport())
+        response = cmd.execute('.', rev_id)
+        bundle = serializer.read_bundle(StringIO(response.body))
+        self.assertEqual((), response.args)
+
+
+class SmartServerRequestHandlerTests(tests.TestCaseWithTransport):
+    """Test that call directly into the handler logic, bypassing the network."""
+
+    def setUp(self):
+        super(SmartServerRequestHandlerTests, self).setUp()
+        self._captureVar('NO_SMART_VFS', None)
+
+    def build_handler(self, transport):
+        """Returns a handler for the commands in protocol version one."""
+        return request.SmartServerRequestHandler(transport, request.request_handlers)
+
+    def test_construct_request_handler(self):
+        """Constructing a request handler should be easy and set defaults."""
+        handler = request.SmartServerRequestHandler(None, None)
+        self.assertFalse(handler.finished_reading)
+
+    def test_hello(self):
+        handler = self.build_handler(None)
+        handler.dispatch_command('hello', ())
+        self.assertEqual(('ok', '1'), handler.response.args)
+        self.assertEqual(None, handler.response.body)
+        
+    def test_disable_vfs_handler_classes_via_environment(self):
+        # VFS handler classes will raise an error from "execute" if NO_SMART_VFS
+        # is set.
+        handler = vfs.HasRequest(None)
+        # set environment variable after construction to make sure it's
+        # examined.
+        # Note that we can safely clobber NO_SMART_VFS here, because setUp has
+        # called _captureVar, so it will be restored to the right state
+        # afterwards.
+        os.environ['NO_SMART_VFS'] = ''
+        self.assertRaises(errors.DisabledMethod, handler.execute)
 
     def test_readonly_exception_becomes_transport_not_possible(self):
         """The response for a read-only error is ('ReadOnlyError')."""
-        handler = request.SmartServerRequestHandler(self.get_readonly_transport())
+        handler = self.build_handler(self.get_readonly_transport())
         # send a mkdir for foo, with no explicit mode - should fail.
         handler.dispatch_command('mkdir', ('foo', ''))
         # and the failure should be an explicit ReadOnlyError
@@ -1017,14 +1060,14 @@
 
     def test_hello_has_finished_body_on_dispatch(self):
         """The 'hello' command should set finished_reading."""
-        handler = request.SmartServerRequestHandler(None)
+        handler = self.build_handler(None)
         handler.dispatch_command('hello', ())
         self.assertTrue(handler.finished_reading)
         self.assertNotEqual(None, handler.response)
 
     def test_put_bytes_non_atomic(self):
         """'put_...' should set finished_reading after reading the bytes."""
-        handler = request.SmartServerRequestHandler(self.get_transport())
+        handler = self.build_handler(self.get_transport())
         handler.dispatch_command('put_non_atomic', ('a-file', '', 'F', ''))
         self.assertFalse(handler.finished_reading)
         handler.accept_body('1234')
@@ -1038,7 +1081,7 @@
     def test_readv_accept_body(self):
         """'readv' should set finished_reading after reading offsets."""
         self.build_tree(['a-file'])
-        handler = request.SmartServerRequestHandler(self.get_readonly_transport())
+        handler = self.build_handler(self.get_readonly_transport())
         handler.dispatch_command('readv', ('a-file', ))
         self.assertFalse(handler.finished_reading)
         handler.accept_body('2,')
@@ -1053,7 +1096,7 @@
     def test_readv_short_read_response_contents(self):
         """'readv' when a short read occurs sets the response appropriately."""
         self.build_tree(['a-file'])
-        handler = request.SmartServerRequestHandler(self.get_readonly_transport())
+        handler = self.build_handler(self.get_readonly_transport())
         handler.dispatch_command('readv', ('a-file', ))
         # read beyond the end of the file.
         handler.accept_body('100,1')
@@ -1138,10 +1181,11 @@
         self.client_protocol = protocol.SmartClientRequestProtocolOne(
             self.client_medium)
         self.smart_server = InstrumentedServerProtocol(self.server_to_client)
-        self.smart_server_request = request.SmartServerRequestHandler(None)
+        self.smart_server_request = request.SmartServerRequestHandler(
+            None, request.request_handlers)
 
     def assertOffsetSerialisation(self, expected_offsets, expected_serialised,
-        client, smart_server_request):
+        client):
         """Check that smart (de)serialises offsets as expected.
         
         We check both serialisation and deserialisation at the same time
@@ -1150,25 +1194,27 @@
         
         :param expected_offsets: a readv offset list.
         :param expected_seralised: an expected serial form of the offsets.
-        :param smart_server_request: a SmartServerRequestHandler instance.
         """
-        # XXX: 'smart_server_request' should be a SmartServerRequestProtocol in
-        # future.
-        offsets = smart_server_request._deserialise_offsets(expected_serialised)
+        # XXX: '_deserialise_offsets' should be a method of the
+        # SmartServerRequestProtocol in future.
+        readv_cmd = vfs.ReadvRequest(None)
+        offsets = readv_cmd._deserialise_offsets(expected_serialised)
         self.assertEqual(expected_offsets, offsets)
         serialised = client._serialise_offsets(offsets)
         self.assertEqual(expected_serialised, serialised)
 
     def build_protocol_waiting_for_body(self):
         out_stream = StringIO()
-        smart_protocol = protocol.SmartServerRequestProtocolOne(None, out_stream.write)
+        smart_protocol = protocol.SmartServerRequestProtocolOne(None,
+                out_stream.write)
         smart_protocol.has_dispatched = True
-        smart_protocol.request = request.SmartServerRequestHandler(None)
-        def handle_end_of_bytes():
-            self.end_received = True
-            self.assertEqual('abcdefg', smart_protocol.request._body_bytes)
-            smart_protocol.request.response = request.SmartServerResponse(('ok', ))
-        smart_protocol.request._end_of_body_handler = handle_end_of_bytes
+        smart_protocol.request = self.smart_server_request
+        class FakeCommand(object):
+            def do_body(cmd, body_bytes):
+                self.end_received = True
+                self.assertEqual('abcdefg', body_bytes)
+                return request.SmartServerResponse(('ok', ))
+        smart_protocol.request._command = FakeCommand()
         # Call accept_bytes to make sure that internal state like _body_decoder
         # is initialised.  This test should probably be given a clearer
         # interface to work with that will not cause this inconsistency.
@@ -1197,14 +1243,12 @@
         one with the order of reads not increasing (an out of order read), and
         one that should coalesce.
         """
-        self.assertOffsetSerialisation([], '',
-            self.client_protocol, self.smart_server_request)
-        self.assertOffsetSerialisation([(1,2)], '1,2',
-            self.client_protocol, self.smart_server_request)
+        self.assertOffsetSerialisation([], '', self.client_protocol)
+        self.assertOffsetSerialisation([(1,2)], '1,2', self.client_protocol)
         self.assertOffsetSerialisation([(10,40), (0,5)], '10,40\n0,5',
-            self.client_protocol, self.smart_server_request)
+            self.client_protocol)
         self.assertOffsetSerialisation([(1,2), (3,4), (100, 200)],
-            '1,2\n3,4\n100,200', self.client_protocol, self.smart_server_request)
+            '1,2\n3,4\n100,200', self.client_protocol)
 
     def test_accept_bytes_of_bad_request_to_protocol(self):
         out_stream = StringIO()

=== modified directory  // last-changed:andrew.bennetts at canonical.com-200704101
... 41235-wtmjydzktegq64cz
# revision id: andrew.bennetts at canonical.com-20070410141235-wtmjydzktegq64cz
# sha1: 82e5fb83d09d95bd0e2e1b5d8f878fb0448e7244
# inventory sha1: 72d5397020bb122754ed1cc1449d19f9311ddf4a
# parent ids:
#   pqm at pqm.ubuntu.com-20070410074302-cf6b95587a1058cd
# base id: pqm at pqm.ubuntu.com-20070410074302-cf6b95587a1058cd
# properties:
#   branch-nick: hpss-part-two-server-commands



More information about the bazaar mailing list